[PATCH] D59003: [GN] Locate prebuilt binaries correctly.

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

Thanks for the patch1



================
Comment at: llvm/utils/gn/gn.py:41
-    if subprocess.call([gn, '--version'], stdout=open(os.devnull, 'w'),
-                                          stderr=subprocess.STDOUT) != 0:
         # Not on path. See if get.py downloaded a prebuilt binary and run that
----------------
pcc wrote:
> 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.
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)?


================
Comment at: llvm/utils/gn/gn.py:47
+            return True
+    return False
 
----------------
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']...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59003/new/

https://reviews.llvm.org/D59003