Skip to content

[lldb] Remove cmake check for pexpect with LLDB_TEST_USE_VENDOR_PACKAGES #86962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

rupprecht
Copy link
Collaborator

The commit 8bed754 was intended to support the use case where users want to run all the LLDB tests in an environment where pexpect is not installed. Those users can build with -DLLDB_TEST_USER_ARGS=--skip-category=pexpect to skip pexpect tests, but because we still fail in cmake configuration, they must use -DLLDB_TEST_USE_VENDOR_PACKAGES=ON to avoid failing due to pexpect not being available.

I would like to remove LLDB_TEST_USE_VENDOR_PACKAGES now, but first I'd like to make sure users w/o pexpect can pass CI with -DLLDB_TEST_USE_VENDOR_PACKAGES=OFF -DLLDB_TEST_USER_ARGS=--skip-category=pexpect. Once that is done, I am not aware of any other issues caused by the previous commits, so the third party tree should be safe to remove.

…GES is off

The commit 8bed754 was intended to support the use case where users want to run all the LLDB tests in an environment where pexpect is not installed. Those users can build with `-DLLDB_TEST_USER_ARGS=--skip-category=pexpect` to skip pexpect tests, *but* because we still fail in cmake configuration, they must use `-DLLDB_TEST_USE_VENDOR_PACKAGES=ON` to avoid failing due to pexpect not being available.

I would like to remove `LLDB_TEST_USE_VENDOR_PACKAGES` now, but first I'd like to make sure users w/o pexpect can pass CI with `-DLLDB_TEST_USE_VENDOR_PACKAGES=OFF -DLLDB_TEST_USER_ARGS=--skip-category=pexpect`.
@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2024

@llvm/pr-subscribers-lldb

Author: Jordan Rupprecht (rupprecht)

Changes

The commit 8bed754 was intended to support the use case where users want to run all the LLDB tests in an environment where pexpect is not installed. Those users can build with -DLLDB_TEST_USER_ARGS=--skip-category=pexpect to skip pexpect tests, but because we still fail in cmake configuration, they must use -DLLDB_TEST_USE_VENDOR_PACKAGES=ON to avoid failing due to pexpect not being available.

I would like to remove LLDB_TEST_USE_VENDOR_PACKAGES now, but first I'd like to make sure users w/o pexpect can pass CI with -DLLDB_TEST_USE_VENDOR_PACKAGES=OFF -DLLDB_TEST_USER_ARGS=--skip-category=pexpect. Once that is done, I am not aware of any other issues caused by the previous commits, so the third party tree should be safe to remove.


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

1 Files Affected:

  • (modified) lldb/test/CMakeLists.txt (-17)
diff --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index 0ef2eb1c42ce06..b6ec5bc4d819e3 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -31,23 +31,6 @@ if(LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS)
   endforeach()
 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.
-# 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)
-    message(FATAL_ERROR
-      "Python module 'pexpect' not found. Please install it via pip or via "
-      "your operating system's package manager. For a temporary workaround, "
-      "use a version from the LLDB tree with "
-      "`LLDB_TEST_USE_VENDOR_PACKAGES=ON`")
-  endif()
-endif()
-
 if(LLDB_BUILT_STANDALONE)
   # In order to run check-lldb-* we need the correct map_config directives in
   # llvm-lit. Because this is a standalone build, LLVM doesn't know about LLDB,

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for us!

@rupprecht rupprecht merged commit a834076 into llvm:main Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants