[LLDB] Fix tests that fail when using internal shell. #156931
Conversation
These tests were failing on darwin, because the internal shell needs environment var definitions to start with 'env'. This PR (hopefully) fixes that problem.
|
@llvm/pr-subscribers-lldb Author: None (cmtice) ChangesThese tests were failing on darwin, because the internal shell needs environment var definitions to start with 'env'. This PR (hopefully) fixes that problem. Full diff: https://github.com/llvm/llvm-project/pull/156931.diff 3 Files Affected:
diff --git a/lldb/test/Shell/Host/TestCustomShell.test b/lldb/test/Shell/Host/TestCustomShell.test
index 0e948a1b1f7f3..5d3e90162fb28 100644
--- a/lldb/test/Shell/Host/TestCustomShell.test
+++ b/lldb/test/Shell/Host/TestCustomShell.test
@@ -6,7 +6,7 @@
# XFAIL: system-openbsd
# RUN: %clang_host %S/Inputs/simple.c -g -o %t.out
-# RUN: SHELL=bogus not %lldb %t.out -b -o 'process launch -X 1 --' 2>&1 | FileCheck %s --check-prefix ERROR
+# RUN: env SHELL=bogus not %lldb %t.out -b -o 'process launch -X 1 --' 2>&1 | FileCheck %s --check-prefix ERROR
# RUN: env -i ASAN_OPTIONS='detect_container_overflow=0' %lldb %t.out -b -o 'process launch -X 1 --' 2>&1 | FileCheck %s
# ERROR: error: shell expansion failed
diff --git a/lldb/test/Shell/SymbolFile/DWARF/deterministic-build.cpp b/lldb/test/Shell/SymbolFile/DWARF/deterministic-build.cpp
index 9e79f23db2b74..a45f794c73a43 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/deterministic-build.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/deterministic-build.cpp
@@ -3,7 +3,7 @@
// requires the ld64 linker, which clang invokes by default.
// REQUIRES: system-darwin
// RUN: %clang_host %s -g -c -o %t.o
-// RUN: ZERO_AR_DATE=1 %clang_host %t.o -g -o %t
+// RUN: env ZERO_AR_DATE=1 %clang_host %t.o -g -o %t
// RUN: %lldb %t -o "breakpoint set -f %s -l 11" -o run -o exit | FileCheck %s
// CHECK: stop reason = breakpoint
diff --git a/lldb/test/Shell/SymbolFile/add-dsym.test b/lldb/test/Shell/SymbolFile/add-dsym.test
index 52d1a1363feef..9695ddc297aa9 100644
--- a/lldb/test/Shell/SymbolFile/add-dsym.test
+++ b/lldb/test/Shell/SymbolFile/add-dsym.test
@@ -4,5 +4,5 @@
# HELP: Syntax: add-dsym <cmd-options> <filename>
# RUN: yaml2obj %S/Inputs/a.yaml -o %t.out
-# RUN: LLDB_APPLE_DSYMFORUUID_EXECUTABLE=%S/Inputs/dsymforuuid.sh %lldb %t.out -o 'add-dsym -u 41945CA4-5D9D-3CDE-82B4-37E4C09750B5' 2>&1 | FileCheck %s
+# RUN: env LLDB_APPLE_DSYMFORUUID_EXECUTABLE=%S/Inputs/dsymforuuid.sh %lldb %t.out -o 'add-dsym -u 41945CA4-5D9D-3CDE-82B4-37E4C09750B5' 2>&1 | FileCheck %s
# CHECK: UUID information was not found
|
|
I need someone with a darwin system to verify that this actually fixes the breakages from here: |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
lldb/test/Shell/Host/TestCustomShell.test will still fail after this, but this change is still necessary either way. It does fix the other two tests and enables implementing env -i within lit's internal shell to fix lldb/test/Shell/Host/TestCustomShell.test.
I'm hoping to get to implementing env -i within the next hour or two.
| # RUN: %clang_host %S/Inputs/simple.c -g -o %t.out | ||
| # RUN: SHELL=bogus not %lldb %t.out -b -o 'process launch -X 1 --' 2>&1 | FileCheck %s --check-prefix ERROR | ||
| # RUN: env SHELL=bogus not %lldb %t.out -b -o 'process launch -X 1 --' 2>&1 | FileCheck %s --check-prefix ERROR | ||
| # RUN: env -i ASAN_OPTIONS='detect_container_overflow=0' %lldb %t.out -b -o 'process launch -X 1 --' 2>&1 | FileCheck %s |
There was a problem hiding this comment.
This will still fail because lit's internal shell doesn't support env -i yet (and I confirmed that it still fails locally).
|
I've tested this locally on Darwin. I can confirm it fixes part of |
So even if I commit the PR, the GreenDragon builder will continue to fail until you get 'env -i' implemented. Is that acceptable? Or do I need to actually revert the change that makes internal shell the default until the 'env -i' fix is committed. @JDevlieghere , @adrian-prantl what do you think? |
|
Whether or not to revert the internal shell PR is a separate question from whether or not to land this patch. This patch needs to go in either way for us to be able to enable the internal shell. |
Ok, looking into it. If we can't get it fixed soon, we'll go ahead and revert. |
This test was brokem by migrating to the lit internal shell due to a lack of env prefix for setting environment variables. This was fixed in prevented the breakpoint in the test from mapping to anything, causing the test to file. This patch restores the original line numbering.
This test was brokem by migrating to the lit internal shell due to a lack of env prefix for setting environment variables. This was fixed in prevented the breakpoint in the test from mapping to anything, causing the test to file. This patch restores the original line numbering.
4 participants
These tests were failing on darwin, because the internal shell needs environment var definitions to start with 'env'. This PR (hopefully) fixes that problem.