Albacore / albacore

Albacore is a professional quality suite of Rake tasks for building .NET or Mono based systems.
www.albacorebuild.net
221 stars 64 forks source link

CrossPlatformCmd.which does not quote paths when invoking where/which #167

Closed aloker closed 9 years ago

aloker commented 9 years ago

Albacore version: 2.4.2 Ruby version: 2.1 O/S: WIndows 8.1 Pro and Windows Server 2012

Symptom: if msbuild.exe is not in PATH, Albacore's build target will never use version 12 of the .NET build tools, even if they are installed correctly, but instead revert to version 4. Normally, the build task (build.rb) will first check if msbuild or xbuild are available in PATH, otherwise it tries to determine the path to the build tools by looking in the registry, specifically for the MSBuildToolsPath key in HKLM\SOFTWARE\Microsoft\MSBuild\ToolsVersions\{version}paths, looking for the latest version (one of 12.0, 4.0, 3.5, 2.0). Even though version 12 is installed, it won't recognize it. In my case, there's clearly an msbuild.exe in the registered path C:\Program Files (x86)\MSBuild\12.0\bin\

The problem: Albacore fails to verify the location of msbuild.exe v12 in said path because it contains spaces (OTOH, the path for version 4 for example is C:\Windows\Microsoft.NET\Framework\v4.0.30319\, which does not contains spaces).

I tracked down the issue and found that apparently CrossPlatformCmd.which is not able to work with paths that contain whitespaces. The command that is executed does not quote its parameters correctly.

There are currently three possible fixes 1) Change the registry values to 8.3 format, ie. C:\PROGRA~2\MSBuild\12.0\bin\ 2) Add quotes to the path sent to CrossPlatformCmd.which (specifically in Build.heuristic_executable) 3) Quote the paths in CrossPlatformCmd.which

aloker commented 9 years ago

Here's a patch that should fix this issue: https://gist.github.com/aloker/4d53960d0dc9e038522e

haf commented 9 years ago

Could you send a PR with that and tests that exercise the code, please? I'm more than willing to merge such a tests-passing PR.

haf commented 9 years ago

Ping