-
-
Notifications
You must be signed in to change notification settings - Fork 624
Fix crash with pip==25.1
, which changed caching for find_all_candidates
#2178
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just been reading through PackagerFinder in pip and I think this fix will work cleanly. I've hit the CI run button and let's see if it works cleanly -- obviously you don't need to add any new tests, since this will hopefully fix already-broken tests! 😄
I'm going to look into what's going on with all of these failing builds as soon as I have time. I'm pretty sure they have nothing to do with this PR, and are instead caused by some non-reproducible elements in the build envs. We'll potentially merge this even with CI red; like I said, I'm pretty sure it's right and we can back it out if it turns out to be an issue after we unwind the other CI woes. |
I think we need to remove the keyword option cli_runner = CliRunner(mix_stderr=False) AFAIU that option has done nothing for a while and is no longer part of Click. EDIT: From Click's 8.2.0 changelog:
|
Yes, we will need to handle click 8.2+ changes. Since Handling will depend on how exactly things go as I wade through the space of unconstrained packages in the tests. Footnotes
|
a94047f
to
4c46b71
Compare
for more information, see https://pre-commit.ci
In
pip==25.1
, the implementation of caching forfind_all_candidates
changed, and it no longer exposes any method in its public interface to clear its cache. This causespip-compile --generate-hashes
to crash with anAttributeError
.In this PR, I replace the cache clearing with simply constructing a new object. I'm not sufficiently familiar with pip/pip-tools internals to know if that's a bad idea (it's unclear to me how much state lies in the finder object, beyond the cache), but I felt it was potentially cleaner than the alternative approach (catch the
AttributeError
and dofinder._all_candidates.clear()
) suggested in #2176.Fixes #2176
Contributor checklist
Maintainer checklist
backwards incompatible
,feature
,enhancement
,deprecation
,bug
,dependency
,docs
orskip-changelog
as they determine changelog listing.