Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert to token-per-line response file handling #26204

Merged
merged 12 commits into from
Jul 6, 2022
Merged
3 changes: 3 additions & 0 deletions src/Cli/dotnet/CommonLocalizableStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -699,4 +699,7 @@ The default is 'true' if a runtime identifier is specified.</value>
<data name="SelfContainAndNoSelfContainedConflict" xml:space="preserve">
<value>The '--self-contained' and '--no-self-contained' options cannot be used together.</value>
</data>
<data name="ResponseFileNotFound" xml:space="preserve">
<value>Response file '{0}' does not exist.</value>
</data>
</root>
2 changes: 1 addition & 1 deletion src/Cli/dotnet/CommonOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Microsoft.DotNet.Cli
internal static class CommonOptions
{
public static Option<string[]> PropertiesOption =
new ForwardedOption<string[]>(new string[] { "-property", "/p" })
new ForwardedOption<string[]>(new string[] { "-property", "/p", "-p" })
baronfel marked this conversation as resolved.
Show resolved Hide resolved
{
IsHidden = true
}.ForwardAsProperty()
Expand Down
6 changes: 5 additions & 1 deletion src/Cli/dotnet/OptionForwardingExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ public static class OptionForwardingExtensions
public static ForwardedOption<T> ForwardAsSingle<T>(this ForwardedOption<T> option, Func<T, string> format) => option.SetForwardingFunction(format);

public static ForwardedOption<string[]> ForwardAsProperty(this ForwardedOption<string[]> option) => option
.SetForwardingFunction((optionVals) => optionVals.SelectMany(optionVal => new string[] { $"{option.Aliases.FirstOrDefault()}:{optionVal.Replace("roperty:", string.Empty)}" }));
.SetForwardingFunction((optionVals) =>
optionVals
.Select(optionVal => optionVal.Replace(";", "%3B")) // must escape semicolon-delimited property values when forwarding them to MSBuild
baronfel marked this conversation as resolved.
Show resolved Hide resolved
.SelectMany(optionVal => new string[] { $"{option.Aliases.FirstOrDefault()}:{optionVal.Replace("roperty:", string.Empty)}" })
baronfel marked this conversation as resolved.
Show resolved Hide resolved
);

public static Option<T> ForwardAsMany<T>(this ForwardedOption<T> option, Func<T, IEnumerable<string>> format) => option.SetForwardingFunction(format);

Expand Down
34 changes: 34 additions & 0 deletions src/Cli/dotnet/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,39 @@ public static Command GetBuiltInCommand(string commandName)
.FirstOrDefault(c => c.Name.Equals(commandName, StringComparison.OrdinalIgnoreCase));
}

/// <summary>
/// Implements token-per-line response file handling for the CLI. We use this instead of the built-in S.CL handling
/// to ensure backwards-compatibility with MSBuild.
/// </summary>
public static bool TokenPerLine(string tokenToReplace, out IReadOnlyList<string> replacementTokens, out string errorMessage) {
baronfel marked this conversation as resolved.
Show resolved Hide resolved
var filePath = Path.GetFullPath(tokenToReplace);
if (File.Exists(filePath)) {
var lines = File.ReadAllLines(filePath);
var trimmedLines =
lines
// Remove content in the lines that contain #, starting from the point of the #
.Select(line => {
var hashPos = line.IndexOf('#');
if (hashPos == -1) {
return line;
} else if (hashPos == 0) {
return "";
} else {
return line.Substring(hashPos).Trim();
}
})
// Remove empty lines
.Where(line => line.Length > 0);
replacementTokens = trimmedLines.ToArray();
errorMessage = null;
return true;
} else {
replacementTokens = null;
errorMessage = string.Format(CommonLocalizableStrings.ResponseFileNotFound, tokenToReplace);
return false;
}
}

public static System.CommandLine.Parsing.Parser Instance { get; } = new CommandLineBuilder(ConfigureCommandLine(RootCommand))
.UseExceptionHandler(ExceptionHandler)
.UseHelp()
Expand All @@ -118,6 +151,7 @@ public static Command GetBuiltInCommand(string commandName)
.UseSuggestDirective()
.DisablePosixBinding()
.EnableLegacyDoubleDashBehavior()
.UseTokenReplacer(TokenPerLine)
.Build();

private static void ExceptionHandler(Exception exception, InvocationContext context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ public MSBuildForwardingApp(IEnumerable<string> argsToForward, string msbuildPat
}
}

public IEnumerable<string> MSBuildArguments { get { return _forwardingAppWithoutLogging.GetAllArguments(); } }
baronfel marked this conversation as resolved.
Show resolved Hide resolved

public void EnvironmentVariable(string name, string value)
{
_forwardingAppWithoutLogging.EnvironmentVariable(name, value);
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.cs.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">Balíček</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">Možnosti --self-contained a --no-self-contained nelze použít společně.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.de.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">Paket</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">Die Optionen „--self-contained“ und „--no-self-contained“ können nicht gemeinsam verwendet werden.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.es.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">Paquete</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">Las opciones '--self-contained' y '--no-self-contained' no se pueden usar juntas.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.fr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">Package</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">Les options « --autonome » et « --non-autonome » ne peuvent pas être utilisées ensemble.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.it.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">Pacchetto</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">Non è possibile usare contemporaneamente le opzioni '--self-contained' e ‘--no-self-contained'.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.ja.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">パッケージ</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">'--self-contained' と '--no-self-contained' オプションは同時に使用できません。</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.ko.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">패키지</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">'--self-contained' 및 '--no-self-contained' 옵션은 함께 사용할 수 없습니다.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.pl.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">Pakiet</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">Opcji „--self-contained” i „--no-self-contained” nie można używać razem.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.pt-BR.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">Pacote</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">As opções '--auto--contidas ' e '--não-independentes ' não podem ser usadas juntas.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.ru.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">Пакет</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">Параметры "--self-contained" и "--no-self-contained" нельзя использовать вместе.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.tr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">Paket</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">'--self-contained' ve '--no-self-contained' seçenekleri birlikte kullanılamaz.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.zh-Hans.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ EOF
<target state="translated">打包</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">“--自包含”和“--非自包含”选项不能一起使用。</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CommonLocalizableStrings.zh-Hant.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
<target state="translated">套件</target>
<note />
</trans-unit>
<trans-unit id="ResponseFileNotFound">
<source>Response file '{0}' does not exist.</source>
<target state="new">Response file '{0}' does not exist.</target>
<note />
</trans-unit>
<trans-unit id="SelfContainAndNoSelfContainedConflict">
<source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
<target state="translated">不能同時使用 '--self-contained' 和 '--no-self-contained' 選項。</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.DotNet.Tools;
using Microsoft.DotNet.Tools.Build;
using Microsoft.DotNet.Tools.Publish;
using System.Linq;
using Xunit;
using Xunit.Abstractions;

Expand All @@ -31,9 +32,10 @@ public void MSBuildArgumentsAreForwardedCorrectly(string[] arguments, bool build
RestoringCommand command = buildCommand ?
BuildCommand.FromArgs(arguments) :
PublishCommand.FromArgs(arguments);
var expectedArguments = arguments.Select(a => a.Replace("-p:", "-property:"));
baronfel marked this conversation as resolved.
Show resolved Hide resolved
command.GetArgumentsToMSBuild().Split(' ')
.Should()
.Contain(arguments);
.Contain(expectedArguments);
}
}
}
52 changes: 52 additions & 0 deletions src/Tests/dotnet.Tests/ParserTests/ResponseFileTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// 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 FluentAssertions;
using Microsoft.DotNet.Cli;
using Microsoft.NET.TestFramework;
using Parser = Microsoft.DotNet.Cli.Parser;
using System.CommandLine.Parsing;
using System.IO;
using System.Linq;
using Xunit;
using Xunit.Abstractions;

namespace Microsoft.DotNet.Tests.ParserTests
{
public class ResponseFileTests : SdkTest
{
public ResponseFileTests(ITestOutputHelper output) : base(output)
{
}

[Fact]
public void Can_safely_expand_response_file_lines() {
var tempFileDir = _testAssetsManager.CreateTestDirectory().Path;
var tempFilePath = Path.Combine(tempFileDir, "params.rsp");
var lines = new[] {
"build",
"a b",
"-p:VSTestTestAdapterPath=\".;/opt/buildagent/plugins/dotnet/tools/vstest15\""
};
File.WriteAllLines(tempFilePath, lines);

var parser = Parser.Instance;
var parseResult = parser.Parse(new []{ "dotnet", $"@{tempFilePath}" });

var tokens = parseResult.Tokens.Select(t => t.Value);
var tokenString = string.Join(", ", tokens);
var bc = Microsoft.DotNet.Tools.Build.BuildCommand.FromParseResult(parseResult);
var tokenized = new [] {
"build",
"a b",
"-p",
"VSTestTestAdapterPath=\".;/opt/buildagent/plugins/dotnet/tools/vstest15\""
};

Log.WriteLine($"MSbuild Args are {string.Join(" ", bc.MSBuildArguments)}");
Log.WriteLine($"Parse Diagram is {parseResult.Diagram()}");
Log.WriteLine($"Token string is {tokenString}");
tokens.Skip(1).Should().BeEquivalentTo(tokenized);
baronfel marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public void MsbuildInvocationIsCorrect(string[] args, string expectedAdditionalA

[Theory]
[InlineData(new string[] { "-f", "tfm" }, "-target:Restore", "-property:TargetFramework=tfm")]
[InlineData(new string[] { "-p:TargetFramework=tfm" }, "-target:Restore", "-p:TargetFramework=tfm")]
[InlineData(new string[] { "-p:TargetFramework=tfm" }, "-target:Restore", "-property:TargetFramework=tfm")]
baronfel marked this conversation as resolved.
Show resolved Hide resolved
[InlineData(new string[] { "/p:TargetFramework=tfm" }, "-target:Restore", "-property:TargetFramework=tfm")]
[InlineData(new string[] { "-t:Run", "-f", "tfm" }, "-target:Restore", "-property:TargetFramework=tfm -t:Run")]
[InlineData(new string[] { "/t:Run", "-f", "tfm" }, "-target:Restore", "-property:TargetFramework=tfm /t:Run")]
Expand Down