[GN] Locate prebuilt binaries correctly.

[hctim]

Summary

Currently downloading a prebuilt binary and not having it on $PATH fails ungracefully, as subprocess.call doesn't like being called to a binary that doesn't exist. This should hopefully solve this problem for Linux, OSX and Windows, but I can only test the Linux component.

[pcc]

llvm/utils/gn/gn.py

41

Maybe you could pass shell=True here instead? On my machine at least, that causes the shell to fail if the command is not found instead of an exception being thrown.

[thakis]

Thanks for the patch1

llvm/utils/gn/gn.py

41

I like this suggestion, but shell=True means that the args array is interpreted differently on Win and non-Win. On non-Win, the args are passed to the shell and you have to make the first item something like 'gn --version' as a single string, while on Windows iirc you have to have two list entries (or the other way round)?

47

There's distutils.spawn.find_executable() which does this, but it has the problem that if you happen to have depot_tools on your path, that includes a dummy gn binary that tries to forward to some real gn binary that's present in depot_tools using projects but not in llvm. So this would claim that the system binary is available but it would then fail to run.

So you'd have to check not system_binary_available(gn) or subprocess.call([gn, '--version']...

[pcc]

llvm/utils/gn/gn.py

41

A quick test on Linux and Windows reveals that passing a string instead of an array for args appears to do the same thing on both platforms. This is also how the documentation suggests using subprocess.call with shell=True:
https://docs.python.org/2/library/subprocess.html

[thakis]

llvm/utils/gn/gn.py

41

hctim, does using subprocess.call('gn --version', ..., shell=True) work for you?

[hctim]

llvm/utils/gn/gn.py

41

Yep, adding shell=True works fine for me. Would you prefer that oneliner to be committed or this change?

[thakis]

llvm/utils/gn/gn.py

41

I'd prefer the oneliner since this change as-is breaks the depot_tools-with-existing-but-broken-gn-binary-on-path use case.

[hctim]

llvm/utils/gn/gn.py

41

No problems. I've sumitted the small change as rL355645. Will abandon this change :)

llvm/utils/gn/gn.py

 
 def print_no_gn(mention_get):
     print('gn binary not found in PATH')
     if mention_get:
         print('run llvm/utils/gn/get.py to download a binary and try again, or')
     print('follow https://gn.googlesource.com/gn/#getting-started')
     return 1
 
+def system_binary_available(binary_name):
+    if sys.platform == 'win32':
+        if subprocess.call(['WHERE', '/q', binary_name],
+                           stdout=open(os.devnull, 'w'),
+                           stderr=subprocess.STDOUT) == 0:
+            return True
+    elif os.name == 'posix':
+        if subprocess.call(['which', binary_name],
+                           stdout=open(os.devnull, 'w'),
+                           stderr=subprocess.STDOUT) == 0:
+            return True
+    return False
 
 def main():
     # Find real gn executable.
     gn = 'gn'
-    if subprocess.call([gn, '--version'], stdout=open(os.devnull, 'w'),
-                                          stderr=subprocess.STDOUT) != 0:
+    if not system_binary_available(gn):
         # Not on path. See if get.py downloaded a prebuilt binary and run that
         # if it's there, or suggest to run get.py if it isn't.
         platform = get_platform()
         if not platform:
             return print_no_gn(mention_get=False)
         gn = os.path.join(os.path.dirname(__file__), 'bin', platform, 'gn')
         if not os.path.exists(gn + ('.exe' if platform == 'windows' else '')):
             return print_no_gn(mention_get=True)