Skip to content

Conversation

@boomanaiden154
Copy link
Contributor

@boomanaiden154 boomanaiden154 commented Dec 4, 2025

This reverts commit 4cfbc44.

There was an issue setting up the PATH variable in system-include-extractor.test which prevented chmod from being resolved. Ubuntu 18.04 puts all the standard Linux utilities in /bin which sits at the end of $PATH on that system. We were not removing the newline after python's print of $PATH, which caused lit to not be able to properly resolve the path.

@boomanaiden154 boomanaiden154 force-pushed the users/boomanaiden154/clangd-reland branch from 394c2c7 to d5a1eef Compare December 4, 2025 15:43
This reverts commit 4cfbc44.

There was an issue setting up the PATH variable in
system-include-extractor.test which prevented chmod from being resolved.
@boomanaiden154 boomanaiden154 force-pushed the users/boomanaiden154/clangd-reland branch from d5a1eef to ca50832 Compare December 4, 2025 15:55
@boomanaiden154 boomanaiden154 added the skip-precommit-approval PR for CI feedback, not intended for review label Dec 4, 2025
@boomanaiden154 boomanaiden154 marked this pull request as ready for review December 4, 2025 16:02
@boomanaiden154 boomanaiden154 enabled auto-merge (squash) December 4, 2025 16:02
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2025

@llvm/pr-subscribers-clangd

Author: Aiden Grossman (boomanaiden154)

Changes

This reverts commit 4cfbc44.

There was an issue setting up the PATH variable in system-include-extractor.test which prevented chmod from being resolved. Ubuntu 18.04 puts all the standard Linux utilities in /bin which sits at the end of $PATH on that system. We were not removing the newline after python's print of $PATH, which caused lit to not be able to properly resolve the path.


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

2 Files Affected:

  • (modified) clang-tools-extra/clangd/test/lit.cfg.py (+14-1)
  • (modified) clang-tools-extra/clangd/test/system-include-extractor.test (+1-1)
diff --git a/clang-tools-extra/clangd/test/lit.cfg.py b/clang-tools-extra/clangd/test/lit.cfg.py
index 05a0f5e7383e9..bd721c41861b2 100644
--- a/clang-tools-extra/clangd/test/lit.cfg.py
+++ b/clang-tools-extra/clangd/test/lit.cfg.py
@@ -1,3 +1,5 @@
+import os
+
 import lit.llvm
 import lit.util
 
@@ -5,10 +7,21 @@
 lit.llvm.llvm_config.clang_setup()
 lit.llvm.llvm_config.use_default_substitutions()
 
+# 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)
+
 config.name = "Clangd"
 config.suffixes = [".test"]
 config.excludes = ["Inputs"]
-config.test_format = lit.formats.ShTest(not lit.llvm.llvm_config.use_lit_shell)
+config.test_format = lit.formats.ShTest(not use_lit_shell)
 config.test_source_root = config.clangd_source_dir + "/test"
 config.test_exec_root = config.clangd_binary_dir + "/test"
 
diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test b/clang-tools-extra/clangd/test/system-include-extractor.test
index 3314be806a801..861d2e188a1f8 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -5,7 +5,7 @@
 
 # Create a bin directory to store the mock-driver and add it to the path
 # RUN: mkdir -p %t.dir/bin
-# RUN: %python -c "print(__import__('os').environ['PATH'])" > %t.path
+# RUN: %python -c "print(__import__('os').environ['PATH'])" | tr -d '\n' > %t.path
 # RUN: export PATH=%t.dir/bin:%{readfile:%t.path}
 # Generate a mock-driver that will print %temp_dir%/my/dir and
 # %temp_dir%/my/dir2 as include search paths.

@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2025

@llvm/pr-subscribers-clang-tools-extra

Author: Aiden Grossman (boomanaiden154)

Changes

This reverts commit 4cfbc44.

There was an issue setting up the PATH variable in system-include-extractor.test which prevented chmod from being resolved. Ubuntu 18.04 puts all the standard Linux utilities in /bin which sits at the end of $PATH on that system. We were not removing the newline after python's print of $PATH, which caused lit to not be able to properly resolve the path.


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

2 Files Affected:

  • (modified) clang-tools-extra/clangd/test/lit.cfg.py (+14-1)
  • (modified) clang-tools-extra/clangd/test/system-include-extractor.test (+1-1)
diff --git a/clang-tools-extra/clangd/test/lit.cfg.py b/clang-tools-extra/clangd/test/lit.cfg.py
index 05a0f5e7383e9..bd721c41861b2 100644
--- a/clang-tools-extra/clangd/test/lit.cfg.py
+++ b/clang-tools-extra/clangd/test/lit.cfg.py
@@ -1,3 +1,5 @@
+import os
+
 import lit.llvm
 import lit.util
 
@@ -5,10 +7,21 @@
 lit.llvm.llvm_config.clang_setup()
 lit.llvm.llvm_config.use_default_substitutions()
 
+# 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)
+
 config.name = "Clangd"
 config.suffixes = [".test"]
 config.excludes = ["Inputs"]
-config.test_format = lit.formats.ShTest(not lit.llvm.llvm_config.use_lit_shell)
+config.test_format = lit.formats.ShTest(not use_lit_shell)
 config.test_source_root = config.clangd_source_dir + "/test"
 config.test_exec_root = config.clangd_binary_dir + "/test"
 
diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test b/clang-tools-extra/clangd/test/system-include-extractor.test
index 3314be806a801..861d2e188a1f8 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -5,7 +5,7 @@
 
 # Create a bin directory to store the mock-driver and add it to the path
 # RUN: mkdir -p %t.dir/bin
-# RUN: %python -c "print(__import__('os').environ['PATH'])" > %t.path
+# RUN: %python -c "print(__import__('os').environ['PATH'])" | tr -d '\n' > %t.path
 # RUN: export PATH=%t.dir/bin:%{readfile:%t.path}
 # Generate a mock-driver that will print %temp_dir%/my/dir and
 # %temp_dir%/my/dir2 as include search paths.

@boomanaiden154 boomanaiden154 merged commit 8eb089d into main Dec 4, 2025
13 of 14 checks passed
@boomanaiden154 boomanaiden154 deleted the users/boomanaiden154/clangd-reland branch December 4, 2025 16:05
kcloudy0717 pushed a commit to kcloudy0717/llvm-project that referenced this pull request Dec 4, 2025
This reverts commit 4cfbc44.

There was an issue setting up the PATH variable in
system-include-extractor.test which prevented chmod from being resolved.
Ubuntu 18.04 puts all the standard Linux utilities in `/bin` which sits
at the end of `$PATH` on that system. We were not removing the newline
after python's print of `$PATH`, which caused lit to not be able to
properly resolve the path.
honeygoyal pushed a commit to honeygoyal/llvm-project that referenced this pull request Dec 9, 2025
This reverts commit 4cfbc44.

There was an issue setting up the PATH variable in
system-include-extractor.test which prevented chmod from being resolved.
Ubuntu 18.04 puts all the standard Linux utilities in `/bin` which sits
at the end of `$PATH` on that system. We were not removing the newline
after python's print of `$PATH`, which caused lit to not be able to
properly resolve the path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang-tools-extra clangd skip-precommit-approval PR for CI feedback, not intended for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants