Remove the custom SDK download on the CI, use a later version of dotnet.exe in our testing #3924
Conversation
There was a problem hiding this comment.
Thanks for fixing this issue!
After this fix, we're not able to install and use .NET SDK in early preview stage(not the same version installed by VS), but only use the .NET SDK installed on the CI machine.
But this should only be concerned if we need to build with very early preview version, which happens only if NuGet needs to use the new APIs from early preview version. So this situation should be very rare from my understanding.
|
Ready again @heng-liu |
| } | ||
|
|
||
| throw new ArgumentException(string.Format(Strings.ListPkg_InvalidOptions, firstOption, incompatibleOption)); | ||
| throw new ArgumentException(string.Format(Strings.ListPkg_InvalidOptions)); |
There was a problem hiding this comment.
Product changes and CI changes in the same PR can increase the effort required to cherry-pick onto old release branches. Consider splitting the PR. This change, for example, will cause product issues if cherry-picked onto a branch before --deprecated and --vulnerable were added.
There was a problem hiding this comment.
This was a test fix.
Honestly, this product change is a dead codepath.
The parsing is happening on the SDK side, ours is there for our internal testing. In production scenarios, only the dotnet.exe code gets hit.
Alternatively, I can just change the string that the test asserts from the resource to a hardcoded string.
lmk wyt.
There was a problem hiding this comment.
The two installation scrips seems useless after this change.
https://github.com/NuGet/NuGet.Client/blob/dev-nkolev92-useLatestSDK/scripts/utils/InstallCLIforBuild.ps1
https://github.com/NuGet/NuGet.Client/blob/dev-nkolev92-useLatestSDK/scripts/utils/InstallCLIforBuild.sh
Bug
Fixes: https://github.com/NuGet/Client.Engineering/issues/739
Regression? Last working version:
Description
The good practice is to always be testing against the active release of the SDK.
When signing was added, we were targeting net5.0, while net5.0 was in development, so we needed to do some fancy work in order to make everything work.
This change tries to bring us back to a simpler world.
Removes the SDK download for building. It'd be a good practice to choose our own SDK, but that raises too many questions given that the build machines are not owned by us.
The SDK is built with the ability to build older versions of the runtime, so we shouldn't run into issues.
Moves the SDK to test against to 5.0.2xx. You might ask why not 5.0.3xx? Well it's not exactly available/stable enough yet.
There are some script behavior changes as we migrated to the new script, so I had to add a workaround for that. Note that I did file 2 respective issues.
dotnet/install-scripts#152 and dotnet/install-scripts#153.
Finally I had to match a test to have the correct expectations now that we're using a dotnet.exe with the same functionality.
The particular test is dotnet list package and here's the SDK side implementation: https://github.com/dotnet/sdk/pull/14352/files#diff-0c2d181894818092e8d9bd3388ffd12bf2c22bcd0e4e48bc282e1b1c51b7d82fR159
At a later point we can move to a newer SDK.
PR Checklist
PR has a meaningful title
PR has a linked issue.
Described changes
Tests
Documentation