Use the correct Python lib for each build configuration generated by the Visual Studio CMake generator

[enlight]

Summary

Previously CMAKE_BUILD_TYPE was used to determine whether to link in python27.lib or python27_d.lib, unfortunately this only works reliably when using a CMake generator that generates a single build configuration (e.g. Ninja). The Visual Studio CMake generator generates four build configurations at once (Debug, Release, RelWithDebInfo, MinSizeRel), so if CMAKE_BUILD_TYPE is set to Debug all four build configurations end up linking in python27_d.lib, this is clearly undesirable.

To ensure that the correct Python lib is used for each build configuration the value of PYTHON_LIBRARY is now determined using generator expressions that evaluate to either the debug or release Python lib. The values of PYTHON_EXECUTABLE and PYTHON_DLL are now likewise determined using generator expressions.

Note that these changes only apply to the Windows build.

[brucem]

This looks good to me and like the correct fix. Leaving it for zturner to give the official nod of approval though.

[zturner]

cmake/modules/LLDBConfig.cmake

61 ↗

(On Diff #35942)

This line is hard to parse mentally, and I'm not sure I've seen this kind of nested generator expression. I trust you when you say it's right, but can you explain what this does?

[enlight]

cmake/modules/LLDBConfig.cmake

61 ↗

(On Diff #35942)

Sure thing.

1. $<CONFIG:Debug> evaluates to 1 when the Debug configuration is being generated, or 0 in all other cases.
2. $<$<CONFIG:Debug>:${PYTHON_DEBUG_EXE}> expands to ${PYTHON_DEBUG_EXE} when the Debug configuration is being generated, or nothing (literally) in all other cases.
3. $<$<NOT:$<CONFIG:Debug>>:${PYTHON_RELEASE_EXE}> expands to ${PYTHON_RELEASE_EXE} when any configuration other than Debug is being generated, or nothing in all other cases.

Since the conditionals in 2 & 3 are mutually exclusive, and a conditional expression that evaluates to 0 yields no value at all, it's possible to concatenate them to obtain a single value. This value will be ${PYTHON_DEBUG_EXE} when generating the Debug configuration, or ${PYTHON_RELEASE_EXE} when generating any other configuration.

[zturner]

If the above if / else is equivalent, I prefer if you change it to that before comitting. If not, feel free to commit as is.

cmake/modules/LLDBConfig.cmake

61 ↗

(On Diff #35942)

Ahh, I understand. Is it equivalent to write this:

if ($<CONFIG:Debug> == 1)
  set(PYTHON_EXECUTABLE ${PYTHON_DEBUG_EXE})
else()
  set(PYTHON_EXECUTABLE ${PYTHON_RELEASE_EXE})
endif()

? If so, I kind of prefer this way, if nothing else so that other people will be able to understand it in the future if they go to edit this code.

[enlight]

I'll submit a revised patch that contains an explanation of the generator expressions to aid future maintainers/contributors.

cmake/modules/LLDBConfig.cmake

61 ↗

(On Diff #35942)

Unfortunately that's not currently possible because the if command doesn't support generator expressions (not that the docs mention it or anything). So while CMake won't complain if you did this:

if ($<CONFIG:Debug>)
  set (PYTHON_EXECUTABLE ${PYTHON_DEBUG_EXE})
else ()
  set (PYTHON_EXECUTABLE ${PYTHON_RELEASE_EXE})
endif ()

It wouldn't actually work as intended because the if/else is only evaluated once during the configuration step rather than per-build-configuration (during the generation step).

[enlight]

Added an explanation of the generator expression used in this patch.

[zturner]

Ok, looks good

[zturner]

Hi Vadim, when I run CMake with this change I get the following output:

[1/1] Re-running CMake...

• Warning: Did not find file Compiler/MSVC-ASM
• Target triple: i686-pc-win32
• Native target architecture is X86
• Threads enabled.
• Doxygen disabled.
• Sphinx disabled.
• Go bindings disabled.
• Could NOT find OCaml (missing: OCAMLFIND OCAML_VERSION

OCAML_STDLIB_PATH)

• OCaml bindings disabled.
• Using Debug VC++ CRT: MDd
• Found PythonInterp: C:/Python27/python.exe (found version "2.7.8")
• Constructing LLVMBuild project information
• LLVMHello ignored -- Loadable modules not supported on this platform.
• Targeting AArch64
• Targeting AMDGPU
• Targeting ARM
• Targeting BPF
• Targeting CppBackend
• Targeting Hexagon
• Targeting Mips
• Targeting MSP430
• Targeting NVPTX
• Targeting PowerPC
• Targeting Sparc
• Targeting SystemZ
• Targeting X86
• Targeting XCore
• Clang version: 3.8.0
• SampleAnalyzerPlugin ignored -- Loadable modules not supported on this

platform.

• PrintFunctionNames ignored -- Loadable modules not supported on this

platform.

• LLD version: 3.8.0
• *Found PythonLibs:

$<$<CONFIG:Debug>:C:/Python27_LLDB/x86/libs/python27_d.lib>$<$<NOT:$<CONFIG:Debug>>:C:/Python27_LLDB/x86/libs/python27.lib>*

• LLDB version: 3.8.0
• Could NOT find LibXml2 (missing: LIBXML2_LIBRARIES LIBXML2_INCLUDE_DIR)
• Could NOT find Doxygen (missing: DOXYGEN_EXECUTABLE)
• *Found PythonInterp:

$<$<CONFIG:Debug>:C:/Python27_LLDB/x86/python_d.exe>$<$<NOT:$<CONFIG:Debug>>:C:/Python27_LLDB/x86/python.exe>*
(found version "1.4")

This doesn't seem right to me. Can you take a look and fix this?

[enlight]

That looks fine to me. CMake prints those messages during the configuration step, at that point the generator expressions haven't been expanded yet. I could prettify those messages a bit to read: Found PythonLibs: C:/Python27_LLDB/x86/libs/python27_d.lib and C:/Python27_LLDB/x86/libs/python27.lib.

[zturner]

Yea that's what I meant. Everything works, but the configuration step is
supposed to print out stuff that people can read at a glance. So
prettifying it would be desirable.

[zturner]

My change to support Python 3 is in. Now that that's in, would you mind
trying to prettify the output messages when you get a chance?

lldb/trunk/cmake/modules/LLDBConfig.cmake

if(UNIX)
     if(CMAKE_SIZEOF_VOID_P EQUAL 8)
       set(CMAKE_LIBRARY_ARCHITECTURE "x86_64-linux-gnu")
     endif()
   endif()
 
   if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Windows")
     if (NOT "${PYTHON_HOME}" STREQUAL "")
       file(TO_CMAKE_PATH "${PYTHON_HOME}" PYTHON_HOME)
-      if ("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
-        file(TO_CMAKE_PATH "${PYTHON_HOME}/python_d.exe" PYTHON_EXECUTABLE)
-        file(TO_CMAKE_PATH "${PYTHON_HOME}/libs/python27_d.lib" PYTHON_LIBRARY)
-        file(TO_CMAKE_PATH "${PYTHON_HOME}/python27_d.dll" PYTHON_DLL)
-      else()
-        file(TO_CMAKE_PATH "${PYTHON_HOME}/python.exe" PYTHON_EXECUTABLE)
-        file(TO_CMAKE_PATH "${PYTHON_HOME}/libs/python27.lib" PYTHON_LIBRARY)
-        file(TO_CMAKE_PATH "${PYTHON_HOME}/python27.dll" PYTHON_DLL)
-      endif()
+      file(TO_CMAKE_PATH "${PYTHON_HOME}/python_d.exe" PYTHON_DEBUG_EXE)
+      file(TO_CMAKE_PATH "${PYTHON_HOME}/libs/python27_d.lib" PYTHON_DEBUG_LIB)
+      file(TO_CMAKE_PATH "${PYTHON_HOME}/python27_d.dll" PYTHON_DEBUG_DLL)
+
+      file(TO_CMAKE_PATH "${PYTHON_HOME}/python.exe" PYTHON_RELEASE_EXE)
+      file(TO_CMAKE_PATH "${PYTHON_HOME}/libs/python27.lib" PYTHON_RELEASE_LIB)
+      file(TO_CMAKE_PATH "${PYTHON_HOME}/python27.dll" PYTHON_RELEASE_DLL)
+
+      # Generator expressions are evaluated in the context of each build configuration generated
+      # by CMake. Here we use the $<CONFIG:Debug>:VALUE logical generator expression to ensure
+      # that the debug Python library, DLL, and executable are used in the Debug build configuration.
+      #
+      # Generator expressions can be difficult to grok at first so here's a breakdown of the one
+      # used for PYTHON_LIBRARY:
+      #
+      # 1. $<CONFIG:Debug> evaluates to 1 when the Debug configuration is being generated,
+      #    or 0 in all other cases.
+      # 2. $<$<CONFIG:Debug>:${PYTHON_DEBUG_LIB}> expands to ${PYTHON_DEBUG_LIB} when the Debug
+      #    configuration is being generated, or nothing (literally) in all other cases.
+      # 3. $<$<NOT:$<CONFIG:Debug>>:${PYTHON_RELEASE_LIB}> expands to ${PYTHON_RELEASE_LIB} when
+      #    any configuration other than Debug is being generated, or nothing in all other cases.
+      # 4. The conditionals in 2 & 3 are mutually exclusive.
+      # 5. A logical expression with a conditional that evaluates to 0 yields no value at all.
+      #
+      # Due to 4 & 5 it's possible to concatenate 2 & 3 to obtain a single value specific to each
+      # build configuration. In this example the value will be ${PYTHON_DEBUG_LIB} when generating the
+      # Debug configuration, or ${PYTHON_RELEASE_LIB} when generating any other configuration.
+      # Note that it's imperative that there is no whitespace between the two expressions, otherwise
+      # CMake will insert a semicolon between the two.
+
+      set (PYTHON_EXECUTABLE $<$<CONFIG:Debug>:${PYTHON_DEBUG_EXE}>$<$<NOT:$<CONFIG:Debug>>:${PYTHON_RELEASE_EXE}>)
+      set (PYTHON_LIBRARY $<$<CONFIG:Debug>:${PYTHON_DEBUG_LIB}>$<$<NOT:$<CONFIG:Debug>>:${PYTHON_RELEASE_LIB}>)
+      set (PYTHON_DLL $<$<CONFIG:Debug>:${PYTHON_DEBUG_DLL}>$<$<NOT:$<CONFIG:Debug>>:${PYTHON_RELEASE_DLL}>)
 
       file(TO_CMAKE_PATH "${PYTHON_HOME}/Include" PYTHON_INCLUDE_DIR)
       if (NOT LLDB_RELOCATABLE_PYTHON)
         add_definitions( -DLLDB_PYTHON_HOME="${PYTHON_HOME}" )
       endif()
     else()
       message("Embedding Python on Windows without specifying a value for PYTHON_HOME is deprecated.  Support for this will be dropped soon.")