-
Notifications
You must be signed in to change notification settings - Fork 217
Switch to setuptools and find_packages
#5065
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
base: develop
Are you sure you want to change the base?
Conversation
| keywords="software build building installation installing compilation HPC scientific", | ||
| url="https://easybuild.io", | ||
| packages=easybuild_packages, | ||
| packages=find_packages(), |
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.
I will need to check this thoroughly, since I don't want to me up a release due to this change...
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.
I did a test locally: find_packages() and easybuild_packages were the same
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.
Which Python/setuptools version was used here matters a lot...
I don't think we can generally assume that find_packages will behave the way we expect it to in all possible circumstances
| from distutils.core import setup | ||
| except ImportError: | ||
| from setuptools import setup | ||
| from setuptools import setup, find_packages |
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.
I'm not happy with introducing a hard requirement on setuptools, since it's not part of the Python standard library, and often includes backwards-incompatible changes in new versions...
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.
We already have a hard dependency on that for Python 3.12 where distutils doesn't exist anymore.
According to various sources setuptools should be available with any reasonably modern Python installation, i.e. Python 3.something. e.g. via ensurepip. And we don't have CI (for newer Pythons) without setuptools installed, so not sure if EB even works completely without it.
And as we don't use any advanced features I doubt that those setuptools changes affect us.
|
What exactly were the issues? I only found #2988 referencing https://lists.ugent.be/wws/arc/easybuild/2019-08/msg00057.html which doesn't open for me. Maybe >6yrs later those issues are gone/rare enough? |
|
Copying @Flamefire's post from #5063 here to avoid splintering the discussion:
#2988 gives a hint, indeed. Unfortunately the EasyBuild mailing list isn't publicly available currently (requires UGent VPN), hopefully that can be restored soon.
It's very hard to determine whether Generally speaking, I recall frequently being frustrated with In fact, more broadly speaking we have a standing rule that EasyBuild should not have a hard dependency on any Python package outside of the Python standard library. That's a rule that was imposed a while ago, after being fed up with dealing with a whole range of weird/annoying problems with various 3rd party dependencies. So, introducing a hard requirement for @Flamefire Please rework #5063 so it doesn't require these changes. |
|
I checked the first issue (https://www.mail-archive.com/[email protected]/msg04884.html) and it is not related to setuptools at all:
The other is:
So also unrelated (and only mentions are distutils and easy_install) and even not relevant anymore given that vsc-* is no longer a dependency. In fact we don't have dependencies anymore.
Can you point out what exactly is the foot-gun here? Where it is mentioned the most times is https://jwodder.github.io/kbits/posts/pypkg-mistakes/#top-level-tests-directory-in-wheel which we are currently(!) guilty! We install a
Given it is the tool that is recommended for a long time and our issues where with Python 2.x I'd say yes. I tested the oldest/first setuptools (v17.0) that works with Python 3.6 and that worked. Same for 34.0.2 mentioned in the issue and the latest (59.6.0, for Python 3.6). So I'd really switch from distutils right now for all Python versions, not just for 3.12+ as we currently do. Having to maintain 2 versions/utilities doesn't sound easier to me. |
|
Only requiring Not requiring We'll see how things work out with Python 3.12+, where we can't avoid The default Python in RHEL10 and derivatives (Rocky Linux 10, etc.) is Python 3.12, so we'll have a better view on things soon-ish. For Python < 3.12, I'm strongly in favor of never requiring "Those Who Cannot Remember the Past Are Condemned To Repeat It" |
My take is that we should test using setuptools as long as most people are using a Python version where they have an alternative in the (now unlikely) case there is an issue with that. This allows us to gather experience without (hard)breaking anyone.
As argued above: None of the previous issues is actually related to using setuptools for installation. The only issue related to setuptools was requiring setuptools as a runtime dependency in a specific version that wasn't available. |
Any reasonably modern Python distribution includes setuptools. So the workaround, that was useful for Python 2.x, is no longer necessary and can be removed. This allows using `find_packages` instead of maintaining an explicit list of packages which is error-prone.
b74c54b to
8e3c9a1
Compare
Any reasonably modern Python distribution includes setuptools. So the workaround, that was useful for Python 2.x, is no longer necessary and can be removed.
This allows using
find_packagesinstead of maintaining an explicit list of packages which is error-prone.I ran into this when adding the vendored
tomlipackagesee easybuilders/easybuild-easyblocks#4006 & easybuilders/easybuild-easyconfigs#24747