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

[BUG] Setuptools crashes when implementing find_distributions in a custom Finder in python 3.8 #3319

Closed
konstin opened this issue May 16, 2022 · 6 comments · Fixed by #3832
Closed
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.

Comments

@konstin
Copy link

konstin commented May 16, 2022

setuptools version

setuptools==62.2.0

Python version

3.8

OS

Ubuntu 20.04

Additional environment information

No response

Description

When implementing find_distributions on a custom Finder, setuptools crashes because it expects the returned PathDistribution instances to have attributes only present in its vendored importlib_metadata but not in python 3.8's importlib.metadata.

See https://github.com/konstin/setuptools-reproducer:

virtualenv -p 3.8 .venv
.venv/bin/pip install -r requirements.txt # pinned versions of setuptools and virtualenv
.venv/bin/python main.py
import sys
from importlib.abc import MetaPathFinder
from importlib.machinery import PathFinder, ModuleSpec
from importlib.metadata import DistributionFinder, PathDistribution
from importlib.util import spec_from_file_location
from pathlib import Path

from typing import Iterable, Optional


class CustomFinder(PathFinder, MetaPathFinder):
    """As example, we load tqdm from a custom location"""

    tqdm_folder = Path(__file__).parent.joinpath("tqdm_folder")

    def find_spec(self, fullname, path=None, target=None) -> Optional[ModuleSpec]:
        if fullname == "tqdm":
            init_py = self.tqdm_folder.joinpath("tqdm").joinpath("__init__.py")
            return spec_from_file_location(fullname, init_py)

    def find_distributions(
        self, context: DistributionFinder.Context = ...
    ) -> Iterable[PathDistribution]:
        dist_info = self.tqdm_folder.joinpath("tqdm_folder").joinpath(
            "tqdm-4.64.0.dist-info"
        )
        # Here, we return a normal python3.8 importlib.metadata.PathDistribution
        return iter([PathDistribution(dist_info)])


sys.meta_path.append(CustomFinder())

# this works - we successfully made tqdm importable
# noinspection PyUnresolvedReferences
import tqdm
print(tqdm.__version__)
# this doesn't
# noinspection PyUnresolvedReferences
import virtualenv
print(virtualenv.__version__)

Latest main is also affected

Expected behavior

setuptools accepts importlib.metadata.PathDistribution instances.

How to Reproduce

See https://github.com/konstin/setuptools-reproducer reproduce.sh

Output

Traceback (most recent call last):
  File "main.py", line 39, in <module>
    import virtualenv
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/virtualenv/__init__.py", line 3, in <module>
    from .run import cli_run, session_via_cli
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/virtualenv/run/__init__.py", line 14, in <module>
    from .plugin.creators import CreatorSelector
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/virtualenv/run/plugin/creators.py", line 6, in <module>
    from virtualenv.create.via_global_ref.builtin.builtin_way import VirtualenvBuiltin
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/virtualenv/create/via_global_ref/builtin/builtin_way.py", line 7, in <module>
    from virtualenv.create.creator import Creator
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/virtualenv/create/creator.py", line 15, in <module>
    from virtualenv.discovery.cached_py_info import LogCmd
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/virtualenv/discovery/cached_py_info.py", line 23, in <module>
    _CACHE[Path(sys.executable)] = PythonInfo()
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/virtualenv/discovery/py_info.py", line 86, in __init__
    self.distutils_install = {u(k): u(v) for k, v in self._distutils_install().items()}
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/virtualenv/discovery/py_info.py", line 152, in _distutils_install
    d = dist.Distribution({"script_args": "--no-user-cfg"})  # conf files not parsed so they do not hijack paths
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/setuptools/dist.py", line 470, in __init__
    for ep in metadata.entry_points(group='distutils.setup_keywords'):
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/setuptools/_vendor/importlib_metadata/__init__.py", line 999, in entry_points
    return SelectableGroups.load(eps).select(**params)
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/setuptools/_vendor/importlib_metadata/__init__.py", line 449, in load
    ordered = sorted(eps, key=by_group)
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/setuptools/_vendor/importlib_metadata/__init__.py", line 996, in <genexpr>
    eps = itertools.chain.from_iterable(
  File "/home/konsti/setuptools-reproducer/.venv/lib/python3.8/site-packages/setuptools/_vendor/importlib_metadata/_itertools.py", line 16, in unique_everseen
    k = key(element)
AttributeError: 'PathDistribution' object has no attribute '_normalized_name'
@konstin konstin added bug Needs Triage Issues that need to be evaluated for severity and status. labels May 16, 2022
@konstin konstin changed the title [BUG] [BUG] Setuptools crashes when implementing find_distributions in a custom Finder in python 3.8 May 16, 2022
@jaraco
Copy link
Member

jaraco commented Jul 14, 2022

The reported issue is not with Setuptools, except for the fact that Setuptools is using importlib_metadata 4.3+.

Your description is correct. The issue is that importlib_metadata 4.3+ and as found in Python 3.10+ is expecting a _normalized_name to be present on PathDistributions.

Perhaps importlib_metadata could be more lenient and provide a compatibility layer when newer versions of importlib metadata encounter distributions without a _normalized_name.

That wouldn't be necessary, however, if you updated the reproducer to rely on importlib_metadata>=4.3 on Python <3.10.

That would be my preference - to reduce/eliminate reliance on the implementation as found in python 3.8 and 3.9 (because it was inadequate for that and other concerns).

I understand that it's awkward that setuptools has access to a newer version of importlib_metadata that you can't use. That's an unfortunate consequence of having these vendored dependencies.

I created python/importlib_metadata#396 to track the broader issue within importlib_metadata, but I'd love to know more about your use case and how feasible it might be to simply depend on the more modern implementations of importlib metadata.

@konstin
Copy link
Author

konstin commented Jul 14, 2022

Depending on importlib_metadata would be feasible through vendoring; It's a bit of a pity though because I otherwise get away with zero dependencies.

I'm more concerned about how to make sure that the version I depend on matches what setuptools uses (or whatever other tool calls find_distributions), and more generally, what the type signature of find_distributions should be. Solving python/importlib_metadata#396 and specifying that you return the std Distribution would be great long term solution imho. In the meantime, what would you need to specify to get setuptools compatible importlib_metadata? Currently I'm just importing it from setuptools._vendor if setuptools is present because that guarantees that's the correct version and i get away without a dependency

@abravalheri
Copy link
Contributor

This seems to be possible to solve with #3832:

> docker run --rm -it python:3.8 /bin/bash
git clone https://github.com/konstin/setuptools-reproducer /tmp/setuptools-reproducer
cd /tmp/setuptools-reproducer/
python3.8 -m venv .venv
sed -i 's~setuptools==62.2.0~setuptools @ git+https://github.com/abravalheri/setuptools@update-importlib~' requirements.txt
sed -i 's~joinpath("tqdm_folder").~~' main.py  # this patch to be required (maybe some debugging that was left behind)
.venv/bin/pip install -r requirements.txt
.venv/bin/python main.py
# 4.64.0
# 20.14.1

@konstin
Copy link
Author

konstin commented Feb 21, 2023

Will this remain fixed as setuptools and importlib_metadata/importlib.metadata get updated and the signature of find_distributions potentially changes again?

@abravalheri
Copy link
Contributor

abravalheri commented Feb 21, 2023

Will this remain fixed as setuptools and importlib_metadata/importlib.metadata get updated and the signature of find_distributions potentially changes again?

For this specific error, I added a regression test to importlib-metadata's test suite in my PR:

https://github.com/python/importlib_metadata/pull/397/files#diff-464cf44c19d12bb82319f93e23cdee3407583e11c0eb868bfb9a670235a3a2dd

If you want to ensure any other backward compatibility, I suggest opening a PR to importlib-metadata with a similar check...

@konstin
Copy link
Author

konstin commented Feb 21, 2023

oh i missed that, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants