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

Fix pip cache remove pattern matching. #13094

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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/13086.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix ``pip cache remove`` pattern matching.
55 changes: 45 additions & 10 deletions src/pip/_internal/commands/cache.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import fnmatch
import os
import re
import textwrap
from optparse import Values
from typing import Any, List
Expand Down Expand Up @@ -204,22 +206,55 @@ def _find_http_files(self, options: Values) -> List[str]:
def _find_wheels(self, options: Values, pattern: str) -> List[str]:
wheel_dir = self._cache_dir(options, "wheels")

# The wheel filename format, as specified in PEP 427, is:
# The wheel filename format, as originally specified in PEP 427, is:
# {distribution}-{version}(-{build})?-{python}-{abi}-{platform}.whl
#
# Additionally, non-alphanumeric values in the distribution are
# normalized to underscores (_), meaning hyphens can never occur
# before `-{version}`.
#
# Given that information:
# - If the pattern we're given contains a hyphen (-), the user is
# providing at least the version. Thus, we can just append `*.whl`
# to match the rest of it.
# - If the pattern we're given doesn't contain a hyphen (-), the
# user is only providing the name. Thus, we append `-*.whl` to
# match the hyphen before the version, followed by anything else.
# - If the pattern we're given is not a glob:
# + We attempt project name normalization such that pyyaml matches PyYAML,
# etc. as outlined here:
# https://packaging.python.org/specifications/name-normalization.
# The one difference is we normalize non-alphanumeric to `_` instead of `-`
# since that is how the wheel filename components are normalized
# (facepalm).
# + If the pattern we're given contains a hyphen (-), the user is providing
# at least the version. Thus, we can just append `*.whl` to match the rest
# of it if it doesn't already glob to the end of the wheel file name.
# + If the pattern we're given is not a glob and doesn't contain a
# hyphen (-), the user is only providing the name. Thus, we append `-*.whl`
# to match the hyphen before the version, followed by anything else.
# - If the pattern is a glob, we ensure `*.whl` is appended if needed but
# cowardly do not attempt glob parsing / project name normalization. The user
# is on their own at that point.
#
# PEP 427: https://www.python.org/dev/peps/pep-0427/
pattern = pattern + ("*.whl" if "-" in pattern else "-*.whl")

return filesystem.find_files(wheel_dir, pattern)
# And: https://packaging.python.org/specifications/binary-distribution-format/
Comment on lines 234 to +235
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For Pip / PyPA knowledgeable reviewers - what the heck is up with this by the way? I've always found it off-putting since the PEP banners have started appearing on some of the PyPA PEPs re-directing to these new standards pages. The fact that some PEPs get a new page and some don't and what process the ones with the new page go through to get updated is all mysterious feeling.

uses_glob_patterns = any(
glob_pattern_char in pattern for glob_pattern_char in ("[", "*", "?")
)
if not uses_glob_patterns:
project_name, sep, rest = pattern.partition("-")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

N.B. I documented punting on deciphering glob patterns with respect to name normalization. Someone braver than I might try that later. I assume forward progress here is better than no progress though.

I did not, however, call the shot on version normalization. Again, I was cowardly, but that problem still remains. If a wheel in the cache is foo-1.0-py3-none-any.whl, then, in my opinion, pip cache remove foo-1.0.0 should remove it, but does not today.

# N.B.: This only normalizes non-alphanumeric characters in the name, which
# is 1/2 the job. Stripping case sensitivity is the other 1/2 and is
# handled below in the conversion from a glob to a regex executed with the
# IGNORECASE flag active.
partially_normalized_project_name = re.sub(r"[^\w.]+", "_", project_name)
if not sep:
sep = "-"
pattern = f"{partially_normalized_project_name}{sep}{rest}"

if not pattern.endswith((".whl", "*")):
pattern = f"{pattern}*.whl"

regex = fnmatch.translate(pattern)

return [
os.path.join(root, filename)
for root, _, files in os.walk(wheel_dir)
for filename in files
if re.match(regex, filename, flags=re.IGNORECASE)
]
88 changes: 69 additions & 19 deletions tests/functional/test_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,26 @@ def populate_http_cache(http_cache_dir: str) -> List[Tuple[str, str]]:
return files


@pytest.fixture
def populate_wheel_cache(wheel_cache_dir: str) -> List[Tuple[str, str]]:
def _populate_wheel_cache(
wheel_cache_dir: str, *name_and_versions: str
) -> List[Tuple[str, str]]:
destination = os.path.join(wheel_cache_dir, "arbitrary", "pathname")
os.makedirs(destination)

files = [
("yyy-1.2.3", os.path.join(destination, "yyy-1.2.3-py3-none-any.whl")),
("zzz-4.5.6", os.path.join(destination, "zzz-4.5.6-py3-none-any.whl")),
("zzz-4.5.7", os.path.join(destination, "zzz-4.5.7-py3-none-any.whl")),
("zzz-7.8.9", os.path.join(destination, "zzz-7.8.9-py3-none-any.whl")),
]

for _name, filename in files:
wheel_info = []
for name_and_version in name_and_versions:
filename = os.path.join(destination, f"{name_and_version}-py3-none-any.whl")
with open(filename, "w"):
pass
wheel_info.append((name_and_version, filename))
return wheel_info

return files

@pytest.fixture
def populate_wheel_cache(wheel_cache_dir: str) -> List[Tuple[str, str]]:
return _populate_wheel_cache(
wheel_cache_dir, "yyy-1.2.3", "zzz-4.5.6", "zzz-4.5.7", "zzz-7.8.9"
)


@pytest.fixture
Expand Down Expand Up @@ -332,34 +335,81 @@ def test_cache_remove_too_many_args(script: PipTestEnvironment) -> None:
script.pip("cache", "remove", "aaa", "bbb", expect_error=True)


@pytest.mark.parametrize(
"pattern",
[
"zzz",
"ZZZ",
"zZz-*-py3-none-any.whl",
"zZz-*.whl",
"zzz-*",
"zzz*",
"zz[a-zA-Z]?",
"[!y]",
],
)
@pytest.mark.usefixtures("populate_wheel_cache")
def test_cache_remove_name_match(
script: PipTestEnvironment, remove_matches_wheel: RemoveMatches
script: PipTestEnvironment, pattern: str, remove_matches_wheel: RemoveMatches
) -> None:
"""Running `pip cache remove zzz` should remove zzz-4.5.6 and zzz-7.8.9,
but nothing else."""
result = script.pip("cache", "remove", "zzz", "--verbose")
"""Running `pip cache remove <zzz matching pattern>` should remove zzz-4.5.6,
zzz-4.5.7 and zzz-7.8.9, but nothing else."""
result = script.pip("cache", "remove", pattern, "--verbose")

assert not remove_matches_wheel("yyy-1.2.3", result)
assert remove_matches_wheel("zzz-4.5.6", result)
assert remove_matches_wheel("zzz-4.5.7", result)
assert remove_matches_wheel("zzz-7.8.9", result)


@pytest.mark.parametrize(
"pattern",
[
"zzz-4.5.6",
"ZZZ-4.5.6",
"zZz-4.5.6-py3-none-any.whl",
"zZz-4.5.6-*.whl",
"zZz-4.5.[0-6]",
],
)
@pytest.mark.usefixtures("populate_wheel_cache")
def test_cache_remove_name_and_version_match(
script: PipTestEnvironment, remove_matches_wheel: RemoveMatches
script: PipTestEnvironment, pattern: str, remove_matches_wheel: RemoveMatches
) -> None:
"""Running `pip cache remove zzz-4.5.6` should remove zzz-4.5.6, but
nothing else."""
result = script.pip("cache", "remove", "zzz-4.5.6", "--verbose")
"""Running `pip cache remove <zzz-4.5.6 matching pattern>` should remove zzz-4.5.6,
but nothing else."""
result = script.pip("cache", "remove", pattern, "--verbose")

assert not remove_matches_wheel("yyy-1.2.3", result)
assert remove_matches_wheel("zzz-4.5.6", result)
assert not remove_matches_wheel("zzz-4.5.7", result)
assert not remove_matches_wheel("zzz-7.8.9", result)


@pytest.mark.parametrize(
"pattern",
[
"pyfftw",
"pyfftw-0.15",
"pyfftw-0.15.0",
"pyFFTW",
"pyFFTW-0.15",
"pyFFTW-0.15.0",
],
)
def test_issue_13086_op_case(
script: PipTestEnvironment,
wheel_cache_dir: str,
pattern: str,
remove_matches_wheel: RemoveMatches,
) -> None:
_populate_wheel_cache(wheel_cache_dir, "pyFFTW-0.15.0", "foo-1.0")
result = script.pip("cache", "remove", pattern, "--verbose")

assert remove_matches_wheel("pyFFTW-0.15.0", result)
assert not remove_matches_wheel("foo-1.0", result)


@pytest.mark.usefixtures("populate_http_cache", "populate_wheel_cache")
def test_cache_purge(
script: PipTestEnvironment,
Expand Down
Loading