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

Import setuptools before distutils #400

Closed
wants to merge 1 commit into from

Conversation

stefanor
Copy link

setuptools 60 uses its own bundled version of distutils, by default. It injects this into sys.modules, at import time. So we need to make sure that it is imported, before anything else imports distutils, to ensure everything is using the same distutils version.

This change in setuptools is to prepare for Python 3.12, which will drop distutils.

Fixes: https://bugs.debian.org/1022491

setuptools 60 uses its own bundled version of distutils, by default. It
injects this into sys.modules, at import time. So we need to make sure
that it is imported, before anything else imports distutils, to ensure
everything is using the same distutils version.

This change in setuptools is to prepare for Python 3.12, which will drop
distutils.

Fixes: https://bugs.debian.org/1022491
@DjLegolas
Copy link
Contributor

I don't think we need this fix, or at least not in the current state:

  1. it breaks linting tests.
  2. when I have imported distutils when I had an old version of setuptools, the imported package was from the stdlib.
    after updating setuptools to latest, and doing the same, I got distutils from the setuptools package.

anyway, I might be mistaken.
and of curse, thanks for this 😄

@stefanor
Copy link
Author

after updating setuptools to latest, and doing the same, I got distutils from the setuptools package.

That depends on whether you have already imported setuptools or not. When you import it, it injects its distutils module into sys.modules.

@stefanor
Copy link
Author

Obviously the best change to make is to replace any distutils imports with something from pure setuptools.

@DjLegolas
Copy link
Contributor

after updating setuptools to latest, and doing the same, I got distutils from the setuptools package.

That depends on whether you have already imported setuptools or not. When you import it, it injects its distutils module into sys.modules.

Well, I don't see this behavior, (tested also on ubuntu):

C:\Users\dj_le>py
Python 3.10.6 (tags/v3.10.6:9c7b4bd, Aug  1 2022, 21:53:49) [MSC v.1932 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import distutils
>>> distutils.__file__
'C:\\Python310\\lib\\site-packages\\setuptools\\_distutils\\__init__.py'
>>>

Obviously the best change to make is to replace any distutils imports with something from pure setuptools.

Agreed. But not all of the functionality is supported by setuptools yet, i.e. all of the distutils.commands we use in deluge

@stefanor
Copy link
Author

stefanor commented Dec 1, 2022

Well, I don't see this behavior, (tested also on ubuntu):

Thanks, I had been surprised that I was in the vanguard of filing these issues. But I see now that Debian is missing distutils-precedence.pth that does the magic to hook distutils at interpreter startup time. Obviously at a small performance penalty.

So, you won't see it unless you have setuptools > 60 and don't have distutils-precedence.pth.

If you do want to reproduce the issue, use Debian unstable outside a virtualenv. Or remove distutils-precedence.pth from the virtualenv.

I totally understand if you don't take the patch, because this is a bug in Debian, I'd say it is, I just filed https://bugs.debian.org/1025216

@cas--
Copy link
Member

cas-- commented Dec 1, 2022

Thanks for providing a potential fix, I was surprised that a fragile import order change was required. Based on your latest findings I feel it probably is better that debian fixes the issue rather than attempt to fix every project importing distutils.

Decomposing the setup.py file and replacing distutils has been on my mind for a long time but not got around to it...

@cas-- cas-- closed this Dec 1, 2022
@stefanor stefanor deleted the setuptools-60 branch December 1, 2022 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants