Prompt for tool exec only if tool hasn't been downloaded before

Author: dsplaisted

src/Cli/dotnet/CliStrings.resx

@@ -317,7 +317,7 @@
     <value>More than one command is defined for the tool.</value>
   </data>
   <data name="ToolSettingsUnsupportedRunner" xml:space="preserve">
-    <value>Command '{0}' uses unsupported runner '{1}'."</value>
+    <value>Tool '{0}' uses unsupported runner '{1}'."</value>
   </data>
   <data name="ToolUnsupportedRuntimeIdentifier" xml:space="preserve">
     <value>The tool does not support the current architecture or operating system ({0}). Supported runtimes: {1}</value>

src/Cli/dotnet/CommandFactory/CommandResolution/LocalToolsCommandResolver.cs

@@ -3,6 +3,7 @@
 
 #nullable disable
 
+using Microsoft.DotNet.Cli.Commands.Tool;
 using Microsoft.DotNet.Cli.ToolManifest;
 using Microsoft.DotNet.Cli.ToolPackage;
 using Microsoft.DotNet.Cli.Utils;
@@ -91,31 +92,8 @@ private CommandSpec GetPackageCommandSpecUsingMuxer(CommandResolverArguments arg
                     toolCommandName.ToString()));
             }
 
-            if (toolCommand.Runner == "dotnet")
-            {
-                if (toolManifestPackage.RollForward || allowRollForward)
-                {
-                    arguments.CommandArguments = ["--allow-roll-forward", .. arguments.CommandArguments];
-                }
-
-                return MuxerCommandSpecMaker.CreatePackageCommandSpecUsingMuxer(
-                    toolCommand.Executable.Value,
-                    arguments.CommandArguments);
-            }
-            else if (toolCommand.Runner == "executable")
-            {
-                var escapedArgs = ArgumentEscaper.EscapeAndConcatenateArgArrayForProcessStart(
-                    arguments.CommandArguments);
-
-                return new CommandSpec(
-                    toolCommand.Executable.Value,
-                    escapedArgs);
-            }
-            else
-            {
-                throw new GracefulException(string.Format(CliStrings.ToolSettingsUnsupportedRunner,
-                    toolCommand.Name, toolCommand.Runner));
-            }
+            return ToolCommandSpecCreator.CreateToolCommandSpec(toolCommand.Name.Value, toolCommand.Executable.Value, toolCommand.Runner,
+                toolManifestPackage.RollForward || allowRollForward, arguments.CommandArguments);
         }
         else
         {

src/Cli/dotnet/Commands/CliCommandStrings.resx

@@ -2486,7 +2486,7 @@ To display a value, specify the corresponding command-line option without provid
     <value>Missing package ID</value>
   </data>
   <data name="ToolRunFromSourceUserConfirmationPrompt" xml:space="preserve">
-    <value>Tool not found on the system. Do you want to run it from source? [y/n]</value>
+    <value>Tool package {0}@{1} will be downloaded from source {2}.  Proceed? [y/n]</value>
   </data>
   <data name="ToolRunFromSourceUserConfirmationFailed" xml:space="preserve">
     <value>Run from source approval denied by the user</value>

src/Cli/dotnet/Commands/Tool/Execute/ToolExecuteCommand.cs

@@ -16,6 +16,7 @@
 
 using Microsoft.Extensions.EnvironmentAbstractions;
 using NuGet.Common;
+using NuGet.Configuration;
 using NuGet.Packaging.Core;
 using NuGet.Versioning;
 
@@ -87,40 +88,54 @@ public override int Execute()
                 }
             }
 
+            var packageLocation = new PackageLocation(
+                    nugetConfig: _configFile != null ? new(_configFile) : null,
+                    sourceFeedOverrides: _sources,
+                    additionalFeeds: _addSource);
 
-            if (!UserAgreedToRunFromSource())
-            {
-                throw new GracefulException(CliCommandStrings.ToolRunFromSourceUserConfirmationFailed, isUserError: true);
-            }
+            var restoreActionConfig = new RestoreActionConfig(
+                    IgnoreFailedSources: _ignoreFailedSources,
+                    Interactive: _interactive);
 
-            if (_allowRollForward)
-            {
-                _forwardArguments.Append("--allow-roll-forward");
-            }
+            (var bestVersion, var packageSource) = _toolPackageDownloader.GetNuGetVersion(packageLocation, packageId, _verbosity, versionRange, restoreActionConfig);
 
+            IToolPackage toolPackage;
 
-            string tempDirectory = NuGetEnvironment.GetFolderPath(NuGetFolderPath.Temp);
+            //  TODO: Add framework argument
+            if (!_toolPackageDownloader.TryGetDownloadedTool(packageId, bestVersion, targetFramework: null, out toolPackage))
+            {
+                if (!UserAgreedToRunFromSource(packageId, bestVersion, packageSource))
+                {
+                    //  TODO: Refactor this to print a better message, and probably a different message depending on whether the user selected no
+                    //  or whether interactive mode was off
+                    throw new GracefulException(CliCommandStrings.ToolRunFromSourceUserConfirmationFailed, isUserError: true);
+                }
 
-            IToolPackage toolPackage = _toolPackageDownloader.InstallPackage(
-                new PackageLocation(
+                //  We've already determined which source we will use and displayed that in a confirmation message to the user.
+                //  So set the package location here to override the source feeds to just the source we already resolved to.
+                //  This does mean that we won't work with feeds that have a primary package but where the RID-specific packages are on
+                //  other feeds, but this is probably OK.
+                var downloadPackageLocation = new PackageLocation(
                     nugetConfig: _configFile != null ? new(_configFile) : null,
-                    sourceFeedOverrides: _sources,
-                    additionalFeeds: _addSource),
-                packageId: packageId,
-                verbosity: _verbosity,
-                versionRange: versionRange,
-                isGlobalToolRollForward: _allowRollForward, // Needed to update .runtimeconfig.json
-                restoreActionConfig: new(
-                    IgnoreFailedSources: _ignoreFailedSources,
-                    Interactive: _interactive));
+                    sourceFeedOverrides: [packageSource.Source],
+                    additionalFeeds: _addSource);
+
+                toolPackage = _toolPackageDownloader.InstallPackage(
+                    downloadPackageLocation,
+                    packageId: packageId,
+                    verbosity: _verbosity,
+                    versionRange: new VersionRange(bestVersion, true, bestVersion, true),
+                    isGlobalToolRollForward: false,
+                    restoreActionConfig: restoreActionConfig);
+            }
 
-            CommandSpec commandSpec = MuxerCommandSpecMaker.CreatePackageCommandSpecUsingMuxer(toolPackage.Command.Executable.ToString(), _forwardArguments);
+            var commandSpec = ToolCommandSpecCreator.CreateToolCommandSpec(toolPackage.Command.Name.Value, toolPackage.Command.Executable.Value, toolPackage.Command.Runner, _allowRollForward, _forwardArguments);
             var command = CommandFactoryUsingResolver.Create(commandSpec);
             var result = command.Execute();
             return result.ExitCode;
         }
 
-        private bool UserAgreedToRunFromSource()
+        private bool UserAgreedToRunFromSource(PackageId packageId, NuGetVersion version, PackageSource source)
         {
             if (_yes)
             {
@@ -132,19 +147,22 @@ private bool UserAgreedToRunFromSource()
                 return false;
             }
 
-            // TODO: Use a better way to ask for user input
-            Console.Write(CliCommandStrings.ToolRunFromSourceUserConfirmationPrompt);
+            //  TODO: Use a better way to ask for user input
+            //  TODO: How to localize y/n and interpret keys correctly?  Does Spectre.Console handle this?
+            string promptMessage = string.Format(CliCommandStrings.ToolRunFromSourceUserConfirmationPrompt, packageId, version.ToString(), source.Source);
+
+            Console.Write(promptMessage);
             bool userAccepted = Console.ReadKey().Key == ConsoleKey.Y;
 
             if (_verbosity >= VerbosityOptions.detailed)
             {
                 Console.WriteLine();
-                Console.WriteLine(new String('-', CliCommandStrings.ToolRunFromSourceUserConfirmationPrompt.Length));
+                Console.WriteLine(new String('-', promptMessage.Length));
             }
             else
             {
                 // Clear the line
-                Console.Write("\r" + new string(' ', CliCommandStrings.ToolRunFromSourceUserConfirmationPrompt.Length + 1) + "\r");
+                Console.Write("\r" + new string(' ', promptMessage.Length + 1) + "\r");
             }
 
             return userAccepted;

src/Cli/dotnet/Commands/Tool/Execute/ToolExecuteCommandParser.cs

@@ -58,6 +58,8 @@ private static Command ConstructCommand()
             command.Options.Add(YesOption);
             command.Options.Add(VerbosityOption);
 
+            //  TODO: Framework?
+
             command.SetAction((parseResult) => new ToolExecuteCommand(parseResult).Execute());
 
             return command;

src/Cli/dotnet/Commands/Tool/Install/ParseResultExtension.cs

@@ -36,6 +36,8 @@ public static VersionRange GetVersionRange(this ParseResult parseResult)
 
         if (prerelease)
         {
+            // TODO: This is probably broken if you try to specify --prerelease together with a version.  Should that be allowed?
+            // It seems like it would make sense to specify a version of something like 2.* and want to allow prerelease versions.
             packageVersion = "*-*";
         }
 

src/Cli/dotnet/Commands/Tool/ToolCommandSpecCreator.cs

@@ -0,0 +1,47 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.CommandLine;
+using System.Text;
+using Microsoft.DotNet.Cli.CommandFactory;
+using Microsoft.DotNet.Cli.CommandFactory.CommandResolution;
+using Microsoft.DotNet.Cli.ToolManifest;
+using Microsoft.DotNet.Cli.ToolPackage;
+using Microsoft.DotNet.Cli.Utils;
+
+namespace Microsoft.DotNet.Cli.Commands.Tool
+{
+    internal class ToolCommandSpecCreator
+    {
+        public static CommandSpec CreateToolCommandSpec(string toolName, string toolExecutable, string toolRunner, bool allowRollForward, IEnumerable<string> commandArguments)
+        {
+            if (toolRunner == "dotnet")
+            {
+                if (allowRollForward)
+                {
+                    commandArguments = ["--allow-roll-forward", .. commandArguments];
+                }
+
+                return MuxerCommandSpecMaker.CreatePackageCommandSpecUsingMuxer(
+                    toolExecutable,
+                    commandArguments);
+            }
+            else if (toolRunner == "executable")
+            {
+                var escapedArgs = ArgumentEscaper.EscapeAndConcatenateArgArrayForProcessStart(
+                    commandArguments);
+
+                return new CommandSpec(
+                    toolExecutable,
+                    escapedArgs);
+            }
+            else
+            {
+                throw new GracefulException(string.Format(CliStrings.ToolSettingsUnsupportedRunner,
+                    toolName, toolRunner));
+            }
+        }
+    }
+}