diff --git a/src/dotnet/ForwardingApp.cs b/src/dotnet/ForwardingApp.cs index 607c0f43a..2ae292b75 100644 --- a/src/dotnet/ForwardingApp.cs +++ b/src/dotnet/ForwardingApp.cs @@ -67,6 +67,11 @@ namespace Microsoft.DotNet.Cli } public int Execute() + { + return GetProcessStartInfo().Execute(); + } + + public ProcessStartInfo GetProcessStartInfo() { var processInfo = new ProcessStartInfo { @@ -83,15 +88,7 @@ namespace Microsoft.DotNet.Cli } } - var process = new Process - { - StartInfo = processInfo - }; - - process.Start(); - process.WaitForExit(); - - return process.ExitCode; + return processInfo; } public ForwardingApp WithEnvironmentVariable(string name, string value) diff --git a/src/dotnet/ProcessStartInfoExtensions.cs b/src/dotnet/ProcessStartInfoExtensions.cs new file mode 100644 index 000000000..721e0ec55 --- /dev/null +++ b/src/dotnet/ProcessStartInfoExtensions.cs @@ -0,0 +1,26 @@ +using System; +using System.Diagnostics; + +namespace Microsoft.DotNet.Cli +{ + internal static class ProcessStartInfoExtensions + { + public static int Execute(this ProcessStartInfo startInfo) + { + if (startInfo == null) + { + throw new ArgumentNullException(nameof(startInfo)); + } + + var process = new Process + { + StartInfo = startInfo + }; + + process.Start(); + process.WaitForExit(); + + return process.ExitCode; + } + } +} diff --git a/src/dotnet/commands/dotnet-build/Program.cs b/src/dotnet/commands/dotnet-build/Program.cs index 785d3b944..b29348757 100644 --- a/src/dotnet/commands/dotnet-build/Program.cs +++ b/src/dotnet/commands/dotnet-build/Program.cs @@ -5,21 +5,29 @@ using System.Collections.Generic; using Microsoft.DotNet.Cli.CommandLine; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.Tools.MSBuild; +using System.Diagnostics; +using System; +using Microsoft.DotNet.Cli; namespace Microsoft.DotNet.Tools.Build { public class BuildCommand { - public static int Run(string[] args) - { - DebugHelper.HandleDebugSwitch(ref args); + private MSBuildForwardingApp _forwardingApp; + public BuildCommand(IEnumerable msbuildArgs, string msbuildPath = null) + { + _forwardingApp = new MSBuildForwardingApp(msbuildArgs, msbuildPath); + } + + public static BuildCommand FromArgs(string[] args, string msbuildPath = null) + { CommandLineApplication app = new CommandLineApplication(throwOnUnexpectedArg: false); app.Name = "dotnet build"; app.FullName = LocalizableStrings.AppFullName; app.Description = LocalizableStrings.AppDescription; app.ArgumentSeparatorHelpText = HelpMessageStrings.MSBuildAdditionalArgsHelpText; - app.HandleRemainingArguments = true; + app.HandleRemainingArguments = true; app.HelpOption("-h|--help"); CommandArgument projectArgument = app.Argument($"<{LocalizableStrings.ProjectArgumentValueName}>", LocalizableStrings.ProjectArgumentDescription); @@ -36,9 +44,12 @@ namespace Microsoft.DotNet.Tools.Build CommandOption noDependenciesOption = app.Option("--no-dependencies", LocalizableStrings.NoDependenciesOptionDescription, CommandOptionType.NoValue); CommandOption verbosityOption = MSBuildForwardingApp.AddVerbosityOption(app); + List msbuildArgs = null; app.OnExecute(() => { - List msbuildArgs = new List(); + // this delayed initialization is here intentionally + // this code will not get run in some cases (i.e. --help) + msbuildArgs = new List(); if (!string.IsNullOrEmpty(projectArgument.Value)) { @@ -93,10 +104,53 @@ namespace Microsoft.DotNet.Tools.Build msbuildArgs.AddRange(app.RemainingArguments); - return new MSBuildForwardingApp(msbuildArgs).Execute(); + return 0; }); - return app.Execute(args); + int exitCode = app.Execute(args); + if (msbuildArgs == null) + { + throw new CommandCreationException(exitCode); + } + + return new BuildCommand(msbuildArgs, msbuildPath); + } + + public static int Run(string[] args) + { + DebugHelper.HandleDebugSwitch(ref args); + + BuildCommand cmd; + try + { + cmd = FromArgs(args); + } + catch (CommandCreationException e) + { + return e.ExitCode; + } + + return cmd.Execute(); + } + + public ProcessStartInfo GetProcessStartInfo() + { + return _forwardingApp.GetProcessStartInfo(); + } + + public int Execute() + { + return GetProcessStartInfo().Execute(); + } + + private class CommandCreationException : Exception + { + public int ExitCode { get; private set; } + + public CommandCreationException(int exitCode) + { + ExitCode = exitCode; + } } } } diff --git a/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs b/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs index 6b2b0f47f..ee9e9aba8 100644 --- a/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs +++ b/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs @@ -9,6 +9,7 @@ using System.Reflection; using System.Runtime.InteropServices; using Microsoft.DotNet.Cli; using Microsoft.DotNet.Cli.CommandLine; +using System.Diagnostics; namespace Microsoft.DotNet.Tools.MSBuild { @@ -33,7 +34,7 @@ namespace Microsoft.DotNet.Tools.MSBuild private readonly IEnumerable _msbuildRequiredParameters = new List { "/m", "/v:m" }; - public MSBuildForwardingApp(IEnumerable argsToForward) + public MSBuildForwardingApp(IEnumerable argsToForward, string msbuildPath = null) { if (Telemetry.CurrentSessionId != null) { @@ -54,23 +55,21 @@ namespace Microsoft.DotNet.Tools.MSBuild } _forwardingApp = new ForwardingApp( - GetMSBuildExePath(), + msbuildPath ?? GetMSBuildExePath(), _msbuildRequiredParameters.Concat(argsToForward.Select(Escape)), environmentVariables: _msbuildRequiredEnvironmentVariables); } + public ProcessStartInfo GetProcessStartInfo() + { + return _forwardingApp + .WithEnvironmentVariable(TelemetrySessionIdEnvironmentVariableName, Telemetry.CurrentSessionId) + .GetProcessStartInfo(); + } + public int Execute() { - try - { - Environment.SetEnvironmentVariable(TelemetrySessionIdEnvironmentVariableName, Telemetry.CurrentSessionId); - - return _forwardingApp.Execute(); - } - finally - { - Environment.SetEnvironmentVariable(TelemetrySessionIdEnvironmentVariableName, null); - } + return GetProcessStartInfo().Execute(); } internal static CommandOption AddVerbosityOption(CommandLineApplication app) diff --git a/test/Microsoft.DotNet.Tools.Tests.Utilities/NonWindowsOnlyFactAttribute.cs b/test/Microsoft.DotNet.Tools.Tests.Utilities/NonWindowsOnlyFactAttribute.cs index d50d0a89e..7eda24788 100644 --- a/test/Microsoft.DotNet.Tools.Tests.Utilities/NonWindowsOnlyFactAttribute.cs +++ b/test/Microsoft.DotNet.Tools.Tests.Utilities/NonWindowsOnlyFactAttribute.cs @@ -1,3 +1,6 @@ +// 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 Microsoft.DotNet.PlatformAbstractions; using Xunit; @@ -9,7 +12,7 @@ namespace Microsoft.DotNet.Tools.Test.Utilities { if (RuntimeEnvironment.OperatingSystemPlatform == Platform.Windows) { - this.Skip = "This test requires windows to run"; + this.Skip = "This test requires non-Windows to run"; } } } diff --git a/test/Microsoft.DotNet.Tools.Tests.Utilities/NonWindowsOnlyTheoryAttribute.cs b/test/Microsoft.DotNet.Tools.Tests.Utilities/NonWindowsOnlyTheoryAttribute.cs new file mode 100644 index 000000000..ca75b47dd --- /dev/null +++ b/test/Microsoft.DotNet.Tools.Tests.Utilities/NonWindowsOnlyTheoryAttribute.cs @@ -0,0 +1,19 @@ +// 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 Microsoft.DotNet.PlatformAbstractions; +using Xunit; + +namespace Microsoft.DotNet.Tools.Test.Utilities +{ + public class NonWindowsOnlyTheoryAttribute : TheoryAttribute + { + public NonWindowsOnlyTheoryAttribute() + { + if (RuntimeEnvironment.OperatingSystemPlatform == Platform.Windows) + { + this.Skip = "This test requires non-Windows to run"; + } + } + } +} diff --git a/test/Microsoft.DotNet.Tools.Tests.Utilities/WindowsOnlyFactAttribute.cs b/test/Microsoft.DotNet.Tools.Tests.Utilities/WindowsOnlyFactAttribute.cs index 74318bb9e..7f9abcccb 100644 --- a/test/Microsoft.DotNet.Tools.Tests.Utilities/WindowsOnlyFactAttribute.cs +++ b/test/Microsoft.DotNet.Tools.Tests.Utilities/WindowsOnlyFactAttribute.cs @@ -12,7 +12,7 @@ namespace Microsoft.DotNet.Tools.Test.Utilities { if (RuntimeEnvironment.OperatingSystemPlatform != Platform.Windows) { - this.Skip = "This test requires windows to run"; + this.Skip = "This test requires Windows to run"; } } } diff --git a/test/Microsoft.DotNet.Tools.Tests.Utilities/WindowsOnlyTheoryAttribute.cs b/test/Microsoft.DotNet.Tools.Tests.Utilities/WindowsOnlyTheoryAttribute.cs index d1e16f025..8a663433f 100644 --- a/test/Microsoft.DotNet.Tools.Tests.Utilities/WindowsOnlyTheoryAttribute.cs +++ b/test/Microsoft.DotNet.Tools.Tests.Utilities/WindowsOnlyTheoryAttribute.cs @@ -1,4 +1,7 @@ -using Microsoft.DotNet.PlatformAbstractions; +// 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 Microsoft.DotNet.PlatformAbstractions; using Xunit; namespace Microsoft.DotNet.Tools.Test.Utilities @@ -9,7 +12,7 @@ namespace Microsoft.DotNet.Tools.Test.Utilities { if (RuntimeEnvironment.OperatingSystemPlatform != Platform.Windows) { - this.Skip = "This test requires windows to run"; + this.Skip = "This test requires Windows to run"; } } } diff --git a/test/dotnet-msbuild.Tests/GivenDotnetBuildInvocation.cs b/test/dotnet-msbuild.Tests/GivenDotnetBuildInvocation.cs new file mode 100644 index 000000000..457297d45 --- /dev/null +++ b/test/dotnet-msbuild.Tests/GivenDotnetBuildInvocation.cs @@ -0,0 +1,33 @@ +using Microsoft.DotNet.Tools.Build; +using FluentAssertions; +using Xunit; + +namespace Microsoft.DotNet.Cli.MSBuild.Tests +{ + public class GivenDotnetBuildInvocation + { + [Theory] + [InlineData(new string[] { }, @"exec /m /v:m /t:Build /clp:Summary")] + [InlineData(new string[] { "-o", "foo" }, @"exec /m /v:m /t:Build /p:OutputPath=foo /clp:Summary")] + [InlineData(new string[] { "--output", "foo" }, @"exec /m /v:m /t:Build /p:OutputPath=foo /clp:Summary")] + [InlineData(new string[] { "-o", "foo1 foo2" }, @"exec /m /v:m /t:Build ""/p:OutputPath=foo1 foo2"" /clp:Summary")] + [InlineData(new string[] { "--no-incremental" }, @"exec /m /v:m /t:Rebuild /clp:Summary")] + [InlineData(new string[] { "-f", "framework" }, @"exec /m /v:m /t:Build /p:TargetFramework=framework /clp:Summary")] + [InlineData(new string[] { "--framework", "framework" }, @"exec /m /v:m /t:Build /p:TargetFramework=framework /clp:Summary")] + [InlineData(new string[] { "-r", "runtime" }, @"exec /m /v:m /t:Build /p:RuntimeIdentifier=runtime /clp:Summary")] + [InlineData(new string[] { "--runtime", "runtime" }, @"exec /m /v:m /t:Build /p:RuntimeIdentifier=runtime /clp:Summary")] + [InlineData(new string[] { "-c", "configuration" }, @"exec /m /v:m /t:Build /p:Configuration=configuration /clp:Summary")] + [InlineData(new string[] { "--configuration", "configuration" }, @"exec /m /v:m /t:Build /p:Configuration=configuration /clp:Summary")] + [InlineData(new string[] { "--version-suffix", "mysuffix" }, @"exec /m /v:m /t:Build /p:VersionSuffix=mysuffix /clp:Summary")] + [InlineData(new string[] { "--no-dependencies" }, @"exec /m /v:m /t:Build /p:BuildProjectReferences=false /clp:Summary")] + [InlineData(new string[] { "-v", "verbosity" }, @"exec /m /v:m /t:Build /verbosity:verbosity /clp:Summary")] + [InlineData(new string[] { "--verbosity", "verbosity" }, @"exec /m /v:m /t:Build /verbosity:verbosity /clp:Summary")] + [InlineData(new string[] { "--no-incremental", "-o", "myoutput", "-r", "myruntime", "-v", "diag" }, @"exec /m /v:m /t:Rebuild /p:OutputPath=myoutput /p:RuntimeIdentifier=myruntime /verbosity:diag /clp:Summary")] + public void MsbuildInvocationIsCorrect(string[] args, string expectedCommand) + { + var msbuildPath = ""; + BuildCommand.FromArgs(args, msbuildPath) + .GetProcessStartInfo().Arguments.Should().Be(expectedCommand); + } + } +} diff --git a/test/dotnet-msbuild.Tests/GivenMsbuildForwardingApp.cs b/test/dotnet-msbuild.Tests/GivenMsbuildForwardingApp.cs new file mode 100644 index 000000000..b04887e2b --- /dev/null +++ b/test/dotnet-msbuild.Tests/GivenMsbuildForwardingApp.cs @@ -0,0 +1,95 @@ +using System.IO; +using Microsoft.DotNet.Tools.MSBuild; +using FluentAssertions; +using Xunit; +using static Microsoft.DotNet.Tools.Test.Utilities.DirectoryInfoExtensions; +using WindowsOnlyFactAttribute = Microsoft.DotNet.Tools.Test.Utilities.WindowsOnlyFactAttribute; +using NonWindowsOnlyFactAttribute = Microsoft.DotNet.Tools.Test.Utilities.NonWindowsOnlyFactAttribute; + +namespace Microsoft.DotNet.Cli.MSBuild.Tests +{ + public class GivenMsbuildForwardingApp + { + [WindowsOnlyFact] + public void DotnetExeIsExecuted() + { + var msbuildPath = ""; + new MSBuildForwardingApp(new string[0], msbuildPath) + .GetProcessStartInfo().FileName.Should().Be("dotnet.exe"); + } + + [NonWindowsOnlyFact] + public void DotnetIsExecuted() + { + var msbuildPath = ""; + new MSBuildForwardingApp(new string[0], msbuildPath) + .GetProcessStartInfo().FileName.Should().Be("dotnet"); + } + + [Theory] + [InlineData("MSBuildExtensionsPath")] + [InlineData("CscToolExe")] + [InlineData("MSBuildSDKsPath")] + [InlineData("DOTNET_CLI_TELEMETRY_SESSIONID")] + public void ItSetsEnvironmentalVariables(string envVarName) + { + var msbuildPath = ""; + var startInfo = new MSBuildForwardingApp(new string[0], msbuildPath).GetProcessStartInfo(); + startInfo.Environment.ContainsKey(envVarName).Should().BeTrue(); + } + + [Fact] + public void ItSetsMSBuildExtensionPathToExistingPath() + { + var msbuildPath = ""; + var envVar = "MSBuildExtensionsPath"; + new DirectoryInfo(new MSBuildForwardingApp(new string[0], msbuildPath) + .GetProcessStartInfo() + .Environment[envVar]) + .Should() + .Exist(); + } + + [Fact] + public void ItSetsMSBuildSDKsPathToExistingPath() + { + var msbuildPath = ""; + var envVar = "MSBuildSDKsPath"; + new DirectoryInfo(new MSBuildForwardingApp(new string[0], msbuildPath) + .GetProcessStartInfo() + .Environment[envVar]) + .Should() + .Exist(); + } + + [Fact] + public void ItSetsCscToolExePathToValidPath() + { + var msbuildPath = ""; + var envVar = "CscToolExe"; + new FileInfo(new MSBuildForwardingApp(new string[0], msbuildPath) + .GetProcessStartInfo() + .Environment[envVar]) + .Should().NotBeNull("constructor will throw on invalid path"); + } + + [Fact] + public void ItSetsOrIgnoresTelemetrySessionId() + { + var msbuildPath = ""; + var envVar = "DOTNET_CLI_TELEMETRY_SESSIONID"; + var startInfo = new MSBuildForwardingApp(new string[0], msbuildPath) + .GetProcessStartInfo(); + (startInfo.Environment[envVar] == null || int.TryParse(startInfo.Environment[envVar], out _)) + .Should().BeTrue("DOTNET_CLI_TELEMETRY_SESSIONID should be null or current session id"); + } + + [Fact] + public void ItDoesNotSetCurrentWorkingDirectory() + { + var msbuildPath = ""; + var startInfo = new MSBuildForwardingApp(new string[0], msbuildPath) + .GetProcessStartInfo().WorkingDirectory.Should().Be(""); + } + } +}