-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
[lit] Remove legacy python version checks (NFC) #105948
[lit] Remove legacy python version checks (NFC) #105948
Conversation
@llvm/pr-subscribers-testing-tools Author: Keith Smiley (keith) ChangesFull diff: https://github.com/llvm/llvm-project/pull/105948.diff 1 Files Affected:
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 2d9af9fbbb3634..b56d103f718955 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1227,7 +1227,7 @@ 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;")
- if sys.version_info > (3, 0) and mode == "wb":
+ if mode == "wb":
f.write(bytes("{ " + "; } &&\n{ ".join(commands) + "; }", "utf-8"))
else:
f.write("{ " + "; } &&\n{ ".join(commands) + "; }")
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Is there a reason you're only touching this version check? A cursory search seems to show ~30 files that are still checking for Python versions that are now below the LLVM minimum requirements.
this one was just spotted in #105759, I updated this PR to include all of them i found in lit |
Thanks! The rest can probably be cleaned up separately when someone gets to them and can ping maintainers. |
No description provided.