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

pip cache remove should also work with a package name #13086

Open
paugier opened this issue Nov 19, 2024 · 6 comments · May be fixed by #13094
Open

pip cache remove should also work with a package name #13086

paugier opened this issue Nov 19, 2024 · 6 comments · May be fixed by #13094
Labels
C: cache Dealing with cache and files in it state: awaiting PR Feature discussed, PR is needed type: enhancement Improvements to functionality

Comments

@paugier
Copy link

paugier commented Nov 19, 2024

pip cache remove uses a pattern for the wheels filename and not the package name (as noted in #12954 (comment)):

pip cache remove pyfftw
WARNING: No matching packages for pattern "pyfftw"

does not work because the wheel name is pyFFTW-0.15.0-cp313-cp313-linux_x86_64.whl (whereas the package name is pyfftw).

One has to use

pip cache remove pyFFTW

It seems to me that pip cache remove pyfftw should remove the wheels for the package pyfftw.

Originally posted by @paugier in #12954 (comment)

@sbidoul sbidoul added type: enhancement Improvements to functionality C: cache Dealing with cache and files in it state: awaiting PR Feature discussed, PR is needed labels Nov 19, 2024
@jsirois
Copy link
Contributor

jsirois commented Nov 19, 2024

Pex would be happy about this too. It currently goes through both PEP-503 project name normalization and PEP-440 version normalization matching against results from pip cache list to ensure Pip caches for certain projects are cleared:
https://github.com/pex-tool/pex/blob/3361e7976d0c3a5d6348b5ebcd9da5b332363035/pex/cli/commands/cache/command.py#L621-L661

@jsirois
Copy link
Contributor

jsirois commented Nov 27, 2024

I'm starting to look at this. I'll note the existing implementation seems to come with a bug:

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:
# {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.
#
# 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)

Since glob expressions allow [a-z]-style wildcards, the comment assertion about a pattern with a - in it is false. So if I provide the pattern foo[0-9] to just delete wheels named foo0, foo1, ... foo9, I'll also delete foo1bob and that seems to go against what the author intended. @duckinator do I have this right?

Worked example:

:; python3.8
Python 3.8.20 (default, Sep  7 2024, 12:06:35)
[GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import fnmatch
>>> fnmatch.fnmatch("foo0bob-1.0-py3-none-any.whl", "foo[0-9]*.whl")
True
>>>

Notes for follow up:

@duckinator
Copy link
Contributor

That's definitely not intended. Good catch.

@jsirois
Copy link
Contributor

jsirois commented Nov 27, 2024

Thanks for confirming @duckinator. There are a few more warts on the glob expression side of "<pattern> can be a glob expression or a package name.", for example:

:; pip cache list wheel
Cache contents:

 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.44.0-py3-none-any.whl (67 kB)
 - wheel-0.45.0-py3-none-any.whl (72 kB)
 - wheel-0.45.0-py3-none-any.whl (72 kB)
 - wheel-0.45.0-py3-none-any.whl (72 kB)
 - wheel-0.45.1-py3-none-any.whl (72 kB)
 - wheel-0.45.1-py3-none-any.whl (72 kB)

:; pip cache list wheel-*.whl
No locally built wheels cached.

:; pip cache list wheel-0.45.1-py3-none-any.whl
No locally built wheels cached.

I'll see if I can't fix those up as well in the course of working this issue.

jsirois added a commit to jsirois/pip that referenced this issue Nov 27, 2024
Previously, glob patterns were not properly accounted for, which could
lead to `pip cache remove wheel-0.45.1-py3-none-any.whl` failing, for
example, when exactly that file was shown to exist in the cache via
`pip cache list`.

Additionally, non-glob patterns previously only literally matched wheel
project names; now they match the normalized name. For example,
`pip cache remove pyfftw` will now remove a cached
`pyFFTW-0.15.0-cp313-cp313-linux_x86_64.whl` wheel since the `pyfftw`
pattern matches the normalized project name.

Fixes pypa#13086
@jsirois jsirois linked a pull request Nov 27, 2024 that will close this issue
@jsirois
Copy link
Contributor

jsirois commented Nov 27, 2024

@duckinator if you have the bandwidth, I'd definitely value your review on #13094.

jsirois added a commit to jsirois/pip that referenced this issue Nov 27, 2024
@duckinator
Copy link
Contributor

@jsirois that PR definitely makes it a lot more in-line with my original intentions. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: cache Dealing with cache and files in it state: awaiting PR Feature discussed, PR is needed type: enhancement Improvements to functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants