From 5721c041c228eb7b898d75a9a7540b2839ef91e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Wed, 2 Oct 2024 21:10:26 +0200 Subject: [PATCH 1/7] [lldb] Fix deps loading for lldb-python on Windows and Python3.8+ --- lldb/bindings/python/CMakeLists.txt | 11 +++++++++++ lldb/bindings/python/python.swig | 12 +++++++++++- lldb/bindings/python/static-binding/lldb.py | 10 ++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lldb/bindings/python/CMakeLists.txt b/lldb/bindings/python/CMakeLists.txt index e48fcb35aba3b..aa5a18d94f7d3 100644 --- a/lldb/bindings/python/CMakeLists.txt +++ b/lldb/bindings/python/CMakeLists.txt @@ -198,6 +198,17 @@ function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_tar COMMENT "Copying Python DLL to LLDB binaries directory.") endif() + # Since Python3.8 the Windows runtime now longer loads dependent DLLs from Path or CWD. Instead, + # candidate locations must be set explicit via os.add_dll_directory(). Here, we add a file with + # all such paths to lldb-python. Module import code reads and registers them. + if (WIN32) + add_custom_command( + TARGET ${swig_target} + POST_BUILD + COMMAND ${CMAKE_COMMAND} -E echo "${LLVM_SHLIB_OUTPUT_INTDIR}" > ${lldb_python_target_dir}/dll_dependents_paths.txt + COMMENT "Generating dll_dependents_paths.txt" + VERBATIM) + endif() endfunction() diff --git a/lldb/bindings/python/python.swig b/lldb/bindings/python/python.swig index 278c0eed2bab2..e8f535c89e237 100644 --- a/lldb/bindings/python/python.swig +++ b/lldb/bindings/python/python.swig @@ -44,7 +44,17 @@ us to override the module import logic to suit our needs. This does that. Older swig versions will simply ignore this setting. */ %define MODULEIMPORT -"try: +"import sys +if sys.platform == "win32" and sys.version_info >= (3, 8): + from pathlib import Path + dll_paths_file = Path(__file__).parent / "dll_dependents_paths.txt" + if dll_paths_file.exists(): + with dll_paths_file.open("r") as f: + dirs = [line.strip() for line in f if Path(line.strip()).is_dir()] + for d in dirs: + import os + os.add_dll_directory(d) +try: # Try an absolute import first. If we're being loaded from lldb, # _lldb should be a built-in module. import $module diff --git a/lldb/bindings/python/static-binding/lldb.py b/lldb/bindings/python/static-binding/lldb.py index 9b57cae9a6b29..a9a1804ceb17c 100644 --- a/lldb/bindings/python/static-binding/lldb.py +++ b/lldb/bindings/python/static-binding/lldb.py @@ -33,6 +33,16 @@ """ from sys import version_info as _swig_python_version_info +import sys +if sys.platform == "win32" and sys.version_info >= (3, 8): + from pathlib import Path + dll_paths_file = Path(__file__).parent / "dll_dependents_paths.txt" + if dll_paths_file.exists(): + with dll_paths_file.open("r") as f: + dirs = [line.strip() for line in f if Path(line.strip()).is_dir()] + for d in dirs: + import os + os.add_dll_directory(d) try: # Try an absolute import first. If we're being loaded from lldb, # _lldb should be a built-in module. From 193818805daefc192e3aba6822adf611c48c192e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Mon, 7 Oct 2024 15:22:52 +0200 Subject: [PATCH 2/7] Copy dependent DLLs into the Python package --- lldb/bindings/python/CMakeLists.txt | 15 ++++++--------- lldb/bindings/python/python.swig | 12 +----------- lldb/bindings/python/static-binding/lldb.py | 10 ---------- 3 files changed, 7 insertions(+), 30 deletions(-) diff --git a/lldb/bindings/python/CMakeLists.txt b/lldb/bindings/python/CMakeLists.txt index aa5a18d94f7d3..3d2f9eefdc074 100644 --- a/lldb/bindings/python/CMakeLists.txt +++ b/lldb/bindings/python/CMakeLists.txt @@ -198,16 +198,13 @@ function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_tar COMMENT "Copying Python DLL to LLDB binaries directory.") endif() - # Since Python3.8 the Windows runtime now longer loads dependent DLLs from Path or CWD. Instead, - # candidate locations must be set explicit via os.add_dll_directory(). Here, we add a file with - # all such paths to lldb-python. Module import code reads and registers them. + # Since Python3.8 the Windows runtime loads dependent DLLs only from the directory of the binary + # itself (and not Path). Thus, we must copy all our DLLs into the Python package. This is a best + # practice on Windows anyway. if (WIN32) - add_custom_command( - TARGET ${swig_target} - POST_BUILD - COMMAND ${CMAKE_COMMAND} -E echo "${LLVM_SHLIB_OUTPUT_INTDIR}" > ${lldb_python_target_dir}/dll_dependents_paths.txt - COMMENT "Generating dll_dependents_paths.txt" - VERBATIM) + add_custom_command(TARGET ${swig_target} POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy -t ${lldb_python_target_dir}> $ + COMMAND_EXPAND_LISTS) endif() endfunction() diff --git a/lldb/bindings/python/python.swig b/lldb/bindings/python/python.swig index e8f535c89e237..278c0eed2bab2 100644 --- a/lldb/bindings/python/python.swig +++ b/lldb/bindings/python/python.swig @@ -44,17 +44,7 @@ us to override the module import logic to suit our needs. This does that. Older swig versions will simply ignore this setting. */ %define MODULEIMPORT -"import sys -if sys.platform == "win32" and sys.version_info >= (3, 8): - from pathlib import Path - dll_paths_file = Path(__file__).parent / "dll_dependents_paths.txt" - if dll_paths_file.exists(): - with dll_paths_file.open("r") as f: - dirs = [line.strip() for line in f if Path(line.strip()).is_dir()] - for d in dirs: - import os - os.add_dll_directory(d) -try: +"try: # Try an absolute import first. If we're being loaded from lldb, # _lldb should be a built-in module. import $module diff --git a/lldb/bindings/python/static-binding/lldb.py b/lldb/bindings/python/static-binding/lldb.py index a9a1804ceb17c..9b57cae9a6b29 100644 --- a/lldb/bindings/python/static-binding/lldb.py +++ b/lldb/bindings/python/static-binding/lldb.py @@ -33,16 +33,6 @@ """ from sys import version_info as _swig_python_version_info -import sys -if sys.platform == "win32" and sys.version_info >= (3, 8): - from pathlib import Path - dll_paths_file = Path(__file__).parent / "dll_dependents_paths.txt" - if dll_paths_file.exists(): - with dll_paths_file.open("r") as f: - dirs = [line.strip() for line in f if Path(line.strip()).is_dir()] - for d in dirs: - import os - os.add_dll_directory(d) try: # Try an absolute import first. If we're being loaded from lldb, # _lldb should be a built-in module. From 641d3234a929404eb2d90ca10af61f022bb8e582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Mon, 7 Oct 2024 16:17:51 +0200 Subject: [PATCH 3/7] Guard the TARGET_RUNTIME_DLLS genex with an explicit CMake version check for now --- lldb/bindings/python/CMakeLists.txt | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lldb/bindings/python/CMakeLists.txt b/lldb/bindings/python/CMakeLists.txt index 3d2f9eefdc074..45b4f48f5b44b 100644 --- a/lldb/bindings/python/CMakeLists.txt +++ b/lldb/bindings/python/CMakeLists.txt @@ -202,9 +202,19 @@ function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_tar # itself (and not Path). Thus, we must copy all our DLLs into the Python package. This is a best # practice on Windows anyway. if (WIN32) - add_custom_command(TARGET ${swig_target} POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy -t ${lldb_python_target_dir}> $ - COMMAND_EXPAND_LISTS) + # TARGET_RUNTIME_DLLS is supported in CMake 3.21+ + if ("${CMAKE_VERSION}" VERSION_LESS "3.21.0") + if (LLDB_INCLUDE_TESTS) + message(SEND_ERROR + "Your CMake version is ${CMAKE_VERSION}. In order to run LLDB tests " + "on Windows please upgrade to 3.21.0 at least (or disable tests with " + "LLDB_INCLUDE_TESTS=Off)") + endif() + else() + add_custom_command(TARGET ${swig_target} POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy -t ${lldb_python_target_dir}> $ + COMMAND_EXPAND_LISTS) + endif() endif() endfunction() From 73300f53be78573b6a7f819cc01b8ceb3ce24bfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Mon, 7 Oct 2024 20:23:35 +0200 Subject: [PATCH 4/7] Include swiftCore.dll as well: transitive but no direct dependency --- lldb/bindings/python/CMakeLists.txt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lldb/bindings/python/CMakeLists.txt b/lldb/bindings/python/CMakeLists.txt index 45b4f48f5b44b..ef73060512746 100644 --- a/lldb/bindings/python/CMakeLists.txt +++ b/lldb/bindings/python/CMakeLists.txt @@ -199,9 +199,9 @@ function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_tar endif() # Since Python3.8 the Windows runtime loads dependent DLLs only from the directory of the binary - # itself (and not Path). Thus, we must copy all our DLLs into the Python package. This is a best - # practice on Windows anyway. - if (WIN32) + # itself (and not Path). Windows has no notion RPATHs, so we must copy all DLLs that we depend on + # into the Python package. Plus swiftCore which is a transitive (but no direct) dependency. + if (WIN32 AND LLDB_ENABLE_SWIFT_SUPPORT) # TARGET_RUNTIME_DLLS is supported in CMake 3.21+ if ("${CMAKE_VERSION}" VERSION_LESS "3.21.0") if (LLDB_INCLUDE_TESTS) @@ -211,8 +211,9 @@ function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_tar "LLDB_INCLUDE_TESTS=Off)") endif() else() + add_dependencies(${swig_target} swiftCore-windows) add_custom_command(TARGET ${swig_target} POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy -t ${lldb_python_target_dir}> $ + COMMAND ${CMAKE_COMMAND} -E copy -t ${lldb_python_target_dir} $ ${SWIFT_BINARY_DIR}/bin/swiftCore.dll COMMAND_EXPAND_LISTS) endif() endif() From 9c18af136b3417988e3f64d6402a2ae8b5df9edc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Tue, 8 Oct 2024 12:50:47 +0200 Subject: [PATCH 5/7] Drop swiftCore dependency and add a note --- lldb/bindings/python/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/bindings/python/CMakeLists.txt b/lldb/bindings/python/CMakeLists.txt index ef73060512746..9ee82cc2cb5c9 100644 --- a/lldb/bindings/python/CMakeLists.txt +++ b/lldb/bindings/python/CMakeLists.txt @@ -211,7 +211,8 @@ function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_tar "LLDB_INCLUDE_TESTS=Off)") endif() else() - add_dependencies(${swig_target} swiftCore-windows) + # swiftCore is defined in swift/stdlib/public/core/CMakeLists.txt + # It's not configured yet, so we cannot depend on it. add_custom_command(TARGET ${swig_target} POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy -t ${lldb_python_target_dir} $ ${SWIFT_BINARY_DIR}/bin/swiftCore.dll COMMAND_EXPAND_LISTS) From 6bf629df0bd3c3c000816175379d5ee2b1a3cab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Fri, 11 Oct 2024 13:40:25 +0200 Subject: [PATCH 6/7] Drop special handling for swiftCore.dll We have to copy it over in build.ps1 since it doesn't exist yet during the initial build. --- lldb/bindings/python/CMakeLists.txt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lldb/bindings/python/CMakeLists.txt b/lldb/bindings/python/CMakeLists.txt index 9ee82cc2cb5c9..25bd64a0485c0 100644 --- a/lldb/bindings/python/CMakeLists.txt +++ b/lldb/bindings/python/CMakeLists.txt @@ -211,10 +211,8 @@ function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_tar "LLDB_INCLUDE_TESTS=Off)") endif() else() - # swiftCore is defined in swift/stdlib/public/core/CMakeLists.txt - # It's not configured yet, so we cannot depend on it. add_custom_command(TARGET ${swig_target} POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy -t ${lldb_python_target_dir} $ ${SWIFT_BINARY_DIR}/bin/swiftCore.dll + COMMAND ${CMAKE_COMMAND} -E copy -t ${lldb_python_target_dir} $ COMMAND_EXPAND_LISTS) endif() endif() From 6e5ef15b7bf155cd50104e4389a4870d194dc06d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Fri, 11 Oct 2024 13:57:18 +0200 Subject: [PATCH 7/7] Drop LLDB_ENABLE_SWIFT_SUPPORT condition and fix comment --- lldb/bindings/python/CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/bindings/python/CMakeLists.txt b/lldb/bindings/python/CMakeLists.txt index 25bd64a0485c0..42299064fd8f7 100644 --- a/lldb/bindings/python/CMakeLists.txt +++ b/lldb/bindings/python/CMakeLists.txt @@ -199,9 +199,9 @@ function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_tar endif() # Since Python3.8 the Windows runtime loads dependent DLLs only from the directory of the binary - # itself (and not Path). Windows has no notion RPATHs, so we must copy all DLLs that we depend on - # into the Python package. Plus swiftCore which is a transitive (but no direct) dependency. - if (WIN32 AND LLDB_ENABLE_SWIFT_SUPPORT) + # itself (and not Path). Windows has no RPATHs, so we must copy all DLLs that we depend on into + # the Python package. + if (WIN32) # TARGET_RUNTIME_DLLS is supported in CMake 3.21+ if ("${CMAKE_VERSION}" VERSION_LESS "3.21.0") if (LLDB_INCLUDE_TESTS)