| << Back to previous view |
|
[QB-2711] Regression in MSBuild properties - no longer quoted
|
|
| Status: | Resolved |
| Project: | QuickBuild |
| Component/s: | None |
| Affects Version/s: | 6.1.14 |
| Fix Version/s: | 6.1.15 |
| Type: | Bug | Priority: | Major |
| Reporter: | Scott Hunter | Assigned To: | Unassigned |
| Resolution: | Fixed | Votes: | 0 |
| Remaining Estimate: | Unknown | Time Spent: | Unknown |
| Original Estimate: | Unknown | ||
| Environment: | Windows | ||
| Description |
|
After upgrading to 6.1.14 our MSBuild tasks no longer run. It looks like this is due to an incorrect change in the way properties are passed on the command line. The double-quoting of property values no longer works properly.
In our case, we have a variable named NuGetPath which is set to C:\BuildToolsQB\windows\nuget\2.8.60717.93\NuGet.exe this is passed to MSBuild using the Build Properties list with name NuGetPath and value ${vars.getValue("NuGetPath")} after upgrading, I can see that the command line is no longer quoted, and the backslashes are messed up: /property:NuGetPath=C:BuildToolsQBwindows\nuget.8.60717.93NuGet.exe Looking in the source, I can see that the setProperties method in MSBuildCommand was changed in a recent version, and the new version of the code handles quoting of properties incorrectly. The previous version of the method works correctly by using addArgValue to quote the entire command line argument which is the correct approach. |
| Comments |
| Comment by Scott Hunter [ 05/May/16 05:31 PM ] |
|
Upon further examination, the previous version did not quote the arguments either (in this case they are not necessary).
The problem is still in the use of addArgLine though. I believe the correct code would be: for (Property each : properties) { String str = String.format("/property:%s=%s", each.getName(), each.getValue()); addArgValue(str); } Each property should be an individual argument. Let ProcessBuilder handle double-quoting if necessary (e.g. if name or value contain whitespace). |
| Comment by Steve Luo [ 06/May/16 11:12 AM ] |
|
Hi Scott,
You are right. We intended to fix issue |