[CMake] Python bindings generation polishing

[sgraenitz]

Summary

Simplify SWIG invocation and handling of generated files.

The swig_wrapper target can generate LLDBWrapPython.cpp and lldb.py in its own binary directory, so we can get rid of a few global variables and their logic. We can use the swig_wrapper's BINARY_DIR target property to refer to it and liblldb's LIBRARY_OUTPUT_DIRECTORY to refer to the framework/shared object output directory.

[sgraenitz]

CMakeLists.txt

143

We depend on swig_wrapper which depends on it/generates it.

[sgraenitz]

Avoid conflicts: updating diff for recent changes on master

[stella.stamenova]

CMakeLists.txt

134

I have a vague recollection that using TARGET_PROPERTY didn't work for some changes I was working on a few months ago, but I don't remember the details. It would be good to make sure that this works on mac, linux and windows all before you commit.

[sgraenitz]

Thanks for taking a look!

It would be good to make sure that this works on mac, linux and windows all before you commit.

Shortly tested an in-tree lldb with clang, compiler-rt and lld on Windows using the below config and successfully built the BUILD_ALL project (Release), but I didn't manage to run check-lldb as Visual Studio failed to find the executable. How do you run the test suite on Windows?

cmake -G "Visual Studio 15 2017 Win64" -DLLVM_TARGETS_TO_BUILD=host -DPYTHON_HOME="%PYTHON_HOME%" ../llvm

[JDevlieghere]

This LGTM if it works on Windows.

CMakeLists.txt

134

IIRC and if we’re talking about the same thing, the generator expressions didn’t work because we were using them in a configured file (the lldb-dotest wrapper). Anyway, still worth double checking here.

[sgraenitz]

Account for changes in parent revision to keep individual commits compiling.

[sgraenitz]

Address feedback and minor improvements

[sgraenitz]

CMakeLists.txt

134

Yes, whenever we use a generator expression, there must be a processing step at generation time that expands the expression. Debugging can become hairy, because message($<GENEX:target>) doesn't have such a step (but the COMMAND in add_custom_target has one).

Anyway, I had another look and indeed the genex's are not necessary here. I would really like to keep the info in the target properties though, because that's where CMake needs it and we can avoid duplicating it to global variables.

I will double check that this works on the common platforms before I commit all my related changes.

[sgraenitz]

CMakeLists.txt

134

BTW removed generator expressions here and now use get_target_property to fetch property values at configuration time.

[ted]

@sgraenitz I've found an issue with this patch, using the Visual Studio 2015 generator.

In scripts/CMakeLists.txt the old code (for Windows):

if (CMAKE_CONFIGURATION_TYPES)
  set(SWIG_PYTHON_DIR ${CMAKE_BINARY_DIR}/\${CMAKE_INSTALL_CONFIG_NAME}/lib${LLVM_LIBDIR_SUFFIX}/site-packages)
else()
  set(SWIG_PYTHON_DIR ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/site-packages)
endif()

Was changed to (for everything):

set(SWIG_PYTHON_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${swig_python_subdir})

Which changes <build>/tools/lldb/scripts/cmake_install.cmake from

if("${CMAKE_INSTALL_COMPONENT}" STREQUAL "Unspecified" OR NOT CMAKE_INSTALL_COMPONENT)
  file(INSTALL DESTINATION "${CMAKE_INSTALL_PREFIX}/lib" TYPE DIRECTORY FILES "i:/obj/${CMAKE_INSTALL_CONFIG_NAME}/lib/site-packages")
endif()

to

if("${CMAKE_INSTALL_COMPONENT}" STREQUAL "Unspecified" OR NOT CMAKE_INSTALL_COMPONENT)
  file(INSTALL DESTINATION "${CMAKE_INSTALL_PREFIX}/lib" TYPE DIRECTORY FILES "I:/obj/$(Configuration)/lib/site-packages")
endif()

"$(Configuration)" is a Visual Studio variable, and not available to cmake, so our bots error out with this error:

CMake Error at tools/lldb/scripts/cmake_install.cmake:31 (file):
  file INSTALL cannot find "I:/obj/$(Configuration)/lib/site-packages".
Call Stack (most recent call first):
  tools/lldb/cmake_install.cmake:41 (include)
  tools/cmake_install.cmake:41 (include)
  cmake_install.cmake:45 (include)

Verified with the latest CMake (3.13.3).

Will switching back to the old code for Windows cause any issues?

CMakeLists.txt

if (NOT LLDB_DISABLE_PYTHON)
     if(LLDB_USE_SYSTEM_SIX)
       set(use_six_py_from_system --useSystemSix)
     endif()
 
     # Add a Post-Build Event to copy over Python files and create the symlink
     # to liblldb.so for the Python API(hardlink on Windows)
     add_custom_target(finish_swig ALL
         COMMAND
-           ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
+           ${PYTHON_EXECUTABLE} ${LLDB_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
                --srcRoot=${LLDB_SOURCE_DIR}
-               --targetDir=${LLDB_PYTHON_TARGET_DIR}
-               --cfgBldDir=${LLDB_PYTHON_TARGET_DIR}
+               --targetDir=$<TARGET_PROPERTY:liblldb,LIBRARY_OUTPUT_DIRECTORY>
+               --cfgBldDir=$<TARGET_PROPERTY:swig_wrapper,BINARY_DIR>
                --prefix=${CMAKE_BINARY_DIR}
                --cmakeBuildConfiguration=${CMAKE_CFG_INTDIR}
                --lldbLibDir=lib${LLVM_LIBDIR_SUFFIX}
                ${use_python_wrapper_from_src_dir}
                ${use_six_py_from_system}
         VERBATIM
-        DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
-        DEPENDS ${LLDB_PYTHON_TARGET_DIR}/lldb.py
+        DEPENDS ${LLDB_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
         COMMENT "Python script sym-linking LLDB Python API")
 
 
     if (TARGET readline)
       set(readline_dep readline)
     endif()
     add_dependencies(finish_swig swig_wrapper liblldb lldb-argdumper ${readline_dep})
 

scripts/CMakeLists.txt

 file(GLOB SWIG_INTERFACES interface/*.i)
 file(GLOB_RECURSE SWIG_SOURCES *.swig)
 set(SWIG_HEADERS
   ${LLDB_SOURCE_DIR}/include/lldb/API/SBDefines.h
   ${LLDB_SOURCE_DIR}/include/lldb/lldb-defines.h
   ${LLDB_SOURCE_DIR}/include/lldb/lldb-enumerations.h
   ${LLDB_SOURCE_DIR}/include/lldb/lldb-forward.h
   ${LLDB_SOURCE_DIR}/include/lldb/lldb-types.h
   ${LLDB_SOURCE_DIR}/include/lldb/lldb-versioning.h
 )
 
 include(FindPythonInterp)
 
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
-  set(SWIG_PYTHON_DIR
-    ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/python${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR})
-else()
-  set(SWIG_PYTHON_DIR ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/site-packages)
-endif()
-
-set(SWIG_INSTALL_DIR lib${LLVM_LIBDIR_SUFFIX})
-
-# Generating the LLDB framework correctly is a bit complicated because the
-# framework depends on the swig output.
 if(LLDB_BUILD_FRAMEWORK)
   set(framework_arg --framework --target-platform Darwin)
-  set(SWIG_PYTHON_DIR
-    ${LLDB_PYTHON_TARGET_DIR}/${LLDB_FRAMEWORK_RESOURCE_DIR}/Python)
-  set(SWIG_INSTALL_DIR
-    ${LLDB_FRAMEWORK_INSTALL_DIR}/${LLDB_FRAMEWORK_RESOURCE_DIR})
 endif()
 
-get_filename_component(CFGBLDDIR ${LLDB_WRAP_PYTHON} DIRECTORY)
-
 find_package(SWIG REQUIRED)
 add_custom_command(
-  OUTPUT ${LLDB_WRAP_PYTHON}
-  OUTPUT ${LLDB_PYTHON_TARGET_DIR}/lldb.py
+  OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/LLDBWrapPython.cpp
+  OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/lldb.py
   DEPENDS ${SWIG_SOURCES}
   DEPENDS ${SWIG_INTERFACES}
   DEPENDS ${SWIG_HEADERS}
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/Python/prepare_binding_Python.py
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/Python/modify-python-lldb.py
   COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/prepare_bindings.py
       ${framework_arg}
       --srcRoot=${LLDB_SOURCE_DIR}
-      --targetDir=${LLDB_PYTHON_TARGET_DIR}
-      --cfgBldDir=${CFGBLDDIR}
+      --targetDir=${CMAKE_CURRENT_BINARY_DIR}
+      --cfgBldDir=${CMAKE_CURRENT_BINARY_DIR}
       --prefix=${CMAKE_BINARY_DIR}
       --swigExecutable=${SWIG_EXECUTABLE}
   VERBATIM
   COMMENT "Python script building LLDB Python wrapper")
-add_custom_target(swig_wrapper ALL DEPENDS ${LLDB_WRAP_PYTHON})
 
-set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/lldb.py PROPERTIES GENERATED 1)
+add_custom_target(swig_wrapper ALL DEPENDS
+  ${CMAKE_CURRENT_BINARY_DIR}/LLDBWrapPython.cpp
+  ${CMAKE_CURRENT_BINARY_DIR}/lldb.py
+)
+
+# For the liblldb target we need to mark LLDBWrapPython.cpp generated. Doing it
+# here has no effect as set_source_files_properties are visible only to targets
+# added in the same directory.
+set_property(GLOBAL PROPERTY LLDB_PYTHON_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
+if(NOT LLDB_BUILD_FRAMEWORK)
+  if(CMAKE_SYSTEM_NAME MATCHES "Windows")
+    set(swig_python_subdir site-packages)
+  else()
+    set(swig_python_subdir python${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR})
+  endif()
 
+  set(SWIG_PYTHON_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${swig_python_subdir})
+  set(SWIG_INSTALL_DIR lib${LLVM_LIBDIR_SUFFIX})
 
   # Install the LLDB python module
   install(DIRECTORY ${SWIG_PYTHON_DIR} DESTINATION ${SWIG_INSTALL_DIR})
+endif()
 
 # build Python modules
 add_subdirectory(Python/modules)

source/API/CMakeLists.txt

 if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
   add_definitions( -DEXPORT_LIBLLDB )
 endif()
 
 get_property(LLDB_ALL_PLUGINS GLOBAL PROPERTY LLDB_PLUGINS)
 
+if(NOT LLDB_DISABLE_PYTHON)
+  # We could get this path from $<TARGET_PROPERTY:swig_wrapper,BINARY_DIR>
+  # but set_source_files_properties() doesn't accept generator expressions.
+  get_property(lldb_python_dir GLOBAL PROPERTY LLDB_PYTHON_BINARY_DIR)
+
+  set(lldb_python_wrapper ${lldb_python_dir}/LLDBWrapPython.cpp)
+  set_source_files_properties(${lldb_python_wrapper} PROPERTIES GENERATED ON)
+endif()
+
 add_lldb_library(liblldb SHARED
   SBAddress.cpp
   SBAttachInfo.cpp
   SBBlock.cpp
   SBBreakpoint.cpp
   SBBreakpointLocation.cpp
   SBBreakpointName.cpp
   SBBreakpointOptionCommon.cpp
add_lldb_library(liblldb SHARED
   SBTypeSummary.cpp
   SBTypeSynthetic.cpp
   SBValue.cpp
   SBValueList.cpp
   SBVariablesOptions.cpp
   SBWatchpoint.cpp
   SBUnixSignals.cpp
   SystemInitializerFull.cpp
-  ${LLDB_WRAP_PYTHON}
+  ${lldb_python_wrapper}
 
   LINK_LIBS
     lldbBase
     lldbBreakpoint
     lldbCore
     lldbDataFormatters
     lldbExpression
     lldbHost
     lldbInitialization
     lldbInterpreter
     lldbSymbol
     lldbTarget
     lldbUtility
     ${LLDB_ALL_PLUGINS}
   LINK_COMPONENTS
     Support
   )
 
-if(LLDB_WRAP_PYTHON)
+if(lldb_python_wrapper)
   add_dependencies(liblldb swig_wrapper)
 
   if (MSVC)
-    set_property(SOURCE ${LLDB_WRAP_PYTHON} APPEND_STRING PROPERTY COMPILE_FLAGS " /W0")
+    set_property(SOURCE ${lldb_python_wrapper} APPEND_STRING PROPERTY COMPILE_FLAGS " /W0")
   else()
-    set_property(SOURCE ${LLDB_WRAP_PYTHON} APPEND_STRING PROPERTY COMPILE_FLAGS " -w")
+    set_property(SOURCE ${lldb_python_wrapper} APPEND_STRING PROPERTY COMPILE_FLAGS " -w")
   endif()
 
   if (CLANG_CL)
-    set_property(SOURCE ${LLDB_WRAP_PYTHON} APPEND_STRING
+    set_property(SOURCE ${lldb_python_wrapper} APPEND_STRING
       PROPERTY COMPILE_FLAGS " -Wno-unused-function")
   endif()
   if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND
       NOT "${CMAKE_SYSTEM_NAME}" MATCHES "Darwin")
-    set_property(SOURCE ${LLDB_WRAP_PYTHON} APPEND_STRING
+    set_property(SOURCE ${lldb_python_wrapper} APPEND_STRING
       PROPERTY COMPILE_FLAGS " -Wno-sequence-point -Wno-cast-qual")
   endif ()
 endif()
 
 set_target_properties(liblldb
   PROPERTIES
   VERSION ${LLDB_VERSION}
 )