-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[release/3.1] Fix dotnet-ef RuntimeFrameworkVersion #20007
Conversation
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.
Change looks reasonable, but could you add a description of why it's needed? Do we need to take this for 3.1.3 (the march release)?
(Description updated) |
Verification: Installed a local 3.1.3 build of dotnet-ef with only runtime 3.1.1 available and it worked where dotnet-ef 3.1.2 failed. Also verified that the runtimeconfig contains 3.1.0 instead of the latest patch version. |
If we merge this today it shouldn't affect our ability to get a 3.1.3 build by tomorrow. Tell-mode should be fine since this is an infrastructure change, but let's still wait for @Pilchie's signoff |
I don't totally understand the fix, but I'm supportive of the scenario. I think we should still email Tactics, since this was an issue we discussed today. One thing we could consider is publishing this package earlier than the full 3.1.3 release too. |
I'll send email to tactics and we'll get this in for 3.1.3 today. @Pilchie If an earlier train arrives, then we should hop on it. |
Similar code existed before the move to Arcade: Line 38 in 2f07f47
efcore/Directory.Build.targets Lines 2 to 6 in 2f07f47
|
I'd like to clean this up more in master--just doing the minimal thing for patches |
This is approved for 3.1.3 by tactics. @bricelam Let's get this merged today. Also can you file an issue (or use an existing one) so we can have an issue tracking this in 3.1.3. |
Description
The dotnet-ef global tool was built incorrectly:
Customer Impact
This will break common VS scenarios as well as people who install the latest tool on the command line.
How found
CTI validation using VS following the patch release, and also customer-reported on the command line.
Test coverage
We have done manual validation on the new PR and will validate again once it is in the build.
Going forward we need to make sure that we can reproduce this in the CTI validation before the new packages are pushed to NuGet.
Also, since we knew about this before and thought it was fixed we will investigate what happened in the process that allowed this.
Regression?
Yes, from 3.0.
Risk
Low.
We had some dead code in the project files: (I'm not exactly sure what happened, but it looks like it broke while migrating to Arcade.)
Fixes errors like:
Verification:
Installed a local 3.1.3 build of dotnet-ef with only runtime 3.1.1 available and it worked where dotnet-ef 3.1.2 failed. Also verified that the runtimeconfig contains 3.1.0 instead of the latest patch version.
Fixes #20005