[BUG] DistutilsMetaFinder appears multiple times on sys.meta_path

[nitzmahone]

setuptools version

setuptools==60.0.4

Python version

python3.8

OS

Any

Additional environment information

N/A

Description

The _DistutilsMetaFinder meta_path finder shim is installed multiple times, at least twice by site.py via the .pth shim, and once by ensure_local_distutils when setuptools is actually imported. It's unlikely to be a problem, especially since it's literally the same instance, but add_shim should be idempotent instead of blindly inserting the shim each time it's called. It also appears that the intent was to remove the shim entirely once distutils has been loaded, but since it's added more than once, the removal in the import leaves two references to it in sys.meta_path after setuptools has been imported.

Expected behavior

After setuptools has been imported, the distutils shim finder is no longer on the meta path.

How to Reproduce

(with setuptools>60 installed):
python -c "import sys; print(f'Before: {sys.meta_path}'); import setuptools; print(f'After: {sys.meta_path}')"

Output

Before: [<_distutils_hack.DistutilsMetaFinder object at 0x7fe4d2688700>, <_distutils_hack.DistutilsMetaFinder object at 0x7fe4d2688700>, <_virtualenv._Finder object at 0x7fe4d2806f70>, <class '_frozen_importlib.BuiltinImporter'>, <class '_frozen_importlib.FrozenImporter'>, <class '_frozen_importlib_external.PathFinder'>]
After: [<_distutils_hack.DistutilsMetaFinder object at 0x7fe4d2688700>, <_distutils_hack.DistutilsMetaFinder object at 0x7fe4d2688700>, <_virtualenv._Finder object at 0x7fe4d2806f70>, <class '_frozen_importlib.BuiltinImporter'>, <class '_frozen_importlib.FrozenImporter'>, <class '_frozen_importlib_external.PathFinder'>, <pkg_resources.extern.VendorImporter object at 0x7fe4d21c5460>, <setuptools.extern.VendorImporter object at 0x7fe4d1f2a100>]

Code of Conduct

• I agree to follow the PSF Code of Conduct

[jaraco]

I observed this behavior today too (https://github.com/pypa/pip/issues/10743#issuecomment-999768459). My instinct was that under normal circumstances that it would just get installed once. I was surprised to see that as a matter of course, it could get installed two or three times. I agree it might be preferable to be idempotent. My concern was with stability if it's added twice but removed once - what should happen?

One correction:

It also appears that the intent was to remove the shim entirely once distutils has been loaded, but since it's added more than once, the removal in the import leaves two references to it in sys.meta_path after setuptools has been imported.

Originally, Setuptools was being cautious about the override and only invoking it during setuptools operations, but it became clear that it's necessary to supply 'distutils' more broadly and before setuptools is imported. At the same time, Setuptools wanted to be sure that if enabled, the local copy would be guaranteed to be present/imported, even if the shim had somehow not been installed previously (such as with python -S). So the short explanation is that it's complicated and there's no simple expectation but more of a mishmash of expectations.

I'd be happy to make it idempotent, but I'd also like to be sure the instance added by the .pth file is not removed by ensure_local_distutils.

[jaraco]

I'll probably get to this in the future, but for now, there are more pressing issues with the distutils adoption, so I welcome others to devise an implementation that meets the above goals.

[nitzmahone]

So the short explanation is that it's complicated and there's no simple expectation but more of a mishmash of expectations.

@jaraco Yeah, I wondered about that, especially with a couple of the other methods in the metafinder that alter the behavior depending on what else is going on. There's also the question to whether you want it to be resilient to things like people clearing out sys.modules then re-importing it - not that anyone would ever do such a silly thing... 😜

#2959 is the dirt-simplest thing that keeps it from appearing multiple times, though to jive with your previous statement, it should probably also remove the code from ensure_local_distutils that zaps the loader off the metapath?

[jaraco]

Yep. That's simple and would likely work, but isn't quite what I had in mind. Let me see if I can throw something together.