cake-build / cake

:cake: Cake (C# Make) is a cross platform build automation system.
https://cakebuild.net
MIT License
3.84k stars 722 forks source link

MSBuild properties are incorrectly formatted #4297

Open Lordfirespeed opened 3 months ago

Lordfirespeed commented 3 months ago

Prerequisites

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 commented 3 months ago

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 commented 3 months ago

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

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