From 2cc1c059be27e6d1c9e0eed540738afe2ff2a8d4 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Tue, 23 Jul 2024 15:32:42 -0700 Subject: [PATCH] Do not go beyond the root when searching for a previously installed tool Fixes #40655 (#40800) Fixes #40655 dotnet tool install currently looks for a previously installed tools manifest. It reads manifests it finds in looking for one that has the manifest it's trying to install but does not check whether that manifest 'isRoot', which means it keeps going outside the root. We don't use a tool beyond the root, which means we can't install then use a tool as would be expected. --- src/Cli/Microsoft.DotNet.Cli.Utils/Command.cs | 2 +- .../dotnet/ToolManifest/ToolManifestFinder.cs | 18 ++++++++- .../dotnet/ToolPackage/ToolPackageFactory.cs | 4 +- .../install/ToolInstallLocalCommand.cs | 5 ++- .../install/ToolInstallLocalInstaller.cs | 5 ++- .../dotnet-tool/run/ToolRunCommand.cs | 2 +- .../ToolsetUtils.cs | 8 +--- .../TestContext.cs | 11 ++++++ .../CommandTests/ToolInstallCommandTests.cs | 37 ++++++++++++++++++- 9 files changed, 74 insertions(+), 18 deletions(-) diff --git a/src/Cli/Microsoft.DotNet.Cli.Utils/Command.cs b/src/Cli/Microsoft.DotNet.Cli.Utils/Command.cs index b69ef82b6ea9..2d22a0edd774 100644 --- a/src/Cli/Microsoft.DotNet.Cli.Utils/Command.cs +++ b/src/Cli/Microsoft.DotNet.Cli.Utils/Command.cs @@ -26,7 +26,7 @@ public Command(Process process, bool trimtrailingNewlines = false) public CommandResult Execute() { - return Execute(_ => { }); + return Execute(null); } public CommandResult Execute(Action processStarted) { diff --git a/src/Cli/dotnet/ToolManifest/ToolManifestFinder.cs b/src/Cli/dotnet/ToolManifest/ToolManifestFinder.cs index 5259c4bdf6fc..1d3ef122a66b 100644 --- a/src/Cli/dotnet/ToolManifest/ToolManifestFinder.cs +++ b/src/Cli/dotnet/ToolManifest/ToolManifestFinder.cs @@ -249,18 +249,32 @@ public IReadOnlyList FindByPackageId(PackageId packageId) { var result = new List(); bool findAnyManifest = false; + DirectoryPath? rootPath = null; foreach ((FilePath possibleManifest, DirectoryPath correspondingDirectory) in EnumerateDefaultAllPossibleManifests()) { + if (rootPath is not null) + { + if (!correspondingDirectory.Value.Equals(rootPath.Value)) + { + break; + } + } + if (_fileSystem.File.Exists(possibleManifest.Value)) { findAnyManifest = true; - if (_toolManifestEditor.Read(possibleManifest, correspondingDirectory).content - .Any(t => t.PackageId.Equals(packageId))) + (List content, bool isRoot) = _toolManifestEditor.Read(possibleManifest, correspondingDirectory); + if (content.Any(t => t.PackageId.Equals(packageId))) { result.Add(possibleManifest); } + + if (isRoot) + { + rootPath = correspondingDirectory; + } } } diff --git a/src/Cli/dotnet/ToolPackage/ToolPackageFactory.cs b/src/Cli/dotnet/ToolPackage/ToolPackageFactory.cs index 2daf1d966e9e..ad8d8be753c9 100644 --- a/src/Cli/dotnet/ToolPackage/ToolPackageFactory.cs +++ b/src/Cli/dotnet/ToolPackage/ToolPackageFactory.cs @@ -13,10 +13,10 @@ namespace Microsoft.DotNet.ToolPackage internal static class ToolPackageFactory { public static (IToolPackageStore, IToolPackageStoreQuery, IToolPackageDownloader) CreateToolPackageStoresAndDownloader( - DirectoryPath? nonGlobalLocation = null, IEnumerable additionalRestoreArguments = null) + DirectoryPath? nonGlobalLocation = null, IEnumerable additionalRestoreArguments = null, string runtimeJsonPathForTests = null) { ToolPackageStoreAndQuery toolPackageStore = CreateConcreteToolPackageStore(nonGlobalLocation); - var toolPackageDownloader = new ToolPackageDownloader(toolPackageStore); + var toolPackageDownloader = new ToolPackageDownloader(toolPackageStore, runtimeJsonPathForTests); return (toolPackageStore, toolPackageStore, toolPackageDownloader); } diff --git a/src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallLocalCommand.cs b/src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallLocalCommand.cs index c8ba4735c3df..c0ce8078427c 100644 --- a/src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallLocalCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallLocalCommand.cs @@ -37,7 +37,8 @@ public ToolInstallLocalCommand( IToolManifestFinder toolManifestFinder = null, IToolManifestEditor toolManifestEditor = null, ILocalToolsResolverCache localToolsResolverCache = null, - IReporter reporter = null + IReporter reporter = null, + string runtimeJsonPathForTests = null ) : base(parseResult) { @@ -54,7 +55,7 @@ public ToolInstallLocalCommand( new ToolManifestFinder(new DirectoryPath(Directory.GetCurrentDirectory())); _toolManifestEditor = toolManifestEditor ?? new ToolManifestEditor(); _localToolsResolverCache = localToolsResolverCache ?? new LocalToolsResolverCache(); - _toolLocalPackageInstaller = new ToolInstallLocalInstaller(parseResult, toolPackageDownloader); + _toolLocalPackageInstaller = new ToolInstallLocalInstaller(parseResult, toolPackageDownloader, runtimeJsonPathForTests); _toolPackageDownloader = toolPackageDownloader; _allowRollForward = parseResult.GetValue(ToolInstallCommandParser.RollForwardOption); _allowPackageDowngrade = parseResult.GetValue(ToolInstallCommandParser.AllowPackageDowngradeOption); diff --git a/src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallLocalInstaller.cs b/src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallLocalInstaller.cs index 48d65d78801e..4a52d585dac7 100644 --- a/src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallLocalInstaller.cs +++ b/src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallLocalInstaller.cs @@ -26,7 +26,8 @@ internal class ToolInstallLocalInstaller public ToolInstallLocalInstaller( ParseResult parseResult, - IToolPackageDownloader toolPackageDownloader = null) + IToolPackageDownloader toolPackageDownloader = null, + string runtimeJsonPathForTests = null) { _parseResult = parseResult; _packageVersion = parseResult.GetValue(ToolInstallCommandParser.VersionOption); @@ -38,7 +39,7 @@ public ToolInstallLocalInstaller( IToolPackageStoreQuery, IToolPackageDownloader downloader) toolPackageStoresAndDownloader = ToolPackageFactory.CreateToolPackageStoresAndDownloader( - additionalRestoreArguments: parseResult.OptionValuesToBeForwarded(ToolInstallCommandParser.GetCommand())); + additionalRestoreArguments: parseResult.OptionValuesToBeForwarded(ToolInstallCommandParser.GetCommand()), runtimeJsonPathForTests: runtimeJsonPathForTests); _toolPackageStore = toolPackageStoresAndDownloader.store; _toolPackageDownloader = toolPackageDownloader ?? toolPackageStoresAndDownloader.downloader; diff --git a/src/Cli/dotnet/commands/dotnet-tool/run/ToolRunCommand.cs b/src/Cli/dotnet/commands/dotnet-tool/run/ToolRunCommand.cs index 389311fa02e0..4d315de15106 100644 --- a/src/Cli/dotnet/commands/dotnet-tool/run/ToolRunCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-tool/run/ToolRunCommand.cs @@ -39,7 +39,7 @@ public override int Execute() CommandName = $"dotnet-{_toolCommandName}", CommandArguments = _forwardArgument, - }, _allowRollForward); ; + }, _allowRollForward); if (commandspec == null) { diff --git a/test/Microsoft.NET.Build.Containers.IntegrationTests/ToolsetUtils.cs b/test/Microsoft.NET.Build.Containers.IntegrationTests/ToolsetUtils.cs index 599d95280157..41c9d703d415 100644 --- a/test/Microsoft.NET.Build.Containers.IntegrationTests/ToolsetUtils.cs +++ b/test/Microsoft.NET.Build.Containers.IntegrationTests/ToolsetUtils.cs @@ -11,13 +11,7 @@ internal static class ToolsetUtils /// internal static string GetRuntimeGraphFilePath() { - string dotnetRoot = TestContext.Current.ToolsetUnderTest.DotNetRoot; - - DirectoryInfo sdksDir = new(Path.Combine(dotnetRoot, "sdk")); - - var lastWrittenSdk = sdksDir.EnumerateDirectories().OrderByDescending(di => di.LastWriteTime).First(); - - return lastWrittenSdk.GetFiles("RuntimeIdentifierGraph.json").Single().FullName; + return TestContext.GetRuntimeGraphFilePath(); } internal static IManifestPicker RidGraphManifestPicker { get; } = new RidGraphManifestPicker(GetRuntimeGraphFilePath()); diff --git a/test/Microsoft.NET.TestFramework/TestContext.cs b/test/Microsoft.NET.TestFramework/TestContext.cs index 2bbf15713bfc..690b7767d1a8 100644 --- a/test/Microsoft.NET.TestFramework/TestContext.cs +++ b/test/Microsoft.NET.TestFramework/TestContext.cs @@ -51,6 +51,17 @@ public static TestContext Current public const string LatestRuntimePatchForNetCoreApp2_0 = "2.0.9"; + public static string GetRuntimeGraphFilePath() + { + string dotnetRoot = TestContext.Current.ToolsetUnderTest.DotNetRoot; + + DirectoryInfo sdksDir = new(Path.Combine(dotnetRoot, "sdk")); + + var lastWrittenSdk = sdksDir.EnumerateDirectories().OrderByDescending(di => di.LastWriteTime).First(); + + return lastWrittenSdk.GetFiles("RuntimeIdentifierGraph.json").Single().FullName; + } + public void AddTestEnvironmentVariables(IDictionary environment) { environment["DOTNET_MULTILEVEL_LOOKUP"] = "0"; diff --git a/test/dotnet.Tests/CommandTests/ToolInstallCommandTests.cs b/test/dotnet.Tests/CommandTests/ToolInstallCommandTests.cs index aa48eadc6403..4cc3c425575e 100644 --- a/test/dotnet.Tests/CommandTests/ToolInstallCommandTests.cs +++ b/test/dotnet.Tests/CommandTests/ToolInstallCommandTests.cs @@ -1,17 +1,25 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.IO; +using System.Linq; +using System.Reflection; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.Tools.Tool.Install; +using Microsoft.DotNet.Tools.Tool.Run; using LocalizableStrings = Microsoft.DotNet.Tools.Tool.Install.LocalizableStrings; using Parser = Microsoft.DotNet.Cli.Parser; namespace Microsoft.DotNet.Tests.Commands.Tool { - public class ToolInstallCommandTests + public class ToolInstallCommandTests : SdkTest { private const string PackageId = "global.tool.console.demo"; + public ToolInstallCommandTests(ITestOutputHelper log) : base(log) + { + } + [Fact] public void WhenRunWithBothGlobalAndToolPathShowErrorMessage() { @@ -28,6 +36,33 @@ public void WhenRunWithBothGlobalAndToolPathShowErrorMessage() "--global --tool-path")); } + [Fact] + public void WhenRunWithRoot() + { + Directory.CreateDirectory("/tmp/folder/sub"); + var directory = Directory.GetCurrentDirectory(); + var ridGraphPath = TestContext.GetRuntimeGraphFilePath(); + try + { + Directory.SetCurrentDirectory("/tmp/folder"); + + new DotnetNewCommand(Log, "tool-manifest").WithCustomHive("/tmp/folder").WithWorkingDirectory("/tmp/folder").Execute().Should().Pass(); + var parseResult = Parser.Instance.Parse("tool install dotnetsay"); + new ToolInstallLocalCommand(parseResult, runtimeJsonPathForTests: ridGraphPath).Execute().Should().Be(0); + + Directory.SetCurrentDirectory("/tmp/folder/sub"); + new DotnetNewCommand(Log, "tool-manifest").WithCustomHive("/tmp/folder/sub").WithWorkingDirectory("/tmp/folder/sub").Execute().Should().Pass(); + parseResult = Parser.Instance.Parse("tool install dotnetsay"); + new ToolInstallLocalCommand(parseResult, runtimeJsonPathForTests: ridGraphPath).Execute().Should().Be(0); + + new ToolRunCommand(Parser.Instance.Parse($"tool run dotnetsay")).Execute().Should().Be(0); + } + finally + { + Directory.SetCurrentDirectory(directory); + } + } + [Fact] public void WhenRunWithBothGlobalAndLocalShowErrorMessage() {