[debuginfo-tests] Don't look for Python 3 if we already have it

[rnk]

Summary

LLDB already requires Python 3 on Windows, so I already configure it
that way. For some reason CMake fails to find the one that Visual Studio
automatically installs at this standard location:

C:/Program Files (x86)/Microsoft Visual Studio/Shared/Python37_64/python.exe

CMake prefers the python on path, which happens to be python 2.7.

[aprantl]

I thought we already had a check in place that refuses to run debuginfo-tests for Python < 3? Can this be removed entirely?

[rnk]

In D69684#1729308, @aprantl wrote:

I thought we already had a check in place that refuses to run debuginfo-tests for Python < 3? Can this be removed entirely?

Yes, this is that check. I've already configured cmake with Python 3, but it throws all that info away and re-runs find_package, which finds Python 2 instead of 3. The intention of this logic is to... just not do anything if we already have python 3.

[rnk]

Nevermind, I've done it poorly in my haste to fix the cmake error. :(

[rnk]

• Do a better job.

[aprantl]

Ah. So this code allows debuginfo-tests to use python3 even if LLVM was configured with python 2.7 and you have both versions installed.

[jmorse]

Ah yep, that's broken, in fact when re-configuring I needed this locally too :/.

[aprantl]

I'm running into an issue running clang tests that can be traced back to https://reviews.llvm.org/D68708.

I'm configuring with debuginfo-tests enabled and python2.7 as the python executable, but with a python3 also installed. The end result is that tools/clang/test/lit.site.cfg.py defines config.python_executable = "", which results in the weirdest of errors. That file is expanded from clang/test/lit.site.cfg.py.in where it is config.python_executable = "@PYTHON_EXECUTABLE@". I git-bisected it to D68708, which seems to make sense. I don't claim to fully understand what is going on here, but I believe that unseting PYTHON_EXECUTABLE is dangerous. Could we initialize a PYTHON3_EXECUTABLE variable here, or even better, just error out if PYTHON_EXECUTABLE is not python 3? Which users/bot are we supporting with this code?

[aprantl]

(green dragon uses an all-python3 configuration now)

[rnk]

Would it be OK if we removed all the logic to redo the python search here, and replace it with a simple version check? So far as I know, most of the rest of LLVM is Python 3 ready, so even if you use python 2 on PATH, there's no reason not to tell cmake to use Python 3.

[JDevlieghere]

In D69684#1737796, @rnk wrote:

Would it be OK if we removed all the logic to redo the python search here, and replace it with a simple version check? So far as I know, most of the rest of LLVM is Python 3 ready, so even if you use python 2 on PATH, there's no reason not to tell cmake to use Python 3.

Very strong +1. Getting Python right (at least in CMake < 3.12) is pretty tricky as we've seen many times in LLDB.

[aprantl]

(green dragon uses an all-python3 configuration now)

In D69684#1737796, @rnk wrote:

Would it be OK if we removed all the logic to redo the python search here, and replace it with a simple version check? So far as I know, most of the rest of LLVM is Python 3 ready, so even if you use python 2 on PATH, there's no reason not to tell cmake to use Python 3.

I would very much prefer a hard error over anything automagic. I just wanted to make sure no-one else ( @jmorse ) relies on this functionality.

[jmorse]

In D69684#1738039, @aprantl wrote:

I would very much prefer a hard error over anything automagic. I just wanted to make sure no-one else ( @jmorse ) relies on this functionality.

We don't rely on it either, the testing-for-python-3 rather than a hard dependency was mostly motivated by being un-intrusive on peoples environments. It looks like that's way harder than I expected though.

[aprantl]

Good. I removed it in d0416b91f0390f6e69dacf3d5d076531221e0767.

debuginfo-tests/CMakeLists.txt

 # Debug Info tests.  These tests invoke clang to generate programs with
 # various types of debug info, and then run those programs under a debugger
 # such as GDB or LLDB to verify the results.
 
 set(DEBUGINFO_TESTS_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 set(DEBUGINFO_TESTS_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
 
 set(DEBUGINFO_TEST_DEPS
   clang
   FileCheck
   count
   llvm-objdump
   not
   )
 
-# Wipe, uh, previous results
+# If we don't already have Python 3, throw away any previous results and try to
+# find it again.
+set(DEBUGINFO_UNSET_PYTHON3 OFF)
+if (PYTHON_VERSION_MAJOR LESS 3)
   unset(PYTHONINTERP_FOUND CACHE)
   unset(PYTHON_EXECUTABLE CACHE)
   unset(PYTHON_LIBRARY CACHE)
   unset(PYTHON_DLL CACHE)
   unset(PYTHON_INCLUDE_DIR CACHE)
   unset(PYTHON_VERSION_STRING CACHE)
   unset(PYTHON_VERSION_MAJOR CACHE)
   unset(PYTHON_VERSION_MINOR CACHE)
   unset(PYTHON_VERSION_PATCH CACHE)
   unset(PYTHONLIBS_VERSION_STRING CACHE)
 
   # Try to find python3. If it doesn't exist, dexter tests can't run.
   find_package(PythonInterp "3")
+  set(DEBUGINFO_UNSET_PYTHON3 ON)
+endif()
+
 if (NOT DEFINED PYTHON_EXECUTABLE)
   message(FATAL_ERROR "Cannot run debuginfo-tests without python")
 elseif(PYTHON_VERSION_MAJOR LESS 3)
   message(FATAL_ERROR "Cannot run debuginfo-tests without python 3")
 else()
    configure_lit_site_cfg(
     ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
     ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
     MAIN_CONFIG
     ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py
     )
 
   add_lit_testsuite(check-debuginfo "Running debug info integration tests"
     ${CMAKE_CURRENT_BINARY_DIR}
     DEPENDS ${DEBUGINFO_TEST_DEPS}
     )
-  set_target_properties(check-debuginfo PROPERTIES FOLDER "Debug info tests") 
+  set_target_properties(check-debuginfo PROPERTIES FOLDER "Debug info tests")
 endif()
 
-# Prevent the rest of llvm observing our secret python3-ness
+# Prevent the rest of llvm observing our secret python3-ness, if that wasn't
+# what was originally found.
+if (DEBUGINFO_UNSET_PYTHON3)
   unset(PYTHONINTERP_FOUND CACHE)
   unset(PYTHON_EXECUTABLE CACHE)
   unset(PYTHON_LIBRARY CACHE)
   unset(PYTHON_DLL CACHE)
   unset(PYTHON_INCLUDE_DIR CACHE)
   unset(PYTHON_VERSION_STRING CACHE)
   unset(PYTHON_VERSION_MAJOR CACHE)
   unset(PYTHON_VERSION_MINOR CACHE)
   unset(PYTHON_VERSION_PATCH CACHE)
   unset(PYTHONLIBS_VERSION_STRING CACHE)
+endif()