MSBuild.build generates an incorrect command line (FAKE 5)

[softlion]

Description

The following script part generates a command line which fails, because the /p switch is inside a string:

Target.create "SchemaCompare" (fun _ ->
    let setParams (defaults:MSBuildParams) =
            { defaults with
                Verbosity = Some(Quiet)
                Targets = ["SqlSchemaCompare"]
                Properties =
                    [
                        "SqlScmpFilePath", scmpFilePath
                        "source", sourceDacpac
                        "target", targetConnectionString
                        "Deploy","true"
                    ]
            }

    MSBuild.build setParams "myproj.sqlproj"
)

Generates this command line, which fails:

"C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\MSBuild.exe" "myproj.sqlproj" "/t:SqlSchemaCompare" "/m" "/nodeReuse:False" "/v:q" "/p:RestorePackages=False" "/p:SqlScmpFilePath=C:\xxx\xxx.scmp" "/p:source=C:\xxx\Output\xxx.dacpac" "/p:target=Data Source=xxx,1433;Initial Catalog=xxx;User Id=xxx;Password=xxx;Integrated Security=False;Persist Security Info=True;Connect Timeout=30;Encrypt=True;MultipleActiveResultSets=True" "/p:Deploy=true" "/bl:C:\xxx\Temp\tmpxxxx.tmp.binlog"

Repro steps

see above.

Expected behavior

no fail.

Actual behavior

MSBUILD : error MSB1006: property is not valid
Switch : 1433

Known workarounds

A command which works would be the following one. The only difference is the quote.

/p:"target=..." instead of "/p:target=..."

"C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\MSBuild.exe" "myproj.sqlproj" "/t:SqlSchemaCompare" "/m" "/nodeReuse:False" "/v:q" "/p:RestorePackages=False" "/p:SqlScmpFilePath=C:\xxx\xxx.scmp" "/p:source=C:\xxx\Output\xxx.dacpac" /p:"target=Data Source=xxx,1433;Initial Catalog=xxx;User Id=xxx;Password=xxx;Integrated Security=False;Persist Security Info=True;Connect Timeout=30;Encrypt=True;MultipleActiveResultSets=True" "/p:Deploy=true" "/bl:C:\xxx\Temp\tmpxxxx.tmp.binlog"

Related information

• Operating system: Win10
• Branch: stable
• Indications of severity: no workaround!
• Version of FAKE: 5.X (dotnet tool install fake-cli -g)

[matthid]

From a command line escaping perspective there should be no difference between using the quotes after or before :

[softlion]

It seems the problem is caused by the comma ",1433"

[softlion]

If i write this command in terminal, and the comma ',1433' is removed, it does not work but it does not complain. If the comma is there, it complains. If the /p: is outside the quotes, it works fine.

[matthid]

maybe comma is a special syntax to start another argument in msbuild and there need to be additional quotes around the property value, no idea

[matthid]

By the way I think one workaround is to use environment variables instead of cli arguments. I think even dotnet cli is doing that for some stuff as there are dome edge cases impossible to escape properly

[softlion]

Dunno how to do that with env vars.

[softlion]

The workaround is working fine

    let msBuildParams, argsString = MSBuild.buildArgs(setParams)
    let args = Process.toParam project + " " + argsString.Replace("\"/p:","/p:\"")
    
    let callMsBuildExe args =
        let result =
            Process.execWithResult (fun info ->
            { info with
                FileName = msBuildParams.ToolPath
                Arguments = args }
            |> Process.setEnvironment msBuildParams.Environment) TimeSpan.MaxValue
        if not result.OK then
            failwithf "msbuild failed with exitcode '%d'" result.ExitCode
        String.Join("\n", result.Messages)

    let handleAfterRun command exitCode project =
        let msgs =
                []
        if exitCode <> 0 then
            let errors =
                msgs
                |> List.choose (fun m -> if m.IsError then Some m.Message else None)
            let errorMessage = sprintf "'%s %s' failed with exitcode %d." command project exitCode
            raise (MSBuildException(errorMessage, errors))

    //let binlogPath, args = addBinaryLogger msBuildParams.ToolPath callMsBuildExe args msBuildParams.DisableInternalBinLog
    Trace.tracefn "Building project: %s\n  %s %s" project msBuildParams.ToolPath args
    let exitCode =
        Process.execSimple (fun info ->
        { info with
            FileName = msBuildParams.ToolPath
            Arguments = args }
        |> Process.setEnvironment msBuildParams.Environment) TimeSpan.MaxValue
    handleAfterRun "msbuild" exitCode project
    __.MarkSuccess()

[matthid]

maybe @vbfox has an idea what's going on there?

[matthid]

@softlion Are you sure your workaround is doing what you want?

I just tried to reproduce this with:

<Project>
  <Target Name="Test">
    <Message Importance="high" Text="Prop: $(MyProp)" />  
  </Target>
</Project>

>msbuild test.proj /t:Test /p:MyProp=asdas
Prop: asdas
>msbuild test.proj /t:Test /p:"target=Data Source=xxx,1433;Initial Catalog=xxx;User Id=xxx;Password=xxx;
Integrated Security=False;Persist Security Info=True;Connect Timeout=30;Encrypt=True;MultipleActiveResultSets=True"
Prop:

And the other escaping indeed leads to the error you did report.

[matthid]

It seems with comma you can assign multiple properties with a single /p:

<Project>
  <Target Name="Test">
    <Message Importance="high" Text="$Prop: $(MyProp)" />
    <Message Importance="high" Text="$Other: $(Other)" />
  </Target>
</Project>

>msbuild test.proj /t:Test /p:MyProp=asdas,Other=asd
$Prop: asdas
$Other: asd

[softlion]

Yes the msbuild command is running fine when /p: is outside the double quotes. If it is inside, even if i remove the comma of ,1433 it does not work. Either with a message, or with no message at all (which is strange). I copy/pasted the command manually to verify while creating the workaround.

[matthid]

Ok I think I figured it out: dotnet/msbuild#2999

The correct thing to to is to use:
msbuild test.proj /t:Test "/p:MyProp=\"Data Source=xxx,1433;Initial Catalog=xxx;User Id=xxx;Password=xxx;Integrated Security=False;Persist Security Info=True;Connect Timeout=30;Encrypt=True;MultipleActiveResultSets=True\""

(ie set the "value" of the properties in quotes as well) However, we need to figure out how msbuild expectes escaping there

[matthid]

I think we will just add msbuild escaping here https://docs.microsoft.com/en-us/visualstudio/msbuild/msbuild-special-characters?view=vs-2017 (including comma) from my quick tests that seems to work the best

[matthid]

@softlion Should be fixed in the next release, thanks for reporting