[lldb][test] Use $(STRIP) instead of strip in API tests (Darwin-only change) #111816

dzhidzhoev · 2024-10-10T11:50:52Z

This makes tests more portable.
Make variables for LLVM utils are passed to make on Darwin as well.

This allows to make tests more portable. Environment variables for LLVM utils are passed to `make` on Darwin as well.

llvmbot · 2024-10-10T11:51:17Z

@llvm/pr-subscribers-lldb

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes

This makes tests more portable.
Environment variables for LLVM utils are passed to make on Darwin as well.

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

7 Files Affected:

(modified) lldb/packages/Python/lldbsuite/test/builders/builder.py (+23-23)
(modified) lldb/test/API/lang/objc/hidden-ivars/Makefile (+2-2)
(modified) lldb/test/API/lang/objc/objc-ivar-stripped/Makefile (+1-1)
(modified) lldb/test/API/lang/objc/objc-static-method-stripped/Makefile (+1-1)
(modified) lldb/test/API/macosx/add-dsym/Makefile (+1-1)
(modified) lldb/test/API/tools/lldb-dap/module/Makefile (+1-1)
(modified) lldb/test/API/tools/lldb-dap/terminated-event/Makefile (+1-1)

diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index f813d68e46e82a..d399a5b228c131 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -169,31 +169,31 @@ def getToolchainUtil(util_name):
         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 = {}
-                # Assembly a toolchain side tool cmd based on passed CC.
-                for var, name in util_names.items():
-                    # Do not override explicity specified tool from the cmd line.
-                    if not os.getenv(var):
-                        util_paths[var] = getToolchainUtil("llvm-" + name)
-                    else:
-                        util_paths[var] = os.getenv(var)
-                utils.extend(["AR=%s" % util_paths["ARCHIVER"]])
-
-                # Look for llvm-dwp or gnu dwp
-                if not lldbutil.which(util_paths["DWP"]):
-                    util_paths["DWP"] = getToolchainUtil("llvm-dwp")
-                if not lldbutil.which(util_paths["DWP"]):
-                    util_paths["DWP"] = lldbutil.which("llvm-dwp")
+        if cc_type in ["clang", "cc", "gcc"]:
+            util_paths = {}
+            # Assembly a toolchain side tool cmd based on passed CC.
+            for var, name in util_names.items():
+                # Do not override explicity specified tool from the cmd line.
+                if not os.getenv(var):
+                    util_paths[var] = getToolchainUtil("llvm-" + name)
+                else:
+                    util_paths[var] = os.getenv(var)
+            utils.extend(["AR=%s" % util_paths["ARCHIVER"]])
+
+            # Look for llvm-dwp or gnu dwp
+            if not lldbutil.which(util_paths["DWP"]):
+                util_paths["DWP"] = getToolchainUtil("llvm-dwp")
+            if not lldbutil.which(util_paths["DWP"]):
+                util_paths["DWP"] = lldbutil.which("llvm-dwp")
+            if not util_paths["DWP"]:
+                util_paths["DWP"] = lldbutil.which("dwp")
                 if not util_paths["DWP"]:
-                    util_paths["DWP"] = lldbutil.which("dwp")
-                    if not util_paths["DWP"]:
-                        del util_paths["DWP"]
+                    del util_paths["DWP"]
 
-                for var, path in util_paths.items():
-                    utils.append("%s=%s" % (var, path))
-        else:
+            for var, path in util_paths.items():
+                utils.append("%s=%s" % (var, path))
+
+        if lldbplatformutil.platformIsDarwin():
             utils.extend(["AR=%slibtool" % os.getenv("CROSS_COMPILE", "")])
 
         return [
diff --git a/lldb/test/API/lang/objc/hidden-ivars/Makefile b/lldb/test/API/lang/objc/hidden-ivars/Makefile
index 283e8a118fb16a..c94c0dee1b9ce9 100644
--- a/lldb/test/API/lang/objc/hidden-ivars/Makefile
+++ b/lldb/test/API/lang/objc/hidden-ivars/Makefile
@@ -14,8 +14,8 @@ endif
 
 stripped: a.out libInternalDefiner.dylib
 	mkdir stripped
-	strip -Sx a.out -o stripped/a.out
-	strip -Sx libInternalDefiner.dylib -o stripped/libInternalDefiner.dylib
+	$(STRIP) -Sx a.out -o stripped/a.out
+	$(STRIP) -Sx libInternalDefiner.dylib -o stripped/libInternalDefiner.dylib
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -fs - stripped/a.out
 endif
diff --git a/lldb/test/API/lang/objc/objc-ivar-stripped/Makefile b/lldb/test/API/lang/objc/objc-ivar-stripped/Makefile
index 8b63215d6d9da6..eed66d2a965d11 100644
--- a/lldb/test/API/lang/objc/objc-ivar-stripped/Makefile
+++ b/lldb/test/API/lang/objc/objc-ivar-stripped/Makefile
@@ -6,7 +6,7 @@ all:        a.out.stripped
 include Makefile.rules
 
 a.out.stripped: a.out.dSYM
-	strip -o a.out.stripped a.out
+	$(STRIP) -o a.out.stripped a.out
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -fs - a.out.stripped
 endif
diff --git a/lldb/test/API/lang/objc/objc-static-method-stripped/Makefile b/lldb/test/API/lang/objc/objc-static-method-stripped/Makefile
index ed312938c9cd11..4936553c56f7c0 100644
--- a/lldb/test/API/lang/objc/objc-static-method-stripped/Makefile
+++ b/lldb/test/API/lang/objc/objc-static-method-stripped/Makefile
@@ -4,7 +4,7 @@ LD_EXTRAS := -lobjc -framework Foundation
 default:        a.out.stripped
 
 a.out.stripped: a.out.dSYM
-	strip -o a.out.stripped a.out
+	$(STRIP) -o a.out.stripped a.out
 	ln -sf a.out.dSYM a.out.stripped.dSYM
 
 include Makefile.rules
diff --git a/lldb/test/API/macosx/add-dsym/Makefile b/lldb/test/API/macosx/add-dsym/Makefile
index 4e1ec2202d0b09..b949b308d3acce 100644
--- a/lldb/test/API/macosx/add-dsym/Makefile
+++ b/lldb/test/API/macosx/add-dsym/Makefile
@@ -8,7 +8,7 @@ hide.app/Contents/a.out.dSYM:
 	mkdir hide.app
 	mkdir hide.app/Contents
 	mv a.out.dSYM hide.app/Contents
-	strip -x a.out
+	$(STRIP) -x a.out
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -fs - a.out
 endif
diff --git a/lldb/test/API/tools/lldb-dap/module/Makefile b/lldb/test/API/tools/lldb-dap/module/Makefile
index b30baf48b972ef..c7d626a1a7e4c1 100644
--- a/lldb/test/API/tools/lldb-dap/module/Makefile
+++ b/lldb/test/API/tools/lldb-dap/module/Makefile
@@ -10,7 +10,7 @@ include Makefile.rules
 all: a.out.stripped
 
 a.out.stripped:
-	strip -o a.out.stripped a.out
+	$(STRIP) -o a.out.stripped a.out
 
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -fs - a.out.stripped
diff --git a/lldb/test/API/tools/lldb-dap/terminated-event/Makefile b/lldb/test/API/tools/lldb-dap/terminated-event/Makefile
index b30baf48b972ef..c7d626a1a7e4c1 100644
--- a/lldb/test/API/tools/lldb-dap/terminated-event/Makefile
+++ b/lldb/test/API/tools/lldb-dap/terminated-event/Makefile
@@ -10,7 +10,7 @@ include Makefile.rules
 all: a.out.stripped
 
 a.out.stripped:
-	strip -o a.out.stripped a.out
+	$(STRIP) -o a.out.stripped a.out
 
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -fs - a.out.stripped

Michael137 · 2024-10-10T11:58:10Z

could you mention that this is aDarwin-only change in the commit/PR title?

Otherwise, LGTM. Will help with the situation i ran into a while ago: https://reviews.llvm.org/D143842

labath · 2024-10-10T12:00:45Z

Environment variables for LLVM utils are passed to make on Darwin as well.

Technically, these are make variables, right?

dzhidzhoev · 2024-10-10T12:06:25Z

Environment variables for LLVM utils are passed to make on Darwin as well.

Technically, these are make variables, right?

Yes. I'll fix it in the description.

Michael137 · 2024-10-10T12:41:25Z

I think we need to make sure to pass xcrun -f strip to dotest on macOS

A follow-up for llvm#111816. This is to fix buildbot failure https://lab.llvm.org/staging/#/builders/195/builds/4242. TestSymbolFileJSON.py doesn't pass with llvm-strip on macOS. Apparently, llvm-strip/llvm-objcopy can't clean symbols from Mach-O nlists.

dzhidzhoev · 2024-10-10T14:31:56Z

I think we need to make sure to pass xcrun -f strip to dotest on macOS

Should it look like that? #111842

A follow-up for #111816. This is to fix buildbot failure https://lab.llvm.org/staging/#/builders/195/builds/4242. TestSymbolFileJSON.py doesn't pass with llvm-strip on macOS. Apparently, llvm-strip/llvm-objcopy can't clean symbols from Mach-O nlists.

…change) (llvm#111816) This makes tests more portable. Make variables for LLVM utils are passed to `make` on Darwin as well. Co-authored-by: Vladimir Vereschaka <[email protected]>

A follow-up for llvm#111816. This is to fix buildbot failure https://lab.llvm.org/staging/#/builders/195/builds/4242. TestSymbolFileJSON.py doesn't pass with llvm-strip on macOS. Apparently, llvm-strip/llvm-objcopy can't clean symbols from Mach-O nlists.

[lldb][test] Use $(STRIP) instead of strip in API tests.

050b000

This allows to make tests more portable. Environment variables for LLVM utils are passed to `make` on Darwin as well.

dzhidzhoev added the lldb label Oct 10, 2024

dzhidzhoev requested review from DavidSpickett, adrian-prantl, labath and jasonmolenda October 10, 2024 11:50

dzhidzhoev self-assigned this Oct 10, 2024

dzhidzhoev requested a review from JDevlieghere as a code owner October 10, 2024 11:50

dzhidzhoev changed the title ~~[lldb][test] Use $(STRIP) instead of strip in API tests.~~ [lldb][test] Use $(STRIP) instead of strip in API tests Oct 10, 2024

dzhidzhoev changed the title ~~[lldb][test] Use $(STRIP) instead of strip in API tests~~ [lldb][test] Use $(STRIP) instead of strip in API tests (Darwin-only change) Oct 10, 2024

Michael137 approved these changes Oct 10, 2024

View reviewed changes

dzhidzhoev merged commit b773da0 into llvm:main Oct 10, 2024
7 of 8 checks passed

dzhidzhoev mentioned this pull request Oct 10, 2024

[lldb][test] Use xcrun -f strip for API tests on Darwin #111842

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb][test] Use $(STRIP) instead of strip in API tests (Darwin-only change) #111816

[lldb][test] Use $(STRIP) instead of strip in API tests (Darwin-only change) #111816

Uh oh!

dzhidzhoev commented Oct 10, 2024 •

edited

Loading

Uh oh!

llvmbot commented Oct 10, 2024

Uh oh!

Michael137 commented Oct 10, 2024

Uh oh!

labath commented Oct 10, 2024

Uh oh!

dzhidzhoev commented Oct 10, 2024

Uh oh!

Uh oh!

Michael137 commented Oct 10, 2024

Uh oh!

dzhidzhoev commented Oct 10, 2024

Uh oh!

Uh oh!

[lldb][test] Use $(STRIP) instead of strip in API tests (Darwin-only change) #111816

[lldb][test] Use $(STRIP) instead of strip in API tests (Darwin-only change) #111816

Uh oh!

Conversation

dzhidzhoev commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 10, 2024

Uh oh!

Michael137 commented Oct 10, 2024

Uh oh!

labath commented Oct 10, 2024

Uh oh!

dzhidzhoev commented Oct 10, 2024

Uh oh!

Uh oh!

Michael137 commented Oct 10, 2024

Uh oh!

dzhidzhoev commented Oct 10, 2024

Uh oh!

Uh oh!

dzhidzhoev commented Oct 10, 2024 •

edited

Loading