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

feat: Use setuptools over setuptools._distutils.core #1843

Merged

Conversation

matthewfeickert
Copy link
Contributor

@matthewfeickert matthewfeickert commented Nov 30, 2022

Resolves #1830

Drop all usage of setuptools._distutils in favor of pure setuptools. This effectively solves the problems that were present in the attempt in PR #1585 and PR #1700. With this, it is now possible to go further and drop direct usage of distutils in favor of setuptools. This effectively happens with the removal of setuptools._distutils as

try:
    from setuptools import setup, Extension
    ...

should always pass, and so this just makes this explicitly clear by removing the try/except block.

Drop all usage of setuptools._distutils in favor of pure setuptools.
This effectively solves the problems that were present in the attempt
in xrootd#1585 and
xrootd#1700.

Co-authored-by: Henry Schreiner <henry.fredrick.schreiner@cern.ch>
@matthewfeickert
Copy link
Contributor Author

From the CI we can see this also works with:

  • setuptools v39.2.0
  • setuptools v59.6.0
  • setuptools v65.6.3

@matthewfeickert matthewfeickert marked this pull request as ready for review November 30, 2022 20:37
@matthewfeickert
Copy link
Contributor Author

Note that by this change

# sysconfig v48.0.0+ is incompatible for Python 3.6 only, so fall back to distutils.
# Instead of checking setuptools.__version__ explicitly, use the knowledge that
# to get here in the 'try' block requires setuptools v48.0.0+.
# FIXME: When support for Python 3.6 is dropped simplify this
import sys
if sys.version_info < (3, 7):
from distutils import sysconfig
else:
import sysconfig

is no longer explicitly true, so I'm inclined to just remove this comment that I originally wrote and move forward with the

import sys
if sys.version_info < (3, 7):
from distutils import sysconfig
else:
import sysconfig

apparently being sufficient.

@matthewfeickert
Copy link
Contributor Author

@simonmichal this is ready for review.

Reviews welcome from @henryiii, @amadio, and @adriansev as well, as extra eyes and testing configurations are always welcome.

Things will get even simpler when Python 3.6 support is dropped.

@adriansev
Copy link
Contributor

@matthewfeickert Thanks a lot for all the work!! I can only test tomorrow (if not later in the night) but one comment that i would have is that as long GRID (and especially WLCG software) is centos 7 based, the python 3.6 compatibility MUST be preserved (IMHO).

@matthewfeickert
Copy link
Contributor Author

matthewfeickert commented Nov 30, 2022

but one comment that i would have is that as long GRID (and especially WLCG software) is centos 7 based, the python 3.6 compatibility MUST be preserved (IMHO).

I don't claim to have any idea how difficult it is to try to keep things like WLCG up let alone running as well as it does (I know it is super hard and that people work tirelessly with no thanks to make this happen), but as a field we should not be happy to keep using EOL software indefinitely.

$ pipx install norwegianblue
$ eol python
| cycle |  release   | latest | latest release |    eol     |
|:------|:----------:|:-------|:--------------:|:----------:|
| 3.11  | 2022-10-24 | 3.11.0 |   2022-10-24   | 2027-10-24 |
| 3.10  | 2021-10-04 | 3.10.8 |   2022-10-11   | 2026-10-04 |
| 3.9   | 2020-10-05 | 3.9.15 |   2022-10-11   | 2025-10-05 |
| 3.8   | 2019-10-14 | 3.8.15 |   2022-10-11   | 2024-10-14 |
| 3.7   | 2018-06-26 | 3.7.15 |   2022-10-10   | 2023-06-27 |
| 3.6   | 2016-12-22 | 3.6.15 |   2021-09-03   | 2021-12-23 |
| 3.5   | 2015-09-12 | 3.5.10 |   2020-09-05   | 2020-09-13 |
| 3.4   | 2014-03-15 | 3.4.10 |   2019-03-18   | 2019-03-18 |
| 3.3   | 2012-09-29 | 3.3.7  |   2017-09-19   | 2017-09-29 |
| 2.7   | 2010-07-03 | 2.7.18 |   2020-04-19   | 2020-01-01 |
| 2.6   | 2008-10-01 | 2.6.9  |   2013-10-29   | 2013-10-29 |
$ eol centos
| cycle    |  release   | latest   |  support   |    eol     | link                                                      |
|:---------|:----------:|:---------|:----------:|:----------:|:----------------------------------------------------------|
| 9        | 2021-09-15 | 9        | 2027-05-31 | 2027-05-31 |                                                           |
| stream-8 | 2019-09-24 | 8        | 2024-05-31 | 2024-05-31 | https://wiki.centos.org/Manuals/ReleaseNotes/CentOSStream |
| 8        | 2019-09-24 | 8 (2111) | 2021-12-31 | 2021-12-31 | https://wiki.centos.org/Manuals/ReleaseNotes/CentOS8.2111 |
| 7        | 2014-07-07 | 7 (2009) | 2020-08-06 | 2024-06-30 | https://wiki.centos.org/Manuals/ReleaseNotes/CentOS7.2009 |
| 6        | 2011-07-10 | 6.10     | 2017-05-10 | 2020-11-30 | https://wiki.centos.org/Manuals/ReleaseNotes/CentOS6.10   |

But that's all I'll say here, especially as the two of us have discussed these ideas before and this PR does nothing to Python 3.6. :)

@adriansev
Copy link
Contributor

@matthewfeickert

But that's all I'll say here, especially as the two of us have discussed these ideas before and this PR does nothing to Python 3.6. :)

yeah, exactly :) it was just a comment and, for the sake of completeness, i must state that i mentioned that for the sake of generic usage as from experiment point of view we (ALICE) moved already to 3.9
and yupii, all checks are green :)

* Now that setuptools._distutils.core usage has been dropped, it is
  possible to go further and drop direct usage of distutils
  in favor of setuptools. This has already happened with the removal
  of setuptools._distutils as

  > from setuptools import setup, Extension

  should always pass, and so this commit just makes this explicitly
  clear by removing the try/except block.
Copy link
Member

@amadio amadio left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks.

@simonmichal
Copy link
Contributor

@matthewfeickert : thanks a lot for the PR, looks good :-)

@simonmichal simonmichal merged commit d5732ef into xrootd:master Dec 2, 2022
@matthewfeickert matthewfeickert deleted the feat/switch-to-setuptools branch December 2, 2022 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

python pip package no longer installs on ubuntu 20
4 participants