Skip to content
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

[lldb] Migrate distutils.version.LooseVersion to packaging #82066

Merged
merged 1 commit into from
Feb 19, 2024

Conversation

JDevlieghere
Copy link
Member

The distutils package has been deprecated and was removed from Python 3.12. The migration page [1] advises to use the packaging module instead. Since Python 3.6 that's vendored into pkg_resources.

[1] https://peps.python.org/pep-0632/#migration-advice

@llvmbot
Copy link
Member

llvmbot commented Feb 16, 2024

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

The distutils package has been deprecated and was removed from Python 3.12. The migration page [1] advises to use the packaging module instead. Since Python 3.6 that's vendored into pkg_resources.

[1] https://peps.python.org/pep-0632/#migration-advice


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

5 Files Affected:

  • (modified) lldb/packages/Python/lldbsuite/test/decorators.py (+7-5)
  • (modified) lldb/packages/Python/lldbsuite/test/lldbplatformutil.py (+13-5)
  • (modified) lldb/test/API/sanity/TestSettingSkipping.py (+16-16)
  • (modified) lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py (+2-2)
  • (modified) lldb/test/Shell/helper/build.py (+2-2)
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index a5d7a7a25879df..b691f82b90652c 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -1,6 +1,6 @@
 # System modules
-from distutils.version import LooseVersion
 from functools import wraps
+from pkg_resources import packaging
 import ctypes
 import locale
 import os
@@ -65,10 +65,10 @@ def fn_neq(x, y):
         ">=": fn_geq,
         "<=": fn_leq,
     }
-    expected_str = ".".join([str(x) for x in expected])
-    actual_str = ".".join([str(x) for x in actual])
 
-    return op_lookup[comparison](LooseVersion(actual_str), LooseVersion(expected_str))
+    return op_lookup[comparison](
+        packaging.version.parse(actual), packaging.version.parse(expected)
+    )
 
 
 def _match_decorator_property(expected, actual):
@@ -238,7 +238,9 @@ def fn(actual_debug_info=None):
             )
         )
         skip_for_py_version = (py_version is None) or _check_expected_version(
-            py_version[0], py_version[1], sys.version_info
+            py_version[0],
+            py_version[1],
+            "{}.{}".format(sys.version_info.major, sys.version_info.minor),
         )
         skip_for_macos_version = (macos_version is None) or (
             (platform.mac_ver()[0] != "")
diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index bd92d03e0e2212..f159e53a98429e 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -7,8 +7,8 @@
 import subprocess
 import sys
 import os
-from distutils.version import LooseVersion
 from urllib.parse import urlparse
+from pkg_resources import packaging
 
 # LLDB modules
 import lldb
@@ -308,13 +308,21 @@ def expectedCompilerVersion(compiler_version):
         return operator in [">", ">=", "!", "!=", "not"]
 
     if operator == ">":
-        return LooseVersion(test_compiler_version) > LooseVersion(version)
+        return packaging.version.parse(test_compiler_version) > packaging.version.parse(
+            version
+        )
     if operator == ">=" or operator == "=>":
-        return LooseVersion(test_compiler_version) >= LooseVersion(version)
+        return packaging.version.parse(
+            test_compiler_version
+        ) >= packaging.version.parse(version)
     if operator == "<":
-        return LooseVersion(test_compiler_version) < LooseVersion(version)
+        return packaging.version.parse(test_compiler_version) < packaging.version.parse(
+            version
+        )
     if operator == "<=" or operator == "=<":
-        return LooseVersion(test_compiler_version) <= LooseVersion(version)
+        return packaging.version.parse(
+            test_compiler_version
+        ) <= packaging.version.parse(version)
     if operator == "!=" or operator == "!" or operator == "not":
         return str(version) not in str(test_compiler_version)
     return str(version) in str(test_compiler_version)
diff --git a/lldb/test/API/sanity/TestSettingSkipping.py b/lldb/test/API/sanity/TestSettingSkipping.py
index 5f58ec2638456d..369948b2b49463 100644
--- a/lldb/test/API/sanity/TestSettingSkipping.py
+++ b/lldb/test/API/sanity/TestSettingSkipping.py
@@ -1,8 +1,7 @@
 """
-This is a sanity check that verifies that test can be sklipped based on settings.
+This is a sanity check that verifies that test can be skipped based on settings.
 """
 
-
 import lldb
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.decorators import *
@@ -10,24 +9,25 @@
 
 class SettingSkipSanityTestCase(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
+    REAL_PYTHON_VERSION = "3.0"
+    FUTURE_PYTHON_VERSION = "4.0"
 
-    @skipIf(py_version=(">=", (3, 0)))
+    @skipIf(py_version=(">=", REAL_PYTHON_VERSION))
     def testSkip(self):
-        """This setting is on by default"""
-        self.assertTrue(False, "This test should not run!")
-
-    @skipIf(py_version=("<", (3, 0)))
-    def testNoMatch(self):
-        self.assertTrue(True, "This test should run!")
+        self.assertTrue(False, "This test should not run and fail (SKIPPED)")
 
-    @skipIf(setting=("target.i-made-this-one-up", "true"))
-    def testNotExisting(self):
-        self.assertTrue(True, "This test should run!")
+    @skipIf(py_version=("<", FUTURE_PYTHON_VERSION))
+    def testNoSKip(self):
+        self.assertTrue(True, "This test should run and pass(PASS)")
 
-    @expectedFailureAll(py_version=(">=", (3, 0)))
+    @expectedFailureAll(py_version=("<", FUTURE_PYTHON_VERSION))
     def testXFAIL(self):
-        self.assertTrue(False, "This test should run and fail!")
+        self.assertTrue(False, "This test should expectedly fail (XFAIL)")
 
-    @expectedFailureAll(py_version=("<", (3, 0)))
+    @expectedFailureAll(py_version=(">=", FUTURE_PYTHON_VERSION))
     def testNotXFAIL(self):
-        self.assertTrue(True, "This test should run and succeed!")
+        self.assertTrue(True, "This test should pass (PASS)")
+
+    @skipIf(setting=("target.i-made-this-one-up", "true"))
+    def testNotExisting(self):
+        self.assertTrue(True, "This test should run!")
diff --git a/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py b/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
index a8c2f93aed0ae3..1ec70b085f8c3d 100644
--- a/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
+++ b/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
@@ -61,9 +61,9 @@ def check_simulator_ostype(self, sdk, platform_name, arch=platform.machine()):
 
         # Older versions of watchOS (<7.0) only support i386
         if platform_name == "watchos":
-            from distutils.version import LooseVersion
+            from pkg_resources import packaging
 
-            if LooseVersion(vers) < LooseVersion("7.0"):
+            if packaging.version.parse(vers) < packaging.version.parse("7.0"):
                 arch = "i386"
 
         triple = "-".join([arch, "apple", platform_name + vers, "simulator"])
diff --git a/lldb/test/Shell/helper/build.py b/lldb/test/Shell/helper/build.py
index 073198a6df2df3..d3c25bd944e983 100755
--- a/lldb/test/Shell/helper/build.py
+++ b/lldb/test/Shell/helper/build.py
@@ -519,9 +519,9 @@ def _find_windows_sdk_in_registry_view(self, view):
 
             # Windows SDK version numbers consist of 4 dotted components, so we
             # have to use LooseVersion, as StrictVersion supports 3 or fewer.
-            from distutils.version import LooseVersion
+            from pkg_resources import packaging
 
-            sdk_versions.sort(key=lambda x: LooseVersion(x), reverse=True)
+            sdk_versions.sort(key=lambda x: packaging.version.parse(x), reverse=True)
             option_value_name = "OptionId.DesktopCPP" + self.msvc_arch_str
             for v in sdk_versions:
                 try:

@JDevlieghere JDevlieghere changed the title [lldb] Migrate distutils.version.LooseVersion to packaging.LooseVersion [lldb] Migrate distutils.version.LooseVersion to packaging Feb 16, 2024
@@ -308,13 +308,21 @@ def expectedCompilerVersion(compiler_version):
return operator in [">", ">=", "!", "!=", "not"]

if operator == ">":
return LooseVersion(test_compiler_version) > LooseVersion(version)
return packaging.version.parse(test_compiler_version) > packaging.version.parse(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these would probably be easier to read if we factored out both parse calls into a variable, as they are repeated everywhere. Don't feel strongly about it though


@expectedFailureAll(py_version=("<", (3, 0)))
@expectedFailureAll(py_version=(">=", FUTURE_PYTHON_VERSION))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this right? what used to be <3.0 is now >=4.0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had a bug in the decorator and I messed around with the conditions to test a few more cases, but the original ones were easier to understand. I'll keep the original comparisons.

The distutils package has been deprecated and was removed from Python
3.12. The migration page [1] advises to use the packaging module
instead. Since Python 3.6 that's vendored into pkg_resources.

[1] https://peps.python.org/pep-0632/#migration-advice
Copy link
Member

@medismailben medismailben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@felipepiovezan felipepiovezan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this

@JDevlieghere JDevlieghere merged commit 13dce35 into llvm:main Feb 19, 2024
4 checks passed
@JDevlieghere JDevlieghere deleted the distutils-migration branch February 19, 2024 23:31
JDevlieghere added a commit that referenced this pull request Feb 20, 2024
…82297)

Reverts #82066 because the following tests started
failing after:


[lldb-api.commands/expression/import-std-module/deque-basic.TestDequeFromStdModule.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_deque-basic/TestDequeFromStdModule_py/)

[lldb-api.commands/expression/import-std-module/deque-dbg-info-content.TestDbgInfoContentDequeFromStdModule.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule_py/)

[lldb-api.commands/expression/import-std-module/forward_list.TestForwardListFromStdModule.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_forward_list/TestForwardListFromStdModule_py/)

[lldb-api.commands/expression/import-std-module/forward_list-dbg-info-content.TestDbgInfoContentForwardListFromStdModule.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule_py/)

[lldb-api.commands/expression/import-std-module/list.TestListFromStdModule.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_list/TestListFromStdModule_py/)

[lldb-api.commands/expression/import-std-module/non-module-type-separation.TestNonModuleTypeSeparation.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_non-module-type-separation/TestNonModuleTypeSeparation_py/)

[lldb-api.commands/expression/import-std-module/queue.TestQueueFromStdModule.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_queue/TestQueueFromStdModule_py/)

[lldb-api.commands/expression/import-std-module/retry-with-std-module.TestRetryWithStdModule.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_retry-with-std-module/TestRetryWithStdModule_py/)

[lldb-api.commands/expression/import-std-module/unique_ptr.TestUniquePtrFromStdModule.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_unique_ptr/TestUniquePtrFromStdModule_py/)

[lldb-api.commands/expression/import-std-module/unique_ptr-dbg-info-content.TestUniquePtrDbgInfoContent.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent_py/)

[lldb-api.commands/expression/import-std-module/vector-of-vectors.TestVectorOfVectorsFromStdModule.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/commands_expression_import-std-module_vector-of-vectors/TestVectorOfVectorsFromStdModule_py/)

[lldb-api.functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr.TestDataFormatterLibcxxSharedPtr.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/functionalities_data-formatter_data-formatter-stl_libcxx_shared_ptr/TestDataFormatterLibcxxSharedPtr_py/)

[lldb-api.functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr.TestDataFormatterLibcxxUniquePtr.py](https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/66513/testReport/junit/lldb-api/functionalities_data-formatter_data-formatter-stl_libcxx_unique_ptr/TestDataFormatterLibcxxUniquePtr_py/)
JDevlieghere added a commit that referenced this pull request Feb 20, 2024
The distutils package has been deprecated and was removed from Python
3.12. The migration page [1] advises to use the packaging module
instead. Since Python 3.6 that's vendored into pkg_resources.

[1] https://peps.python.org/pep-0632/#migration-advice
kevinfrei added a commit to kevinfrei/llvm that referenced this pull request Mar 4, 2024
JDevlieghere added a commit to swiftlang/llvm-project that referenced this pull request Apr 17, 2024
The distutils package has been deprecated and was removed from Python
3.12. The migration page [1] advises to use the packaging module
instead. Since Python 3.6 that's vendored into pkg_resources.

[1] https://peps.python.org/pep-0632/#migration-advice

(cherry picked from commit 0c02329)
JDevlieghere added a commit to swiftlang/llvm-project that referenced this pull request Apr 18, 2024
The distutils package has been deprecated and was removed from Python
3.12. The migration page [1] advises to use the packaging module
instead. Since Python 3.6 that's vendored into pkg_resources.

[1] https://peps.python.org/pep-0632/#migration-advice

(cherry picked from commit 0c02329)
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.

4 participants