[PATCH] D78762: build: use `find_package(Python3)` if available
Nico Weber via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 29 13:28:14 PDT 2020
thakis added a comment.
I believe this breaks check-builtins on macOS. See https://bugs.chromium.org/p/chromium/issues/detail?id=1076480 for details.
The `split()` on the subprocess output throws since it wants a b'.' on py3, but that code has an unqualified except block. That means (I think) we always believe we run on macOS 10.0.0 under py3. I don't understand how we then fail to convert that back to an int (see linked bug).
Anyhoo, please take a look, and if it takes a while to fix we might have to go through yet another reland cycled :/
In D78762#2006872 <https://reviews.llvm.org/D78762#2006872>, @compnerd wrote:
> I was trying to do the minimal change to cover llvm. I definitely care about LLD and will do that in a subsequent change.
>
> As to the benefit of this, it is primarily that the detection here explicitly ensures that python3 is used rather than python2. The fallback of aliasing is the easiest way to achieve this without having a lot of complicated logic at each site:
>
> if(NOT Python3_Interpreter_FOUND)
> add_custom_command(...)
> else()
> add_custom_command(...)
> endif()
I would've expected we'd just set some new variable, eg LLVM_PYTHON_INTERPRETER to either the py2 or py3 interpreter and use that everywhere. Seems a bit less confusing, and shouldn't make anything else more complicated. But it's all going away in a few months anyways, so I don't have a strong opinion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78762/new/
https://reviews.llvm.org/D78762
More information about the cfe-commits
mailing list