From 7a3bc96f75ed9b2a360cb332bd4b1321af124f11 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Tue, 15 Nov 2016 11:56:39 -0800 Subject: [PATCH] Fix 4508: CLI verbs that call into msbuild should control their output (#4719) * Fix 4508: CLI verbs that call into msbuild should control their output * fix failing tests + tiny bufix in dotnet test --- .../MSBuildBareBonesProject/BareBones.proj | 6 +- .../MSBuildIntegration/build.proj | 5 ++ src/dotnet/commands/dotnet-build/Program.cs | 6 ++ src/dotnet/commands/dotnet-clean/Program.cs | 6 ++ .../dotnet-msbuild/MSBuildForwardingApp.cs | 8 ++- .../commands/dotnet-pack/PackCommand.cs | 6 ++ src/dotnet/commands/dotnet-publish/Program.cs | 3 + .../commands/dotnet-publish/PublishCommand.cs | 6 ++ src/dotnet/commands/dotnet-restore/Program.cs | 7 ++ src/dotnet/commands/dotnet-test/Program.cs | 12 +++- .../GivenDotnetInvokesMSBuild.cs | 68 +++++++++++++++++++ 11 files changed, 128 insertions(+), 5 deletions(-) diff --git a/TestAssets/TestProjects/MSBuildBareBonesProject/BareBones.proj b/TestAssets/TestProjects/MSBuildBareBonesProject/BareBones.proj index 89cee1275..e0b870644 100644 --- a/TestAssets/TestProjects/MSBuildBareBonesProject/BareBones.proj +++ b/TestAssets/TestProjects/MSBuildBareBonesProject/BareBones.proj @@ -2,15 +2,15 @@ - + - + - + \ No newline at end of file diff --git a/TestAssets/TestProjects/MSBuildIntegration/build.proj b/TestAssets/TestProjects/MSBuildIntegration/build.proj index bbb04b518..f8323acf9 100644 --- a/TestAssets/TestProjects/MSBuildIntegration/build.proj +++ b/TestAssets/TestProjects/MSBuildIntegration/build.proj @@ -13,6 +13,7 @@ + @@ -28,5 +29,9 @@ + + + + \ No newline at end of file diff --git a/src/dotnet/commands/dotnet-build/Program.cs b/src/dotnet/commands/dotnet-build/Program.cs index 3d0a3f060..f087544ba 100644 --- a/src/dotnet/commands/dotnet-build/Program.cs +++ b/src/dotnet/commands/dotnet-build/Program.cs @@ -36,6 +36,7 @@ namespace Microsoft.DotNet.Tools.Build CommandOption noIncrementalOption = app.Option("--no-incremental", "Set this flag to turn off incremental build", CommandOptionType.NoValue); CommandOption noDependenciesOption = app.Option("--no-dependencies", "Set this flag to ignore project to project references and only build the root project", CommandOptionType.NoValue); + CommandOption verbosityOption = MSBuildForwardingApp.AddVerbosityOption(app); app.OnExecute(() => { @@ -85,6 +86,11 @@ namespace Microsoft.DotNet.Tools.Build msbuildArgs.Add("/p:BuildProjectReferences=false"); } + if (verbosityOption.HasValue()) + { + msbuildArgs.Add($"/verbosity:{verbosityOption.Value()}"); + } + msbuildArgs.AddRange(app.RemainingArguments); return new MSBuildForwardingApp(msbuildArgs).Execute(); diff --git a/src/dotnet/commands/dotnet-clean/Program.cs b/src/dotnet/commands/dotnet-clean/Program.cs index 2c8995166..5d15fc704 100644 --- a/src/dotnet/commands/dotnet-clean/Program.cs +++ b/src/dotnet/commands/dotnet-clean/Program.cs @@ -31,6 +31,7 @@ namespace Microsoft.DotNet.Tools.Clean CommandOption outputOption = app.Option("-o|--output ", "Directory in which the build outputs have been placed", CommandOptionType.SingleValue); CommandOption frameworkOption = app.Option("-f|--framework ", "Clean a specific framework", CommandOptionType.SingleValue); CommandOption configurationOption = app.Option("-c|--configuration ", "Clean a specific configuration", CommandOptionType.SingleValue); + CommandOption verbosityOption = MSBuildForwardingApp.AddVerbosityOption(app); app.OnExecute(() => { @@ -58,6 +59,11 @@ namespace Microsoft.DotNet.Tools.Clean msbuildArgs.Add($"/p:Configuration={configurationOption.Value()}"); } + if (verbosityOption.HasValue()) + { + msbuildArgs.Add($"/verbosity:{verbosityOption.Value()}"); + } + msbuildArgs.AddRange(app.RemainingArguments); return new MSBuildForwardingApp(msbuildArgs).Execute(); diff --git a/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs b/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs index a9b69dd39..5b2b16ff0 100644 --- a/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs +++ b/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Reflection; using System.Runtime.InteropServices; using Microsoft.DotNet.Cli; +using Microsoft.DotNet.Cli.CommandLine; namespace Microsoft.DotNet.Tools.MSBuild { @@ -27,7 +28,7 @@ namespace Microsoft.DotNet.Tools.MSBuild }; private readonly IEnumerable _msbuildRequiredParameters = - new List { "/m" }; + new List { "/m", "/v:m" }; public MSBuildForwardingApp(IEnumerable argsToForward) { @@ -68,6 +69,11 @@ namespace Microsoft.DotNet.Tools.MSBuild } } + internal static CommandOption AddVerbosityOption(CommandLineApplication app) + { + return app.Option("-v|--verbosity", "Set the verbosity level of the command. Allowed values are q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]", CommandOptionType.SingleValue); + } + private static string GetMSBuildExePath() { return Path.Combine( diff --git a/src/dotnet/commands/dotnet-pack/PackCommand.cs b/src/dotnet/commands/dotnet-pack/PackCommand.cs index cb0ad685e..30ea16179 100644 --- a/src/dotnet/commands/dotnet-pack/PackCommand.cs +++ b/src/dotnet/commands/dotnet-pack/PackCommand.cs @@ -49,6 +49,7 @@ namespace Microsoft.DotNet.Tools.Pack var argRoot = cmd.Argument("", "The project to pack, defaults to the project file in the current directory. Can be a path to any project file", multipleValues:true); + CommandOption verbosityOption = MSBuildForwardingApp.AddVerbosityOption(cmd); cmd.OnExecute(() => { @@ -92,6 +93,11 @@ namespace Microsoft.DotNet.Tools.Pack msbuildArgs.Add($"/p:Serviceable=true"); } + if (verbosityOption.HasValue()) + { + msbuildArgs.Add($"/verbosity:{verbosityOption.Value()}"); + } + msbuildArgs.AddRange(argRoot.Values); msbuildArgs.AddRange(cmd.RemainingArguments); diff --git a/src/dotnet/commands/dotnet-publish/Program.cs b/src/dotnet/commands/dotnet-publish/Program.cs index 45174aff6..c1c93b22c 100644 --- a/src/dotnet/commands/dotnet-publish/Program.cs +++ b/src/dotnet/commands/dotnet-publish/Program.cs @@ -3,6 +3,7 @@ using Microsoft.DotNet.Cli.CommandLine; using Microsoft.DotNet.Cli.Utils; +using Microsoft.DotNet.Tools.MSBuild; namespace Microsoft.DotNet.Tools.Publish { @@ -43,6 +44,7 @@ namespace Microsoft.DotNet.Tools.Publish CommandOption versionSuffixOption = app.Option( "--version-suffix ", "Defines the value for the $(VersionSuffix) property in the project", CommandOptionType.SingleValue); + CommandOption verbosityOption = MSBuildForwardingApp.AddVerbosityOption(app); app.OnExecute(() => { @@ -54,6 +56,7 @@ namespace Microsoft.DotNet.Tools.Publish publish.OutputPath = outputOption.Value(); publish.Configuration = configurationOption.Value(); publish.VersionSuffix = versionSuffixOption.Value(); + publish.Verbosity = verbosityOption.Value(); publish.ExtraMSBuildArguments = app.RemainingArguments; return publish.Execute(); diff --git a/src/dotnet/commands/dotnet-publish/PublishCommand.cs b/src/dotnet/commands/dotnet-publish/PublishCommand.cs index 79476f727..2ce171fb8 100644 --- a/src/dotnet/commands/dotnet-publish/PublishCommand.cs +++ b/src/dotnet/commands/dotnet-publish/PublishCommand.cs @@ -16,6 +16,7 @@ namespace Microsoft.DotNet.Tools.Publish public string OutputPath { get; set; } public string Configuration { get; set; } public string VersionSuffix { get; set; } + public string Verbosity { get; set; } public List ExtraMSBuildArguments { get; set; } @@ -59,6 +60,11 @@ namespace Microsoft.DotNet.Tools.Publish msbuildArgs.Add($"/p:VersionSuffix={VersionSuffix}"); } + if (!string.IsNullOrEmpty(Verbosity)) + { + msbuildArgs.Add($"/verbosity:{Verbosity}"); + } + msbuildArgs.AddRange(ExtraMSBuildArguments); return new MSBuildForwardingApp(msbuildArgs).Execute(); diff --git a/src/dotnet/commands/dotnet-restore/Program.cs b/src/dotnet/commands/dotnet-restore/Program.cs index a923be2be..1aa2f5573 100644 --- a/src/dotnet/commands/dotnet-restore/Program.cs +++ b/src/dotnet/commands/dotnet-restore/Program.cs @@ -65,6 +65,8 @@ namespace Microsoft.DotNet.Tools.Restore "Set this flag to ignore project to project references and only restore the root project", CommandOptionType.NoValue); + CommandOption verbosityOption = MSBuildForwardingApp.AddVerbosityOption(cmd); + cmd.OnExecute(() => { var msbuildArgs = new List() @@ -109,6 +111,11 @@ namespace Microsoft.DotNet.Tools.Restore msbuildArgs.Add($"/p:RestoreRecursive=false"); } + if (verbosityOption.HasValue()) + { + msbuildArgs.Add($"/verbosity:{verbosityOption.Value()}"); + } + // Add in arguments msbuildArgs.AddRange(argRoot.Values); diff --git a/src/dotnet/commands/dotnet-test/Program.cs b/src/dotnet/commands/dotnet-test/Program.cs index d8c9e6a00..a200c0960 100644 --- a/src/dotnet/commands/dotnet-test/Program.cs +++ b/src/dotnet/commands/dotnet-test/Program.cs @@ -87,6 +87,8 @@ namespace Microsoft.DotNet.Tools.Test @"Do not build project before testing.", CommandOptionType.NoValue); + CommandOption verbosityOption = MSBuildForwardingApp.AddVerbosityOption(cmd); + cmd.OnExecute(() => { var msbuildArgs = new List() @@ -94,7 +96,6 @@ namespace Microsoft.DotNet.Tools.Test "/t:VSTest" }; - msbuildArgs.Add("/verbosity:quiet"); msbuildArgs.Add("/nologo"); if (settingOption.HasValue()) @@ -147,6 +148,15 @@ namespace Microsoft.DotNet.Tools.Test msbuildArgs.Add($"/p:VSTestNoBuild=true"); } + if (verbosityOption.HasValue()) + { + msbuildArgs.Add($"/verbosity:{verbosityOption.Value()}"); + } + else + { + msbuildArgs.Add("/verbosity:quiet"); + } + string defaultproject = GetSingleTestProjectToRunTestIfNotProvided(argRoot.Value, cmd.RemainingArguments); if(!string.IsNullOrEmpty(defaultproject)) diff --git a/test/msbuild.IntegrationTests/GivenDotnetInvokesMSBuild.cs b/test/msbuild.IntegrationTests/GivenDotnetInvokesMSBuild.cs index c78570150..e6dbc380b 100644 --- a/test/msbuild.IntegrationTests/GivenDotnetInvokesMSBuild.cs +++ b/test/msbuild.IntegrationTests/GivenDotnetInvokesMSBuild.cs @@ -31,5 +31,73 @@ namespace Microsoft.DotNet.Cli.MSBuild.IntegrationTests .Execute(command) .Should().Pass(); } + + [Theory] + [InlineData("build")] + [InlineData("clean")] + [InlineData("pack")] + [InlineData("publish")] + public void When_dotnet_command_invokes_msbuild_with_no_args_verbosity_is_set_to_minimum(string command) + { + var testInstance = TestAssets.Get("MSBuildIntegration") + .CreateInstance(identifier: command) + .WithSourceFiles(); + + var cmd = new DotnetCommand() + .WithWorkingDirectory(testInstance.Root) + .ExecuteWithCapturedOutput(command); + cmd.Should().Pass(); + cmd.StdOut.Should().NotContain("Message with normal importance"); + // sanity check + cmd.StdOut.Should().Contain("Message with high importance"); + } + + [Theory] + [InlineData("build")] + [InlineData("clean")] + [InlineData("pack")] + [InlineData("publish")] + [InlineData("test")] + public void When_dotnet_command_invokes_msbuild_with_diag_verbosity_Then_arg_is_passed(string command) + { + var testInstance = TestAssets.Get("MSBuildIntegration") + .CreateInstance(identifier: command) + .WithSourceFiles(); + + var cmd = new DotnetCommand() + .WithWorkingDirectory(testInstance.Root) + .ExecuteWithCapturedOutput($"{command} -v diag"); + cmd.Should().Pass(); + cmd.StdOut.Should().Contain("Message with low importance"); + } + + [Fact] + public void When_dotnet_test_invokes_msbuild_with_no_args_verbosity_is_set_to_quiet() + { + string command = "test"; + var testInstance = TestAssets.Get("MSBuildIntegration") + .CreateInstance(identifier: command) + .WithSourceFiles(); + + var cmd = new DotnetCommand() + .WithWorkingDirectory(testInstance.Root) + .ExecuteWithCapturedOutput(command); + cmd.Should().Pass(); + cmd.StdOut.Should().NotContain("Message with high importance"); + } + + [Fact] + public void When_dotnet_msbuild_command_is_invoked_with_non_msbuild_switch_Then_it_fails() + { + string command = "msbuild"; + var testInstance = TestAssets.Get("MSBuildIntegration") + .CreateInstance(identifier: command) + .WithSourceFiles(); + + var cmd = new DotnetCommand() + .WithWorkingDirectory(testInstance.Root) + .ExecuteWithCapturedOutput($"{command} -v diag"); + cmd.ExitCode.Should().NotBe(0); + } } }