Fix quoting in MSBuild function in tools.sh

[omajid]

Without this, any arguments that need proper quoting (eg, an argument contains spaces) get mangled.

I can reproduce the issue via the VMR by using something like:

$ ./build.sh "/p:OfficialBuilder=foo bar"

This builds on top of dotnet/installer#19686, which fixed quoting in build.sh.

[omajid]

This isn't an exhaustive fix, just the minimal to get the VMR to build with a space-containing property.

I ran ShellCheck and it flags several other places where we mix arrays and strings and expand arrays incorrectly:

$ shellcheck -S error eng/common/tools.sh

In eng/common/tools.sh line 256:
    local name="$variationName[@]"
                ^-- SC1087 (error): Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).


In eng/common/tools.sh line 276:
  echo "Trying to run '$@' for maximum of $maxRetries attempts."
                       ^-- SC2145 (error): Argument mixes string and array. Use * or separate argument.


In eng/common/tools.sh line 281:
      echo "Ran '$@' successfully."
                 ^-- SC2145 (error): Argument mixes string and array. Use * or separate argument.


In eng/common/tools.sh line 286:
    echo "Failed to execute '$@'. Waiting $timeout seconds before next attempt ($retries out of $maxRetries)." 1>&2
                             ^-- SC2145 (error): Argument mixes string and array. Use * or separate argument.


In eng/common/tools.sh line 290:
  echo "Failed to execute '$@' for $maxRetries times." 1>&2
                           ^-- SC2145 (error): Argument mixes string and array. Use * or separate argument.

For more information:
  https://www.shellcheck.net/wiki/SC1087 -- Use braces when expanding arrays,...
  https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ...

This warning is also concerning, since it flags a variable used in the inner build command for the VMR:

In eng/common/tools.sh line 500:
    export ARCADE_BUILD_TOOL_COMMAND="$_InitializeBuildTool $@"
                                     ^------------------------^ SC2124 (warning): Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.

Although the VMR builds fine in source-build without any other changes.

[omajid]

cc @am11 @MattGal @riarenas @ViktorHofer

[MattGal]

cc @am11 @MattGal @riarenas @ViktorHofer

LGTM but I am no longer on .NET, sadly :(

[am11]

Over at runtime, we support it like this: https://github.com/dotnet/runtime/blob/13d753ce9faa06e7fa77c070b580948759b6bc0e/eng/build.sh#L565-L568. It was done so other usages of this file (namely; almost the entire dotnet org repos) are not affected.

[akoeplinger]

@am11 do you know of any places where fixing it like this would cause issues?

[am11]

I can't say for sure but one way to find out; do it in YOLO mode, fingers crossed 🤞 😅. It was just a caution we took when allowing runtime/build.sh --cmakeargs etc. to have spaces / semicolons in value.

[akoeplinger]

Ok, let's find out then 😄

[akoeplinger]

The only thing it "broke" which needed reacting so far was this code in aspnetcore which passed "/p:RunTemplateTests=false /p:SkipHelixReadyTests=true" as a single string: dotnet/aspnetcore@76c2012#diff-893d2c8bf0a06b7b35a7f446eccfac758030015251187e087b18c44830e8c8fbL576-R576