[lldb][test] Enforce pexpect system availability by default

[rupprecht]

This switches the default of LLDB_TEST_USE_VENDOR_PACKAGES from ON to OFF in preparation for eventually deleting it. All known LLDB buildbots have this package installed, so flipping the default will uncover any other users.

If this breaks anything, the preferred fix is to install pexpect on the host system. The second fix is to build with cmake option -DLLDB_TEST_USE_VENDOR_PACKAGES=ON as a temporary measure until pexpect can be installed. If neither of those work, reverting this patch is OK.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jordan Rupprecht (rupprecht)

Changes

This switches the default of LLDB_TEST_USE_VENDOR_PACKAGES from ON to OFF in preparation for eventually deleting it. All known LLDB buildbots have this package installed, so flipping the default will uncover any other users.

If this breaks anything, the preferred fix is to install pexpect on the host system. The second fix is to build with cmake option -DLLDB_TEST_USE_VENDOR_PACKAGES=ON as a temporary measure until pexpect can be installed. If neither of those work, reverting this patch is OK.

---

Full diff: https://github.com/llvm/llvm-project/pull/84270.diff

2 Files Affected:

• (modified) lldb/cmake/modules/LLDBConfig.cmake (+1-1)
• (modified) lldb/test/CMakeLists.txt (+3-1)

diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake
index 93c8ffe4b7d8a0..5d62213c3f5838 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -68,7 +68,7 @@ option(LLDB_SKIP_DSYM "Whether to skip generating a dSYM when installing lldb."
 option(LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS
   "Fail to configure if certain requirements are not met for testing." OFF)
 option(LLDB_TEST_USE_VENDOR_PACKAGES
-  "Use packages from lldb/third_party/Python/module instead of system deps." ON)
+  "Use packages from lldb/third_party/Python/module instead of system deps." OFF)
 
 set(LLDB_GLOBAL_INIT_DIRECTORY "" CACHE STRING
   "Path to the global lldbinit directory. Relative paths are resolved relative to the
diff --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index 950643a5b8cc8e..0ef2eb1c42ce06 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -34,7 +34,9 @@ endif()
 # The "pexpect" package should come from the system environment, not from the
 # LLDB tree. However, we delay the deletion of it from the tree in case
 # users/buildbots don't have the package yet and need some time to install it.
-if (NOT LLDB_TEST_USE_VENDOR_PACKAGES)
+# Windows is configured to skip all pexpect tests, and guards all
+# "import pexpect" calls, so we do not need pexpect installed there.
+if (NOT LLDB_TEST_USE_VENDOR_PACKAGES AND NOT WIN32)
   unset(PY_pexpect_FOUND CACHE)
   lldb_find_python_module(pexpect)
   if (NOT PY_pexpect_FOUND)

[rupprecht]

btw, it would be nice if LLDB buildbots were clean before landing this. https://green.lab.llvm.org/green/view/LLDB/ is currently giving me ERR_CONNECTION_REFUSED. The others are failing but I'm hoping ec72909 restores that.

[JDevlieghere]

https://green.lab.llvm.org/green/view/LLDB/ is currently giving me ERR_CONNECTION_REFUSED.tores that.

Yeah, unfortunately I don't have an ETA for when this will be back online. It's probably not worth waiting on that. Changing the configuration of these bots is almost instant, so if they come back and it turns out this did cause an issue, I (or our build wrangler) can easily turn it back off just for green dragon while we investigate.

[mysterymath]

Chiming in on behalf of Fuchsia, since this did break us.

We build and ship a custom Python with our LLDB, since we're intended to be a portable (i.e., drop it in a directory) distribution. That Python probably wouldn't have pexpect, unless it's something that comes with the default distribution. If this is currently coming from LLDB, then this change would intrinsically break us pretty hard, and it's not straightfoward to bundle additional software with our Python (we shouldn't access pip in our builders, so we'd need to manually manage the dep as a blob).

[rupprecht]

@mysterymath do you run the API test suites with that custom python, i.e. ninja check-lldb-api? If so, would you be able to add bundle pexpect for your tests even if it's not in what you ship?

The goal is to be able to delete https://github.com/llvm/llvm-project/tree/main/lldb/third_party/Python/module/pexpect-4.6. This library is only used in the API tests.

The goal of the cmake check here is to be able to have an ~immediate feedback loop that pexpect needs to be installed, instead of waiting a long time until ninja check-lldb-api runs the tests and discovers that import pexpect fails. If you don't run these tests, but the cmake warning is still blocking you, maybe we should just remove the check. LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS serves essentially the same purpose but is opt-in.

[mysterymath]

@mysterymath do you run the API test suites with that custom python, i.e. ninja check-lldb-api? If so, would you be able to add bundle pexpect for your tests even if it's not in what you ship?

We do run the tests with the Python we ship, since we don't have direct control of the version that is present on our builders, and Python doesn't have ABI compatibility sufficient for LLDB's purposes across minor versions. We've generally tried to keep our builds hermetic and not build anything against host libraries unless absolutely necessary.

We could possibly add pexpect to our shipped Python, but I haven't scoped what that would entail yet; there may be some bureaucratic hoops to jump through. I wouldn't expect much sympathy on that front of course, but it does seem like adding out-of-tree deps to the Python used by LLDB's test suite does make it more difficult to package a hermetic distribution of LLVM+LLDB.