[WIP] dotnet tool exec <package[@version]> one-shot execution
#48443
Conversation
|
Commented offline that we wanted to start with "dotnet tool run --from-source". We're currently discussing alias options or auto-detection options but the full command is safest to start with. |
|
@edvilme I would recommend patterning this more off of local tools than global tools. Global tools download the packages to a folder structure under Local tools on the other hand run directly from the packages in the NuGet global packages folder. I think it would be easier to run a tool from there without any side effects besides the fact that the package is now in the global packages folder. |
There was a problem hiding this comment.
Things to consider for review when it is finished / good test cases:
- Does it delete the tool, as it says without permanently installing it
- It passes the args to tools
- Version resolution works
- Uses latest if no version specified
- It uses the feed
- Make sure we prompt before downloading something new
- Does it have -y -n
- Supports nuget feed options
I like dotnet toolx personally but ok to see why we are using --from-source for now since it matches the other patterns.
|
As per internal discussions, this should be moved to dotnet tool exec <package[@version]> |
dotnet tool exec <package[@version]> one-shot execution
dotnet tool exec <package[@version]> one-shot executiondotnet tool exec <package[@version]>` one-shot execution
dotnet tool exec <package[@version]>` one-shot executiondotnet tool exec <package[@version]> one-shot execution
|
|
||
| VersionRange versionRange = _parseResult.GetVersionRange(); | ||
|
|
||
| string tempDirectory = PathUtilities.CreateTempSubdirectory(); |
There was a problem hiding this comment.
I may just not understand the feature properly, but I thought marcpopMSFT told me that the plan was to execute the tool directly from the nuget cache? Though it would be pretty fast as long as it'd already been downloaded.
|
/ba-g stuck |
|
Note: Ready for review, failing some test. |
| Arity = ArgumentArity.Zero | ||
| }; | ||
|
|
||
| public static Option<bool> NoOption = |
There was a problem hiding this comment.
I don't think this is ever used? And I don't think it should be.
| return result.ExitCode; | ||
| } | ||
|
|
There was a problem hiding this comment.
nit: some extra blank lines here
| @@ -39,6 +42,7 @@ private static Command ConstructCommand() | |||
|
|
|||
| command.Arguments.Add(CommandNameArgument); | |||
| command.Arguments.Add(CommandArgument); | |||
|
|
|||
There was a problem hiding this comment.
It seems like you were planning to add one-shot execution to tool run then didn't. Can we revert the changes in this file and move the --yes option to the ToolExecuteCommandParser?
| }; | ||
|
|
||
| public static readonly Option<string> VersionOption = ToolInstallCommandParser.VersionOption; | ||
| public static readonly Option<bool> RollForwardOption = ToolInstallCommandParser.RollForwardOption; |
There was a problem hiding this comment.
Why do we have to define these here as opposed to just adding ToolInstallCommandParser.RollForwardOption in ToolExecuteCommandParser.ConstructCommand and using that in the Command?
|
|
||
| public static readonly Argument<IEnumerable<string>> CommandArgument = new("commandArguments") | ||
| { | ||
| Description = "arguments forwarded to the tool" |
There was a problem hiding this comment.
Localize? And if this is actually expected to be seen by people, might want to rephrase
| @@ -2483,6 +2483,12 @@ To display a value, specify the corresponding command-line option without provid | |||
| <data name="ZeroTestsRan" xml:space="preserve"> | |||
| <value>Zero tests ran</value> | |||
| </data> | |||
| <data name="ToolRunFromSourceOptionDescription" xml:space="preserve"> | |||
There was a problem hiding this comment.
I don't think this is ever used?
|
|
||
| public override int Execute() | ||
| { | ||
| if (!UserAgreedToRunFromSource() || _packageToolIdentityArgument is null) |
There was a problem hiding this comment.
How does this show up to the user? Just as "Tool exec command returned 1"? If so, we should make that clearer.
Closes #31103
WIP: Name, and implementation may change