Reconsidering distutils replacement strategy

[pganssle]

I first expressed concerns about the fact that distutils needs to be imported after setuptools in this post, and I did not realize that we had not adequately addressed this before the distutils adoption. I think we should re-consider the way this is done, to avoid any sort of requirement on the sort order.

Currently, when you import setuptools, distutils is injected into sys.modules.

The problem here is that it requires importing setuptools before distutils, which most people will not be doing right now (by almost all common import sort orders, distutils comes first).

@jaraco explains why it's done this way here:

I did consider another approach, where Setuptools could add a .pth file that would import setuptools.distutils_patch. Such an approach would always happen earlier and so would not be subject to the race that's happening here, but it would also happen whether or not setuptools was imported (on any invocation of Python in that environment).

I think one preferred solution would be for us to install a distutils package that would be imported before distutils. The problem is that in the standard sys.path, site-packages comes after the standard library. Since anything we do here will be a tremendous hack, I'm thinking that maybe the most surgical way to accomplish this is to do something like this:

1. Create a setuptools._distutils/import_hack/distutils/__init__.py (where there's no __init__.py in import_hack) that invokes import setuptools.distutils_patch.
2. Add a .pth file that injects $SITEPACKAGES/setuptools/_import_hack/ into sys.path before the standard library.

Another option I think we should consider is stepping up the timetable on providing everything distutils provides directly from the setuptools namespace, as I described here:

I personally would prefer it if we made the setuptools namespace the canonical namespace, and have distutils just import symbols from setuptools and possibly wrap them in deprecation warnings.

Right now, there are things that you must import from distutils, and so we can't say, "You can avoid an issue with the sort order by just not importing distutils". If we provide everything that distutils provides in the setuptools namespace, we can just say, "Just use the setuptools version and you won't have this problem."

Personally, I think we should do a combination of both — make it so importing distutils still works in whatever order you do it in and move everything to the canonical setuptools namespace. That will make it easier to actively deprecate direct use of distutils, and pave the way for us eventually removing the need for the .pth file (though the fastest road to not having the .pth file will be via removing distutils from the standard library).

[jaraco]

I was thinking of something similar:

• move setuptools.distutils_patch to distutils_patch (top-level module/package).
• add .pth that simply has import distutils_patch.

This approach has a bit less indirection and would be less susceptible to issues with path manipulation (platform-specific path separators, interactions with alternate importers such as zipimporter). This approach also allows setuptools._distutils to remain pristine (relative to pypa/distutils).

[pganssle]

@jaraco Wouldn't import distutils_patch mean that we'd need to eagerly import setuptools._distutils on every interpreter startup (even if it doesn't import setuptools directly)?

The approach I've taken in #2260 probably won't work with zipimporter, but it has the huge advantage that the only thing happening in the .pth file is manipulation of sys.path rather than a full import.

[jaraco]

Wouldn't import distutils_patch mean that we'd need to eagerly import setuptools._distutils on every interpreter startup (even if it doesn't import setuptools directly)?

Good point. _distutils would need to be outside setuptools also. Ideally, the .pth file should install a minimal hook that defers all processing until import distutils. I think your design does that.

probably won't work with zipimporter.

Is that an acceptable limitation? I suspect not.

[pganssle]

probably won't work with zipimporter.

Is that an acceptable limitation? I suspect not.

It's certainly not ideal. I think we should do our best to support whatever environments people are running setuptools from; I'll have to look at this use case to see if there's a simple way to address it in _distutils_importer.

That said, I'm not sure what the use cases are where people are running setuptools from a zip file or some other exotic configuration. Worst case scenario, I think the zipimporter people are no worse off with #2260 than they are today — if they want to use both setuptools and distutils and they want to run setuptools from a zip file, they have to import setuptools first (and in many / most cases, it would just be a warning if they do that rather than breaking the package). I'm a lot more comfortable with that requirement than I am with something that says that all setuptools imports must precede distutils imports, particularly since packages doing exotic import stuff tend to have a lot of compatibility shimming going on anyway.

[jaraco]

Another approach to consider:

• in .pth, add a custom loader/importer to front of sys.meta_path
• in custom importer, override finding of distutils

[mattip]

It seems you released setuptools 49.2.1 without resolving this issue (also #2260). Was that on purpose? This is causing no end of problems with downstream projects, and will slow the adoption of new versions of setuptools.

[jaraco]

Yes. #2260 is blocked on a rewrite and request/need for more sophistication in the solution. 49.2.1 fixed the thing that it fixed. I agree this issue is important and I'd like to get it addressed asap.

[yan12125]

Thanks for the fix. However, I noticed that the warning is still raised in setuptools 49.3.0

$ python
Python 3.8.5 (default, Jul 27 2020, 08:42:51) 
[GCC 10.1.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import distutils
>>> import setuptools
/usr/lib/python3.8/site-packages/_distutils_hack/__init__.py:18: UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first.
  warnings.warn(
>>> setuptools.__version__
'49.3.0'

Is the warning expected in the new .pth approach?

[jaraco]

Hmm. Yeah, that's a little annoying, but somewhat intentional.

The pth behavior is only enabled when DISTUTILS_USE_SETUPTOOLS=local. If you set that env var, the warning goes away:

~ $ pip-run -q setuptools==49.3 -- -c "import distutils; import setuptools; print(setuptools.__version__)"
/var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/pip-run-u97o8h9w/_distutils_hack/__init__.py:18: UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first.
  warnings.warn(
49.3.0
~ $ env SETUPTOOLS_USE_DISTUTILS=local pip-run -q setuptools==49.3 -- -c "import distutils; import setuptools; print(setuptools.__version__)
"
49.3.0

I'd like 49.3 to prove stable then switch the default (so SETUPTOOLS_USE_DISTUTILS=stdlib is needed to opt-out instead) in about a week.

[yan12125]

Got it, thanks for clarification!

[gaborbernat]

I'd like 49.3 to prove stable then switch the default (so SETUPTOOLS_USE_DISTUTILS=stdlib is needed to opt-out instead) in about a week.

This is getting really frustrating. It's now the third setutpools release that breaks multiple CIs of mine only because some libraries dare to use distutils LooseVersion and setuptools start spitting out a warning about what no one can do nothing about. Sigh. I really wish we'd stop with this warning craze, and move to something better. End users are the one to see it and suffer with it the most, but they can't address it... only the library maintainers can.

Also, please don't close an issue until it's not fixed. This clearly isn't, not at least the default switch is the other way around.

[pganssle]

Yes this is clearly not fixed until the now hopefully pointless warning is removed.

[pganssle]

Hmm. Yeah, that's a little annoying, but somewhat intentional.

Why would this be intentional? The point of the other fix is so that you wouldn't care about the import order except in very rare situations. I originally removed it entirely, but ended up putting it back in place to point at the problem if the pth file wasn't working properly.

It makes no sense to raise the warning if you aren't even doing the distutils replacement.

[jaraco]

It's intentional because the distutils replacement is currently opt-in. Maybe the warning should only be present if the distutils replacement is opted in. If we are encouraging packagers to import distutils late or not to import it at all, the argument can be made that the warning should be issued unconditionally, as it's still preferable for a packager to present the most compatible interface, even if a particular environment doesn't have the future features enabled.

My instinct is to push forward with making the distutils replacement opt-out (#2255). I'm open to tweaking the warnings too.

[pganssle]

It's intentional because the distutils replacement is currently opt-in. Maybe the warning should only be present if the distutils replacement is opted in. If we are encouraging packagers to import distutils late or not to import it at all, the argument can be made that the warning should be issued unconditionally, as it's still preferable for a packager to present the most compatible interface, even if a particular environment doesn't have the future features enabled.

If we want people to stop importing distutils at all, we should warn whenever distutils is used. The only reason to care about the order in which distutils and setuptools are imported was because it was causing problems with the old mechanism. The entire point of the new .pth-based mechanism was to prevent people from caring about this.

The only real purpose of the warning is to let people know that we have replaced distutils in sys.modules after it was in use. It's particularly incoherent to warn about it only if the distutils replacement is disabled, since that's the one situation where it definitely doesn't matter.

[pganssle]

I disabled this warning except in cases where the distutils import is enabled in #2316, and I've pushed out a v49.3.1 release (which should hit PyPI in an hour or so when the tests pass, I guess).