Skip to content

Conversation

@Meinersbur
Copy link
Member

@Meinersbur Meinersbur commented Aug 21, 2024

The value of LIT_USE_INTERNAL_SHELL is inverted although it is not supposed to. The inversion was introduced in #65415.

Assume the environment variable LIT_USE_INTERNAL_SHELL=0 (i.e. use /bin/sh) is set:

use_lit_shell = True                                          # Use internal shell
lit_shell_env = os.environ.get("LIT_USE_INTERNAL_SHELL")      # lit_shell_env = "0", i.e. use /bin/sh
if lit_shell_env:
  use_lit_shell = not lit.util.pythonize_bool(lit_shell_env)  # use_lit_shell = not bool("0") = True, i.e. use internal shell

config.test_format = lit.formats.ShTest(execute_external=not use_lit_shell) # execute_external = not True = False, i.e. use internal shell even though we explicitly switched off using `LIT_USE_INTERNAL_SHELL=0`.

@Meinersbur Meinersbur requested a review from joker-eph August 21, 2024 08:36
@llvmbot llvmbot added the mlir label Aug 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 21, 2024

@llvm/pr-subscribers-mlir

Author: Michael Kruse (Meinersbur)

Changes

The value of LIT_USE_INTERNAL_SHELL is inverted although it is not supposed to. The inversion was introduced in #65415.

Assume the environment variable LIT_USE_INTERNAL_SHELL=0 (i.e. use /bin/sh) is set:

use_lit_shell = True                                          # Use internal shell
lit_shell_env = os.environ.get("LIT_USE_INTERNAL_SHELL")      # lit_shell_env = "0", i.e. use /bin/sh
if lit_shell_env:
  use_lit_shell = not lit.util.pythonize_bool(lit_shell_env)  # use_lit_shell = not bool("0") = True, i.e. use internal shell

config.test_format = lit.formats.ShTest(execute_external=not use_lit_shell) # execute_external = not True = False, i.e. use internal shell even though we explicitly switched off `LIT_USE_INTERNAL_SHELL`.

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

1 Files Affected:

  • (modified) mlir/test/lit.cfg.py (+1-1)
diff --git a/mlir/test/lit.cfg.py b/mlir/test/lit.cfg.py
index 98d0ddd9a2be11..81a668e73d4b24 100644
--- a/mlir/test/lit.cfg.py
+++ b/mlir/test/lit.cfg.py
@@ -23,7 +23,7 @@
 use_lit_shell = True
 lit_shell_env = os.environ.get("LIT_USE_INTERNAL_SHELL")
 if lit_shell_env:
-  use_lit_shell = not lit.util.pythonize_bool(lit_shell_env)
+  use_lit_shell = lit.util.pythonize_bool(lit_shell_env)
 
 config.test_format = lit.formats.ShTest(execute_external=not use_lit_shell)
 

@github-actions
Copy link

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:
darker --check --diff -r 50f4168e40790bd91123824ee338643ac18ccc0b...d2de4b855c0f56c9700e2841abdb99cc98daa7bb mlir/test/lit.cfg.py
View the diff from darker here.
--- lit.cfg.py	2024-08-21 08:22:52.000000 +0000
+++ lit.cfg.py	2024-08-21 08:39:59.313207 +0000
@@ -21,11 +21,11 @@
 # We prefer the lit internal shell which provides a better user experience on failures
 # 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)
+    use_lit_shell = lit.util.pythonize_bool(lit_shell_env)
 
 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 = [

@Meinersbur
Copy link
Member Author

Meinersbur commented Oct 31, 2024

Deprecated by 18e5505 / #106458

@Meinersbur Meinersbur closed this Oct 31, 2024
@Meinersbur Meinersbur deleted the mlir_LIT_USE_INTERNAL_SHELL branch September 24, 2025 09:17
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.

2 participants