Use msbuild /restore instead of a separate process

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,45 +11,72 @@ namespace Microsoft.DotNet.Tools
 {
     public class RestoringCommand : MSBuildForwardingApp
     {
-        private bool NoRestore { get; }
+        public RestoreCommand SeparateRestoreCommand { get; }
 
-        private IEnumerable<string> ParsedArguments { get; }
-
-        private IEnumerable<string> TrailingArguments { get; }
+        public RestoringCommand(
+            IEnumerable<string> msbuildArgs,
+            IEnumerable<string> parsedArguments,
+            IEnumerable<string> trailingArguments,
+            bool noRestore,
+            string msbuildPath = null)
+            : base(GetCommandArguments(msbuildArgs, parsedArguments, noRestore), msbuildPath)
+        {
+            SeparateRestoreCommand = GetSeparateRestoreCommand(parsedArguments, trailingArguments, noRestore, msbuildPath);
+        }
 
-        private IEnumerable<string> ArgsToForwardToRestore()
+        private static IEnumerable<string> GetCommandArguments(
+            IEnumerable<string> msbuildArgs,
+            IEnumerable<string> parsedArguments,
+            bool noRestore)
         {
-            var restoreArguments = ParsedArguments.Where(a =>
-                !a.StartsWith("/p:TargetFramework"));
+            if (noRestore) 
+            {
+                return msbuildArgs;
+            }
 
-            if (!restoreArguments.Any(a => a.StartsWith("/verbosity:")))
+            if (HasArgumentToExcludeFromRestore(parsedArguments))
             {
-                restoreArguments = restoreArguments.Concat(new string[] { "/verbosity:q" });
+                return Prepend("/nologo", msbuildArgs);
             }
 
-            return restoreArguments.Concat(TrailingArguments);
+            return Prepend("/restore", msbuildArgs);
         }
 
-        private bool ShouldRunImplicitRestore => !NoRestore;
-
-        public RestoringCommand(
-            IEnumerable<string> msbuildArgs,
+        private static RestoreCommand GetSeparateRestoreCommand(
             IEnumerable<string> parsedArguments,
-            IEnumerable<string> trailingArguments,
+            IEnumerable<string> trailingArguments, 
             bool noRestore,
-            string msbuildPath = null)
-            : base(msbuildArgs, msbuildPath)
+            string msbuildPath)
         {
-            NoRestore = noRestore;
-            ParsedArguments = parsedArguments;
-            TrailingArguments = trailingArguments;
+            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<string> Prepend(string argument, IEnumerable<string> arguments)
+            => new[] { argument }.Concat(arguments);
+
+        private static bool HasArgumentToExcludeFromRestore(IEnumerable<string> 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;

src/dotnet/commands/dotnet-build/BuildCommand.cs

@@ -37,6 +37,8 @@ public static BuildCommand FromArgs(string[] args, string msbuildPath = null)
 
             var appliedBuildOptions = result["dotnet"]["build"];
 
+            msbuildArgs.Add($"/clp:Summary");
+
             if (appliedBuildOptions.HasOption("--no-incremental"))
             {
                 msbuildArgs.Add("/t:Rebuild");
@@ -50,8 +52,6 @@ public static BuildCommand FromArgs(string[] args, string msbuildPath = null)
 
             msbuildArgs.AddRange(appliedBuildOptions.Arguments);
 
-            msbuildArgs.Add($"/clp:Summary");
-
             bool noRestore = appliedBuildOptions.HasOption("--no-restore");
 
             return new BuildCommand(

src/dotnet/commands/dotnet-restore/Program.cs

@@ -19,7 +19,7 @@ public RestoreCommand(IEnumerable<string> msbuildArgs, string msbuildPath = null
         {
         }
 
-        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 @@ public static RestoreCommand FromArgs(string[] args, string msbuildPath = null)
 
             var parsedRestore = result["dotnet"]["restore"];
 
-            var msbuildArgs = new List<string>
-            {
-                "/NoLogo",
-                "/t:Restore"
-            };
+            var msbuildArgs = new List<string>();
 
-            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 @@ public static int Run(string[] args)
 
             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:"));
-        }
     }
 }

test/dotnet-msbuild.Tests/GivenDotnetBuildInvocation.cs

@@ -10,7 +10,6 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
     public class GivenDotnetBuildInvocation
     {
         const string ExpectedPrefix = "exec <msbuildpath> /m /v:m";
-        const string ExpectedSuffix = "/clp:Summary";
 
         [Theory]
         [InlineData(new string[] { }, "/t:Build")]
@@ -19,8 +18,6 @@ public class GivenDotnetBuildInvocation
         [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 @@ public class GivenDotnetBuildInvocation
         [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 = "<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 = "<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}");
+
         }
     }
 }

test/dotnet-msbuild.Tests/GivenDotnetPackInvocation.cs

@@ -11,7 +11,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
 {
     public class GivenDotnetPackInvocation
     {
-        const string ExpectedPrefix = "exec <msbuildpath> /m /v:m /t:pack";
+        const string ExpectedPrefix = "exec <msbuildpath> /m /v:m /restore /t:pack";
 
         [Theory]
         [InlineData(new string[] { }, "")]
@@ -33,8 +33,10 @@ public void MsbuildInvocationIsCorrect(string[] args, string expectedAdditionalA
             expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}");
 
             var msbuildPath = "<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}");
         }
     }
 }

test/dotnet-msbuild.Tests/GivenDotnetPublishInvocation.cs

@@ -20,12 +20,10 @@ public GivenDotnetPublishInvocation(ITestOutputHelper output)
             this.output = output;
         }
 
-        const string ExpectedPrefix = "exec <msbuildpath> /m /v:m /t:Publish";
+        const string ExpectedPrefix = "exec <msbuildpath> /m /v:m";
 
         [Theory]
         [InlineData(new string[] { }, "")]
-        [InlineData(new string[] { "-f", "<tfm>" }, "/p:TargetFramework=<tfm>")]
-        [InlineData(new string[] { "--framework", "<tfm>" }, "/p:TargetFramework=<tfm>")]
         [InlineData(new string[] { "-r", "<rid>" }, "/p:RuntimeIdentifier=<rid>")]
         [InlineData(new string[] { "--runtime", "<rid>" }, "/p:RuntimeIdentifier=<rid>")]
         [InlineData(new string[] { "-o", "<publishdir>" }, "/p:PublishDir=<publishdir>")]
@@ -43,10 +41,35 @@ public void MsbuildInvocationIsCorrect(string[] args, string expectedAdditionalA
             expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}");
 
             var msbuildPath = "<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", "<tfm>" }, "/p:TargetFramework=<tfm>")]
+        [InlineData(new string[] { "--framework", "<tfm>" }, "/p:TargetFramework=<tfm>")]
+        public void MsbuildInvocationIsCorrectForSeparateRestore(string[] args, string expectedAdditionalArgs)
+        {
+            expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}");
+
+            var msbuildPath = "<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]

test/dotnet-msbuild.Tests/GivenDotnetRestoreInvocation.cs

@@ -11,10 +11,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
     public class GivenDotnetRestoreInvocation
     {
         private const string ExpectedPrefix =
-            "exec <msbuildpath> /m /v:m /NoLogo /t:Restore";
-
-        private string ExpectedPrefixWithConsoleLoggerParamaters =
-            $"{ExpectedPrefix} /ConsoleLoggerParameters:Verbosity=Minimal";
+            "exec <msbuildpath> /m /v:m /nologo /t:Restore";
 
         [Theory]
         [InlineData(new string[] { }, "")]
@@ -30,19 +27,9 @@ public class GivenDotnetRestoreInvocation
         [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 = "<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}");