-
Notifications
You must be signed in to change notification settings - Fork 15.3k
Reapply "[clangd] Enable lit internal shell by default" #170186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Reapply "[clangd] Enable lit internal shell by default" #170186
Conversation
This reverts commit 4cfbc44. This was failing due to a missing chmod binary on one of the bots (clangd-ubuntu-tsan). This patch fixes that by explicitly checking for the presence of a chmod binary. This should not be necessary (I have added a TODO for myself to update once I have looked at the bot setup which I am currently waiting on access to) as check-llvm works with requiring chmod unconditionally.
|
@llvm/pr-subscribers-clangd Author: Aiden Grossman (boomanaiden154) ChangesThis reverts commit 4cfbc44. This was failing due to a missing chmod binary on one of the bots (clangd-ubuntu-tsan). This patch fixes that by explicitly checking for the presence of a chmod binary. This should not be necessary (I have added a TODO for myself to update once I have looked at the bot setup which I am currently waiting on access to) as check-llvm works with requiring chmod unconditionally. Full diff: https://github.com/llvm/llvm-project/pull/170186.diff 2 Files Affected:
diff --git a/clang-tools-extra/clangd/test/lit.cfg.py b/clang-tools-extra/clangd/test/lit.cfg.py
index 05a0f5e7383e9..0199275c70e1e 100644
--- a/clang-tools-extra/clangd/test/lit.cfg.py
+++ b/clang-tools-extra/clangd/test/lit.cfg.py
@@ -1,3 +1,6 @@
+import os
+import shutil
+
import lit.llvm
import lit.util
@@ -5,10 +8,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"
@@ -41,6 +55,13 @@ def calculate_arch_features(arch_string):
if lit.util.pythonize_bool(config.have_benchmarks):
config.available_features.add("have-benchmarks")
+# This is needed to avoid running a single test (system-include-extractor.test)
+# on a single buildbot (clangd-ubuntu-tsan) and likely should not be needed. We
+# are able to unconditionally assume a chmod binary exists for check-llvm.
+# TODO(boomanaiden154): Fix this after investigating the bot setup.
+if shutil.which("chmod"):
+ config.available_features.add("chmod")
+
# It is not realistically possible to account for all options that could
# possibly be present in system and user configuration files, so disable
# default configs for the test runs.
diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test b/clang-tools-extra/clangd/test/system-include-extractor.test
index 3314be806a801..36e4c581ecad1 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -1,7 +1,7 @@
# RUN: rm -rf %t.dir && mkdir -p %t.dir
# The mock driver below is a shell script:
-# REQUIRES: shell
+# REQUIRES: shell, chmod
# Create a bin directory to store the mock-driver and add it to the path
# RUN: mkdir -p %t.dir/bin
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Aiden Grossman (boomanaiden154) ChangesThis reverts commit 4cfbc44. This was failing due to a missing chmod binary on one of the bots (clangd-ubuntu-tsan). This patch fixes that by explicitly checking for the presence of a chmod binary. This should not be necessary (I have added a TODO for myself to update once I have looked at the bot setup which I am currently waiting on access to) as check-llvm works with requiring chmod unconditionally. Full diff: https://github.com/llvm/llvm-project/pull/170186.diff 2 Files Affected:
diff --git a/clang-tools-extra/clangd/test/lit.cfg.py b/clang-tools-extra/clangd/test/lit.cfg.py
index 05a0f5e7383e9..0199275c70e1e 100644
--- a/clang-tools-extra/clangd/test/lit.cfg.py
+++ b/clang-tools-extra/clangd/test/lit.cfg.py
@@ -1,3 +1,6 @@
+import os
+import shutil
+
import lit.llvm
import lit.util
@@ -5,10 +8,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"
@@ -41,6 +55,13 @@ def calculate_arch_features(arch_string):
if lit.util.pythonize_bool(config.have_benchmarks):
config.available_features.add("have-benchmarks")
+# This is needed to avoid running a single test (system-include-extractor.test)
+# on a single buildbot (clangd-ubuntu-tsan) and likely should not be needed. We
+# are able to unconditionally assume a chmod binary exists for check-llvm.
+# TODO(boomanaiden154): Fix this after investigating the bot setup.
+if shutil.which("chmod"):
+ config.available_features.add("chmod")
+
# It is not realistically possible to account for all options that could
# possibly be present in system and user configuration files, so disable
# default configs for the test runs.
diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test b/clang-tools-extra/clangd/test/system-include-extractor.test
index 3314be806a801..36e4c581ecad1 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -1,7 +1,7 @@
# RUN: rm -rf %t.dir && mkdir -p %t.dir
# The mock driver below is a shell script:
-# REQUIRES: shell
+# REQUIRES: shell, chmod
# Create a bin directory to store the mock-driver and add it to the path
# RUN: mkdir -p %t.dir/bin
|
This reverts commit 4cfbc44.
This was failing due to a missing chmod binary on one of the bots (clangd-ubuntu-tsan). This patch fixes that by explicitly checking for the presence of a chmod binary. This should not be necessary (I have added a TODO for myself to update once I have looked at the bot setup which I am currently waiting on access to) as check-llvm works with requiring chmod unconditionally.