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

2022-02-14 Thread Hans Wennborg via Phabricator via cfe-commits 
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
  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

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

2020-04-29 Thread Nico Weber via Phabricator via cfe-commits 
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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-04-29 Thread Nico Weber via Phabricator via cfe-commits 
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 , @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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-04-27 Thread Nico Weber via Phabricator via cfe-commits 
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 lld/test. Is 
that intentional?


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

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

2020-04-27 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
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 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()

which would be the way to handle that.  Creating another wrapper function is a 
terrible thing just to filter out incidental python usage would not only be 
extremely complicated but a large amount of logic that would also increase 
configure times unnecessarily.

The detection here is far more robust and precise.  `Python3_EXECUTABLE` is the 
CMake variable name, not something that I chose.


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

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

2020-04-27 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
compnerd closed this revision.
compnerd added a comment.

rGcd84bfb8142bc7ff3a07a188ffb809f1d86d1fd7 
 (with the 
Python2 fixes)


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

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

2020-04-27 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
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 2.7)

smeenai wrote:
> Python2
Yeah, I have that fixed locally, there are 2 instances of it.


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

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

2020-04-27 Thread Shoaib Meenai via Phabricator via cfe-commits 
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 earlier comment was made under the impression that we 
> were going to continue supporting Python 2. With that in mind, the burden 
> should fall on LLDB if we want to continue supporting it. Python 2 will be a 
> "special case" then, rather than another first class citizen. With all that 
> said, this LGTM.


To be clear, the plan is to support Python 2 till Jan 2021 or so. Given that 
this has the fallback though, it LGTM (with the comments addressed).

http://lists.llvm.org/pipermail/llvm-dev/2020-January/138730.html is the 
discussion for LLVM's Python 2/3 plans.




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 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

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

2020-04-27 Thread Jonas Devlieghere via Phabricator via cfe-commits 
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 earlier comment was made under the impression that we 
were going to continue supporting Python 2. With that in mind, the burden 
should fall on LLDB if we want to continue supporting it. Python 2 will be a 
"special case" then, rather than another first class citizen. With all that 
said, this LGTM.


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

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

2020-04-27 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
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 the two from being set to `TRUE` 
simultaneously.  If two paths need to be supported, then you must have two 
paths.


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

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

2020-04-27 Thread Jonas Devlieghere via Phabricator via cfe-commits 
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 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 , @ldionne wrote:

> 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` 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

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

2020-04-27 Thread Louis Dionne via Phabricator via cfe-commits 
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` 
> 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.


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

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

2020-04-24 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
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:
  clang-tools-extra/test/lit.site.cfg.py.in
  clang/CMakeLists.txt
  clang/bindings/python/tests/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  clang/utils/perf-training/CMakeLists.txt
  clang/utils/perf-training/lit.site.cfg.in
  clang/utils/perf-training/order-files.lit.site.cfg.in
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/test/lit.site.cfg.py.in
  llvm/tools/llvm-shlib/CMakeLists.txt

Index: llvm/tools/llvm-shlib/CMakeLists.txt
===
--- llvm/tools/llvm-shlib/CMakeLists.txt
+++ llvm/tools/llvm-shlib/CMakeLists.txt
@@ -164,7 +164,7 @@
   endif()
 
   add_custom_command(OUTPUT ${LLVM_EXPORTED_SYMBOL_FILE}
-COMMAND ${PYTHON_EXECUTABLE} ${GEN_SCRIPT} --libsfile ${LIBSFILE} ${GEN_UNDERSCORE} --nm "${llvm_nm}" -o ${LLVM_EXPORTED_SYMBOL_FILE}
+COMMAND "${Python3_EXECUTABLE}" ${GEN_SCRIPT} --libsfile ${LIBSFILE} ${GEN_UNDERSCORE} --nm "${llvm_nm}" -o ${LLVM_EXPORTED_SYMBOL_FILE}
 DEPENDS ${LIB_NAMES} ${llvm_nm_target}
 COMMENT "Generating export list for LLVM-C"
 VERBATIM )
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -12,7 +12,7 @@
 config.llvm_shlib_ext = "@SHLIBEXT@"
 config.llvm_exe_ext = "@EXEEXT@"
 config.lit_tools_dir = path(r"@LLVM_LIT_TOOLS_DIR@")
-config.python_executable = "@PYTHON_EXECUTABLE@"
+config.python_executable = "@Python3_EXECUTABLE@"
 config.gold_executable = "@GOLD_EXECUTABLE@"
 config.ld64_executable = "@LD64_EXECUTABLE@"
 config.ocamlfind_executable = "@OCAMLFIND@"
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -117,7 +117,7 @@
 set(native_export_file "${target_name}.def")
 
 add_custom_command(OUTPUT ${native_export_file}
-  COMMAND ${PYTHON_EXECUTABLE} -c "import sys;print(''.join(['EXPORTS\\n']+sys.stdin.readlines(),))"
+  COMMAND "${Python3_EXECUTABLE}" -c "import sys;print(''.join(['EXPORTS\\n']+sys.stdin.readlines(),))"
 < ${export_file} > ${native_export_file}
   DEPENDS ${export_file}
   VERBATIM
@@ -1001,7 +1001,7 @@
   set(mangling itanium)
 endif()
 add_custom_command(OUTPUT ${exported_symbol_file}
-   COMMAND ${PYTHON_EXECUTABLE} ${LLVM_MAIN_SRC_DIR}/utils/extract_symbols.py --mangling=${mangling} ${static_libs} -o ${exported_symbol_file}
+   COMMAND "${Python3_EXECUTABLE}" ${LLVM_MAIN_SRC_DIR}/utils/extract_symbols.py --mangling=${mangling} ${static_libs} -o ${exported_symbol_file}
WORKING_DIRECTORY ${LLVM_LIBRARY_OUTPUT_INTDIR}
DEPENDS ${LLVM_MAIN_SRC_DIR}/utils/extract_symbols.py ${static_libs}
VERBATIM
@@ -1402,7 +1402,7 @@
   # empty list entries. So escape the ;s in the list and do the splitting
   # ourselves. cmake has no relpath function, so use Python for that.
   string(REPLACE ";" "\\;" pathlist_escaped "${pathlist}")
-  execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c" "\n
+  execute_process(COMMAND "${Python3_EXECUTABLE}" "-c" "\n
 import os, sys\n
 base = sys.argv[1]
 def haslink(p):\n
@@ -1477,7 +1477,6 @@
   # SHLIBDIR points the build tree.
   string(REPLACE "${CMAKE_CFG_INTDIR}" "${LLVM_BUILD_MODE}" SHLIBDIR "${LLVM_SHLIB_OUTPUT_INTDIR}")
 
-  set(PYTHON_EXECUTABLE ${PYTHON_EXECUTABLE})
   # FIXME: "ENABLE_SHARED" doesn't make sense, since it is used just for
   # plugins. We may rename it.
   if(LLVM_ENABLE_PLUGINS)
@@ -1635,7 +1634,7 @@
 ALLOW_EXTERNAL
 )
 
-  set(LIT_COMMAND "${PYTHON_EXECUTABLE};${lit_base_dir}/${lit_file_name}")
+  set(LIT_COMMAND "${Python3_EXECUTABLE};${lit_base_dir}/${lit_file_name}")
   list(APPEND LIT_COMMAND ${LIT_ARGS})
   foreach(param ${ARG_PARAMS})
 list(APPEND LIT_COMMAND --param ${param})
Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -639,7 +639,7 @@
 return()
   endif()
 
-  execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c" "import ${module}"
+  execute_process(COMMAND "${Python3_EXECUTABLE}" "-c" "import ${module}"
 RESULT_VARIABLE status
 ERROR_QUIET)
 
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -672,16 +672,38 @@
 
 include(HandleLLVMOptions)
 
-include(FindPythonInterp)
-if( NOT PYTHONINTERP_FOUND )
-  message(FATAL_ERROR
-"Unable to find Python interpreter, required

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

2020-04-24 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
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:
  clang-tools-extra/test/lit.site.cfg.py.in
  clang/CMakeLists.txt
  clang/bindings/python/tests/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  clang/utils/perf-training/CMakeLists.txt
  clang/utils/perf-training/lit.site.cfg.in
  clang/utils/perf-training/order-files.lit.site.cfg.in
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/test/lit.site.cfg.py.in

Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -12,7 +12,7 @@
 config.llvm_shlib_ext = "@SHLIBEXT@"
 config.llvm_exe_ext = "@EXEEXT@"
 config.lit_tools_dir = path(r"@LLVM_LIT_TOOLS_DIR@")
-config.python_executable = "@PYTHON_EXECUTABLE@"
+config.python_executable = "@Python3_EXECUTABLE@"
 config.gold_executable = "@GOLD_EXECUTABLE@"
 config.ld64_executable = "@LD64_EXECUTABLE@"
 config.ocamlfind_executable = "@OCAMLFIND@"
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -117,7 +117,7 @@
 set(native_export_file "${target_name}.def")
 
 add_custom_command(OUTPUT ${native_export_file}
-  COMMAND ${PYTHON_EXECUTABLE} -c "import sys;print(''.join(['EXPORTS\\n']+sys.stdin.readlines(),))"
+  COMMAND "${Python3_EXECUTABLE}" -c "import sys;print(''.join(['EXPORTS\\n']+sys.stdin.readlines(),))"
 < ${export_file} > ${native_export_file}
   DEPENDS ${export_file}
   VERBATIM
@@ -1001,7 +1001,7 @@
   set(mangling itanium)
 endif()
 add_custom_command(OUTPUT ${exported_symbol_file}
-   COMMAND ${PYTHON_EXECUTABLE} ${LLVM_MAIN_SRC_DIR}/utils/extract_symbols.py --mangling=${mangling} ${static_libs} -o ${exported_symbol_file}
+   COMMAND "${Python3_EXECUTABLE}" ${LLVM_MAIN_SRC_DIR}/utils/extract_symbols.py --mangling=${mangling} ${static_libs} -o ${exported_symbol_file}
WORKING_DIRECTORY ${LLVM_LIBRARY_OUTPUT_INTDIR}
DEPENDS ${LLVM_MAIN_SRC_DIR}/utils/extract_symbols.py ${static_libs}
VERBATIM
@@ -1402,7 +1402,7 @@
   # empty list entries. So escape the ;s in the list and do the splitting
   # ourselves. cmake has no relpath function, so use Python for that.
   string(REPLACE ";" "\\;" pathlist_escaped "${pathlist}")
-  execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c" "\n
+  execute_process(COMMAND "${Python3_EXECUTABLE}" "-c" "\n
 import os, sys\n
 base = sys.argv[1]
 def haslink(p):\n
@@ -1477,7 +1477,6 @@
   # SHLIBDIR points the build tree.
   string(REPLACE "${CMAKE_CFG_INTDIR}" "${LLVM_BUILD_MODE}" SHLIBDIR "${LLVM_SHLIB_OUTPUT_INTDIR}")
 
-  set(PYTHON_EXECUTABLE ${PYTHON_EXECUTABLE})
   # FIXME: "ENABLE_SHARED" doesn't make sense, since it is used just for
   # plugins. We may rename it.
   if(LLVM_ENABLE_PLUGINS)
@@ -1635,7 +1634,7 @@
 ALLOW_EXTERNAL
 )
 
-  set(LIT_COMMAND "${PYTHON_EXECUTABLE};${lit_base_dir}/${lit_file_name}")
+  set(LIT_COMMAND "${Python3_EXECUTABLE};${lit_base_dir}/${lit_file_name}")
   list(APPEND LIT_COMMAND ${LIT_ARGS})
   foreach(param ${ARG_PARAMS})
 list(APPEND LIT_COMMAND --param ${param})
Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -639,7 +639,7 @@
 return()
   endif()
 
-  execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c" "import ${module}"
+  execute_process(COMMAND "${Python3_EXECUTABLE}" "-c" "import ${module}"
 RESULT_VARIABLE status
 ERROR_QUIET)
 
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -672,16 +672,38 @@
 
 include(HandleLLVMOptions)
 
-include(FindPythonInterp)
-if( NOT PYTHONINTERP_FOUND )
-  message(FATAL_ERROR
-"Unable to find Python interpreter, required for builds and testing.
+if(CMAKE_VERSION VERSION_LESS 3.12)
+  include(FindPythonInterp)
+  if( NOT PYTHONINTERP_FOUND )
+message(FATAL_ERROR
+  "Unable to find Python interpreter, required for builds and testing.
 
-Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
-endif()
+  Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
+  endif()
+
+  if( ${PYTHON_VERSION_STRING} VERSION_LESS 2.7 )
+message(FATAL_ERROR "Python 2.7 or newer is required")
+  endif()
 
-if( ${PYTHON_VERSION_STRING} VERSION_LESS 2.7 )
-  message(FATAL_ERROR "Python 2.7 or newer is required")
+  add_executable(Python3::Interpreter IMPORTED)
+

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

2020-04-24 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
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 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.


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

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

2020-04-24 Thread Jonas Devlieghere via Phabricator via cfe-commits 
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 make it really hard for us to keep doing so (even internally) if 
upstream decides to drop support. I have looked into this change myself a while 
ago with these limitations in mind, and believe we can achieve the same in a 
way that would make this easier.

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?


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

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

2020-04-24 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
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 ACTION
  https://reviews.llvm.org/D78762/new/

https://reviews.llvm.org/D78762

Files:
  clang/CMakeLists.txt
  clang/bindings/python/tests/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  clang/utils/perf-training/CMakeLists.txt
  clang/utils/perf-training/lit.site.cfg.in
  clang/utils/perf-training/order-files.lit.site.cfg.in
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/test/lit.site.cfg.py.in

Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -12,7 +12,7 @@
 config.llvm_shlib_ext = "@SHLIBEXT@"
 config.llvm_exe_ext = "@EXEEXT@"
 config.lit_tools_dir = path(r"@LLVM_LIT_TOOLS_DIR@")
-config.python_executable = "@PYTHON_EXECUTABLE@"
+config.python_executable = "@Python3_EXECUTABLE@"
 config.gold_executable = "@GOLD_EXECUTABLE@"
 config.ld64_executable = "@LD64_EXECUTABLE@"
 config.ocamlfind_executable = "@OCAMLFIND@"
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -117,7 +117,7 @@
 set(native_export_file "${target_name}.def")
 
 add_custom_command(OUTPUT ${native_export_file}
-  COMMAND ${PYTHON_EXECUTABLE} -c "import sys;print(''.join(['EXPORTS\\n']+sys.stdin.readlines(),))"
+  COMMAND "${Python3_EXECUTABLE}" -c "import sys;print(''.join(['EXPORTS\\n']+sys.stdin.readlines(),))"
 < ${export_file} > ${native_export_file}
   DEPENDS ${export_file}
   VERBATIM
@@ -1001,7 +1001,7 @@
   set(mangling itanium)
 endif()
 add_custom_command(OUTPUT ${exported_symbol_file}
-   COMMAND ${PYTHON_EXECUTABLE} ${LLVM_MAIN_SRC_DIR}/utils/extract_symbols.py --mangling=${mangling} ${static_libs} -o ${exported_symbol_file}
+   COMMAND "${Python3_EXECUTABLE}" ${LLVM_MAIN_SRC_DIR}/utils/extract_symbols.py --mangling=${mangling} ${static_libs} -o ${exported_symbol_file}
WORKING_DIRECTORY ${LLVM_LIBRARY_OUTPUT_INTDIR}
DEPENDS ${LLVM_MAIN_SRC_DIR}/utils/extract_symbols.py ${static_libs}
VERBATIM
@@ -1402,7 +1402,7 @@
   # empty list entries. So escape the ;s in the list and do the splitting
   # ourselves. cmake has no relpath function, so use Python for that.
   string(REPLACE ";" "\\;" pathlist_escaped "${pathlist}")
-  execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c" "\n
+  execute_process(COMMAND "${Python3_EXECUTABLE}" "-c" "\n
 import os, sys\n
 base = sys.argv[1]
 def haslink(p):\n
@@ -1477,7 +1477,6 @@
   # SHLIBDIR points the build tree.
   string(REPLACE "${CMAKE_CFG_INTDIR}" "${LLVM_BUILD_MODE}" SHLIBDIR "${LLVM_SHLIB_OUTPUT_INTDIR}")
 
-  set(PYTHON_EXECUTABLE ${PYTHON_EXECUTABLE})
   # FIXME: "ENABLE_SHARED" doesn't make sense, since it is used just for
   # plugins. We may rename it.
   if(LLVM_ENABLE_PLUGINS)
@@ -1635,7 +1634,7 @@
 ALLOW_EXTERNAL
 )
 
-  set(LIT_COMMAND "${PYTHON_EXECUTABLE};${lit_base_dir}/${lit_file_name}")
+  set(LIT_COMMAND "${Python3_EXECUTABLE};${lit_base_dir}/${lit_file_name}")
   list(APPEND LIT_COMMAND ${LIT_ARGS})
   foreach(param ${ARG_PARAMS})
 list(APPEND LIT_COMMAND --param ${param})
Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -639,7 +639,7 @@
 return()
   endif()
 
-  execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c" "import ${module}"
+  execute_process(COMMAND "${Python3_EXECUTABLE}" "-c" "import ${module}"
 RESULT_VARIABLE status
 ERROR_QUIET)
 
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -672,16 +672,38 @@
 
 include(HandleLLVMOptions)
 
-include(FindPythonInterp)
-if( NOT PYTHONINTERP_FOUND )
-  message(FATAL_ERROR
-"Unable to find Python interpreter, required for builds and testing.
+if(CMAKE_VERSION VERSION_LESS 3.12)
+  include(FindPythonInterp)
+  if( NOT PYTHONINTERP_FOUND )
+message(FATAL_ERROR
+  "Unable to find Python interpreter, required for builds and testing.
 
-Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
-endif()
+  Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
+  endif()
+
+  if( ${PYTHON_VERSION_STRING} VERSION_LESS 2.7 )
+message(FATAL_ERROR "Python 2.7 or newer is required")
+  endif()
 
-if( ${PYTHON_VERSION_STRING} VERSION_LESS 2.7 )
-  message(FATAL_ERROR "Python 2.7 or newer is required")
+