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

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

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

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

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?

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)
+
+      # Note that it's imperative that there is no whitespace between the debug and not-debug generator expressions
+      # below, otherwise CMake will replace the whitespace with a semicolon in some contexts (which would stuff things up).
+      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.")