From 10e743e8e5ef1e6c4993aca26ac928f8697fea5e Mon Sep 17 00:00:00 2001 From: Livar Cunha Date: Thu, 23 Mar 2017 22:20:47 -0700 Subject: [PATCH 1/3] Switched to using a FallbackPackagePathResolver to find the tool dll, because now with Fallback folders, we can have the tool dll in the fallback folder but its assets file in the user nuget cache. This happens because Nuget never writes to the fallback folder, including the tool's assets file that it generates during restore. --- .../dotnet-fallbackfoldertool/Program.cs | 15 ++++++ .../dotnet-fallbackfoldertool.csproj | 17 ++++++ ...AppWithFallbackFolderToolDependency.csproj | 14 +++++ .../NuGet.Config | 6 +++ .../Program.cs | 15 ++++++ build/test/TestPackageProjects.targets | 9 ++++ .../IPackagedCommandSpecFactory.cs | 9 ++++ .../PackagedCommandSpecFactory.cs | 49 +++++++++++++---- .../ProjectToolsCommandResolver.cs | 14 ++--- .../GivenAProjectToolsCommandResolver.cs | 53 +++++++++++++++++++ 10 files changed, 182 insertions(+), 19 deletions(-) create mode 100644 TestAssets/TestPackages/dotnet-fallbackfoldertool/Program.cs create mode 100644 TestAssets/TestPackages/dotnet-fallbackfoldertool/dotnet-fallbackfoldertool.csproj create mode 100755 TestAssets/TestProjects/AppWithFallbackFolderToolDependency/AppWithFallbackFolderToolDependency.csproj create mode 100644 TestAssets/TestProjects/AppWithFallbackFolderToolDependency/NuGet.Config create mode 100644 TestAssets/TestProjects/AppWithFallbackFolderToolDependency/Program.cs diff --git a/TestAssets/TestPackages/dotnet-fallbackfoldertool/Program.cs b/TestAssets/TestPackages/dotnet-fallbackfoldertool/Program.cs new file mode 100644 index 000000000..c936da285 --- /dev/null +++ b/TestAssets/TestPackages/dotnet-fallbackfoldertool/Program.cs @@ -0,0 +1,15 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace ConsoleApplication +{ + public class Program + { + public static void Main(string[] args) + { + Console.WriteLine("Hello Fallback folder World!"); + } + } +} diff --git a/TestAssets/TestPackages/dotnet-fallbackfoldertool/dotnet-fallbackfoldertool.csproj b/TestAssets/TestPackages/dotnet-fallbackfoldertool/dotnet-fallbackfoldertool.csproj new file mode 100644 index 000000000..b7049a0ca --- /dev/null +++ b/TestAssets/TestPackages/dotnet-fallbackfoldertool/dotnet-fallbackfoldertool.csproj @@ -0,0 +1,17 @@ + + + + + netcoreapp2.0 + dotnet-fallbackfoldertool + Exe + + $(CLI_SharedFrameworkVersion) + + + + + $(ProjectRuntimeConfigFilePath) + + + diff --git a/TestAssets/TestProjects/AppWithFallbackFolderToolDependency/AppWithFallbackFolderToolDependency.csproj b/TestAssets/TestProjects/AppWithFallbackFolderToolDependency/AppWithFallbackFolderToolDependency.csproj new file mode 100755 index 000000000..706240e90 --- /dev/null +++ b/TestAssets/TestProjects/AppWithFallbackFolderToolDependency/AppWithFallbackFolderToolDependency.csproj @@ -0,0 +1,14 @@ + + + + + netcoreapp2.0 + Exe + $(CLI_SharedFrameworkVersion) + netcoreapp2.0 + + + + + + diff --git a/TestAssets/TestProjects/AppWithFallbackFolderToolDependency/NuGet.Config b/TestAssets/TestProjects/AppWithFallbackFolderToolDependency/NuGet.Config new file mode 100644 index 000000000..b8e876fcb --- /dev/null +++ b/TestAssets/TestProjects/AppWithFallbackFolderToolDependency/NuGet.Config @@ -0,0 +1,6 @@ + + + + + + diff --git a/TestAssets/TestProjects/AppWithFallbackFolderToolDependency/Program.cs b/TestAssets/TestProjects/AppWithFallbackFolderToolDependency/Program.cs new file mode 100644 index 000000000..2130cd0a7 --- /dev/null +++ b/TestAssets/TestProjects/AppWithFallbackFolderToolDependency/Program.cs @@ -0,0 +1,15 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace ConsoleApplication +{ + public class Program + { + public static void Main(string[] args) + { + Console.WriteLine("Hello World!"); + } + } +} diff --git a/build/test/TestPackageProjects.targets b/build/test/TestPackageProjects.targets index 1d900b84a..88b80d3b4 100644 --- a/build/test/TestPackageProjects.targets +++ b/build/test/TestPackageProjects.targets @@ -127,6 +127,15 @@ True + + dotnet-fallbackfoldertool + dotnet-fallbackfoldertool.csproj + True + True + 1.0.0 + + True + dotnet-prefercliruntime dotnet-prefercliruntime.csproj diff --git a/src/Microsoft.DotNet.Cli.Utils/CommandResolution/IPackagedCommandSpecFactory.cs b/src/Microsoft.DotNet.Cli.Utils/CommandResolution/IPackagedCommandSpecFactory.cs index f3f4b5eda..20f8a3302 100644 --- a/src/Microsoft.DotNet.Cli.Utils/CommandResolution/IPackagedCommandSpecFactory.cs +++ b/src/Microsoft.DotNet.Cli.Utils/CommandResolution/IPackagedCommandSpecFactory.cs @@ -18,5 +18,14 @@ namespace Microsoft.DotNet.Cli.Utils string depsFilePath, string runtimeConfigPath); + CommandSpec CreateCommandSpecFromLibrary( + LockFileTargetLibrary toolLibrary, + string commandName, + IEnumerable commandArguments, + IEnumerable allowedExtensions, + IEnumerable packageFolders, + CommandResolutionStrategy commandResolutionStrategy, + string depsFilePath, + string runtimeConfigPath); } } diff --git a/src/Microsoft.DotNet.Cli.Utils/CommandResolution/PackagedCommandSpecFactory.cs b/src/Microsoft.DotNet.Cli.Utils/CommandResolution/PackagedCommandSpecFactory.cs index ca3ce324a..84a754248 100644 --- a/src/Microsoft.DotNet.Cli.Utils/CommandResolution/PackagedCommandSpecFactory.cs +++ b/src/Microsoft.DotNet.Cli.Utils/CommandResolution/PackagedCommandSpecFactory.cs @@ -31,6 +31,27 @@ namespace Microsoft.DotNet.Cli.Utils CommandResolutionStrategy commandResolutionStrategy, string depsFilePath, string runtimeConfigPath) + { + return CreateCommandSpecFromLibrary( + toolLibrary, + commandName, + commandArguments, + allowedExtensions, + new List { nugetPackagesRoot }, + commandResolutionStrategy, + depsFilePath, + runtimeConfigPath); + } + + public CommandSpec CreateCommandSpecFromLibrary( + LockFileTargetLibrary toolLibrary, + string commandName, + IEnumerable commandArguments, + IEnumerable allowedExtensions, + IEnumerable packageFolders, + CommandResolutionStrategy commandResolutionStrategy, + string depsFilePath, + string runtimeConfigPath) { Reporter.Verbose.WriteLine(string.Format( LocalizableStrings.AttemptingToFindCommand, @@ -51,7 +72,7 @@ namespace Microsoft.DotNet.Cli.Utils return null; } - var commandPath = GetCommandFilePath(nugetPackagesRoot, toolLibrary, toolAssembly); + var commandPath = GetCommandFilePath(packageFolders, toolLibrary, toolAssembly); if (!File.Exists(commandPath)) { @@ -68,17 +89,22 @@ namespace Microsoft.DotNet.Cli.Utils commandArguments, depsFilePath, commandResolutionStrategy, - nugetPackagesRoot, + packageFolders, runtimeConfigPath); } private string GetCommandFilePath( - string nugetPackagesRoot, + IEnumerable packageFolders, LockFileTargetLibrary toolLibrary, LockFileItem runtimeAssembly) { - var packageDirectory = new VersionFolderPathResolver(nugetPackagesRoot) - .GetInstallPath(toolLibrary.Name, toolLibrary.Version); + + var packageFoldersCount = packageFolders.Count(); + var userPackageFolder = packageFoldersCount == 1 ? string.Empty : packageFolders.First(); + var fallbackPackageFolders = packageFoldersCount > 1 ? packageFolders.Skip(1) : packageFolders; + + var packageDirectory = new FallbackPackagePathResolver(userPackageFolder, fallbackPackageFolders) + .GetPackageDirectory(toolLibrary.Name, toolLibrary.Version); var filePath = Path.Combine( packageDirectory, @@ -92,7 +118,7 @@ namespace Microsoft.DotNet.Cli.Utils IEnumerable commandArguments, string depsFilePath, CommandResolutionStrategy commandResolutionStrategy, - string nugetPackagesRoot, + IEnumerable packageFolders, string runtimeConfigPath) { var commandExtension = Path.GetExtension(commandPath); @@ -104,7 +130,7 @@ namespace Microsoft.DotNet.Cli.Utils commandArguments, depsFilePath, commandResolutionStrategy, - nugetPackagesRoot, + packageFolders, runtimeConfigPath); } @@ -116,7 +142,7 @@ namespace Microsoft.DotNet.Cli.Utils IEnumerable commandArguments, string depsFilePath, CommandResolutionStrategy commandResolutionStrategy, - string nugetPackagesRoot, + IEnumerable packageFolders, string runtimeConfigPath) { var host = string.Empty; @@ -144,8 +170,11 @@ namespace Microsoft.DotNet.Cli.Utils arguments.Add(depsFilePath); } - arguments.Add("--additionalprobingpath"); - arguments.Add(nugetPackagesRoot); + foreach (var packageFolder in packageFolders) + { + arguments.Add("--additionalprobingpath"); + arguments.Add(packageFolder); + } if(_addAdditionalArguments != null) { diff --git a/src/Microsoft.DotNet.Cli.Utils/CommandResolution/ProjectToolsCommandResolver.cs b/src/Microsoft.DotNet.Cli.Utils/CommandResolution/ProjectToolsCommandResolver.cs index 995620e59..85f82bcd2 100644 --- a/src/Microsoft.DotNet.Cli.Utils/CommandResolution/ProjectToolsCommandResolver.cs +++ b/src/Microsoft.DotNet.Cli.Utils/CommandResolution/ProjectToolsCommandResolver.cs @@ -136,12 +136,10 @@ namespace Microsoft.DotNet.Cli.Utils var toolPackageFramework = project.DotnetCliToolTargetFramework; - string nugetPackagesRoot; var toolLockFile = GetToolLockFile( toolLibraryRange, toolPackageFramework, - possiblePackageRoots, - out nugetPackagesRoot); + possiblePackageRoots); if (toolLockFile == null) { @@ -174,7 +172,8 @@ namespace Microsoft.DotNet.Cli.Utils toolLockFile, depsFileRoot); - var normalizedNugetPackagesRoot = PathUtility.EnsureNoTrailingDirectorySeparator(nugetPackagesRoot); + var packageFolders = toolLockFile.PackageFolders.Select(p => + PathUtility.EnsureNoTrailingDirectorySeparator(p.Path)); Reporter.Verbose.WriteLine(string.Format( LocalizableStrings.AttemptingToCreateCommandSpec, @@ -185,7 +184,7 @@ namespace Microsoft.DotNet.Cli.Utils commandName, args, _allowedCommandExtensions, - normalizedNugetPackagesRoot, + packageFolders, s_commandResolutionStrategy, depsFilePath, null); @@ -215,19 +214,16 @@ namespace Microsoft.DotNet.Cli.Utils private LockFile GetToolLockFile( SingleProjectInfo toolLibrary, NuGetFramework framework, - IEnumerable possibleNugetPackagesRoot, - out string nugetPackagesRoot) + IEnumerable possibleNugetPackagesRoot) { foreach (var packagesRoot in possibleNugetPackagesRoot) { if (TryGetToolLockFile(toolLibrary, framework, packagesRoot, out LockFile lockFile)) { - nugetPackagesRoot = packagesRoot; return lockFile; } } - nugetPackagesRoot = null; return null; } diff --git a/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs b/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs index 5876cb5dd..b934d1258 100644 --- a/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs +++ b/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs @@ -335,6 +335,59 @@ namespace Microsoft.DotNet.Tests result.Args.Should().NotContain("--fx-version"); } + [Fact] + public void ItFindsToolsLocatedInTheNuGetFallbackFolder() + { + var projectToolsCommandResolver = SetupProjectToolsCommandResolver(); + + var testInstance = TestAssets.Get("AppWithFallbackFolderToolDependency") + .CreateInstance() + .WithSourceFiles(); + var testProjectDirectory = testInstance.Root.FullName; + var fallbackFolder = Path.Combine(testProjectDirectory, "fallbackFolder"); + + PopulateFallbackFolder(testProjectDirectory, fallbackFolder); + + var nugetConfig = testInstance.Root.GetFile("NuGet.Config").FullName; + File.WriteAllText( + nugetConfig, + $@" + + + + + + "); + + new RestoreCommand() + .WithWorkingDirectory(testProjectDirectory) + .Execute($"--configfile {nugetConfig}") + .Should() + .Pass(); + + var commandResolverArguments = new CommandResolverArguments() + { + CommandName = "dotnet-fallbackfoldertool", + CommandArguments = null, + ProjectDirectory = testProjectDirectory + }; + + var result = projectToolsCommandResolver.Resolve(commandResolverArguments); + + result.Should().NotBeNull(); + } + + private void PopulateFallbackFolder(string testProjectDirectory, string fallbackFolder) + { + new RestoreCommand() + .WithWorkingDirectory(testProjectDirectory) + .Execute($"--packages {fallbackFolder}") + .Should() + .Pass(); + + Directory.Delete(Path.Combine(fallbackFolder, ".tools"), true); + } + private ProjectToolsCommandResolver SetupProjectToolsCommandResolver() { Environment.SetEnvironmentVariable( From a4300cc069f66745dc33892e4a7630cd4095b760 Mon Sep 17 00:00:00 2001 From: Livar Cunha Date: Thu, 23 Mar 2017 22:26:25 -0700 Subject: [PATCH 2/3] Improving the test slightly by checking that it is finding the correct path to the tool dll. --- .../GivenAProjectToolsCommandResolver.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs b/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs index b934d1258..41c9bef98 100644 --- a/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs +++ b/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs @@ -375,6 +375,15 @@ namespace Microsoft.DotNet.Tests var result = projectToolsCommandResolver.Resolve(commandResolverArguments); result.Should().NotBeNull(); + + var commandPath = result.Args.Trim('"'); + commandPath.Should().Contain(Path.Combine( + fallbackFolder, + "dotnet-fallbackfoldertool", + "1.0.0", + "lib", + "netcoreapp2.0", + "dotnet-fallbackfoldertool.dll")); } private void PopulateFallbackFolder(string testProjectDirectory, string fallbackFolder) From b2116cd2e1b1b91adc9e1e019f661ed8a3f93de5 Mon Sep 17 00:00:00 2001 From: Livar Cunha Date: Sun, 26 Mar 2017 21:52:27 -0700 Subject: [PATCH 3/3] Addressing comments by adding a test covering the case where the tool assemblies are not found. --- .../PackagedCommandSpecFactory.cs | 8 ++- .../LocalizableStrings.cs | 2 + .../GivenAProjectToolsCommandResolver.cs | 63 ++++++++++++++++--- 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.DotNet.Cli.Utils/CommandResolution/PackagedCommandSpecFactory.cs b/src/Microsoft.DotNet.Cli.Utils/CommandResolution/PackagedCommandSpecFactory.cs index 84a754248..e7bd1b33c 100644 --- a/src/Microsoft.DotNet.Cli.Utils/CommandResolution/PackagedCommandSpecFactory.cs +++ b/src/Microsoft.DotNet.Cli.Utils/CommandResolution/PackagedCommandSpecFactory.cs @@ -98,7 +98,6 @@ namespace Microsoft.DotNet.Cli.Utils LockFileTargetLibrary toolLibrary, LockFileItem runtimeAssembly) { - var packageFoldersCount = packageFolders.Count(); var userPackageFolder = packageFoldersCount == 1 ? string.Empty : packageFolders.First(); var fallbackPackageFolders = packageFoldersCount > 1 ? packageFolders.Skip(1) : packageFolders; @@ -106,6 +105,13 @@ namespace Microsoft.DotNet.Cli.Utils var packageDirectory = new FallbackPackagePathResolver(userPackageFolder, fallbackPackageFolders) .GetPackageDirectory(toolLibrary.Name, toolLibrary.Version); + if (packageDirectory == null) + { + throw new GracefulException(string.Format( + LocalizableStrings.CommandAssembliesNotFound, + toolLibrary.Name)); + } + var filePath = Path.Combine( packageDirectory, PathUtility.GetPathWithDirectorySeparator(runtimeAssembly.Path)); diff --git a/src/Microsoft.DotNet.Cli.Utils/LocalizableStrings.cs b/src/Microsoft.DotNet.Cli.Utils/LocalizableStrings.cs index 6bbcac0ea..8a3e20014 100644 --- a/src/Microsoft.DotNet.Cli.Utils/LocalizableStrings.cs +++ b/src/Microsoft.DotNet.Cli.Utils/LocalizableStrings.cs @@ -71,6 +71,8 @@ namespace Microsoft.DotNet.Cli.Utils public const string NoExecutableFoundMatchingCommand = "No executable found matching command \"{0}\""; + public const string CommandAssembliesNotFound = "The command executable for \"{0}\" was not found. The project may not have been restored or restore failed - run `dotnet restore`"; + public const string WaitingForDebuggerToAttach = "Waiting for debugger to attach. Press ENTER to continue"; public const string ProcessId = "Process ID: {0}"; diff --git a/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs b/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs index 41c9bef98..9d9e00539 100644 --- a/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs +++ b/test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs @@ -348,16 +348,7 @@ namespace Microsoft.DotNet.Tests PopulateFallbackFolder(testProjectDirectory, fallbackFolder); - var nugetConfig = testInstance.Root.GetFile("NuGet.Config").FullName; - File.WriteAllText( - nugetConfig, - $@" - - - - - - "); + var nugetConfig = UseNuGetConfigWithFallbackFolder(testInstance, fallbackFolder); new RestoreCommand() .WithWorkingDirectory(testProjectDirectory) @@ -386,6 +377,42 @@ namespace Microsoft.DotNet.Tests "dotnet-fallbackfoldertool.dll")); } + [Fact] + public void ItXXXWhenTheToolDllIsNotFound() + { + var projectToolsCommandResolver = SetupProjectToolsCommandResolver(); + + var testInstance = TestAssets.Get("AppWithFallbackFolderToolDependency") + .CreateInstance() + .WithSourceFiles(); + var testProjectDirectory = testInstance.Root.FullName; + var fallbackFolder = Path.Combine(testProjectDirectory, "fallbackFolder"); + + PopulateFallbackFolder(testProjectDirectory, fallbackFolder); + + var nugetConfig = UseNuGetConfigWithFallbackFolder(testInstance, fallbackFolder); + + new RestoreCommand() + .WithWorkingDirectory(testProjectDirectory) + .Execute($"--configfile {nugetConfig}") + .Should() + .Pass(); + + Directory.Delete(Path.Combine(fallbackFolder, "dotnet-fallbackfoldertool"), true); + + var commandResolverArguments = new CommandResolverArguments() + { + CommandName = "dotnet-fallbackfoldertool", + CommandArguments = null, + ProjectDirectory = testProjectDirectory + }; + + Action action = () => projectToolsCommandResolver.Resolve(commandResolverArguments); + + action.ShouldThrow().WithMessage( + "The command executable for \"dotnet-fallbackfoldertool\" was not found. The project may not have been restored or restore failed - run `dotnet restore`"); + } + private void PopulateFallbackFolder(string testProjectDirectory, string fallbackFolder) { new RestoreCommand() @@ -397,6 +424,22 @@ namespace Microsoft.DotNet.Tests Directory.Delete(Path.Combine(fallbackFolder, ".tools"), true); } + private string UseNuGetConfigWithFallbackFolder(TestAssetInstance testInstance, string fallbackFolder) + { + var nugetConfig = testInstance.Root.GetFile("NuGet.Config").FullName; + File.WriteAllText( + nugetConfig, + $@" + + + + + + "); + + return nugetConfig; + } + private ProjectToolsCommandResolver SetupProjectToolsCommandResolver() { Environment.SetEnvironmentVariable(