From 2ea3af799d08dc156f8e908ca30acde8cb968a9e Mon Sep 17 00:00:00 2001 From: jonsequitur Date: Tue, 31 Jan 2017 16:47:41 -0800 Subject: [PATCH 1/3] escape semicolons when forwarding RestoreSources to MSBuild --- .../dotnet-msbuild/MSBuildForwardingApp.cs | 15 ++++++++--- .../GivenDotnetMSBuildBuildsProjects.cs | 27 ++++++++++++++++++- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs b/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs index 5b5e68232..177f344eb 100644 --- a/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs +++ b/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs @@ -41,10 +41,12 @@ namespace Microsoft.DotNet.Tools.MSBuild { Type loggerType = typeof(MSBuildLogger); - argsToForward = argsToForward.Concat(new[] - { - $"/Logger:{loggerType.FullName},{loggerType.GetTypeInfo().Assembly.Location}" - }); + argsToForward = argsToForward + .Select(Escape) + .Concat(new[] + { + $"/Logger:{loggerType.FullName},{loggerType.GetTypeInfo().Assembly.Location}" + }); } catch (Exception) { @@ -77,6 +79,11 @@ namespace Microsoft.DotNet.Tools.MSBuild return app.Option("-v|--verbosity", LocalizableStrings.VerbosityOptionDescription, CommandOptionType.SingleValue); } + private static string Escape(string arg) => + (arg.StartsWith("/p:RestoreSources=", StringComparison.OrdinalIgnoreCase)) ? + arg.Replace(";", "%3B") : // <-- this is a workaround for https://github.com/Microsoft/msbuild/issues/1622 + arg; + private static string GetMSBuildExePath() { return Path.Combine( diff --git a/test/dotnet-msbuild.Tests/GivenDotnetMSBuildBuildsProjects.cs b/test/dotnet-msbuild.Tests/GivenDotnetMSBuildBuildsProjects.cs index dde27668c..2c00d40c9 100644 --- a/test/dotnet-msbuild.Tests/GivenDotnetMSBuildBuildsProjects.cs +++ b/test/dotnet-msbuild.Tests/GivenDotnetMSBuildBuildsProjects.cs @@ -5,6 +5,7 @@ using System; using System.Linq; using System.Linq.Expressions; using System.Reflection; +using System.Runtime.InteropServices; using FluentAssertions; using Microsoft.DotNet.Configurer; using Microsoft.DotNet.Tools.MSBuild; @@ -76,6 +77,30 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests } } + [Fact] + public void WhenRestoreSourcesStartsWithUnixPathThenHttpsSourceIsParsedCorrectly() + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return; + } + + // this is a workaround for https://github.com/Microsoft/msbuild/issues/1622 + var testInstance = TestAssets.Get("MSBuildTestApp") + .CreateInstance() + .WithSourceFiles() + .WithRestoreFiles(); + + var root = testInstance.Root; + var somePathThatExists = "/usr/local/bin"; + + var result = new DotnetCommand() + .WithWorkingDirectory(root) + .Execute($"msbuild /p:RestoreSources={somePathThatExists};https://api.nuget.org/v3/index.json /t:restore MSBuildTestApp.csproj"); + + result.Should().Pass(); + } + [Fact] public void WhenDotnetRunHelpIsInvokedAppArgumentsTextIsIncludedInOutput() { @@ -85,7 +110,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests var result = new TestCommand("dotnet") .WithWorkingDirectory(projectDirectory.Path) .ExecuteWithCapturedOutput("run --help"); - + result.ExitCode.Should().Be(0); result.StdOut.Should().Contain(AppArgumentsText); } From 667f1cf018d7bc3fa5777542326efde1425c52ac Mon Sep 17 00:00:00 2001 From: jonsequitur Date: Wed, 1 Feb 2017 10:09:54 -0800 Subject: [PATCH 2/3] also escape :// --- src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs | 4 +++- test/dotnet.Tests/PackagedCommandTests.cs | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs b/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs index 177f344eb..7f9120791 100644 --- a/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs +++ b/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs @@ -80,8 +80,10 @@ namespace Microsoft.DotNet.Tools.MSBuild } private static string Escape(string arg) => + // this is a workaround for https://github.com/Microsoft/msbuild/issues/1622 (arg.StartsWith("/p:RestoreSources=", StringComparison.OrdinalIgnoreCase)) ? - arg.Replace(";", "%3B") : // <-- this is a workaround for https://github.com/Microsoft/msbuild/issues/1622 + arg.Replace(";", "%3B") + .Replace("://", ":%2F%2F") : arg; private static string GetMSBuildExePath() diff --git a/test/dotnet.Tests/PackagedCommandTests.cs b/test/dotnet.Tests/PackagedCommandTests.cs index d1f8b0c93..52d58a9e7 100644 --- a/test/dotnet.Tests/PackagedCommandTests.cs +++ b/test/dotnet.Tests/PackagedCommandTests.cs @@ -41,7 +41,6 @@ namespace Microsoft.DotNet.Tests }; } } - public static IEnumerable LibraryDependencyToolArguments { get From 905db127a695131849f21a55a33e5efedede495d Mon Sep 17 00:00:00 2001 From: jonsequitur Date: Wed, 1 Feb 2017 14:12:34 -0800 Subject: [PATCH 3/3] use missing package test to verify correct package feed parsing --- ...aryWithUnresolvablePackageReference.csproj | 11 ++++++++ .../dotnet-msbuild/MSBuildForwardingApp.cs | 3 +-- .../GivenDotnetMSBuildBuildsProjects.cs | 27 ++++++++++++++----- 3 files changed, 33 insertions(+), 8 deletions(-) create mode 100755 TestAssets/TestProjects/LibraryWithUnresolvablePackageReference/LibraryWithUnresolvablePackageReference.csproj diff --git a/TestAssets/TestProjects/LibraryWithUnresolvablePackageReference/LibraryWithUnresolvablePackageReference.csproj b/TestAssets/TestProjects/LibraryWithUnresolvablePackageReference/LibraryWithUnresolvablePackageReference.csproj new file mode 100755 index 000000000..66c164859 --- /dev/null +++ b/TestAssets/TestProjects/LibraryWithUnresolvablePackageReference/LibraryWithUnresolvablePackageReference.csproj @@ -0,0 +1,11 @@ + + + + netstandard1.5 + + + + + + + diff --git a/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs b/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs index 7f9120791..6b2b0f47f 100644 --- a/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs +++ b/src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs @@ -42,7 +42,6 @@ namespace Microsoft.DotNet.Tools.MSBuild Type loggerType = typeof(MSBuildLogger); argsToForward = argsToForward - .Select(Escape) .Concat(new[] { $"/Logger:{loggerType.FullName},{loggerType.GetTypeInfo().Assembly.Location}" @@ -56,7 +55,7 @@ namespace Microsoft.DotNet.Tools.MSBuild _forwardingApp = new ForwardingApp( GetMSBuildExePath(), - _msbuildRequiredParameters.Concat(argsToForward), + _msbuildRequiredParameters.Concat(argsToForward.Select(Escape)), environmentVariables: _msbuildRequiredEnvironmentVariables); } diff --git a/test/dotnet-msbuild.Tests/GivenDotnetMSBuildBuildsProjects.cs b/test/dotnet-msbuild.Tests/GivenDotnetMSBuildBuildsProjects.cs index 2c00d40c9..117f34bae 100644 --- a/test/dotnet-msbuild.Tests/GivenDotnetMSBuildBuildsProjects.cs +++ b/test/dotnet-msbuild.Tests/GivenDotnetMSBuildBuildsProjects.cs @@ -12,12 +12,20 @@ using Microsoft.DotNet.Tools.MSBuild; using Microsoft.DotNet.Tools.Test.Utilities; using NuGet.Protocol; using Xunit; +using Xunit.Abstractions; using MSBuildCommand = Microsoft.DotNet.Tools.Test.Utilities.MSBuildCommand; namespace Microsoft.DotNet.Cli.MSBuild.Tests { public class GivenDotnetMSBuildBuildsProjects : TestBase { + private readonly ITestOutputHelper _output; + + public GivenDotnetMSBuildBuildsProjects(ITestOutputHelper output) + { + _output = output; + } + [Fact] public void ItRunsSpecifiedTargetsWithPropertiesCorrectly() { @@ -86,19 +94,26 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests } // this is a workaround for https://github.com/Microsoft/msbuild/issues/1622 - var testInstance = TestAssets.Get("MSBuildTestApp") + var testInstance = TestAssets.Get("LibraryWithUnresolvablePackageReference") .CreateInstance() - .WithSourceFiles() - .WithRestoreFiles(); + .WithSourceFiles(); var root = testInstance.Root; var somePathThatExists = "/usr/local/bin"; var result = new DotnetCommand() .WithWorkingDirectory(root) - .Execute($"msbuild /p:RestoreSources={somePathThatExists};https://api.nuget.org/v3/index.json /t:restore MSBuildTestApp.csproj"); - - result.Should().Pass(); + .Execute($"msbuild /p:RestoreSources={somePathThatExists};https://api.nuget.org/v3/index.json /t:restore LibraryWithUnresolvablePackageReference.csproj"); + + _output.WriteLine($"[STDOUT]\n{result.StdOut}\n[STDERR]\n{result.StdErr}"); + + result.Should().Fail(); + + result.StdOut.Should() + .ContainVisuallySameFragment( +@"Feeds used: + /usr/local/bin + https://api.nuget.org/v3/index.json"); } [Fact]