Skip to content
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

dotnet tool run and install roll-forward option #37231

Merged
merged 52 commits into from
Jan 27, 2024

Conversation

JL03-Yue
Copy link
Member

@JL03-Yue JL03-Yue commented Nov 29, 2023

! This pr does not contains the detection of compatible runtimes as it is still being discussed

dotnet tool run and install --roll-forward option

dotnet tool run --allow-roll-forward option

For dotnet tool run command, a --allow-roll-forward Option to parse the --allow-roll-forward option anywhere in the command using System.CommandLine is included, and it feed the value of this option (if any) into the actual Process.Start calls to launch tools (e.g. in the ToolRunCommand. Note that the --allow-roll-forward parsed is always inserted into the 'final' dotnet command first for positional argument purposes.

Use case
When the user has the local tool sample installed, and they invoke is using dotnet tool run sample --allow-roll-forward LatestMajor. This code rewrites that into dotnet --roll-forward Major <path to sampleEntryPoint.dll in the tool storage location>.

dotnet tool install --allow-roll-forward option for global tools

For dotnet tool install command for global tools, a --allow-roll-forward option to parse the --allow-roll-forward option anywhere in the dotnet tool install command for global tools is included. It updates the runtime config file for the tool to include a rollForward property set to Major according to the documentation of roll forward selection.

Use case
When the user is installing the global tool sample, and they install it using dotnet tool install sample -g --allow-roll-forward. This code will rewrite the runtimeConfig the for tool and set rollForward property to Major.

dotnet tool install --allow-roll-forward option for local tools

For dotnet tool install command for local tools, a --allow-roll-forward option to parse the --allow-roll-forward option anywhere in the dotnet tool install command for local tools is included. It stores the rollForward state in the local tool manifest file using the Add function in ToolManifestEditor, and read from the ToolManifestPackage using ToolManifestFinder. The rollForward is set to Major according to the documentation of roll forward selection.

Note that for tools in the same directory with same tool command, it returns the tool that has been added first. For tools installed then later installed with the --allow-roll-forward option. It updates the manifest to change the rollForward property to True. Note that to run the tool with dotnet [toolname], the rollForward does not work.

Use case
When the user is installing the local tool sample, and they install it using dotnet tool install sample --allow-roll-forward. This code will store the state in the manifest file and read from manifest file when running dotnet tool run [sampleToolCommand]

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Tools untriaged Request triage from a team member labels Nov 29, 2023
@JL03-Yue JL03-Yue changed the base branch from main to release/8.0.2xx November 29, 2023 23:15
@JL03-Yue JL03-Yue changed the title dotnet tool run roll-forward option dotnet tool run and install roll-forward option Nov 30, 2023
@KalleOlaviNiemitalo
Copy link
Contributor

Seems related to #26824 but that one is for all runtime-options, not just --roll-forward.

@KalleOlaviNiemitalo
Copy link
Contributor

How about dotnet tool run foo -- --roll-forward major; I think that should not treat --roll-forward as a dotnet option and should instead pass it to the tool only. But I didn't find a test for this case.

@JL03-Yue
Copy link
Member Author

Seems related to #26824 but that one is for all runtime-options, not just --roll-forward.

Yes, it is a similar solution. The other issue discussion is at #30336.

@baronfel
Copy link
Member

How about dotnet tool run foo -- --roll-forward major; I think that should not treat --roll-forward as a dotnet option and should instead pass it to the tool only. But I didn't find a test for this case.

This is a good thing to verify - -- should signal that everything afterwards is forwarded to the called tool.

Copy link
Contributor

@KalleOlaviNiemitalo KalleOlaviNiemitalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd want to use Major roll-forward rather than LatestMajor, to minimize the risk that installing a later version of .NET Runtime breaks some tool that previously worked via roll-forward. RollForwardOptionDescription in install/LocalizableStrings.resx looks like --roll-forward=Major should work; but ToolInstallCommandParser.RollForwardOption is defined as a CliOption<bool>, and ToolPackageDownloader.UpdateRuntimeConfigFile can only set "LatestMajor".

src/Cli/dotnet/ToolPackage/ToolPackageDownloader.cs Outdated Show resolved Hide resolved
Copy link
Member Author

@JL03-Yue JL03-Yue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more tests to be covered

@JL03-Yue JL03-Yue changed the base branch from release/8.0.2xx to release/8.0.3xx January 24, 2024 00:01
Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the detailed description and coverage of the various scenarios! I'm excited for this first step along the road of making tools easier to use.

@JL03-Yue JL03-Yue merged commit dc2a5fe into dotnet:release/8.0.3xx Jan 27, 2024
16 checks passed
@JL03-Yue JL03-Yue deleted the local-tool-rollforward branch January 27, 2024 07:48
0xced added a commit to serilog-contrib/serilog-formatting-log4net that referenced this pull request Jul 8, 2024
Implemented in [dotnet tool run and install roll-forward option][1].

Supported [since the .NET SDK v8.0.300][2].

Documented in [
.NET tool roll-forward][3].

[1]: dotnet/sdk#37231
[2]: dotnet/sdk@dc2a5fe
[3]: https://learn.microsoft.com/en-us/dotnet/core/whats-new/dotnet-9/sdk#net-tool-roll-forward
0xced added a commit to serilog-contrib/serilog-formatting-log4net that referenced this pull request Jul 8, 2024
Implemented in [dotnet tool run and install roll-forward option][1].

Supported [since the .NET SDK v8.0.300][2].

Documented in [
.NET tool roll-forward][3].

[1]: dotnet/sdk#37231
[2]: dotnet/sdk@dc2a5fe
[3]: https://learn.microsoft.com/en-us/dotnet/core/whats-new/dotnet-9/sdk#net-tool-roll-forward
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Tools untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants