[lldb] Drop client-side Python pre-load and remove PythonPathSetup#200533
Open
JDevlieghere wants to merge 1 commit into
Open
[lldb] Drop client-side Python pre-load and remove PythonPathSetup#200533JDevlieghere wants to merge 1 commit into
JDevlieghere wants to merge 1 commit into
Conversation
The previous arrangement required clients of liblldb to call into the PythonRuntimeLoader before any scripting work, which only held for in-tree clients (lldb, lldb-dap) that link lldbHost directly. External clients reaching liblldb through the LLDB.framework / RPC boundary (e.g. Xcode) cannot do that and would fail to load the script interpreter plugin. The Python load is now centralized in PluginManager::PluginInfo::Create right before dlopen'ing the script interpreter plugin, so any consumer that drives PluginManager::Initialize gets the same behavior with no client-side cooperation. This commit removes the now-redundant client calls and the helper they used to depend on: - lldb driver: drops the Windows-only pre-load entirely; the central call covers it. - lldb-dap: drops the post-version pre-load for the same reason. Keeps --check-python (which intentionally surfaces the resolved path for IDE integrations) and rewrites it on PythonRuntimeLoader's Load() / GetLoadedPath() class API. - lldbHostPythonPathSetup is removed: PythonRuntimeLoaderWindows.cpp has subsumed its responsibilities, so neither the driver, lldb-dap, nor lldbHost link it any longer. The lldb/Host/windows/PythonPathSetup sources and header are deleted with it.
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesThe previous arrangement required clients of liblldb to call into the PythonRuntimeLoader before any scripting work, which only held for in-tree clients (lldb, lldb-dap) that link lldbHost directly. External clients reaching liblldb through the LLDB.framework / RPC boundary (e.g. Xcode) cannot do that and would fail to load the script interpreter plugin. The Python load is now centralized in PluginManager::PluginInfo::Create right before dlopen'ing the script interpreter plugin, so any consumer that drives PluginManager::Initialize gets the same behavior with no client-side cooperation. This commit removes the now-redundant client calls and the helper they used to depend on:
Full diff: https://github.com/llvm/llvm-project/pull/200533.diff 8 Files Affected:
diff --git a/lldb/include/lldb/Host/windows/PythonPathSetup/PythonPathSetup.h b/lldb/include/lldb/Host/windows/PythonPathSetup/PythonPathSetup.h
deleted file mode 100644
index ca46c8b823cd1..0000000000000
--- a/lldb/include/lldb/Host/windows/PythonPathSetup/PythonPathSetup.h
+++ /dev/null
@@ -1,52 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_SOURCE_HOST_PYTHONPATHSETUP_H
-#define LLDB_SOURCE_HOST_PYTHONPATHSETUP_H
-
-#include "llvm/Support/Error.h"
-
-#ifdef LLDB_PYTHON_DLL_RELATIVE_PATH
-/// Resolve the full path of the directory defined by
-/// LLDB_PYTHON_DLL_RELATIVE_PATH. If it exists, add it to the list of DLL
-/// search directories.
-///
-/// \return `true` if the library was added to the search path.
-/// `false` otherwise.
-bool AddPythonDLLToSearchPath();
-#endif
-
-/// Attempts to setup the DLL search path for the Python runtime library.
-///
-/// In the following paragraphs, python3xx.dll refers to the Python runtime
-/// library name which is defined by LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME, e.g.
-/// python311.dll for Python 3.11.
-///
-/// The setup flow depends on which macros are defined:
-///
-/// - If only LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME is defined, checks whether
-/// python3xx.dll can be loaded. Returns an error if it cannot.
-///
-/// - If only LLDB_PYTHON_DLL_RELATIVE_PATH is defined, attempts to resolve the
-/// relative path and add it to the DLL search path. Returns an error if this
-/// fails. Note that this may succeed even if python3xx.dll is not present in
-/// the added search path.
-///
-/// - If both LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME and
-/// LLDB_PYTHON_DLL_RELATIVE_PATH are defined, first checks if python3xx.dll
-/// can be loaded. If successful, returns immediately. Otherwise, attempts to
-/// resolve the relative path and add it to the DLL search path, then checks
-/// again if python3xx.dll can be loaded.
-///
-/// \return If LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME is defined, return the
-/// absolute path of the Python shared library which was resolved or an error if
-/// it could not be found. If LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME and
-/// LLDB_PYTHON_DLL_RELATIVE_PATH are not defined, return an empty string.
-llvm::Expected<std::string> SetupPythonRuntimeLibrary();
-
-#endif // LLDB_SOURCE_HOST_PYTHONPATHSETUP_H
diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index f7bd00457613a..9e0d60c149550 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -66,7 +66,6 @@ add_host_subdirectory(posix
)
if (CMAKE_SYSTEM_NAME MATCHES "Windows")
- add_subdirectory(windows/PythonPathSetup)
add_host_subdirectory(windows
windows/ConnectionConPTYWindows.cpp
windows/ConnectionGenericFileWindows.cpp
diff --git a/lldb/source/Host/windows/PythonPathSetup/CMakeLists.txt b/lldb/source/Host/windows/PythonPathSetup/CMakeLists.txt
deleted file mode 100644
index 96f2b482b376f..0000000000000
--- a/lldb/source/Host/windows/PythonPathSetup/CMakeLists.txt
+++ /dev/null
@@ -1,10 +0,0 @@
-add_lldb_library(lldbHostPythonPathSetup STATIC
- PythonPathSetup.cpp
-)
-
-if(DEFINED LLDB_PYTHON_DLL_RELATIVE_PATH)
- target_compile_definitions(lldbHostPythonPathSetup PRIVATE LLDB_PYTHON_DLL_RELATIVE_PATH="${LLDB_PYTHON_DLL_RELATIVE_PATH}")
-endif()
-if(DEFINED LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME)
- target_compile_definitions(lldbHostPythonPathSetup PRIVATE LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME="${LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME}")
-endif()
diff --git a/lldb/source/Host/windows/PythonPathSetup/PythonPathSetup.cpp b/lldb/source/Host/windows/PythonPathSetup/PythonPathSetup.cpp
deleted file mode 100644
index 0da2267e8c78a..0000000000000
--- a/lldb/source/Host/windows/PythonPathSetup/PythonPathSetup.cpp
+++ /dev/null
@@ -1,103 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Host/windows/PythonPathSetup/PythonPathSetup.h"
-
-#include "lldb/Host/windows/windows.h"
-#include "llvm/Support/Windows/WindowsSupport.h"
-
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/Support/ConvertUTF.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Path.h"
-#include <pathcch.h>
-
-using namespace llvm;
-
-#if defined(LLDB_PYTHON_DLL_RELATIVE_PATH) || \
- defined(LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME)
-static std::string GetModulePath(HMODULE module) {
- std::vector<WCHAR> buffer(MAX_PATH);
- while (buffer.size() <= PATHCCH_MAX_CCH) {
- DWORD len = GetModuleFileNameW(module, buffer.data(), buffer.size());
- if (len == 0)
- return "";
- if (len < buffer.size()) {
- std::string buffer_utf8;
- if (convertWideToUTF8(std::wstring(buffer.data(), len), buffer_utf8))
- return buffer_utf8;
- return "";
- }
- if (::GetLastError() == ERROR_INSUFFICIENT_BUFFER)
- buffer.resize(buffer.size() * 2);
- }
- return "";
-}
-#endif
-
-#ifdef LLDB_PYTHON_DLL_RELATIVE_PATH
-/// Returns the full path to the lldb.exe executable.
-static std::string GetPathToExecutable() { return GetModulePath(nullptr); }
-
-bool AddPythonDLLToSearchPath() {
- std::string path_str = GetPathToExecutable();
- if (path_str.empty())
- return false;
-
- SmallVector<char, MAX_PATH> path(path_str.begin(), path_str.end());
- sys::path::remove_filename(path);
- sys::path::append(path, LLDB_PYTHON_DLL_RELATIVE_PATH);
- sys::fs::make_absolute(path);
-
- SmallVector<wchar_t, 1> path_wide;
- if (sys::windows::widenPath(path.data(), path_wide))
- return false;
-
- if (sys::fs::exists(path))
- return SetDllDirectoryW(path_wide.data());
- return false;
-}
-#endif
-
-#ifdef LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME
-std::optional<std::string> GetPythonDLLPath() {
-#define WIDEN2(x) L##x
-#define WIDEN(x) WIDEN2(x)
- HMODULE h = LoadLibraryW(WIDEN(LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME));
- if (!h)
- return std::nullopt;
-
- std::string path = GetModulePath(h);
- FreeLibrary(h);
-
- return path;
-#undef WIDEN2
-#undef WIDEN
-}
-#endif
-
-llvm::Expected<std::string> SetupPythonRuntimeLibrary() {
-#ifdef LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME
- if (std::optional<std::string> python_path = GetPythonDLLPath())
- return *python_path;
-#ifdef LLDB_PYTHON_DLL_RELATIVE_PATH
- if (AddPythonDLLToSearchPath()) {
- if (std::optional<std::string> python_path = GetPythonDLLPath())
- return *python_path;
- }
-#endif
- return createStringError(
- inconvertibleErrorCode(),
- "unable to find '" LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME "'");
-#elif defined(LLDB_PYTHON_DLL_RELATIVE_PATH)
- if (!AddPythonDLLToSearchPath())
- return createStringError(inconvertibleErrorCode(),
- "unable to find the Python runtime library");
-#endif
- return "";
-}
diff --git a/lldb/tools/driver/CMakeLists.txt b/lldb/tools/driver/CMakeLists.txt
index 1c93ed9fad927..0534ddc9a58c3 100644
--- a/lldb/tools/driver/CMakeLists.txt
+++ b/lldb/tools/driver/CMakeLists.txt
@@ -34,10 +34,6 @@ set(LLDB_DRIVER_LINK_LIBS
lldbUtility
)
-if(WIN32)
- list(APPEND LLDB_DRIVER_LINK_LIBS lldbHostPythonPathSetup)
-endif()
-
add_lldb_tool(lldb
Driver.cpp
Platform.cpp
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index e58286f9ff41e..6020e5a279079 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -33,10 +33,6 @@
#include "llvm/Support/WithColor.h"
#include "llvm/Support/raw_ostream.h"
-#ifdef _WIN32
-#include "lldb/Host/windows/PythonPathSetup/PythonPathSetup.h"
-#endif
-
#include <algorithm>
#include <atomic>
#include <bitset>
@@ -735,13 +731,6 @@ int main(int argc, char const *argv[]) {
"~/Library/Logs/DiagnosticReports/.\n");
#endif
-#ifdef _WIN32
- auto python_path_or_err = SetupPythonRuntimeLibrary();
- if (!python_path_or_err)
- llvm::WithColor::error()
- << llvm::toString(python_path_or_err.takeError()) << '\n';
-#endif
-
// Parse arguments.
LLDBOptTable T;
unsigned MissingArgIndex;
diff --git a/lldb/tools/lldb-dap/tool/CMakeLists.txt b/lldb/tools/lldb-dap/tool/CMakeLists.txt
index fbd3cf0cd4377..33263cb5adb6b 100644
--- a/lldb/tools/lldb-dap/tool/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/tool/CMakeLists.txt
@@ -4,10 +4,6 @@ add_public_tablegen_target(LLDBDAPOptionsTableGen)
set(LLDB_DAP_LINK_LIBS lldbDAP liblldb lldbHost)
-if(WIN32)
- list(APPEND LLDB_DAP_LINK_LIBS lldbHostPythonPathSetup)
-endif()
-
add_lldb_tool(lldb-dap
lldb-dap.cpp
diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
index c78eb2d1f23a7..91c00f5fc357b 100644
--- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
@@ -76,7 +76,7 @@
#include <io.h>
typedef int socklen_t;
#include "lldb/Host/windows/ProcessLauncherWindows.h"
-#include "lldb/Host/windows/PythonPathSetup/PythonPathSetup.h"
+#include "lldb/Host/PythonRuntimeLoader.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/Program.h"
#else
@@ -771,13 +771,12 @@ int main(int argc, char *argv[]) {
llvm::errs() << "lldb-dap was not built with Python support" << '\n';
return EXIT_SUCCESS;
#endif
- auto python_path_or_err = SetupPythonRuntimeLibrary();
- if (!python_path_or_err) {
- llvm::WithColor::error()
- << llvm::toString(python_path_or_err.takeError()) << '\n';
+ if (llvm::Error err = lldb_private::PythonRuntimeLoader::Load()) {
+ llvm::WithColor::error() << llvm::toString(std::move(err)) << '\n';
return EXIT_FAILURE;
}
- std::string python_path = *python_path_or_err;
+ llvm::StringRef python_path =
+ lldb_private::PythonRuntimeLoader::GetLoadedPath();
if (python_path.empty()) {
llvm::WithColor::error()
<< "unable to look for the Python shared library" << '\n';
@@ -786,11 +785,6 @@ int main(int argc, char *argv[]) {
llvm::outs() << python_path << '\n';
return EXIT_SUCCESS;
}
-
- auto python_path_or_err = SetupPythonRuntimeLibrary();
- if (!python_path_or_err)
- llvm::WithColor::error()
- << llvm::toString(python_path_or_err.takeError()) << '\n';
#endif
if (input_args.hasArg(OPT_client)) {
|
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp -- lldb/tools/driver/Driver.cpp lldb/tools/lldb-dap/tool/lldb-dap.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
index 91c00f5fc..7bb41c03e 100644
--- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
@@ -75,8 +75,8 @@
#undef GetObject
#include <io.h>
typedef int socklen_t;
-#include "lldb/Host/windows/ProcessLauncherWindows.h"
#include "lldb/Host/PythonRuntimeLoader.h"
+#include "lldb/Host/windows/ProcessLauncherWindows.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/Program.h"
#else
|
12 tasks
🪟 Windows x64 Test ResultsThe build failed before running any tests. Click on a failure below to see the details. [code=1] tools/lldb/tools/lldb-dap/tool/CMakeFiles/lldb-dap.dir/lldb-dap.cpp.objIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The previous arrangement required clients of liblldb to call into the PythonRuntimeLoader before any scripting work, which only held for in-tree clients (lldb, lldb-dap) that link lldbHost directly. External clients reaching liblldb through the LLDB.framework / RPC boundary (e.g. Xcode) cannot do that and would fail to load the script interpreter plugin.
The Python load is now centralized in PluginManager::PluginInfo::Create right before dlopen'ing the script interpreter plugin, so any consumer that drives PluginManager::Initialize gets the same behavior with no client-side cooperation. This commit removes the now-redundant client calls and the helper they used to depend on: