-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,10 +127,8 @@ jobs: | |
| pip install --upgrade pip | ||
| pip --version | ||
| pip install -r requirements.txt | ||
| if ! python -c "import distutils" 2> /dev/null; then | ||
| # we need setuptools for distutils in Python 3.12+, needed for python setup.py sdist | ||
| pip install --upgrade setuptools | ||
| fi | ||
| pip install setuptools | ||
|
|
||
| # git config is required to make actual git commits (cfr. tests for GitRepository) | ||
| git config --global user.name "Github Actions" | ||
| git config --global user.email "[email protected]" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,10 +31,7 @@ | |
| import glob | ||
| import os | ||
| import logging | ||
| try: | ||
| from distutils.core import setup | ||
| except ImportError: | ||
| from setuptools import setup | ||
| from setuptools import setup, find_packages | ||
|
|
||
| from easybuild.tools.version import VERSION | ||
|
|
||
|
|
@@ -68,33 +65,6 @@ def find_rel_test(): | |
| return res | ||
|
|
||
|
|
||
| easybuild_packages = [ | ||
| "easybuild", "easybuild.base", | ||
| "easybuild.framework", "easybuild.framework.easyconfig", "easybuild.framework.easyconfig.format", | ||
| "easybuild.toolchains", "easybuild.toolchains.compiler", "easybuild.toolchains.mpi", | ||
| "easybuild.toolchains.fft", "easybuild.toolchains.linalg", "easybuild.tools", "easybuild.tools.containers", | ||
| "easybuild.tools.deprecated", "easybuild.tools.job", "easybuild.tools.toolchain", | ||
| "easybuild.tools.module_naming_scheme", "easybuild.tools.package", "easybuild.tools.package.package_naming_scheme", | ||
| "easybuild.tools.py2vs3", "easybuild.tools.repository", "easybuild.tools.tomllib", "easybuild.tools.tomllib.tomli", | ||
| "test.framework", "test", | ||
| ] | ||
|
|
||
| # Verify the above list is complete, if setuptools is installed | ||
| try: | ||
| import setuptools | ||
| except ImportError: | ||
| pass | ||
| else: | ||
| packages = set(setuptools.find_packages()) | ||
| easybuild_packages_set = set(easybuild_packages) | ||
| if easybuild_packages_set != packages: | ||
| # Warning only | ||
| print("="*80 + "\n" | ||
| "=== WARNING: Wrong list of easybuild_packages.\n" | ||
| f"Missing: {packages - easybuild_packages_set}\n" | ||
| f"Unneccessary: {easybuild_packages_set - packages}" | ||
| "\n" + "="*80 + "\n") | ||
|
|
||
| setup( | ||
| name="easybuild-framework", | ||
| version=str(VERSION), | ||
|
|
@@ -105,7 +75,7 @@ def find_rel_test(): | |
| license="GPLv2", | ||
| keywords="software build building installation installing compilation HPC scientific", | ||
| url="https://easybuild.io", | ||
| packages=easybuild_packages, | ||
| packages=find_packages(), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did a test locally:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| package_dir={'test.framework': 'test/framework'}, | ||
| package_data={'test.framework': find_rel_test()}, | ||
| scripts=[ | ||
|
|
||
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...Uh oh!
There was an error while loading. Please reload this page.
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.