[CMake] Don't set Python_ADDITIONAL_VERSIONS

[JDevlieghere]

Summary

Until recently, Python_ADDITIONAL_VERSIONS was used to limit LLVM's Python support to 2.7. Now that both LLVM and LLDB both support Python 3, there's no longer a need to put an arbitrary limit on this. However, instead of removing the variable, D64443 expanded the list , which has the (presumably unintentional) side-effect of expression preference for Python 3. Instead, as Michal proposed in the original differential, we should just not set the list at all, and let CMake pick whatever Python interpreter you have in your path.

[JDevlieghere]

Without this change, it is impossible to configure CMake with Python 2.7 on macOS, without either setting the paths manually, or completely removing any trace of Python 3 on the machine.

[tstellar]

I'm in favor of this, but we should make sure there is documentation telling users how to explicitly choose the python version they want. I'm also curious, how does cmake determine which version to use if more than on interpreter is found?

[JDevlieghere]

In D64894#1590516, @tstellar wrote:

I'm in favor of this, but we should make sure there is documentation telling users how to explicitly choose the python version they want.

Agreed, I'll update the docs.

I'm also curious, how does cmake determine which version to use if more than on interpreter is found?

If you don't specify a version (which we don't) it just picks the first one it finds (in your PATH).

[JDevlieghere]

Update the docs with information on the default behavior with regards to Python and how to overwrite it.

[mgorny]

Makes sense. I suppose you want to update LLDBStandalone as well. And probably Python_ADDITIONAL_VERSIONS in clang & co are outdated.

[JosephTremoulet]

llvm/docs/GettingStarted.rst

601–604 ↗

(On Diff #210451)

FWIW, my experience yesterday was that I needed to also set PYTHON_LIBRARY and PYTHON_INCLUDE_DIR to avoid just erroring out with mismatches (but setting all three did work, once I figured out that PYTHON_LIBRARY needs to point to a file while the other two need to point to directories). It might be worth mentioning something along the lines of "You may also need to set PYTHON_LIBRARY and PYTHON_INCLUDE_DIR"? I phrased that as a question b/c I don't know if it's something that others will run into, or if I just have something funny in my environment somehow.

[JDevlieghere]

llvm/docs/GettingStarted.rst

601–604 ↗

(On Diff #210451)

This shouldn't be necessary anymore after this change!

[mgorny]

llvm/docs/GettingStarted.rst

601–604 ↗

(On Diff #210451)

Actually, I think it will be necessary if you change PYTHON_EXECUTABLE while having the other two cached. However, I'd rather fix the code to do a second lookup than expect users to override everything. However, I still need to think how we could achieve that.

Hmm, maybe instead of erroring out immediately we could clean cache vars and retry find_package()?

[cbiesinger]

Preferring Python 3, if present, was actually intentional on my behalf on the basis that we should use the newer Python if we can. But I did not realize this would break MacOS :( (and, it sounds like, any system where you have a python 3 binary but not devel package?)

I can confirm that -DPYTHON_EXECUTABLE=/usr/bin/python3 does work (I only tried with a deleted CMakeCache.txt). It is unfortunate that on most Linux systems we'd still default to Python 2

[cbiesinger]

Should this be merged to LLVM 9?

[raj.khem]

somehow when doing stage2 build, it is stubborn and does not respect any of PYTHON_EXECUTABLE. PYTHON_LIBRARY or PYTHON_INCLUDE_DIR that were passed with -D on cmake invocation even after they are added to -DDCLANG_BOOTSTRAP_PASSTHROUGH list. llvm finds its own python during stage2. in stage1 it respects the above settings.

cfe/trunk/CMakeLists.txt

if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )
   include_directories("${LLVM_BINARY_DIR}/include" "${LLVM_MAIN_INCLUDE_DIR}")
   link_directories("${LLVM_LIBRARY_DIR}")
 
   set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin )
   set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
   set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
 
   if(LLVM_INCLUDE_TESTS)
-    set(Python_ADDITIONAL_VERSIONS 2.7)
     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()
 

compiler-rt/trunk/CMakeLists.txt

if (COMPILER_RT_STANDALONE_BUILD)
   load_llvm_config()
   if (TARGET intrinsics_gen)
     # Loading the llvm config causes this target to be imported so place it
     # under the appropriate folder in an IDE.
     set_target_properties(intrinsics_gen PROPERTIES FOLDER "Compiler-RT Misc")
   endif()
 
   # Find Python interpreter.
-  set(Python_ADDITIONAL_VERSIONS 2.7 2.6 2.5)
   include(FindPythonInterp)
   if(NOT PYTHONINTERP_FOUND)
     message(FATAL_ERROR "
       Unable to find Python interpreter required testing. Please install Python
       or specify the PYTHON_EXECUTABLE CMake variable.")
   endif()
 
   # Ensure that fat libraries are built correctly on Darwin

lld/trunk/CMakeLists.txt

if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
 
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)
 
   if(LLVM_INCLUDE_TESTS)
-    set(Python_ADDITIONAL_VERSIONS 2.7)
     include(FindPythonInterp)
     if(NOT PYTHONINTERP_FOUND)
       message(FATAL_ERROR
 "Unable to find Python interpreter, required for testing.
 
 Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
     endif()
 

llvm/trunk/CMakeLists.txt

if(WIN32 OR CYGWIN)
   endif()
 else()
   set(LLVM_ENABLE_PLUGINS_default ${LLVM_ENABLE_PIC})
 endif()
 option(LLVM_ENABLE_PLUGINS "Enable plugin support" ${LLVM_ENABLE_PLUGINS_default})
 
 include(HandleLLVMOptions)
 
-# We support both Python 2 and 3.
-set(Python_ADDITIONAL_VERSIONS 3.7 3.6 3.5 2.7)
 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()
 

llvm/trunk/docs/GettingStarted.rst

 | CMAKE_BUILD_TYPE        | Tells ``cmake`` what type of build you are trying  |
 |                         | to generate files for. Valid options are Debug,    |
 |                         | Release, RelWithDebInfo, and MinSizeRel. Default   |
 |                         | is Debug.                                          |
 +-------------------------+----------------------------------------------------+
 | CMAKE_INSTALL_PREFIX    | Specifies the install directory to target when     |
 |                         | running the install action of the build files.     |
 +-------------------------+----------------------------------------------------+
+| PYTHON_EXECUTABLE       | Forces CMake to use a specific Python version by   |
+|                         | passing a path to a Python interpreter. By default |
+|                         | the Python version of the interpreter in your PATH |
+|                         | is used.                                           |
++-------------------------+----------------------------------------------------+
 | LLVM_TARGETS_TO_BUILD   | A semicolon delimited list controlling which       |
 |                         | targets will be built and linked into llvm.        |
 |                         | The default list is defined as                     |
 |                         | ``LLVM_ALL_TARGETS``, and can be set to include    |
 |                         | out-of-tree targets. The default value includes:   |
 |                         | ``AArch64, AMDGPU, ARM, BPF, Hexagon, Mips,        |
 |                         | MSP430, NVPTX, PowerPC, Sparc, SystemZ, X86,       |
 |                         | XCore``.                                           |