diff --git a/src/dotnet/commands/RestoringCommand.cs b/src/dotnet/commands/RestoringCommand.cs index 0239fd69b..8d4f3c923 100644 --- a/src/dotnet/commands/RestoringCommand.cs +++ b/src/dotnet/commands/RestoringCommand.cs @@ -1,6 +1,7 @@ // 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; using System.Collections.Generic; using System.Linq; using Microsoft.DotNet.Tools.MSBuild; @@ -10,26 +11,7 @@ namespace Microsoft.DotNet.Tools { public class RestoringCommand : MSBuildForwardingApp { - private bool NoRestore { get; } - - private IEnumerable ParsedArguments { get; } - - private IEnumerable TrailingArguments { get; } - - private IEnumerable ArgsToForwardToRestore() - { - var restoreArguments = ParsedArguments.Where(a => - !a.StartsWith("/p:TargetFramework")); - - if (!restoreArguments.Any(a => a.StartsWith("/verbosity:"))) - { - restoreArguments = restoreArguments.Concat(new string[] { "/verbosity:q" }); - } - - return restoreArguments.Concat(TrailingArguments); - } - - private bool ShouldRunImplicitRestore => !NoRestore; + public RestoreCommand SeparateRestoreCommand { get; } public RestoringCommand( IEnumerable msbuildArgs, @@ -37,18 +19,64 @@ namespace Microsoft.DotNet.Tools IEnumerable trailingArguments, bool noRestore, string msbuildPath = null) - : base(msbuildArgs, msbuildPath) + : base(GetCommandArguments(msbuildArgs, parsedArguments, noRestore), msbuildPath) { - NoRestore = noRestore; - ParsedArguments = parsedArguments; - TrailingArguments = trailingArguments; + SeparateRestoreCommand = GetSeparateRestoreCommand(parsedArguments, trailingArguments, noRestore, msbuildPath); } + private static IEnumerable GetCommandArguments( + IEnumerable msbuildArgs, + IEnumerable parsedArguments, + bool noRestore) + { + if (noRestore) + { + return msbuildArgs; + } + + if (HasArgumentToExcludeFromRestore(parsedArguments)) + { + return Prepend("/nologo", msbuildArgs); + } + + return Prepend("/restore", msbuildArgs); + } + + private static RestoreCommand GetSeparateRestoreCommand( + IEnumerable parsedArguments, + IEnumerable trailingArguments, + bool noRestore, + string msbuildPath) + { + if (noRestore || !HasArgumentToExcludeFromRestore(parsedArguments)) + { + return null; + } + + var restoreArguments = parsedArguments + .Where(a => !IsExcludedFromRestore(a)) + .Concat(trailingArguments); + + return RestoreCommand.FromArgs( + restoreArguments.ToArray(), + msbuildPath, + noLogo: false); + } + + private static IEnumerable Prepend(string argument, IEnumerable arguments) + => new[] { argument }.Concat(arguments); + + private static bool HasArgumentToExcludeFromRestore(IEnumerable arguments) + => arguments.Any(a => IsExcludedFromRestore(a)); + + private static bool IsExcludedFromRestore(string argument) + => argument.StartsWith("/p:TargetFramework=", StringComparison.Ordinal); + public override int Execute() { - if (ShouldRunImplicitRestore) + if (SeparateRestoreCommand != null) { - int exitCode = RestoreCommand.Run(ArgsToForwardToRestore().ToArray()); + int exitCode = SeparateRestoreCommand.Execute(); if (exitCode != 0) { return exitCode; diff --git a/src/dotnet/commands/dotnet-build/BuildCommand.cs b/src/dotnet/commands/dotnet-build/BuildCommand.cs index c7127353b..3fcee0925 100644 --- a/src/dotnet/commands/dotnet-build/BuildCommand.cs +++ b/src/dotnet/commands/dotnet-build/BuildCommand.cs @@ -37,6 +37,8 @@ namespace Microsoft.DotNet.Tools.Build var appliedBuildOptions = result["dotnet"]["build"]; + msbuildArgs.Add($"/clp:Summary"); + if (appliedBuildOptions.HasOption("--no-incremental")) { msbuildArgs.Add("/t:Rebuild"); @@ -50,8 +52,6 @@ namespace Microsoft.DotNet.Tools.Build msbuildArgs.AddRange(appliedBuildOptions.Arguments); - msbuildArgs.Add($"/clp:Summary"); - bool noRestore = appliedBuildOptions.HasOption("--no-restore"); return new BuildCommand( diff --git a/src/dotnet/commands/dotnet-restore/Program.cs b/src/dotnet/commands/dotnet-restore/Program.cs index b0269370f..b432150c3 100644 --- a/src/dotnet/commands/dotnet-restore/Program.cs +++ b/src/dotnet/commands/dotnet-restore/Program.cs @@ -19,7 +19,7 @@ namespace Microsoft.DotNet.Tools.Restore { } - public static RestoreCommand FromArgs(string[] args, string msbuildPath = null) + public static RestoreCommand FromArgs(string[] args, string msbuildPath = null, bool noLogo = true) { DebugHelper.HandleDebugSwitch(ref args); @@ -31,17 +31,15 @@ namespace Microsoft.DotNet.Tools.Restore var parsedRestore = result["dotnet"]["restore"]; - var msbuildArgs = new List - { - "/NoLogo", - "/t:Restore" - }; + var msbuildArgs = new List(); - if (!HasVerbosityOption(parsedRestore)) + if (noLogo) { - msbuildArgs.Add("/ConsoleLoggerParameters:Verbosity=Minimal"); + msbuildArgs.Add("/nologo"); } + msbuildArgs.Add("/t:Restore"); + msbuildArgs.AddRange(parsedRestore.OptionValuesToBeForwarded()); msbuildArgs.AddRange(parsedRestore.Arguments); @@ -65,12 +63,5 @@ namespace Microsoft.DotNet.Tools.Restore return cmd.Execute(); } - - private static bool HasVerbosityOption(AppliedOption parsedRestore) - { - return parsedRestore.HasOption("verbosity") || - parsedRestore.Arguments.Any(a => a.Contains("/v:")) || - parsedRestore.Arguments.Any(a => a.Contains("/verbosity:")); - } } } \ No newline at end of file diff --git a/test/dotnet-msbuild.Tests/GivenDotnetBuildInvocation.cs b/test/dotnet-msbuild.Tests/GivenDotnetBuildInvocation.cs index 259d5e257..64b7ecda5 100644 --- a/test/dotnet-msbuild.Tests/GivenDotnetBuildInvocation.cs +++ b/test/dotnet-msbuild.Tests/GivenDotnetBuildInvocation.cs @@ -10,7 +10,6 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests public class GivenDotnetBuildInvocation { const string ExpectedPrefix = "exec /m /v:m"; - const string ExpectedSuffix = "/clp:Summary"; [Theory] [InlineData(new string[] { }, "/t:Build")] @@ -19,8 +18,6 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests [InlineData(new string[] { "--output", "foo" }, "/t:Build /p:OutputPath=foo")] [InlineData(new string[] { "-o", "foo1 foo2" }, "/t:Build \"/p:OutputPath=foo1 foo2\"")] [InlineData(new string[] { "--no-incremental" }, "/t:Rebuild")] - [InlineData(new string[] { "-f", "tfm" }, "/t:Build /p:TargetFramework=tfm")] - [InlineData(new string[] { "--framework", "tfm" }, "/t:Build /p:TargetFramework=tfm")] [InlineData(new string[] { "-r", "rid" }, "/t:Build /p:RuntimeIdentifier=rid")] [InlineData(new string[] { "--runtime", "rid" }, "/t:Build /p:RuntimeIdentifier=rid")] [InlineData(new string[] { "-c", "config" }, "/t:Build /p:Configuration=config")] @@ -29,14 +26,45 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests [InlineData(new string[] { "--no-dependencies" }, "/t:Build /p:BuildProjectReferences=false")] [InlineData(new string[] { "-v", "diag" }, "/t:Build /verbosity:diag")] [InlineData(new string[] { "--verbosity", "diag" }, "/t:Build /verbosity:diag")] - [InlineData(new string[] { "--no-incremental", "-o", "myoutput", "-r", "myruntime", "-v", "diag" }, "/t:Rebuild /p:OutputPath=myoutput /p:RuntimeIdentifier=myruntime /verbosity:diag")] + [InlineData(new string[] { "--no-incremental", "-o", "myoutput", "-r", "myruntime", "-v", "diag", "/ArbitrarySwitchForMSBuild" }, + "/t:Rebuild /p:OutputPath=myoutput /p:RuntimeIdentifier=myruntime /verbosity:diag /ArbitrarySwitchForMSBuild")] public void MsbuildInvocationIsCorrect(string[] args, string expectedAdditionalArgs) { expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}"); var msbuildPath = ""; - BuildCommand.FromArgs(args, msbuildPath) - .GetProcessStartInfo().Arguments.Should().Be($"{ExpectedPrefix}{expectedAdditionalArgs} {ExpectedSuffix}"); + var command = BuildCommand.FromArgs(args, msbuildPath); + + command.SeparateRestoreCommand.Should().BeNull(); + + command.GetProcessStartInfo() + .Arguments.Should() + .Be($"{ExpectedPrefix} /restore /clp:Summary{expectedAdditionalArgs}"); + } + + [Theory] + [InlineData(new string[] { "-f", "tfm" }, "/t:Restore", "/t:Build /p:TargetFramework=tfm")] + [InlineData(new string[] { "-o", "myoutput", "-f", "tfm", "-v", "diag", "/ArbitrarySwitchForMSBuild" }, + "/t:Restore /p:OutputPath=myoutput /verbosity:diag /ArbitrarySwitchForMSBuild", + "/t:Build /p:OutputPath=myoutput /p:TargetFramework=tfm /verbosity:diag /ArbitrarySwitchForMSBuild")] + public void MsbuildInvocationIsCorrectForSeparateRestore( + string[] args, + string expectedAdditionalArgsForRestore, + string expectedAdditionalArgs) + { + expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}"); + + var msbuildPath = ""; + var command = BuildCommand.FromArgs(args, msbuildPath); + + command.SeparateRestoreCommand.GetProcessStartInfo() + .Arguments.Should() + .Be($"{ExpectedPrefix} {expectedAdditionalArgsForRestore}"); + + command.GetProcessStartInfo() + .Arguments.Should() + .Be($"{ExpectedPrefix} /nologo /clp:Summary{expectedAdditionalArgs}"); + } } } diff --git a/test/dotnet-msbuild.Tests/GivenDotnetPackInvocation.cs b/test/dotnet-msbuild.Tests/GivenDotnetPackInvocation.cs index aba1b15a6..74559a6fa 100644 --- a/test/dotnet-msbuild.Tests/GivenDotnetPackInvocation.cs +++ b/test/dotnet-msbuild.Tests/GivenDotnetPackInvocation.cs @@ -11,7 +11,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests { public class GivenDotnetPackInvocation { - const string ExpectedPrefix = "exec /m /v:m /t:pack"; + const string ExpectedPrefix = "exec /m /v:m /restore /t:pack"; [Theory] [InlineData(new string[] { }, "")] @@ -33,8 +33,10 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}"); var msbuildPath = ""; - PackCommand.FromArgs(args, msbuildPath) - .GetProcessStartInfo().Arguments.Should().Be($"{ExpectedPrefix}{expectedAdditionalArgs}"); + var command = PackCommand.FromArgs(args, msbuildPath); + + command.SeparateRestoreCommand.Should().BeNull(); + command.GetProcessStartInfo().Arguments.Should().Be($"{ExpectedPrefix}{expectedAdditionalArgs}"); } } } diff --git a/test/dotnet-msbuild.Tests/GivenDotnetPublishInvocation.cs b/test/dotnet-msbuild.Tests/GivenDotnetPublishInvocation.cs index f7cf3838d..4ece567bc 100644 --- a/test/dotnet-msbuild.Tests/GivenDotnetPublishInvocation.cs +++ b/test/dotnet-msbuild.Tests/GivenDotnetPublishInvocation.cs @@ -20,12 +20,10 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests this.output = output; } - const string ExpectedPrefix = "exec /m /v:m /t:Publish"; + const string ExpectedPrefix = "exec /m /v:m"; [Theory] [InlineData(new string[] { }, "")] - [InlineData(new string[] { "-f", "" }, "/p:TargetFramework=")] - [InlineData(new string[] { "--framework", "" }, "/p:TargetFramework=")] [InlineData(new string[] { "-r", "" }, "/p:RuntimeIdentifier=")] [InlineData(new string[] { "--runtime", "" }, "/p:RuntimeIdentifier=")] [InlineData(new string[] { "-o", "" }, "/p:PublishDir=")] @@ -43,10 +41,35 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}"); var msbuildPath = ""; - PublishCommand.FromArgs(args, msbuildPath) - .GetProcessStartInfo() - .Arguments.Should() - .Be($"{ExpectedPrefix}{expectedAdditionalArgs}"); + var command = PublishCommand.FromArgs(args, msbuildPath); + + command.SeparateRestoreCommand + .Should() + .BeNull(); + + command.GetProcessStartInfo() + .Arguments.Should() + .Be($"{ExpectedPrefix} /restore /t:Publish{expectedAdditionalArgs}"); + } + + [Theory] + [InlineData(new string[] { "-f", "" }, "/p:TargetFramework=")] + [InlineData(new string[] { "--framework", "" }, "/p:TargetFramework=")] + public void MsbuildInvocationIsCorrectForSeparateRestore(string[] args, string expectedAdditionalArgs) + { + expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}"); + + var msbuildPath = ""; + var command = PublishCommand.FromArgs(args, msbuildPath); + + command.SeparateRestoreCommand + .GetProcessStartInfo() + .Arguments.Should() + .Be($"{ExpectedPrefix} /t:Restore"); + + command.GetProcessStartInfo() + .Arguments.Should() + .Be($"{ExpectedPrefix} /nologo /t:Publish{expectedAdditionalArgs}"); } [Theory] diff --git a/test/dotnet-msbuild.Tests/GivenDotnetRestoreInvocation.cs b/test/dotnet-msbuild.Tests/GivenDotnetRestoreInvocation.cs index dd55e2abd..eb8ea6163 100644 --- a/test/dotnet-msbuild.Tests/GivenDotnetRestoreInvocation.cs +++ b/test/dotnet-msbuild.Tests/GivenDotnetRestoreInvocation.cs @@ -11,10 +11,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests public class GivenDotnetRestoreInvocation { private const string ExpectedPrefix = - "exec /m /v:m /NoLogo /t:Restore"; - - private string ExpectedPrefixWithConsoleLoggerParamaters = - $"{ExpectedPrefix} /ConsoleLoggerParameters:Verbosity=Minimal"; + "exec /m /v:m /nologo /t:Restore"; [Theory] [InlineData(new string[] { }, "")] @@ -30,19 +27,9 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests [InlineData(new string[] { "--no-cache" }, "/p:RestoreNoCache=true")] [InlineData(new string[] { "--ignore-failed-sources" }, "/p:RestoreIgnoreFailedSources=true")] [InlineData(new string[] { "--no-dependencies" }, "/p:RestoreRecursive=false")] - public void MsbuildInvocationWithConsoleLoggerParametersIsCorrect(string[] args, string expectedAdditionalArgs) - { - expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}"); - - var msbuildPath = ""; - RestoreCommand.FromArgs(args, msbuildPath) - .GetProcessStartInfo().Arguments - .Should().Be($"{ExpectedPrefixWithConsoleLoggerParamaters}{expectedAdditionalArgs}"); - } - [InlineData(new string[] { "-v", "minimal" }, @"/verbosity:minimal")] [InlineData(new string[] { "--verbosity", "minimal" }, @"/verbosity:minimal")] - public void MsbuildInvocationWithVerbosityIsCorrect(string[] args, string expectedAdditionalArgs) + public void MsbuildInvocationIsCorrect(string[] args, string expectedAdditionalArgs) { expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}");