mirrors/dotnet-installer
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

Use msbuild /restore instead of separate invocations where possible

Browse source 
It is not currently possible when there is a -f|--framework argument because
we cannot force a TargetFramework global property on to the restore evaluation.
Doing so completely breaks restore by applying the TargetFramework to all
projects transitively. The correct behavior is to restore for all frameworks,
then build/publish/etc for the given target framework. Achieving that still
requires two distinct msbuild invocations.

This also changes the verbosity of implicit restore from quiet to that
of the subsequent command (default=minimal). Similar to global properties,
we cannot specify a distinct console verbosity for the /restore portion of
the overall execution. For consistency, we apply the same verbosity change
to the case where we still use two separate msbuild invocations.

This also fixes an issue where the separate restore invocation's msbuild log
would be overwritten by the subsequent command execution. However, this remains
unfixed in the case where we still use two separate msbuild invocations.
This commit is contained in:
Nick Guerrera 2017-10-24 15:50:43 -07:00
parent 8fead788d7
commit 35b7ad2789
6 changed files with 132 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

80
src/dotnet/commands/RestoringCommand.cs
View file

@ -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<string> ParsedArguments { get; }
private IEnumerable<string> TrailingArguments { get; }
private IEnumerable<string> 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<string> msbuildArgs,
@ -37,18 +19,64 @@ namespace Microsoft.DotNet.Tools
IEnumerable<string> 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<string> GetCommandArguments(
IEnumerable<string> msbuildArgs,
IEnumerable<string> parsedArguments,
bool noRestore)
{
if (noRestore)
{
return msbuildArgs;
}
if (HasArgumentToExcludeFromRestore(parsedArguments))
{
return Prepend("/nologo", msbuildArgs);
}
return Prepend("/restore", msbuildArgs);
}
private static RestoreCommand GetSeparateRestoreCommand(
IEnumerable<string> parsedArguments,
IEnumerable<string> 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<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;

11
src/dotnet/commands/dotnet-restore/Program.cs
View file

@ -31,11 +31,14 @@ namespace Microsoft.DotNet.Tools.Restore
var parsedRestore = result["dotnet"]["restore"];
var msbuildArgs = new List<string>
var msbuildArgs = new List<string>();
if (noLogo)
{
"/NoLogo",
"/t:Restore"
};
msbuildArgs.Add("/nologo");
}
msbuildArgs.Add("/t:Restore");
msbuildArgs.AddRange(parsedRestore.OptionValuesToBeForwarded());

41
test/dotnet-msbuild.Tests/GivenDotnetBuildInvocation.cs
View file

@ -9,7 +9,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
{
public class GivenDotnetBuildInvocation
{
const string ExpectedPrefix = "exec <msbuildpath> /m /v:m /clp:Summary";
const string ExpectedPrefix = "exec <msbuildpath> /m /v:m";
[Theory]
[InlineData(new string[] { }, "/t:Build")]
@ -18,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")]
@ -28,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 = "<msbuildpath>";
BuildCommand.FromArgs(args, msbuildPath)
.GetProcessStartInfo().Arguments.Should().Be($"{ExpectedPrefix}{expectedAdditionalArgs}");
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}");
}
}
}

8
test/dotnet-msbuild.Tests/GivenDotnetPackInvocation.cs
View file

@ -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 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
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}");
}
}
}

37
test/dotnet-msbuild.Tests/GivenDotnetPublishInvocation.cs
View file

@ -20,12 +20,10 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
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 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
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]

2
test/dotnet-msbuild.Tests/GivenDotnetRestoreInvocation.cs
View file

@ -11,7 +11,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
public class GivenDotnetRestoreInvocation
{
private const string ExpectedPrefix =
"exec <msbuildpath> /m /v:m /NoLogo /t:Restore";
"exec <msbuildpath> /m /v:m /nologo /t:Restore";
[Theory]
[InlineData(new string[] { }, "")]