Incorrect escaping of semi-colon in property values for MS Build

[georgehemmings]

What You Are Seeing?

When using .WithProperty("DefineConstants", "A=a;B=b".Quote()); the semi-colon is escaped which is invalid. The escaping behaviour was added in #1625

What is Expected?

The semi-colon should not be escaped in this scenario as the resulting command is invalid.

What version of Cake are you using?

0.22.2

Are you running on a 32 or 64 bit system?

64

What environment are you running on? Windows? Linux? Mac?

Windows

Are you running on a CI Server? If so, which one?

TeamCity

How Did You Get This To Happen? (Steps to Reproduce)

MS Build with property.

.WithProperty("DefineConstants", "A=a;B=b".Quote());

Output Log

Executing: "C:/Program Files (x86)/MSBuild/14.0/Bin/MSBuild.exe" /v:minimal /p:Configuration="Release" /p:Platform=x86 /p:WarningLevel=0 /p:DefineConstants="A=a%3BB=b" /target:Rebuild "C:/code/x/src/x/xSetup.wixproj"
Microsoft (R) Build Engine version 14.0.25420.1
Copyright (C) Microsoft Corporation. All rights reserved.

C:\code\x\src\xSetup\x.wxs(15): error CNDL0150: Undefined preprocess
or variable '$(var.ProductName)'. [C:\code\x\src\xSetup\xSetup.wixpr
oj]
An error occurred when executing task 'Package'.
Error: Cake.Core.CakeException: MSBuild: Process returned an error (exit code 1).
   at Cake.Core.Tooling.Tool`1.ProcessExitCode(Int32 exitCode)
   at Cake.Core.Tooling.Tool`1.Run(TSettings settings, ProcessArgumentBuilder arguments, ProcessSettings processSettings, Action`1 postAction)
   at Cake.Common.Tools.MSBuild.MSBuildAliases.MSBuild(ICakeContext context, FilePath solution, MSBuildSettings settings)
   at Submission#0.<.ctor>b__2()
   at Cake.Core.ActionTask.Execute(ICakeContext context)
   at Cake.Core.DefaultExecutionStrategy.Execute(CakeTask task, ICakeContext context)
   at Cake.Core.CakeEngine.ExecuteTask(ICakeContext context, IExecutionStrategy strategy, Stopwatch stopWatch, CakeTask task, CakeReport report)
   at Cake.Core.CakeEngine.RunTarget(ICakeContext context, IExecutionStrategy strategy, String target)
   at Cake.Scripting.BuildScriptHost.RunTarget(String target)
   at Submission#0..ctor(Session session, Object& submissionResult)
   at Submission#0.<Factory>(Session session)
   at Roslyn.Scripting.CommonScriptEngine.Execute[T](String code, String path, DiagnosticBag diagnostics, Session session, Boolean isInteractive)
   at Roslyn.Scripting.Session.Execute(String code)
   at Cake.Core.Scripting.ScriptRunner.Run(IScriptHost host, FilePath scriptPath, IDictionary`2 arguments)
   at Cake.Commands.BuildCommand.Execute(CakeOptions options)
   at Cake.CakeApplication.Run(CakeOptions options)
   at Cake.Program.Main()

[devlead]

@cake-build/cake-team I kinda feel this WithProperty("DefineConstants", "A=a", "B=b"); should generate /p:DefineConstants=A=a;B=b that way we support both escaping and multiple values in a typed way.

Potentially one could have an dictionary of properties that doesn't support multiple properties beginning with DefineConstants.

[dchristensen]

I'm running into this issue as well, and I agree with @devlead that when passing multiple values into WithProperty they should be combined with a separating ; to support this scenario. Defining multiple values for the same parameter name on the command line doesn't make much sense anyway since only the last value will be available in the MSBuild script.

[marcus-n3rd]

I am having the same issue trying to pass the ExcludeFilesFromDeployment property with multiple files as the value and this functionality will help immensely.

[marcus-n3rd]

Though, I did find a way to workaround it using the ArgumentCustomization property:

string excludeFiles = "\"" + string.Join(";", configuration.ExcludeFilesFromDeployment) + "\"";
MSBuildSettings settings = new MSBuildSettings() {
  ArgumentCustomization = args=>args.Append("/p:ExcludeFilesFromDeployment=" + excludeFiles)
};

[wpspires]

Hi team,
How do you want this handled? Should semicolon just not be escaped? I could just remove it from the dictionary of escaped chars. Or are there conditions in which it should be kept vs removed?

[Joe-Dunleavy]

Hi, I'm willing to give this a shot. I will keep you updated on progress.

[Joe-Dunleavy]

Please see #1852 for proposed changes.

If this is not quite what you had in mind -- or if there are any issues, please let me know and I will do my best to amend them.