[CMake] Remove duplicated logic to find Python when doing a standalone build

[JDevlieghere]

Summary

I'm pretty sure there's no need to have this logic living in LLDBStandalone. We always go through LLDBConfig.cmake which has the canonical way to find the Python libs and interpreter.

[amccarth]

Actually, right now I'm trying to figure out where the interpreter is found because cmake is finding different, incompatible versions of the interpreter (2.7) and the libs (3.6).

On Windows, it looks like LLDBConfig.cmake doesn't explicitly look for the interpreter, only the libs. By the time find_python_libs_windows runs, the interpreter is already set to the wrong version.

[JDevlieghere]

In D64821#1588424, @amccarth wrote:

Actually, right now I'm trying to figure out where the interpreter is found because cmake is finding different, incompatible versions of the interpreter (2.7) and the libs (3.6).

On Windows, it looks like LLDBConfig.cmake doesn't explicitly look for the interpreter, only the libs. By the time find_python_libs_windows runs, the interpreter is already set to the wrong version.

For an in-tree build I assume it would be LLVM setting the interpreter. Maybe we should we unset the variables here so we can start clean?

[JDevlieghere]

Indeed, the culprit appears to be https://github.com/llvm/llvm-project/blob/master/llvm/CMakeLists.txt#L645, which prefer Python 3 over Python 2.

[sgraenitz]

I'm pretty sure there's no need to have this logic living in LLDBStandalone. We always go through LLDBConfig.cmake which has the canonical way to find the Python libs and interpreter.

LLDBStandalone is supposed to act like a polyfill for whatever LLVM would usually do, so LLDBConfig (and ideally everything else) can ignore in-tree vs. standalone. Thus, the Python logic here should always behave like its LLVM counterpart. If it turns out to be incorrect, we should either adapt LLDBConfig or change both LLVM and LLDBStandalone.

Indeed, the culprit appears to be https://github.com/llvm/llvm-project/blob/master/llvm/CMakeLists.txt#L645, which prefer Python 3 over Python 2.

This changed quite recently https://reviews.llvm.org/D64443

lldb/cmake/modules/LLDBStandalone.cmake

101 ↗

(On Diff #210177)

The actual difference to https://github.com/llvm/llvm-project/blob/c23bc97e85f6/llvm/CMakeLists.txt#L645 is the condition on PYTHON_EXECUTABLE.

[sgraenitz]

Jonas is right, that there is nothing wrong with this code, but that it's just redundant as we'll do all of this in LLDBConfig anyway. Removing it means adding a potential difference for standalone builds, but in the end it should be fine.

lldb/trunk/cmake/modules/LLDBStandalone.cmake

 # CMake modules to be in that directory as well.
 list(APPEND CMAKE_MODULE_PATH "${LLVM_DIR}")
 include(AddLLVM)
 include(TableGen)
 include(HandleLLVMOptions)
 include(CheckAtomic)
 include(LLVMDistributionSupport)
 
-set(Python_ADDITIONAL_VERSIONS 3.7 3.6 3.5 2.7)
-if (PYTHON_EXECUTABLE STREQUAL "")
-  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()
-else()
-  message(STATUS "Found PythonInterp: ${PYTHON_EXECUTABLE}")
-endif()
-
 set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
 set(LLVM_INCLUDE_TESTS ON CACHE INTERNAL "")
 
 option(LLVM_USE_FOLDERS "Enable solution folders in Visual Studio. Disable for Express versions." ON)
 if(LLVM_USE_FOLDERS)
   set_property(GLOBAL PROPERTY USE_FOLDERS ON)
 endif()