cake icon indicating copy to clipboard operation
cake copied to clipboard

MSBuild properties are incorrectly formatted

Open Lordfirespeed opened this issue 1 year ago • 2 comments

Prerequisites

  • [X] I have written a descriptive issue title
  • [X] I have searched issues to ensure it has not already been reported

Cake runner

Cake Frosting

Cake version

4.0.0

Operating system

Linux

Operating system architecture

64-Bit

CI Server

No response

What are you seeing?

MSBuild arguments are rendered like so:

/property:DefineConstants=foo;bar;baz

Which leads to an error: image

What is expected?

MSBuild properties should be specified in the correct format. For example:

/property:DefineConstants=foo;DefineConstants=bar;DefineConstants=baz

Steps to Reproduce

Create a task with the following Run implementation:

var buildSettings = new DotNetBuildSettings {
    Configuration = "Release",

    MSBuildSettings = new() {
        Properties = {
            {"DefineConstants", ["foo", "bar", "baz"] },
        },
    },
};

context.DotNetBuild(context.RootDirectory, buildSettings);

Ensure that context.RootDirectory is defined.

Run the task.

Notes

Related: #1852 Of note, I believe this comment is incorrect. Perhaps WithProperty("DefineConstants", "A=a", "B=b"); should instead generate:

/p:DefineConstants=A=a;DefineConstants=B=b

Lordfirespeed avatar Mar 10 '24 12:03 Lordfirespeed

Leaving this here for any future unfortunate souls - When working with DefineConstants, semicolons within its property value need to be escaped with %3B.

The solution I proposed above will not work, each subsequence DefineConstants overwrites the value of the previous. The rendered constants ["foo", "bar", "baz'] should loook like:

/p:DefineConstants=foo%3Bbar%3Bbaz

Lordfirespeed avatar Mar 10 '24 12:03 Lordfirespeed

This does beg the question: Why are semicolons explicitly not escaped for DefineConstants?

private static readonly HashSet<string> _propertiesNotEscapeSemicolons = new HashSet<string>
{
    "DefineConstants",
    "ExcludeFilesFromDeployment"
};

Lordfirespeed avatar Mar 10 '24 12:03 Lordfirespeed