Merge pull request #8734 from peterhuene/tool-install-message-cleanup

Remove temp project path from tool install warnings and errors.

src/dotnet/ToolPackage/IToolPackageInstaller.cs

@@ -15,6 +15,7 @@ namespace Microsoft.DotNet.ToolPackage
             VersionRange versionRange = null,
             string targetFramework = null,
             FilePath? nugetConfig = null,
+            DirectoryPath? rootConfigDirectory = null,
             string source = null,
             string verbosity = null);
     }

src/dotnet/ToolPackage/ToolPackageFactory.cs

@@ -16,7 +16,7 @@ namespace Microsoft.DotNet.ToolPackage
             IToolPackageStore toolPackageStore = CreateToolPackageStore(nonGlobalLocation);
             var toolPackageInstaller = new ToolPackageInstaller(
                 toolPackageStore,
-                new ProjectRestorer(Reporter.Output));
+                new ProjectRestorer());
 
             return (toolPackageStore, toolPackageInstaller);
         }

src/dotnet/ToolPackage/ToolPackageInstaller.cs

@@ -35,6 +35,7 @@ namespace Microsoft.DotNet.ToolPackage
             VersionRange versionRange = null,
             string targetFramework = null,
             FilePath? nugetConfig = null,
+            DirectoryPath? rootConfigDirectory = null,
             string source = null,
             string verbosity = null)
         {
@@ -53,7 +54,8 @@ namespace Microsoft.DotNet.ToolPackage
                             packageId: packageId,
                             versionRange: versionRange,
                             targetFramework: targetFramework ?? BundledTargetFramework.GetTargetFrameworkMoniker(),
-                            restoreDirectory: stageDirectory);
+                            restoreDirectory: stageDirectory,
+                            rootConfigDirectory: rootConfigDirectory);
 
                         try
                         {
@@ -115,11 +117,12 @@ namespace Microsoft.DotNet.ToolPackage
             PackageId packageId,
             VersionRange versionRange,
             string targetFramework,
-            DirectoryPath restoreDirectory)
+            DirectoryPath restoreDirectory,
+            DirectoryPath? rootConfigDirectory)
         {
             var tempProject = _tempProject ?? new DirectoryPath(Path.GetTempPath())
                 .WithSubDirectories(Path.GetRandomFileName())
-                .WithFile(Path.GetRandomFileName() + ".csproj");
+                .WithFile("restore.csproj");
 
             if (Path.GetExtension(tempProject.Value) != "csproj")
             {
@@ -135,7 +138,7 @@ namespace Microsoft.DotNet.ToolPackage
                         new XElement("TargetFramework", targetFramework),
                         new XElement("RestorePackagesPath", restoreDirectory.Value),
                         new XElement("RestoreProjectStyle", "DotnetToolReference"), // without it, project cannot reference tool package
-                        new XElement("RestoreRootConfigDirectory", Directory.GetCurrentDirectory()), // config file probing start directory
+                        new XElement("RestoreRootConfigDirectory", rootConfigDirectory?.Value ?? Directory.GetCurrentDirectory()), // config file probing start directory
                         new XElement("DisableImplicitFrameworkReferences", "true"), // no Microsoft.NETCore.App in tool folder
                         new XElement("RestoreFallbackFolders", "clear"), // do not use fallbackfolder, tool package need to be copied to tool folder
                         new XElement("RestoreAdditionalProjectSources", // use fallbackfolder as feed to enable offline

src/dotnet/commands/dotnet-install/dotnet-install-tool/ProjectRestorer.cs

@@ -16,10 +16,14 @@ namespace Microsoft.DotNet.Tools.Install.Tool
     {
         private const string AnyRid = "any";
         private readonly IReporter _reporter;
+        private readonly IReporter _errorReporter;
+        private readonly bool _forceOutputRedirection;
 
         public ProjectRestorer(IReporter reporter = null)
         {
-            _reporter = reporter;
+            _reporter = reporter ?? Reporter.Output;
+            _errorReporter = reporter ?? Reporter.Error;
+            _forceOutputRedirection = reporter != null;
         }
 
         public void Restore(
@@ -56,11 +60,11 @@ namespace Microsoft.DotNet.Tools.Install.Tool
             var command = new DotNetCommandFactory(alwaysRunOutOfProc: true)
                 .Create("restore", argsToPassToRestore);
 
-            if (_reporter != null)
+            if (verbosity == null || _forceOutputRedirection)
             {
                 command = command
-                    .OnOutputLine((line) => _reporter.WriteLine(line))
+                    .OnOutputLine(line => WriteLine(_reporter, line, project))
-                    .OnErrorLine((line) => _reporter.WriteLine(line));
+                    .OnErrorLine(line => WriteLine(_errorReporter, line, project));
             }
 
             var result = command.Execute();
@@ -69,5 +73,28 @@ namespace Microsoft.DotNet.Tools.Install.Tool
                 throw new ToolPackageException(LocalizableStrings.ToolInstallationRestoreFailed);
             }
         }
+
+        private static void WriteLine(IReporter reporter, string line, FilePath project)
+        {
+            line = line ?? "";
+
+            // Remove the temp project prefix if present
+            if (line.StartsWith($"{project.Value} : ", StringComparison.OrdinalIgnoreCase))
+            {
+                line = line.Substring(project.Value.Length + 3);
+            }
+
+            // Note: MSBuild intentionally does not localize "warning" and "error" for diagnostic messages
+            if (line.StartsWith("warning ", StringComparison.Ordinal))
+            {
+                line = line.Yellow();
+            }
+            else if (line.StartsWith("error ", StringComparison.Ordinal))
+            {
+                line = line.Red();
+            }
+
+            reporter.WriteLine(line);
+        }
     }
 }

test/Microsoft.DotNet.ToolPackage.Tests/ToolPackageInstallerTests.cs

@@ -154,21 +154,17 @@ namespace Microsoft.DotNet.ToolPackage.Tests
         }
 
         [Fact]
-        public void GivenAConfigFileInCurrentDirectoryPackageInstallSucceeds()
+        public void GivenAConfigFileRootDirectoryPackageInstallSucceeds()
         {
             var nugetConfigPath = WriteNugetConfigFileToPointToTheFeed();
 
             var (store, installer, reporter, fileSystem) = Setup(useMock: false);
 
-            /*
-             * In test, we don't want NuGet to keep look up, so we point current directory to nugetconfig.
-             */
-            Directory.SetCurrentDirectory(nugetConfigPath.GetDirectoryPath().Value);
-
             var package = installer.InstallPackage(
                 packageId: TestPackageId,
                 versionRange: VersionRange.Parse(TestPackageVersion),
-                targetFramework: _testTargetframework);
+                targetFramework: _testTargetframework,
+                rootConfigDirectory: nugetConfigPath.GetDirectoryPath());
 
             AssertPackageInstall(reporter, fileSystem, package, store);
 
@@ -498,6 +494,32 @@ namespace Microsoft.DotNet.ToolPackage.Tests
             package.Uninstall();
         }
 
+        [Fact]
+        public void GivenANuGetDiagnosticMessageItShouldNotContainTheTempProject()
+        {
+            var nugetConfigPath = WriteNugetConfigFileToPointToTheFeed();
+            var tempProject = GetUniqueTempProjectPathEachTest();
+
+            var (store, installer, reporter, fileSystem) = Setup(
+                useMock: false,
+                tempProject: tempProject);
+
+            var package = installer.InstallPackage(
+                packageId: TestPackageId,
+                versionRange: VersionRange.Parse("1.0.*"),
+                targetFramework: _testTargetframework,
+                nugetConfig: nugetConfigPath);
+
+            reporter.Lines.Should().NotBeEmpty();
+            reporter.Lines.Should().Contain(l => l.Contains("warning"));
+            reporter.Lines.Should().NotContain(l => l.Contains(tempProject.Value));
+            reporter.Lines.Clear();
+
+            AssertPackageInstall(reporter, fileSystem, package, store);
+
+            package.Uninstall();
+        }
+
         private static void AssertPackageInstall(
             BufferedReporter reporter,
             IFileSystem fileSystem,
@@ -615,7 +637,7 @@ namespace Microsoft.DotNet.ToolPackage.Tests
             FilePath? tempProject = null,
             DirectoryPath? offlineFeed = null)
         {
-            var root = new DirectoryPath(Path.Combine(Directory.GetCurrentDirectory(), Path.GetRandomFileName()));
+            var root = new DirectoryPath(Path.Combine(Path.GetFullPath(TempRoot.Root), Path.GetRandomFileName()));
             var reporter = new BufferedReporter();
 
             IFileSystem fileSystem;

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

@@ -40,6 +40,7 @@ namespace Microsoft.DotNet.Tools.Tests.ComponentMocks
             VersionRange versionRange = null,
             string targetFramework = null,
             FilePath? nugetConfig = null,
+            DirectoryPath? rootConfigDirectory = null,
             string source = null,
             string verbosity = null)
         {