Avoid reinstall tool already installed (#41746)

Fixes #40818

Context

Following the changes introduced in #37311, the dotnet tool install --global command began reinstalling tools that were already installed, which involved deleting and then adding the same tool version again. Before this release, it was not possible to install a version that was already installed.

This behavior change was reported in #40818. I propose a modification to avoid reinstalling tools that are already installed.

Change

The current installation flow involves uninstalling all matching tools and then installing them again. My change modifies this flow by first checking if the requested best match package version is already installed. If it is, the flow is stopped, and a message is printed: 'Tool '{0}' is already installed'.

src/Cli/dotnet/ToolPackage/IToolPackageDownloader.cs

@@ -9,12 +9,21 @@ namespace Microsoft.DotNet.Cli.ToolPackage
 {
     internal interface IToolPackageDownloader
     {
-        IToolPackage InstallPackage(PackageLocation packageLocation, PackageId packageId,
+        IToolPackage InstallPackage(PackageLocation packageLocation,
+            PackageId packageId,
             VerbosityOptions verbosity,
             VersionRange versionRange = null,
             string targetFramework = null,
             bool isGlobalTool = false,
             bool isGlobalToolRollForward = false
         );
+
+        NuGetVersion GetNuGetVersion(
+            PackageLocation packageLocation,
+            PackageId packageId,
+            VerbosityOptions verbosity,
+            VersionRange versionRange = null,
+            bool isGlobalTool = false
+        );
     }
 }

src/Cli/dotnet/ToolPackage/ToolPackageDownloader.cs

@@ -377,5 +377,39 @@ namespace Microsoft.DotNet.Cli.ToolPackage
             lockFile.Targets.Add(lockFileTarget);
             new LockFileFormat().Write(Path.Combine(assetFileDirectory.Value, "project.assets.json"), lockFile);
         }
+
+        public NuGetVersion GetNuGetVersion(
+            PackageLocation packageLocation,
+            PackageId packageId,
+            VerbosityOptions verbosity,
+            VersionRange versionRange = null,
+            bool isGlobalTool = false)
+        {
+            ILogger nugetLogger = new NullLogger();
+
+            if (verbosity.IsDetailedOrDiagnostic())
+            {
+                nugetLogger = new NuGetConsoleLogger();
+            }
+
+            if (versionRange == null)
+            {
+                var versionString = "*";
+                versionRange = VersionRange.Parse(versionString);
+            }
+
+            var nugetPackageDownloader = new NuGetPackageDownloader.NuGetPackageDownloader(
+                packageInstallDir: isGlobalTool ? _globalToolStageDir : _localToolDownloadDir,
+                verboseLogger: nugetLogger,
+                isNuGetTool: true,
+                verbosityOptions: verbosity);
+
+            var packageSourceLocation = new PackageSourceLocation(
+                nugetConfig: packageLocation.NugetConfig,
+                rootConfigDirectory: packageLocation.RootConfigDirectory,
+                additionalSourceFeeds: packageLocation.AdditionalFeeds);
+
+            return nugetPackageDownloader.GetBestPackageVersionAsync(packageId, versionRange, packageSourceLocation).GetAwaiter().GetResult();
+        }
     }
 }

src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallGlobalOrToolPathCommand.cs

@@ -19,6 +19,7 @@ using NuGet.Versioning;
 using Microsoft.DotNet.Tools.Tool.List;
 using static System.Formats.Asn1.AsnWriter;
 using System.CommandLine.Parsing;
+using static System.Threading.Lock;
 
 namespace Microsoft.DotNet.Tools.Tool.Install
 {
@@ -157,6 +158,17 @@ namespace Microsoft.DotNet.Tools.Tool.Install
 
             IToolPackage oldPackageNullable = GetOldPackage(toolPackageStoreQuery, packageId);
 
+            if (oldPackageNullable != null)
+            {
+                NuGetVersion nugetVersion = GetBestMatchNugetVersion(packageId, versionRange, toolPackageDownloader);
+
+                if (ToolVersionAlreadyInstalled(oldPackageNullable, nugetVersion))
+                {
+                    _reporter.WriteLine(string.Format(LocalizableStrings.ToolAlreadyInstalled, _packageId, oldPackageNullable.Version.ToNormalizedString()).Green());
+                    return 0;
+                }   
+            }
+
             using (var scope = new TransactionScope(
                 TransactionScopeOption.Required,
                 TimeSpan.Zero))
@@ -226,6 +238,22 @@ namespace Microsoft.DotNet.Tools.Tool.Install
             return 0;
         }
 
+        private NuGetVersion GetBestMatchNugetVersion(PackageId packageId, VersionRange versionRange, IToolPackageDownloader toolPackageDownloader)
+        {
+            return toolPackageDownloader.GetNuGetVersion(
+                packageLocation: new PackageLocation(nugetConfig: GetConfigFile(), additionalFeeds: _source),
+                packageId: packageId,
+                versionRange: versionRange,
+                verbosity: _verbosity,
+                isGlobalTool: true
+            );
+        }
+
+        private static bool ToolVersionAlreadyInstalled(IToolPackage oldPackageNullable, NuGetVersion nuGetVersion)
+        {
+            return oldPackageNullable != null && (oldPackageNullable.Version.Version == nuGetVersion.Version);
+        }
+
         private static void EnsureVersionIsHigher(IToolPackage oldPackageNullable, IToolPackage newInstalledPackage, bool allowDowngrade)
         {
             if (oldPackageNullable != null && (newInstalledPackage.Version < oldPackageNullable.Version && !allowDowngrade))

test/Microsoft.DotNet.PackageInstall.Tests/ToolPackageDownloaderTests.cs

@@ -178,6 +178,53 @@ namespace Microsoft.DotNet.PackageInstall.Tests
             uninstaller.Uninstall(package.PackageDirectory);
         }
 
+        [Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void GivenAllButNoPackageVersionItReturnLatestStableVersion(bool testMockBehaviorIsInSync)
+        {
+            var nugetConfigPath = GenerateRandomNugetConfigFilePath();
+
+            var (store, storeQuery, downloader, uninstaller, reporter, fileSystem) = Setup(
+                useMock: testMockBehaviorIsInSync,
+                writeLocalFeedToNugetConfig: nugetConfigPath);
+
+            var package = downloader.GetNuGetVersion(
+                new PackageLocation(nugetConfig: nugetConfigPath),
+                packageId: TestPackageId,
+                verbosity: TestVerbosity,
+                isGlobalTool: true);
+
+            package.OriginalVersion.Should().Be(TestPackageVersion);
+        }
+
+        [Theory]
+        [InlineData(false, "1.0.0-rc*", TestPackageVersion)]
+        [InlineData(true, "1.0.0-rc*", TestPackageVersion)]
+        [InlineData(false, "1.*", TestPackageVersion)]
+        [InlineData(true, "1.*", TestPackageVersion)]
+        [InlineData(false, TestPackageVersion, TestPackageVersion)]
+        [InlineData(true, TestPackageVersion, TestPackageVersion)]
+        public void GivenASpecificVersionGetCorrectVersion(bool testMockBehaviorIsInSync, string requestedVersion, string expectedVersion)
+        {
+            var nugetConfigPath = GenerateRandomNugetConfigFilePath();
+            var emptySource = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+            Directory.CreateDirectory(emptySource);
+
+            var (store, storeQuery, downloader, uninstaller, reporter, fileSystem) = Setup(
+                useMock: testMockBehaviorIsInSync,
+                writeLocalFeedToNugetConfig: nugetConfigPath);
+
+            var package = downloader.GetNuGetVersion(new PackageLocation(nugetConfig: nugetConfigPath,
+                    additionalFeeds: new[] { emptySource }),
+                packageId: TestPackageId,
+                verbosity: TestVerbosity,
+                versionRange: VersionRange.Parse(requestedVersion),
+                isGlobalTool: true);
+
+            package.OriginalVersion.Should().Be(expectedVersion);
+        }
+
         [Theory]
         [InlineData(false)]
         [InlineData(true)]
@@ -689,6 +736,7 @@ namespace Microsoft.DotNet.PackageInstall.Tests
             new ToolPackageUninstaller(store).Uninstall(package.PackageDirectory);
         }
 
+
         [Theory]
         [InlineData(false)]
         [InlineData(true)]

test/Microsoft.DotNet.Tools.Tests.ComponentMocks/ToolPackageDownloaderMock.cs

@@ -111,7 +111,7 @@ namespace Microsoft.DotNet.Tools.Tests.ComponentMocks
                     _toolDownloadDir = isGlobalTool ? _globalToolStageDir : _localToolDownloadDir;
                     var assetFileDirectory = isGlobalTool ? _globalToolStageDir : _localToolAssetDir;
                     rollbackDirectory = _toolDownloadDir.Value;
-                    
+
                     if (string.IsNullOrEmpty(packageId.ToString()))
                     {
                         throw new ToolPackageException(LocalizableStrings.ToolInstallationRestoreFailed);
@@ -180,10 +180,10 @@ namespace Microsoft.DotNet.Tools.Tests.ComponentMocks
                             PackagedShims = Array.Empty<FilePath>()
                         };
                     }
-                    else 
+                    else
                     {
                         var packageRootDirectory = _toolPackageStore.GetRootPackageDirectory(packageId);
-                       
+
                         _fileSystem.Directory.CreateDirectory(packageRootDirectory.Value);
                         _fileSystem.Directory.Move(_toolDownloadDir.Value, packageDirectory.Value);
                         rollbackDirectory = packageDirectory.Value;
@@ -201,7 +201,7 @@ namespace Microsoft.DotNet.Tools.Tests.ComponentMocks
                                         version: version,
                                         packageDirectory: packageDirectory,
                                         warnings: warnings, packagedShims: packedShims, frameworks: frameworks);
-                    }                   
+                    }
                 },
                 rollback: () =>
                 {
@@ -214,7 +214,7 @@ namespace Microsoft.DotNet.Tools.Tests.ComponentMocks
                     {
                         _fileSystem.Directory.Delete(packageRootDirectory.Value, false);
                     }
-                    
+
                 });
         }
 
@@ -300,6 +300,29 @@ namespace Microsoft.DotNet.Tools.Tests.ComponentMocks
                    || (f.Type == MockFeedType.ExplicitNugetConfig && f.Uri == nugetConfig.Value);
         }
 
+        public NuGetVersion GetNuGetVersion(
+            PackageLocation packageLocation,
+            PackageId packageId,
+            VerbosityOptions verbosity,
+            VersionRange versionRange = null,
+            bool isGlobalTool = false)
+        {
+            versionRange = VersionRange.Parse(versionRange?.OriginalString ?? "*");
+
+            if (string.IsNullOrEmpty(packageId.ToString()))
+            {
+                throw new ToolPackageException(LocalizableStrings.ToolInstallationRestoreFailed);
+            }
+
+            var feedPackage = GetPackage(
+                packageId.ToString(),
+                versionRange,
+                packageLocation.NugetConfig,
+                packageLocation.RootConfigDirectory);
+
+            return NuGetVersion.Parse(feedPackage.Version);
+        }
+
         private class TestToolPackage : IToolPackage
         {
             public PackageId Id { get; set; }

test/dotnet.Tests/CommandTests/ToolInstallGlobalOrToolPathCommandTests.cs

@@ -376,7 +376,7 @@ namespace Microsoft.DotNet.Tests.Commands.Tool
         }
 
         [Fact]
-        public void WhenInstallTheSameVersionTwiceItShouldSucceed()
+        public void WhenInstallTheSpecificSameVersionTwiceItShouldNoop()
         {
             ParseResult result = Parser.Instance.Parse($"dotnet tool install -g {PackageId} --version {PackageVersion}");
 
@@ -402,13 +402,7 @@ namespace Microsoft.DotNet.Tests.Commands.Tool
 
             toolInstallGlobalOrToolPathCommand.Execute().Should().Be(0);
 
-            _reporter
-                .Lines
-                .Should()
-                .Equal(string.Format(
-                    Microsoft.DotNet.Tools.Tool.Update.LocalizableStrings.UpdateSucceededStableVersionNoChange,
-                    PackageId,
-                    PackageVersion).Green());
+            _reporter.Lines.Should().Equal(string.Format(LocalizableStrings.ToolAlreadyInstalled, PackageId, PackageVersion).Green());
         }
 
         [Fact]

test/dotnet.Tests/CommandTests/ToolUpdateGlobalOrToolPathCommandTests.cs

@@ -291,7 +291,7 @@ namespace Microsoft.DotNet.Tests.Commands.Tool
             command.Execute();
 
             _reporter.Lines.First().Should().Contain(string.Format(
-                LocalizableStrings.UpdateSucceededStableVersionNoChange,
+                Microsoft.DotNet.Tools.Tool.Install.LocalizableStrings.ToolAlreadyInstalled,
                 _packageId, HigherPackageVersion));
         }
 
@@ -306,7 +306,7 @@ namespace Microsoft.DotNet.Tests.Commands.Tool
             command.Execute();
 
             _reporter.Lines.First().Should().Contain(string.Format(
-                LocalizableStrings.UpdateSucceededPreVersionNoChange,
+                Microsoft.DotNet.Tools.Tool.Install.LocalizableStrings.ToolAlreadyInstalled,
                 _packageId, HigherPreviewPackageVersion));
         }