From e2d9c2f892e3327374635f75fca6fbe8bea0092a Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Tue, 6 Mar 2018 16:15:27 -0800 Subject: [PATCH] Remove temp project path from tool install warnings and errors. This commit attempts to filter the diagnostic messages emitted during tool installation. The diagnostic messages may be prefixed with the temporary project; since this is an implementation detail that only causes confusion and clutter in the diagnostic messages, the prefix is removed if present. Fixes #8707. --- src/dotnet/ToolPackage/ToolPackageFactory.cs | 2 +- .../ToolPackage/ToolPackageInstaller.cs | 2 +- .../dotnet-install-tool/ProjectRestorer.cs | 35 ++++++++++++++++--- .../ToolPackageInstallerTests.cs | 26 ++++++++++++++ 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/src/dotnet/ToolPackage/ToolPackageFactory.cs b/src/dotnet/ToolPackage/ToolPackageFactory.cs index c619df4a3..f3349db77 100644 --- a/src/dotnet/ToolPackage/ToolPackageFactory.cs +++ b/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); } diff --git a/src/dotnet/ToolPackage/ToolPackageInstaller.cs b/src/dotnet/ToolPackage/ToolPackageInstaller.cs index cf655a301..44a51094e 100644 --- a/src/dotnet/ToolPackage/ToolPackageInstaller.cs +++ b/src/dotnet/ToolPackage/ToolPackageInstaller.cs @@ -119,7 +119,7 @@ namespace Microsoft.DotNet.ToolPackage { var tempProject = _tempProject ?? new DirectoryPath(Path.GetTempPath()) .WithSubDirectories(Path.GetRandomFileName()) - .WithFile(Path.GetRandomFileName() + ".csproj"); + .WithFile("restore.csproj"); if (Path.GetExtension(tempProject.Value) != "csproj") { diff --git a/src/dotnet/commands/dotnet-install/dotnet-install-tool/ProjectRestorer.cs b/src/dotnet/commands/dotnet-install/dotnet-install-tool/ProjectRestorer.cs index 06cc81c38..5ba8a177b 100644 --- a/src/dotnet/commands/dotnet-install/dotnet-install-tool/ProjectRestorer.cs +++ b/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)) - .OnErrorLine((line) => _reporter.WriteLine(line)); + .OnOutputLine(line => WriteLine(_reporter, line, project)) + .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); + } } } diff --git a/test/Microsoft.DotNet.ToolPackage.Tests/ToolPackageInstallerTests.cs b/test/Microsoft.DotNet.ToolPackage.Tests/ToolPackageInstallerTests.cs index 31ecb91dd..f08ca6077 100644 --- a/test/Microsoft.DotNet.ToolPackage.Tests/ToolPackageInstallerTests.cs +++ b/test/Microsoft.DotNet.ToolPackage.Tests/ToolPackageInstallerTests.cs @@ -498,6 +498,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,