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

Replace distutils #4796

Closed
hugovk opened this issue Jul 16, 2020 · 4 comments
Closed

Replace distutils #4796

hugovk opened this issue Jul 16, 2020 · 4 comments
Labels

Comments

@hugovk
Copy link
Member

hugovk commented Jul 16, 2020

There's an effort underway to move the functionality of distutils into Setuptools and other libraries, with a goal to remove distutils from the standard library.

Re:

(Though replacing distutils with setuptools wherever possible is worthwhile irrespective of the distutils injection strategy.)

setuptools will be the canonical implementation of "distutils" in the future. Some of the distutils hierarchy already is duplicated in setuptools (like commands), the parts that are likely to remain supported will likely move into the setuptools (I suspect that will include things like exceptions) namespace, and some stuff that is no longer necessary will be deprecated and removed.

If you need something from the distutils namespace and it's not available in setuptools, continue to use it until we provide an alternative (I would say "raise an issue" but I am not sure if we're going to just do a bulk migration or not), but if there's already an equivalent in setuptools, switching over to using setuptools now is the best thing you can do (and you should report any issues you encounter when doing this).

... setuptools is not exposing its version of distutils as setuptools.distutils (and to the extent that it exists under the setuptools namespace, that location is not necessarily stable). setuptools replaces the top-level distutils module with its own version of distutils, so import distutils pulls the version from setuptools.

This is one phase of an ongoing project to remove distutils from the standard library (and probably eventually retire the project entirely in favor of setuptools).


Here's where we're using disutils:

File Use Replacement
Tests/test_image_access.py and setup.py from distutils import ccompiler, sysconfig #4809, #4814, #4817
Tests/test_imagefont.py distutils.version.StrictVersion packaging.version.parse #4797
setup.py from distutils.command.build_ext import build_ext from setuptools.command.build_ext import build_ext #4829
setup.py from distutils import cygwinccompiler #4890
@radarhere
Copy link
Member

I'm not certain that we need to remove from distutils import cygwinccompiler. Some thoughts.

The import of cygwinccompiler was added in #4019, to resolve #4018 - the Python distutils in the MinGW environment was using "gcc --print-prog-name ld" when looking for ld.

When we moved the MinGW job from AppVeyor to GitHub Actions, this code actually stopped being run by our CIs - the problem doesn't occur on GHA.

setuptools doesn't use gcc to try and find the path. So once distutils is no more, this code can be safely removed.

If we wanted to remove it now, I think there are only two alternatives. Either add code to ensure that setuptools distutils is used, rather than Python distutils, or if a user ever reports it, suggest that they investigate how to correctly connect gcc with ld.

@radarhere
Copy link
Member

I've created msys2/MINGW-packages#6744 to see if the fallback behaviour can be incorporated into the MinGW patches.

@nulano
Copy link
Contributor

nulano commented Aug 30, 2020

I'm not certain that we need to remove from distutils import cygwinccompiler. Some thoughts.

The import of cygwinccompiler was added in #4019, to resolve #4018 - the Python distutils in the MinGW environment was using "gcc --print-prog-name ld" when looking for ld.

When we moved the MinGW job from AppVeyor to GitHub Actions, this code actually stopped being run by our CIs - the problem doesn't occur on GHA.

setuptools doesn't use gcc to try and find the path. So once distutils is no more, this code can be safely removed.

If we wanted to remove it now, I think there are only two alternatives. Either add code to ensure that setuptools distutils is used, rather than Python distutils, or if a user ever reports it, suggest that they investigate how to correctly connect gcc with ld.

While working on #4890, I've just realized the actual cause of #4018, and the proper fix. The issue was that AppVeyor was missing the MSYSTEM=MINGW32 envvar, and getting confused by the default MSYSTEM=MSYS2.

After adding the envvar in my branch nulano/Pillow@master...nulano:mingw-test2, the build passes on AppVeyor.

After removing MSYSTEM=MINGW32 again in my branch nulano/Pillow@mingw-test2...nulano:mingw-test3, the build fails again.

I've therefore removed the GCC workaround in #4890.

@nulano nulano mentioned this issue Aug 30, 2020
@radarhere
Copy link
Member

#4890 was the last step. distutils is no longer imported by Pillow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants