Skip to content

Conversation

boomanaiden154
Copy link
Contributor

This patch enables lit's internal shell by default now that all
REQUIRES: shell tests have been update to not require shell features not
available in lit and all unresolved tests have been fixed.

This should speed up test runtime by a bit and will give nicer error
messaging.

Fixes #102701.

boomanaiden154 added a commit to boomanaiden154/llvm-project that referenced this pull request Aug 29, 2025
This patch enables lit's internal shell by default now that all
REQUIRES: shell tests have been update to not require shell features not
available in lit and all unresolved tests have been fixed.

This should speed up test runtime by a bit and will give nicer error
messaging.

Fixes llvm#102701.

Pull Request: llvm#156083
@llvmbot llvmbot added the BOLT label Aug 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 29, 2025

@llvm/pr-subscribers-bolt

Author: Aiden Grossman (boomanaiden154)

Changes

This patch enables lit's internal shell by default now that all
REQUIRES: shell tests have been update to not require shell features not
available in lit and all unresolved tests have been fixed.

This should speed up test runtime by a bit and will give nicer error
messaging.

Fixes #102701.


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

1 Files Affected:

  • (modified) bolt/test/lit.cfg.py (+12-1)
diff --git a/bolt/test/lit.cfg.py b/bolt/test/lit.cfg.py
index 0d05229be2bf3..bef570ba50a04 100644
--- a/bolt/test/lit.cfg.py
+++ b/bolt/test/lit.cfg.py
@@ -18,11 +18,22 @@
 # name: The name of this test suite.
 config.name = "BOLT"
 
+# TODO: Consolidate the logic for turning on the internal shell by default for all LLVM test suites.
+# See https://github.com/llvm/llvm-project/issues/106636 for more details.
+#
+# We prefer the lit internal shell which provides a better user experience on failures
+# and is faster unless the user explicitly disables it with LIT_USE_INTERNAL_SHELL=0
+# env var.
+use_lit_shell = True
+lit_shell_env = os.environ.get("LIT_USE_INTERNAL_SHELL")
+if lit_shell_env:
+    use_lit_shell = lit.util.pythonize_bool(lit_shell_env)
+
 # testFormat: The test format to use to interpret tests.
 #
 # For now we require '&&' between commands, until they get globally killed and
 # the test runner updated.
-config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
+config.test_format = lit.formats.ShTest(execute_external=not use_lit_shell)
 
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = [

Copy link
Contributor

@maksfb maksfb left a comment

Choose a reason for hiding this comment

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

Thanks!

Created using spr 1.3.6

[skip ci]
Created using spr 1.3.6
@boomanaiden154 boomanaiden154 changed the base branch from users/boomanaiden154/main.bolt-enable-lit-internal-shell-by-default to main August 29, 2025 19:51
@boomanaiden154 boomanaiden154 merged commit 69ccc39 into main Aug 29, 2025
10 of 15 checks passed
@boomanaiden154 boomanaiden154 deleted the users/boomanaiden154/bolt-enable-lit-internal-shell-by-default branch August 29, 2025 19:52
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Aug 29, 2025
This patch enables lit's internal shell by default now that all
REQUIRES: shell tests have been update to not require shell features not
available in lit and all unresolved tests have been fixed.

This should speed up test runtime by a bit and will give nicer error
messaging.

Fixes #102701.

Reviewers:
aaupov, yota9, maksfb, paschalis-mpeis, yozhu, rafaelauler, ayermolo

Reviewed By: maksfb

Pull Request: llvm/llvm-project#156083
@zmodem
Copy link
Collaborator

zmodem commented Sep 2, 2025

We're seeing bolt/test/runtime/instrumentation-indirect-2.c fail (hang) after this. (https://crbug.com/442483657)

I see it still has REQUIRES: shell but that doesn't seem to help:

REQUIRES: system-linux,shell,fuser

I don't see the failure on any of the bolt buildbots, but those bots don't seem to have built anything for months, e.g. https://lab.llvm.org/buildbot/#/builders/113/builds/8245 is the last build on bolt-x86_64-ubuntu-clang and it seems to be a month old.

We'll work around this by excluding the test from our builds for now, but it would be great if someone could look into it.

boomanaiden154 added a commit to boomanaiden154/llvm-project that referenced this pull request Sep 2, 2025
This test was broken by llvm#156083 because it was never ported to the
internal shell. It requires fuser which is not installed by default on
premerge and none of the BOLT buildbots have been online in a while.

This was actually causing a timeout because of llvm#156484, worked around
using a manual bash invocation with a wait call to ensure all of the
subprocesses have exited.
boomanaiden154 added a commit that referenced this pull request Sep 2, 2025
This test was broken by #156083 because it was never ported to the
internal shell. It requires fuser which is not installed by default on
premerge and none of the BOLT buildbots have been online in a while.

This was actually causing a timeout because of #156484, worked around
using a manual bash invocation with a wait call to ensure all of the
subprocesses have exited.
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.

[BOLT][llvm-lit] Enable internal shell for BOLT test suite

4 participants