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.

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

Jump to bottom

Update non PEP 440 wheel filename deprecation notice #12939

Merged
merged 5 commits into from
Oct 12, 2024
Merged

Update non PEP 440 wheel filename deprecation notice #12939

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa25c97
Update non PEP 440 wheel filename deprecation notice
notatallshaw Aug 25, 2024
fbe7924
NEWS ENTRY
notatallshaw Aug 25, 2024
7ddafd0
Merge branch 'main' into update-non-PEP-440-wheel-filename-deprecatio…
notatallshaw Oct 8, 2024
9fd6b86
Merge branch 'main' into update-non-PEP-440-wheel-filename-deprecatio…
notatallshaw Oct 9, 2024
0c4cdf9
Break up different exceptions and report specific error
notatallshaw Oct 12, 2024
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
1 change: 1 addition & 0 deletions news/12939.trivial.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update non PEP 440 wheel filename deprecation notice.
40 changes: 26 additions & 14 deletions src/pip/_internal/models/wheel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@
from typing import Dict, Iterable, List

from pip._vendor.packaging.tags import Tag
from pip._vendor.packaging.utils import (
InvalidVersion,
parse_wheel_filename,
)
from pip._vendor.packaging.utils import (
InvalidWheelFilename as PackagingInvalidWheelName,
)

from pip._internal.exceptions import InvalidWheelFilename
from pip._internal.utils.deprecation import deprecated
Expand All @@ -32,22 +39,27 @@ def __init__(self, filename: str) -> None:
self.name = wheel_info.group("name").replace("_", "-")
_version = wheel_info.group("ver")
if "_" in _version:
deprecated(
reason=(
f"Wheel filename {filename!r} uses an invalid filename format, "
f"as the version part {_version!r} is not correctly normalised, "
"and contains an underscore character. Future versions of pip may "
"fail to recognise this wheel."
),
replacement=(
"rename the wheel to use a correctly normalised version part "
"(this may require updating the version in the project metadata)"
),
gone_in="25.1",
issue=12914,
)
_version = _version.replace("_", "-")

try:
parse_wheel_filename(filename)
except (PackagingInvalidWheelName, InvalidVersion):
Copy link
Member

Choose a reason for hiding this comment

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

By just reading the code here, it's not obvious that all wheel filename parsing errors here are guaranteed to relate to an invalid version part.

Would it make sense to catch the two exception separately with a slightly different deprecation message?

Or is there a guarantee here that all exceptions are due to the version part? In that case a little comment explaining that might be useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not guaranteed but it would have to occur under the following series of events:

  1. The wheel filename passes the existing regex
  2. There is a _ that is valid in what the regex parses as "version" but is not actually part of the version
  3. There is a different error in the filename that packaging catches

This would therefore have to be a currently unknown error that wheel filenames can have that packaging does not support but pip's regex was passing on anyway.

The reason I am catching both exceptions is because I did the same in #12327 and I was borrowing that code, but in that PR the code was scanning a directory that could find arbitrary names and I saw both errors in my testing.

Happy to split catching the two different exceptions up and give an appropriate message for PackagingInvalidWheelName .

deprecated(
reason=(
f"Wheel filename {filename!r} uses an invalid filename format, "
f"as the version part {_version!r} is not correctly "
"normalised, and contains an underscore character. Future "
"versions of pip will fail to recognise this wheel."
),
replacement=(
"rename the wheel to use a correctly normalised "
"version part (this may require updating the version "
"in the project metadata)"
),
gone_in="25.1",
issue=12938,
)

self.version = _version
self.build_tag = wheel_info.group("build")
self.pyversions = wheel_info.group("pyver").split(".")
Expand Down
Loading