Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Fix Python binding generation build step on Windows

Authored by enlight on Oct 8 2016, 8:00 AM.



If Python is installed to a location that contains spaces (e.g. "C:\Program Files\Python3") then the build fails while attempting to run the script because the path to the Python executable is not double-quoted before being passed to the shell. The fix consists of letting Python handle the formatting of the command line, since subprocess.Popen() is perfectly capable of handling paths containing spaces if it's given the command and arguments as a list instead of a single pre-formatted string.

This fixed the build for me on Windows 10, and didn't seem to break the build on Ubuntu 14.10.

Diff Detail


Event Timeline

enlight updated this revision to Diff 74036.Oct 8 2016, 8:00 AM
enlight retitled this revision from to Fix Python binding generation build step on Windows.
enlight updated this object.
enlight added reviewers: zturner, clayborg.
enlight set the repository for this revision to rL LLVM.
enlight added a project: Restricted Project.
clayborg accepted this revision.Oct 10 2016, 12:46 PM
clayborg edited edge metadata.
This revision is now accepted and ready to land.Oct 10 2016, 12:46 PM

@zturner Are you good with this going in?

This revision was automatically updated to reflect the committed changes.
ldrumm added a subscriber: ldrumm.Oct 13 2016, 7:30 AM
ldrumm added inline comments.

Seeing as we're logging a failure to call our command here, perhaps using the repr format string type:

logging.error("failed to run %r: %s", command, script_stderr) would give the exact invocation of the failed command, and means we're not precomputing a "pretty" - possibly false representation of the command line.

We can the get rid of command_line = " ".join(command) as it's no longer needed


Same here.

amccarth added inline comments.

When an error is logged, it's handy to be able to copy and paste the command line in order to reproduce/debug the failure. In that case, the repr format isn't particularly useful. I'd rather have the "pretty" format, even if I have to fix a few quoting problems.