Skip to content
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

[lldb] Fix finding make tool for tests #111980

Merged
merged 1 commit into from
Oct 11, 2024
Merged

Conversation

DavidSpickett
Copy link
Collaborator

Fixes 0e91323 / #111531

For reasons I can't explain, a clean build works fine for me, and all the bots are working fine. But if I rebuild in some way the make tool becomes None.

Looking at the other variables, they had these extra lines so I've added those for make and it seems to solve the problem.

Fixes 0e91323

For reasons I can't explain, a clean build works fine for me,
and all the bots are working fine. But if I rebuild in some
way the make tool becomes None.

Looking at the other variables, they had these extra lines
so I've added those for make and it seems to solve the problem.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 11, 2024

@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)

Changes

Fixes 0e91323 / #111531

For reasons I can't explain, a clean build works fine for me, and all the bots are working fine. But if I rebuild in some way the make tool becomes None.

Looking at the other variables, they had these extra lines so I've added those for make and it seems to solve the problem.


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

2 Files Affected:

  • (modified) lldb/utils/lldb-dotest/CMakeLists.txt (+1)
  • (modified) lldb/utils/lldb-dotest/lldb-dotest.in (+2)
diff --git a/lldb/utils/lldb-dotest/CMakeLists.txt b/lldb/utils/lldb-dotest/CMakeLists.txt
index bb17a2ce017d5d..3b8c88b6dc78cb 100644
--- a/lldb/utils/lldb-dotest/CMakeLists.txt
+++ b/lldb/utils/lldb-dotest/CMakeLists.txt
@@ -34,6 +34,7 @@ set(vars
   LLDB_TEST_EXECUTABLE
   LLDB_TEST_COMPILER
   LLDB_TEST_DSYMUTIL
+  LLDB_TEST_MAKE
   LLDB_LIBS_DIR
   LLVM_TOOLS_DIR
   LIBCXX_LIBRARY_DIR
diff --git a/lldb/utils/lldb-dotest/lldb-dotest.in b/lldb/utils/lldb-dotest/lldb-dotest.in
index 0e9648a6e6dc83..f20859e87b1e0b 100755
--- a/lldb/utils/lldb-dotest/lldb-dotest.in
+++ b/lldb/utils/lldb-dotest/lldb-dotest.in
@@ -9,6 +9,7 @@ arch = '@LLDB_TEST_ARCH@'
 executable = '@LLDB_TEST_EXECUTABLE_CONFIGURED@'
 compiler = '@LLDB_TEST_COMPILER_CONFIGURED@'
 dsymutil = '@LLDB_TEST_DSYMUTIL_CONFIGURED@'
+make = '@LLDB_TEST_MAKE_CONFIGURED@'
 lldb_build_dir = '@LLDB_TEST_BUILD_DIRECTORY_CONFIGURED@'
 lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@"
 lldb_framework_dir = "@LLDB_FRAMEWORK_DIR_CONFIGURED@"
@@ -35,6 +36,7 @@ if __name__ == '__main__':
     cmd.extend(['--executable', executable])
     cmd.extend(['--compiler', compiler])
     cmd.extend(['--dsymutil', dsymutil])
+    cmd.extend(['--make', make])
     cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
     cmd.extend(['--llvm-tools-dir', llvm_tools_dir])
     if has_libcxx:

@DavidSpickett
Copy link
Collaborator Author

Putting this in review because I've zero idea why this fixes it. Without it I get this on a rebuild:

Traceback (most recent call last):
  File "/home/david.spickett/llvm-project/lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py", line 67, in test_reverse_continue_skip_breakpoint_async
    self.reverse_continue_skip_breakpoint_internal(async_mode=True)
  File "/home/david.spickett/llvm-project/lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py", line 70, in reverse_continue_skip_breakpoint_internal
    target, process, initial_threads = self.setup_recording(async_mode)
  File "/home/david.spickett/llvm-project/lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py", line 94, in setup_recording
    self.build()
  File "/home/david.spickett/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1496, in build
    self.runBuildCommand(command)
  File "/home/david.spickett/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1501, in runBuildCommand
    output = check_output(command, stderr=STDOUT, errors="replace")
  File "/usr/lib/python3.10/subprocess.py", line 421, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "/usr/lib/python3.10/subprocess.py", line 503, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/lib/python3.10/subprocess.py", line 971, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.10/subprocess.py", line 1738, in _execute_child
    and os.path.dirname(executable)
  File "/usr/lib/python3.10/posixpath.py", line 152, in dirname
    p = os.fspath(p)
TypeError: expected str, bytes or os.PathLike object, not NoneType
Config=aarch64-/home/david.spickett/build-llvm-aarch64/bin/clang

Because the make passed in command is actually None. Perhaps the cache isn't read properly after the first configuration?

Copy link
Contributor

@weliveindetail weliveindetail left a comment

Choose a reason for hiding this comment

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

Interesting! Your changes look fine, as they are symmetric with what we do here for dsymutil. I just wonder why I didn't hit that. Is this a separate test driver for Linux?

@DavidSpickett
Copy link
Collaborator Author

I don't think so, it's used to generate dotest.py on all platforms I think. Perhaps my cmake is being strict / less strict? The funny thing is, I can see LLDB_TEST_MAKE set in the cache even when the build breaks.

@DavidSpickett DavidSpickett merged commit a2bd5db into llvm:main Oct 11, 2024
9 checks passed
@DavidSpickett DavidSpickett deleted the lldb-make branch October 11, 2024 13:14
ichaer added a commit to splunk/ichaer-llvm-project that referenced this pull request Oct 11, 2024
…ent-indentonly

* llvm-trunk/main: (6379 commits)
  [gn build] Port 1c94388
  [RISCV] Introduce VLOptimizer pass (llvm#108640)
  [mlir][vector] Add more tests for ConvertVectorToLLVM (7/n) (llvm#111895)
  [libc++] Add output groups to run-buildbot (llvm#111739)
  [libc++abi] Remove unused LIBCXXABI_LIBCXX_INCLUDES CMake option (llvm#111824)
  [clang] Ignore inline namespace for `hasName` (llvm#109147)
  [AArch64] Disable consecutive store merging when Neon is unavailable (llvm#111519)
  [lldb] Fix finding make tool for tests (llvm#111980)
  Turn `-Wdeprecated-literal-operator` on by default (llvm#111027)
  [AMDGPU] Rewrite RegSeqNames using !foreach. NFC. (llvm#111994)
  Revert "Reland: [clang] Finish implementation of P0522 (llvm#111711)"
  Revert "[clang] CWG2398: improve overload resolution backwards compat (llvm#107350)"
  Revert "[clang] Implement TTP P0522 pack matching for deduced function template calls. (llvm#111457)"
  [Clang] Replace Intrinsic::getDeclaration with getOrInsertDeclaration (llvm#111990)
  Revert "[NVPTX] Prefer prmt.b32 over bfi.b32 (llvm#110766)"
  [RISCV] Add DAG combine to turn (sub (shl X, 8-Y), (shr X, Y)) into orc.b (llvm#111828)
  [libc] Fix compilation of new trig functions (llvm#111987)
  [NFC] Rename `Intrinsic::getDeclaration` to `getOrInsertDeclaration` (llvm#111752)
  [NFC][CodingStandard] Add additional example for if-else brace rule (llvm#111733)
  CodeGen: Remove redundant REQUIRES registered-target from tests (llvm#111982)
  ...
@labath
Copy link
Collaborator

labath commented Oct 14, 2024

lldb-dotest is a separate tool, which is usually only invoked manually when debugging a specific test (and not invoked by make check-lldb like the bots do). This fact that its args need to stay in sync is annoying an a relatively common problem. I feel that there must be a way to have a single source of truth for the testing args, but I haven't found the time to figure one out yet (patches welcome).

DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
Fixes 0e91323 /
llvm#111531

For reasons I can't explain, a clean build works fine for me, and all
the bots are working fine. But if I rebuild in some way the make tool
becomes None.

Looking at the other variables, they had these extra lines so I've added
those for make and it seems to solve the problem.
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024
Fixes 0e91323 /
llvm#111531

For reasons I can't explain, a clean build works fine for me, and all
the bots are working fine. But if I rebuild in some way the make tool
becomes None.

Looking at the other variables, they had these extra lines so I've added
those for make and it seems to solve the problem.
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.

4 participants