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>
3 changes: 2 additions & 1 deletion src/Cli/dotnet/CommonOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ namespace Microsoft.DotNet.Cli
internal static class CommonOptions
{
public static Option<string[]> PropertiesOption =
new ForwardedOption<string[]>(new string[] { "-property", "/p" })
// these are all of the forms that the property switch can be understood by in MSBuild
new ForwardedOption<string[]>(new string[] { "--property", "-property", "/property", "/p", "-p", "--p" })
{
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
.Select(optionVal => $"{option.Aliases.FirstOrDefault()}:{optionVal}")
);

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(0, 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
2 changes: 1 addition & 1 deletion src/Cli/dotnet/commands/RestoringCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private static IEnumerable<string> Prepend(string argument, IEnumerable<string>
private static bool HasArgumentToExcludeFromRestore(IEnumerable<string> arguments)
=> arguments.Any(a => IsExcludedFromRestore(a));

private static readonly string[] propertyPrefixes = new string[]{ "-", "/" };
private static readonly string[] propertyPrefixes = new string[]{ "-", "/", "--" };

private static bool IsExcludedFromRestore(string argument)
=> propertyPrefixes.Any(prefix => argument.StartsWith($"{prefix}property:TargetFramework=", StringComparison.Ordinal)) ||
Expand Down
15 changes: 9 additions & 6 deletions src/Cli/dotnet/commands/dotnet-complete/ParseCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,29 @@ public static int Run(ParseResult result)
{
result.HandleDebugSwitch();

Console.WriteLine(result.Diagram());
var tokens = result.Tokens.Skip(1).Select(t => t.Value).ToArray();
var reparsed = Microsoft.DotNet.Cli.Parser.Instance.Parse(tokens);
Console.WriteLine(reparsed.Diagram());

if (result.UnparsedTokens.Any())

if (reparsed.UnparsedTokens.Any())
{
Console.WriteLine("Unparsed Tokens: ");
Console.WriteLine(string.Join(" ", result.UnparsedTokens));
Console.WriteLine(string.Join(" ", reparsed.UnparsedTokens));
}

var optionValuesToBeForwarded = result.OptionValuesToBeForwarded(ParseCommandParser.GetCommand());
var optionValuesToBeForwarded = reparsed.OptionValuesToBeForwarded(ParseCommandParser.GetCommand());
if (optionValuesToBeForwarded.Any())
{
Console.WriteLine("Option values to be forwarded: ");
Console.WriteLine(string.Join(" ", optionValuesToBeForwarded));
}
if (result.Errors.Any())
if (reparsed.Errors.Any())
{
Console.WriteLine();
Console.WriteLine("ERRORS");
Console.WriteLine();
foreach (var error in result.Errors)
foreach (var error in reparsed.Errors)
{
Console.WriteLine(error?.Message);
}
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 @@ -2,9 +2,12 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using FluentAssertions;
using Microsoft.DotNet.Cli;
using Microsoft.DotNet.Tools;
using Microsoft.DotNet.Tools.Build;
using Microsoft.DotNet.Tools.Publish;
using System.CommandLine;
using System.Linq;
using Xunit;
using Xunit.Abstractions;

Expand All @@ -22,18 +25,35 @@ public MSBuildArgumentCommandLineParserTests(ITestOutputHelper output)
[Theory]
[InlineData(new string[] { "-property:prop1=true", "-p:prop2=false" }, true)]
[InlineData(new string[] { "-property:prop1=true", "-p:prop2=false" }, false)]
[InlineData(new string[] { "-p:teamcity_buildConfName=\"Build, Test and Publish\"" }, false)]
[InlineData(new string[] { "-p:teamcity_buildConfName=\"Build, Test and Publish\"" }, true)]
[InlineData(new string[] { "-detailedSummary" }, true)]
[InlineData(new string[] { "-clp:NoSummary" }, true)]
[InlineData(new string[] { "-orc" }, true)]
[InlineData(new string[] { "-orc" }, false)]
public void MSBuildArgumentsAreForwardedCorrectly(string[] arguments, bool buildCommand)
{
RestoringCommand command = buildCommand ?
BuildCommand.FromArgs(arguments) :
RestoringCommand command = buildCommand ?
BuildCommand.FromArgs(arguments) :
PublishCommand.FromArgs(arguments);
command.GetArgumentsToMSBuild().Split(' ')
.Should()
.Contain(arguments);
var expectedArguments = arguments.Select(a => a.Replace("-property:", "--property:").Replace("-p:", "--property:"));
var argString = command.MSBuildArguments;

foreach (var expectedArg in expectedArguments)
{
argString.Should().Contain(expectedArg);
}
}

[Theory]
[InlineData(new string[] { "-p:teamcity_buildConfName=\"Build, Test and Publish\"" }, new string[] { "--property:teamcity_buildConfName=\"Build, Test and Publish\"" })]
[InlineData(new string[] { "-p:prop1=true", "-p:prop2=false" }, new string[] { "--property:prop1=true", "--property:prop2=false" })]
[InlineData(new string[] { "-p:prop1=\".;/opt/usr\"" }, new string[] { "--property:prop1=\".%3B/opt/usr\"" })]
public void Can_pass_msbuild_properties_safely(string[] tokens, string[] forwardedTokens) {
var forwardingFunction = (CommonOptions.PropertiesOption as ForwardedOption<string[]>).GetForwardingFunction();
var result = CommonOptions.PropertiesOption.Parse(tokens);
var parsedTokens = forwardingFunction(result);
parsedTokens.Should().BeEquivalentTo(forwardedTokens);
}
}
}
Loading