[lldb][test] Use tools from llvm instead of compiler tools

[dzhidzhoev]

In #102185, toolchain detection for API tests has been rewritten in Python. Tools paths for tests there are determined from compiler path.

Here tools are taken from --llvm-tools-dir dotest.py argument, which by default refers to the LLVM build directory, unless they are explicitly redefined in environment variables. It helps to minimize external dependencies and to maximize the reproducibility of the build.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes

In #102185, toolchain detection for API tests has been rewritten in Python. However, tools paths for tests are determined from compiler path.

Here tools are taken from --llvm-tools-dir dotest.py argument, which by default refers to the LLVM build directory, unless they are explicitly redefined in environment variables. It helps to minimize external dependencies and to maximize the reproducibility of the build.

---

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

5 Files Affected:

• (modified) lldb/packages/Python/lldbsuite/test/builders/builder.py (+11-2)
• (modified) lldb/packages/Python/lldbsuite/test/configuration.py (+3)
• (modified) lldb/packages/Python/lldbsuite/test/dotest.py (+1)
• (modified) lldb/test/API/functionalities/archives/Makefile (-2)
• (modified) lldb/test/API/functionalities/archives/TestBSDArchives.py (-1)

diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index 564918c58b6dd2..90f11fffbb4928 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -110,6 +110,10 @@ def getToolchainSpec(self, compiler):
         if not cc:
             return []
 
+        exe_ext = ""
+        if lldbplatformutil.getHostPlatform() == "windows":
+            exe_ext = ".exe"
+
         cc = cc.strip()
         cc_path = pathlib.Path(cc)
 
@@ -149,9 +153,9 @@ def getToolchainSpec(self, compiler):
         cc_dir = cc_path.parent
 
         def getToolchainUtil(util_name):
-            return cc_dir / (cc_prefix + util_name + cc_ext)
+            return os.path.join(configuration.llvm_tools_dir, util_name + exe_ext)
 
-        cxx = getToolchainUtil(cxx_type)
+        cxx = cc_dir / (cc_prefix + cxx_type + cc_ext)
 
         util_names = {
             "OBJCOPY": "objcopy",
@@ -161,6 +165,11 @@ def getToolchainUtil(util_name):
         }
         utils = []
 
+        # Required by API TestBSDArchives.py tests.
+        # TODO don't forget to fix the test's Makefile when porting to mainline
+        if not os.getenv("LLVM_AR"):
+            utils.extend(["LLVM_AR=%s" % getToolchainUtil("llvm-ar")])
+
         if not lldbplatformutil.platformIsDarwin():
             if cc_type in ["clang", "cc", "gcc"]:
                 util_paths = {}
diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py b/lldb/packages/Python/lldbsuite/test/configuration.py
index 27eef040497d13..1bacd74a968c31 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -118,6 +118,9 @@
 # same base name.
 all_tests = set()
 
+# Path to LLVM tools to be used by tests.
+llvm_tools_dir = None
+
 # LLDB library directory.
 lldb_libs_dir = None
 lldb_obj_root = None
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py
index f14a00a2394b0b..b1ae896d3fd3b4 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -280,6 +280,7 @@ def parseOptionsAndInitTestdirs():
             "xcrun -find -toolchain default dsymutil"
         )
     if args.llvm_tools_dir:
+        configuration.llvm_tools_dir = args.llvm_tools_dir
         configuration.filecheck = shutil.which("FileCheck", path=args.llvm_tools_dir)
         configuration.yaml2obj = shutil.which("yaml2obj", path=args.llvm_tools_dir)
 
diff --git a/lldb/test/API/functionalities/archives/Makefile b/lldb/test/API/functionalities/archives/Makefile
index c4c593e6db0519..4b9696e26b5752 100644
--- a/lldb/test/API/functionalities/archives/Makefile
+++ b/lldb/test/API/functionalities/archives/Makefile
@@ -12,12 +12,10 @@ libfoo.a: a.o b.o
 
 # This tests whether lldb can load a thin archive
 libbar.a: c.o
-	$(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
 	$(eval LLVM_ARFLAGS := -rcsDT)
 	$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
 libfoo-thin.a: a.o b.o
-	$(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
 	$(eval LLVM_ARFLAGS := -rcsUT)
 	$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
diff --git a/lldb/test/API/functionalities/archives/TestBSDArchives.py b/lldb/test/API/functionalities/archives/TestBSDArchives.py
index 1bef8e896e0be7..928e9508617ad6 100644
--- a/lldb/test/API/functionalities/archives/TestBSDArchives.py
+++ b/lldb/test/API/functionalities/archives/TestBSDArchives.py
@@ -25,7 +25,6 @@ def setUp(self):
         oslist=["windows"],
         bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build static libs on Windows",
     )
-    @expectedFailureAll(remote=True)
     def test(self):
         """Break inside a() and b() defined within libfoo.a."""
         self.build()

[DavidSpickett]

Is there some tool that every bot is going to have to add an override for because it's not something in <build>/bin?

If there is such a tool, that doesn't make this change bad, but we may want to delay while that's accounted for.

[DavidSpickett]

A better version of my question, do you have a list of tools this effects, is it just the ones where you have modified lines in this PR?

If so we probably have nothing to worry about.

[dzhidzhoev]

A better version of my question, do you have a list of tools this effects, is it just the ones where you have modified lines in this PR?

If so we probably have nothing to worry about.

These are the tools listed in util_names dictionary

llvm-project/lldb/packages/Python/lldbsuite/test/builders/builder.py

Line 160 in 025276a

util_names = {

[dzhidzhoev]

Is there some tool that every bot is going to have to add an override for because it's not something in <build>/bin?

AFAIK no. All these tools have llvm alternatives.

[DavidSpickett]

Decide what to do with the remote test change but otherwise this LGTM.

[dzhidzhoev]

Decide what to do with the remote test change but otherwise this LGTM.

Some other tests are unexpectedly passing on remote config, so they'll need extra attention.