Refactor -r pylock.toml, support release control and dependency cooldown#14165
Refactor -r pylock.toml, support release control and dependency cooldown#14165sbidoul wants to merge 5 commits into
Conversation
8d4d07e to
ae3b2be
Compare
Insstead of adding a locked_link and locked_version to InstallRequirement, provide a locked link per project name to the PackageFinder. Let get_all_candidate ignore indexes and --find-link when a locked link is known for a project. This naturally supports format control, so the duplicate implementation in constructors.py can be removed. This also naturally supports release control, as well as --uploaded-prior-to (via the upload_time field of pylock.toml). All in all, this is a much simpler and natural implementation.
ae3b2be to
780e67b
Compare
780e67b to
f8278c3
Compare
|
This is ready. I think it makes a lot of sense to involve the PackageFinder to provide the locked links, because a lock file can been seen both as a source of requirements, and as a sort of index page which provides the available versions and artifacts URLs. |
ichard26
left a comment
There was a problem hiding this comment.
I need to do a second pass on this, but I agree this is a much cleaner approach. Kudos!
I have two general notes:
- This seems to regress support for falling back to a sdist over a wheel when the latter is rejected by
--no-binary? I guess that's what the TODO is for. Since this needs a packaging change, it seems unlikely that this will be fixed in time for pip 26.2. Speaking as the RM, I'm fine with that since this use-case seems quite rare and pylock support is still experimental, but I do want to flag this explicitly. - It's a bit unfortunate that some of the new errors don't include which lockfile the locked link comes from. This isn't a showstopper since using multiple lockfiles is likely similarly uncommon, but it could be a source of confusion nonetheless.
I still have opinions on the overall direction and shape of our pylock installation support, but I'll save that for the issue threads (and later because I haven't thought about in a while).
|
Thanks for the review and typo-catching.
It's not a regression, rather something I noticed recently and added a TODO for it.
Same opinion here. This is probably fixable but I don't want to spend time on this right now given the unlikeliness of these errors. Regarding opinions, looking forward to reading them :) I'm aiming at a consistent UX for "lock files as just another requirements source" and I feel it fits well so far, both from a UX and architecture perspective. I believe this makes sense for the low level pip commands we have. This does not preclude the creation of higher level commands such as |
Instead of adding a
locked_linkandlocked_versionto InstallRequirement, provide a locked link per project name to the PackageFinder.Let
get_all_candidatesignore indexes and--find-linkwhen a locked link is known for a project.This naturally supports format control, so the duplicate implementation in
constructors.pycan be removed. This also naturally addresses release control, as well as--uploaded-prior-to(via theupload_timefield ofpylock.toml). Revert the change that I had made toresolvelib/factory.py, which was a bit awkward.All in all, this is a much simpler and natural implementation, I think.
TODO:
closes #13950
closes #13963
closes #14168
PR Checklist: