This is an archive of the discontinued LLVM Phabricator instance.

gn build: Unbreak get.py and gn.py on Windows
ClosedPublic

Authored by thakis on Mar 7 2019, 3:35 PM.

Details

Summary

Inspired by the recent changes here I figured I'd test these on Windows. Looks like I haven't done that in a while, they were both broken.

os.uname() doesn't exist on Windows, so use platform.machine() which returns os.uname()[4] on non-Win and (on 64-bit systems) "AMD64" on Windows. Also use sys.platform instead of platform to check for Windows-ness for the file extension in gn.py (get.py got this right).

Using [gn, '--version'] works on Windows, but I think it does the wrong thing on POSIX where it passes --version to the shell instead of to gn, so I think after r355645 the "broken gn on PATH" case isn't working right on POSIX. I'll leave that to you to fix.

Diff Detail

Event Timeline

thakis created this revision.Mar 7 2019, 3:35 PM
Herald added a project: Restricted Project. · View Herald Transcript
hctim resigned from this revision.Mar 7 2019, 3:45 PM
hctim added a subscriber: hctim.

LGTM, but as I don't have a machine to test this on I'll leave up to @pcc. Will patch up the stringification of the subprocess call and cc you there.

pcc accepted this revision.Mar 7 2019, 3:51 PM

LGTM

This revision is now accepted and ready to land.Mar 7 2019, 3:51 PM
This revision was automatically updated to reflect the committed changes.
thakis added a comment.Mar 8 2019, 5:01 AM

I landed the follow-up to the shell=True change in r355694 since it wasn't done yet and I had three spare minutes this morning.