Replace distutils.version with looseversion since the former was deprecated in python 3.10 and removed in 3.12. #99549
Conversation
…ecated in python 3.10 and removed in 3.12.
|
Change LGTM, but I'm not sure whether LLVM has policies around introducing new developer dependencies like this package. Do you know? |
I'm not sure to be honest. Any ideas who to ask? |
There was a problem hiding this comment.
I asked around on an internal LLVM chat and consensus was that this is probably fine to commit, but it would also be ideal to reimplement the logic to avoid the dependency. There was one idea to limit the scope of the import, e.g.
if dwarf_version_string and gdb_version_string:
if int(dwarf_version_string) >= 5:
try:
from looseversion import LooseVersion
except:
raise Error("running gdb tests requires the looseversion package")
if LooseVersion(gdb_version_string) < LooseVersion("10.1"):
But I don't want to block this :)
Thanks! I'll look into rewriting it into something similar, I just wanted to make sure this got in before the release branching next week since it is causing us a lot of headaches internally. |
|
@mysterymath you may want to take a look at this before it hits Fuchsia CI. |
|
We've a workable solution for this in Fuchsia now, so I think this is good to proceed. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/2785 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/174/builds/2014 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/190/builds/2264 Here is the relevant piece of the build log for the reference: |
|
Reverted this as this breaks a bunch of build bots. This definitely deserves more discussion. For example, we took an alternative approach in LLDB (see #93712) which may be applicable here too? |
@JDevlieghere did this break any of your bots other than the ones reported here? All of the ones listed here are my bots and I'm in the process of updating them. |
The LLDB approach also seems to require an extra package to be installed that is not part of the default python distribution, is that really that different from the approach here? |
Yes, it also broke the GreenDragon bots: https://green.lab.llvm.org/job/llvm.org/view/LLDB/ which you wouldn't be able to update yourself.
It's not, except that maybe one could argue that packaging is maintained by a Python workgroup and has millions of users compared to this package, which seems to be maintained by a single person and has a few thousand users. More importantly though, we should have the same solution across subprojects to avoid bots and developers having to install an increasing number of arbitrary packages. |
Ah, sorry about that.
I did try to look at using the packaging package instead, but it seemed to require more work to make sure everything worked while using looseversion was a drop-in replacement that should just work with minimal changes required. I wasn't aware of the work done in GDB to try and solve a similar problem, but will look into it, thanks for pointing it out (we don't run LLDB internally). If we did want to borrow the same or a similar approach, it would make more sense if both could share the same code, so bug fixes in one would be fixed in both. Is there a way to do that in the current system? |
…ecated in python 3.10 and removed in 3.12. (#99549) Summary: Python deprecated the distutils package in 3.10, and removed it in 3.12 causing problems when trying to run the lit tests with 3.12. https://docs.python.org/3.10/library/distutils.html Replace usage with the looseversion package which should be a drop-in replacement for the original usage. If your testing fails after this commit, you need to install the looseversion package. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251589
…was deprecated in python 3.10 and removed in 3.12." (#99786) Summary: Reverts #99549 because it breaks a bunch of build bots. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251297
Python deprecated the distutils package in 3.10, and removed it in 3.12 causing problems when trying to run the lit tests with 3.12.
https://docs.python.org/3.10/library/distutils.html
Replace usage with the looseversion package which should be a drop-in replacement for the original usage.