Skip to content

Supply Distribution.locate_file, honoring the abstract method. #11685

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 12, 2025
Merged

Supply Distribution.locate_file, honoring the abstract method. #11685

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a59ffc6
Supply Distribution.locate_file, honoring the abstract method. Fixes …
jaraco Jan 1, 2023
0d596aa
Merge branch 'main' into bugfix/11684-implement-locate-file
sbidoul Jan 4, 2025
a15d958
Merge branch 'main' into bugfix/11684-implement-locate-file
ichard26 Jan 12, 2025
255312e
Add type annotations + comment
ichard26 Jan 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/pip/_internal/metadata/importlib/_dists.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import importlib.metadata
import pathlib
import zipfile
from os import PathLike
from typing import (
Collection,
Dict,
Expand Down Expand Up @@ -95,6 +96,11 @@ def read_text(self, filename: str) -> Optional[str]:
raise UnsupportedWheel(error)
return text

def locate_file(self, path: str | PathLike[str]) -> pathlib.Path:
Copy link
Contributor

@jsirois jsirois Jan 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This broke 3.8 & 3.9 compatibility in the 25.0 release:

:; python3.8
Python 3.8.20 (default, Nov 30 2024, 23:56:06) 
[GCC 14.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from os import PathLike
>>> import pathlib
>>> def locate_file(path: str | PathLike[str]) -> pathlib.Path:
...     raise NotImplementedError
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'ABCMeta' object is not subscriptable
>>>

A from __future__ import annotations is needed to fix.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, this didn't fail in CI as the importlib-metadata backend isn't used on Python 3.10 or lower. Sorry about this!

Copy link
Contributor

@jsirois jsirois Jan 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, this file should ~never be imported though unless under Python 3.11+ due to checks in src/pip/_internal/metadata/__init__.py.

I can concoct this failure:

:; /tmp/pip25/bin/python -V
Python 3.8.20

:; /tmp/pip25/bin/python -mpip -V
pip 25.0 from /tmp/pip25/lib/python3.8/site-packages/pip (python 3.8)

:; _PIP_USE_IMPORTLIB_METADATA=True /tmp/pip25/bin/python -mpip wheel --no-binary :all: cowsay
WARNING: There was an error checking the latest version of pip.
ERROR: Exception:
Traceback (most recent call last):
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/cli/base_command.py", line 106, in _run_wrapper
    status = _inner_run()
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/cli/base_command.py", line 97, in _inner_run
    return self.run(options, args)
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/cli/req_command.py", line 67, in wrapper
    return func(self, options, args)
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/commands/wheel.py", line 105, in run
    session = self.get_default_session(options)
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/cli/index_command.py", line 76, in get_default_session
    self._session = self.enter_context(self._build_session(options))
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/cli/index_command.py", line 99, in _build_session
    session = PipSession(
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/network/session.py", line 345, in __init__
    self.headers["User-Agent"] = user_agent()
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/network/session.py", line 177, in user_agent
    setuptools_dist = get_default_environment().get_distribution("setuptools")
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/metadata/__init__.py", line 76, in get_default_environment
    return select_backend().Environment.default()
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/metadata/__init__.py", line 61, in select_backend
    from . import importlib
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/metadata/importlib/__init__.py", line 1, in <module>
    from ._dists import Distribution
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/metadata/importlib/_dists.py", line 41, in <module>
    class WheelDistribution(importlib.metadata.Distribution):
  File "/tmp/pip25/lib/python3.8/site-packages/pip/_internal/metadata/importlib/_dists.py", line 99, in WheelDistribution
    def locate_file(self, path: str | PathLike[str]) -> pathlib.Path:
TypeError: 'ABCMeta' object is not subscriptable

But that seems pretty artificial. I'll investigate more how I'm hitting this codepath without exporting _PIP_USE_IMPORTLIB_METADATA=True and only file an issue if I find a real one along those lines.

Sorry for the noise.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So @ichard26 and @sbidoul I don't think this is actually a real bug that any reasonable use can hit. My _PIP_USE_IMPORTLIB_METADATA=True force is ... forced and the original case I got here from was my own PEBKAC.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what was I thinking? The union operator also doesn't work under Python 3.10. Anyway, a fix is up: #13181

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ichard26.

# This method doesn't make sense for our in-memory wheel, but the API
# requires us to define it.
raise NotImplementedError


class Distribution(BaseDistribution):
def __init__(
Expand Down
Loading