[PATCH] D78762: build: use `find_package(Python3)` if available

hans added a comment. Herald added a project: clang-tools-extra. What's the new cmake flag to set the python executable, and should we update llvm/docs/GettingStarted.rst to not refer to PYTHON_EXECUTABLE anymore? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D78762: build: use `find_package(Python3)` if available

thakis added a comment. I attempted a fix in e071ea48e923651ae21b9b684b0473248630322c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78762/new/ https://reviews.llvm.org/D78762

[PATCH] D78762: build: use `find_package(Python3)` if available

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)

[PATCH] D78762: build: use `find_package(Python3)` if available

thakis added a comment. Is there more background on this somewhere? What's the advantage of using Python3_EXECUTABLE instead of just PYTHON_EXECUTABLE? Having Python3_EXECUTABLE be python 2 seems pretty surprising to me. Also, this doesn't update some PYTHON_EXECUTABLEs, e.g. the one in

[PATCH] D78762: build: use `find_package(Python3)` if available

compnerd added a comment. 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

[PATCH] D78762: build: use `find_package(Python3)` if available

compnerd closed this revision. compnerd added a comment. rGcd84bfb8142bc7ff3a07a188ffb809f1d86d1fd7 (with the Python2 fixes) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D78762: build: use `find_package(Python3)` if available

compnerd marked an inline comment as done. compnerd added inline comments. Comment at: llvm/CMakeLists.txt:696 +message(WARNING "Python3 not found, using python2 as a fallback") +find_package(Python3 COMPONENTS Interpreter REQUIRED) +if(Python2_VERSION VERSION_LESS

[PATCH] D78762: build: use `find_package(Python3)` if available

smeenai added a comment. In D78762#2006406 , @JDevlieghere wrote: > I talked to Saleem and he cleared up some of my concerns. Given that the > community seems to have agreed to support only Python 3, this change seems a > lot more reasonable. My

[PATCH] D78762: build: use `find_package(Python3)` if available

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. I talked to Saleem and he cleared up some of my concerns. Given that the community seems to have agreed to support only Python 3, this change seems a lot more reasonable. My

[PATCH] D78762: build: use `find_package(Python3)` if available

compnerd added a comment. @JDevlieghere I think that for LLDB, which this patch does not touch, the proper thing to do is to have 2 different paths with the ability to explicitly opt into the path, i.e. `LLDB_USE_PYTHON2:BOOL` and `LLDB_USE_PYTHON3:BOOL` with `CMakeDependentOption` to prevent

[PATCH] D78762: build: use `find_package(Python3)` if available

JDevlieghere added a comment. In D78762#2002390 , @compnerd wrote: > @JDevlieghere I think that adding another mechanism for finding the python > executable is not the right approach. You already have the variables that > you are talking about, you

[PATCH] D78762: build: use `find_package(Python3)` if available

ldionne accepted this revision. ldionne added a comment. In D78762#2002305 , @JDevlieghere wrote: > My suggestion is to have 4 new CMake variable that abstract over this: > `LLVM_PYTHON_EXECUTABLE`, `LLVM_PYTHON_LIBRARIES`, `LLVM_PYTHON_INCLUDE_DIRS` >

[PATCH] D78762: build: use `find_package(Python3)` if available

compnerd updated this revision to Diff 260010. compnerd added a comment. Add missed case of `PYTHON_EXECUTABLE` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78762/new/ https://reviews.llvm.org/D78762 Files:

[PATCH] D78762: build: use `find_package(Python3)` if available

compnerd updated this revision to Diff 259987. compnerd added a comment. Adjust clang-tools-extra at the same time Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78762/new/ https://reviews.llvm.org/D78762 Files:

[PATCH] D78762: build: use `find_package(Python3)` if available

compnerd added a comment. @JDevlieghere I dont think that adding another mechanism for finding the python executable is not the right approach. You already have the variables that you are talking about, you just need to specify it in triplicate if you want compatibility across all the

[PATCH] D78762: build: use `find_package(Python3)` if available

JDevlieghere requested changes to this revision. JDevlieghere added a comment. This revision now requires changes to proceed. I would strongly prefer to do this differently. While we hope to drop Python 2 support in LLDB as soon as possible, we are not there yet. This patch as it stands will

[PATCH] D78762: build: use `find_package(Python3)` if available

compnerd updated this revision to Diff 259923. compnerd added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Include clang as some of the CI uses the unified build which fails without the update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST