-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[WIP NO MERGE]: Add support for RuntimeFrameworkName to add an additional implicit package ref for ASP.NET Core 2.1 projects #2404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When set by the project, this property adds an additional PackageReference to Microsoft.AspNetCore.App/All which behaves like Microsoft.NETCore.App. It is the recommended replacement for the versionless PackageReference, which only worked when using Microsoft.NET.Sdk.Web. See dotnet/aspnetcore#3307
xlf regeneration appeared to mess up Strings.ru.xlf. Does this require manual fixup for some reason? |
No manual fixup of xlf should be required. That would be an xliff-tasks bug. I'll investigate. |
Not sure what happened to strings.ru.xlf. I've never seen that before. It's behaving as though it didn't think the file already had content. I'm not sure how to repro it either. I have a feeling the following would solve it, but then we'd lose our repro :(
Given a bunch of things under time pressure in parallel today, I think we should try that. Can you back up your bin/obj directory somewhere first, so that I can take a look for any clues left behind by the bad update. |
This reverts commit 9bcb413.
Ok, I think the xlf files are correct now. When I ran build.cmd the first time while preparing this PR, UpdateXlf failed. I wondered if this might be related to corruption of Strings.ru.xlf, but the error said the issue was with Strings.es.xlf.
|
I think that access issue is a bug that was fixed in xliff-tasks, I'll check if we're not on latest |
Therefore, this uses the same property fro both implicit package references. | ||
--> | ||
<Choose> | ||
<When Condition=" '$(RuntimeFrameworkName)' == 'Microsoft.AspNetCore.All' " > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understood the rationale for why we can have just one property is that we will align the versions of these shared frameworks. It makes me wonder why we have separate logic for setting the version of each of them here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably because when we have the core shared framework, the baseline is still 2.1.0, but we've already set the precedent of a different baseline of 2.1.1 for ASP.NET?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, that's why. I only added the separate properties because aspnetcore's baseline is 2.1.1 (for now.)
Per conversation in person, we are no longer pursuing this fix in 2.1.4xx. |
Part of dotnet/aspnetcore#3307
An alternative to #2394, which doesn't require changes to runtime packages and keeps the baseline for ASP.NET Core at 2.1.1 (which is the baseline in the 2.1.301 SDK).
Changes: