-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
accept bare version as NuGet exact version for .NET tools #36671
accept bare version as NuGet exact version for .NET tools #36671
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
src/Cli/dotnet/commands/dotnet-tool/install/ParseResultExtension.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-tool/install/ParseResultExtension.cs
Outdated
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
This looks great! I think another team member should take a look, but I'm happy with it :)
src/Cli/dotnet/commands/dotnet-tool/install/ParseResultExtension.cs
Outdated
Show resolved
Hide resolved
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.
Looking into this, I realized that it's possible to specify floating versions, for example --version 1.0.*
. Do we have any tests covering that scenario? If not it might be a good idea to add some.
src/Cli/dotnet/commands/dotnet-tool/install/ParseResultExtension.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-tool/install/ParseResultExtension.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-tool/install/ParseResultExtension.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Andy Zivkovic <zivkan@users.noreply.github.com>
src/Cli/dotnet/commands/dotnet-tool/install/ParseResultExtension.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Andy Zivkovic <zivkan@users.noreply.github.com>
Customer Impact
In .NET 8 we made a breaking change that changes the current .NET tool installation mechanism from restore a local temporary project to downloading the .NET tool from NuGet. This change was introduced because this previous mechanism has number of side effects, often show up as flaky restore errors because MSBuild concepts like Directory.Build.props, or other heirarchical notions, pollute the restore.
Per user feedback in #35566, he change, however, disable users from installing unlisted tools. This feature was added in #36021 if specifying exact match. However, since the exact match refers to input
[version]
. (referred to NuGet package version reference) come with bracket, this caused surprise for some users using bare version as exact version match.The intended way to fix this is to allow bare version of .NET tool to interpret as specific version, i.e. version
5.0.0
refers to the exact version match of5.0.0
instead of[5.0.0, )
. To add this feature, a semvar detection is added when getting and parsing the version range inGetVersionRange()
. Changes are validated by tests.Testing
Existing unit tests passed without changes. New tested are added to validate an unlisted bar version tool can be installed with exact version match withtout bracket specified. The new testing scenarios pass with changes in the pull request.
Risk
Low. This change adds a notation that was missed from breaking change without adding complexity to the behavior.
Original description
Per conversation #35566, specifically discussions on comment, this change is made to accept bare versions in .NET tools as NuGet exact version. For example, e.g.
--version 1.2.3
would be interpreted by the NuGet package downloader as[1.2.3]