JDevlieghere added a comment.
In D78762#2002390 <https://reviews.llvm.org/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 just need to specify it in triplicate if you want
> compatibility across all the versions, but specifying `-DPython3_EXECUTABLE=`
> gives you the control over the executable for python3. If you want to use
> python2, `-DPython3_EXECUTABLE=` and `-DPython2_EXECUTABLE=` will ensure that
> python2 is always used. If you are using an older CMake, specifying
> `-DPython3_EXECUTABLE=`, `-DPython2_EXECUTABLE=` and `-DPYTHON_EXECUTABLE=`
> will ensure that the specified version is used. I'm not sure what the
> scenario is that you believe will be made more difficult with this approach.
It only //works// because you're setting Python3_EXECUTABLE to
Python2_EXECUTABLE.
set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
This will be completely opaque to lldb and we'll have to struggle again to
reverse engineer which Python is used, leaving us in the same situation as
today. How are we going to choose between Python 2 and Python 3 based on that
variable? What I envision is to find Python once, like we do with other
dependencies in the LLVM project, and then use that (interpreter, libraries,
include paths) in LLDB. This has to work for in-tree-builds as well as
out-of-tree builds (I'm not a fan but hey they're still supported).
In D78762#2005407 <https://reviews.llvm.org/D78762#2005407>, @ldionne wrote:
> In D78762#2002305 <https://reviews.llvm.org/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` and finally `LLVM_PYTHON_HINT`. We can then
> > populate the first three depending on what machinery we want to use, i.e.
> > the old CMake way of finding Python (until we bump the requirement across
> > llvm), as well as the new `FindPython3` and `FindPython2`. Similarly for
> > the `HINT` variable, having our own abstraction means we don't clobber any
> > of the variables used internally by CMake.
> >
> > What do you think?
>
>
> This is not better IMHO since it assumes that all subprojects are using the
> `LLVM_meow` variable to refer to the Python executable.
This patch is already changing the variable. How is this different/worse from
using `Python3_EXECUTABLE` and having it **maybe** point to a Python 2
interpreter?
================
Comment at: clang/CMakeLists.txt:154
+ message(WARNING "Python3 not found, using python2 as a fallback")
+ find_package(Python3 COMPONENTS Interpreter REQUIRED)
+ if(Python2_VERSION VERSION_LESS 2.7)
----------------
Python2?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78762/new/
https://reviews.llvm.org/D78762
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
