[lit] Export env vars in script to avoid pruning #105759
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-testing-tools Author: Keith Smiley (keith) ChangesOn macOS the dynamic loader prunes dyld specific environment variables such as Full diff: https://github.com/llvm/llvm-project/pull/105759.diff 1 Files Affected:
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index da7fa86fd39173..7ea9fb9594cc32 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1223,6 +1223,11 @@ def executeScript(test, litConfig, tmpBase, commands, cwd):
if test.config.pipefail:
f.write(b"set -o pipefail;" if mode == "wb" else "set -o pipefail;")
f.write(b"set -x;" if mode == "wb" else "set -x;")
+
+ env_str = "\n".join("export {}={};".format(k, shlex.quote(v))
+ for k, v in test.config.environment.items())
+ f.write(bytes(env_str, "utf-8") if mode == "wb" else env_str)
+
if sys.version_info > (3, 0) and mode == "wb":
f.write(bytes("{ " + "; } &&\n{ ".join(commands) + "; }", "utf-8"))
else:
|
|
✅ With the latest revision this PR passed the Python code formatter. |
|
review requesting github recommended folks 🙏🏻 |
|
Just to make sure I understand the problem correctly: on macOS rtld clears these environment variables when we invoking sh/bash to execute the test script? I would like to see this in a comment in the code explaining why we have to export all environment variables. And I'd also prefer if we could just export the ones starting with |
Correct, the first process that is executed, dyld captures the variables and wipes them from the process pre
Correct!
Updated to scope to only variables that start with |
arichardson
approved these changes
Aug 24, 2024
| f.write(b"set -x;" if mode == "wb" else "set -x;") | ||
|
|
||
| if sys.version_info > (3, 0) and mode == "wb": |
There was a problem hiding this comment.
Unrelated to this change but the version check can be dropped.
Nit: the newline before is unrelated to this patch.
There was a problem hiding this comment.
removed whitespace, submitted the version check removal here #105948
On macOS the dynamic loader prunes dyld specific environment variables such as `DYLD_INSERT_LIBRARIES`, `DYLD_LIBRARY_PATH`, etc. If these are set in the lit config it's safe to assume that the user actually wanted their subprocesses to run with these variables, versus the python interpreter that gets executed with them before they are pruned. This change exports all known variables in the shell script instead of relying on them being passed through.
We don't really care to see the exports I don't think
keith
force-pushed
the
ks/lit-export-env-vars-in-script-to-avoid-pruning
branch
from
268e4b1 to
0262886
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/3071 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/41/builds/1487 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/4/builds/1573 Here is the relevant piece of the build log for the reference: |
dmpolukhin
pushed a commit
to dmpolukhin/llvm-project
that referenced
this pull request
Sep 2, 2024
On macOS the dynamic loader prunes dyld specific environment variables such as `DYLD_INSERT_LIBRARIES`, `DYLD_LIBRARY_PATH`, etc. If these are set in the lit config it's safe to assume that the user actually wanted their subprocesses to run with these variables, versus the python interpreter that gets executed with them before they are pruned. This change exports all known variables in the shell script instead of relying on them being passed through.
qiaojbao
pushed a commit
to GPUOpen-Drivers/llvm-project
that referenced
this pull request
Sep 30, 2024
…a9b483f9f Local branch amd-gfx 2f9a9b4 Merged main:be5ecc35efc902a4742669d41a88cfd88babb245 into amd-gfx:fee3baa081ca Remote branch main 65b7cbb [lit] Export env vars in script to avoid pruning (llvm#105759)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On macOS the dynamic loader prunes dyld specific environment variables such as
DYLD_INSERT_LIBRARIES,DYLD_LIBRARY_PATH, etc. If these are set in the lit config it's safe to assume that the user actually wanted their subprocesses to run with these variables, versus the python interpreter that gets executed with them before they are pruned. This change exports all known variables in the shell script instead of relying on them being passed through.