-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lit] Remove Python 2 string support #157979
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 Python 2 string support #157979
Conversation
There are some code paths within lit that still check what string types are supported with the aim of being compatible with Python 2 and 3. Given LLVM's minimum Python version is 3.8 and we do not have any upstream testing for Python 2, I think we can safely drop this.
@llvm/pr-subscribers-testing-tools Author: Aiden Grossman (boomanaiden154) ChangesThere are some code paths within lit that still check what string types are supported with the aim of being compatible with Python 2 and 3. Given LLVM's minimum Python version is 3.8 and we do not have any upstream testing for Python 2, I think we can safely drop this. Full diff: https://github.com/llvm/llvm-project/pull/157979.diff 3 Files Affected:
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 69ca80008e2f9..0fc5658d4ce94 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -905,14 +905,8 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
# Replace uses of /dev/null with temporary files.
if kAvoidDevNull:
- # In Python 2.x, basestring is the base class for all string (including unicode)
- # In Python 3.x, basestring no longer exist and str is always unicode
- try:
- str_type = basestring
- except NameError:
- str_type = str
for i, arg in enumerate(args):
- if isinstance(arg, str_type) and kDevNull in arg:
+ if isinstance(arg, str) and kDevNull in arg:
f = tempfile.NamedTemporaryFile(delete=False)
f.close()
named_temp_files.append(f.name)
diff --git a/llvm/utils/lit/lit/llvm/config.py b/llvm/utils/lit/lit/llvm/config.py
index 56aa5eb64fa36..ea53e74a90049 100644
--- a/llvm/utils/lit/lit/llvm/config.py
+++ b/llvm/utils/lit/lit/llvm/config.py
@@ -230,7 +230,7 @@ def with_environment(self, variable, value, append_path=False):
# For paths, we should be able to take a list of them and process
# all of them.
paths_to_add = value
- if lit.util.is_string(paths_to_add):
+ if isinstance(paths_to_add, str):
paths_to_add = [paths_to_add]
def norm(x):
@@ -259,7 +259,7 @@ def norm(x):
self.config.environment[variable] = value
def with_system_environment(self, variables, append_path=False):
- if lit.util.is_string(variables):
+ if isinstance(variables, str):
variables = [variables]
for v in variables:
value = os.environ.get(v)
@@ -401,7 +401,7 @@ def add_tool_substitutions(self, tools, search_dirs=None):
if not search_dirs:
search_dirs = [self.config.llvm_tools_dir]
- if lit.util.is_string(search_dirs):
+ if isinstance(search_dirs, str):
search_dirs = [search_dirs]
tools = [x if isinstance(x, ToolSubst) else ToolSubst(x) for x in tools]
diff --git a/llvm/utils/lit/lit/util.py b/llvm/utils/lit/lit/util.py
index b03fd8bc22693..ce4c3c2df3436 100644
--- a/llvm/utils/lit/lit/util.py
+++ b/llvm/utils/lit/lit/util.py
@@ -13,14 +13,6 @@
import threading
-def is_string(value):
- try:
- # Python 2 and Python 3 are different here.
- return isinstance(value, basestring)
- except NameError:
- return isinstance(value, str)
-
-
def pythonize_bool(value):
if value is None:
return False
@@ -28,7 +20,7 @@ def pythonize_bool(value):
return value
if isinstance(value, numbers.Number):
return value != 0
- if is_string(value):
+ if isinstance(value, str):
if value.lower() in ("1", "true", "on", "yes"):
return True
if value.lower() in ("", "0", "false", "off", "no"):
|
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.
Looks reasonable to me.
I assume the lit docs don't mention anything about python 2 support?
Nothing that I've seen. I also haven't seen any recent patches keeping things working with Python 2, so I think it's pretty safe to conclude that configuration would have bitrotted at this point. |
There are some code paths within lit that still check what string types are supported with the aim of being compatible with Python 2 and 3. Given LLVM's minimum Python version is 3.8 and we do not have any upstream testing for Python 2, I think we can safely drop this.