Fix dependency locking when Bundler finds incorrect lockfile dependencies #8489
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-conservativeness-when-lockfile-has-incorrect-deps
branch
2 times, most recently
from
582f8b5 to
93c7c96
Compare
4 tasks
Resolver had internal logic to prioritize locked versions when sorting versions, however part of it was not being actually hit because of how unlocking worked in the resolver: a package was allow to be unlocked when that was explicit requested or when the list of unlocks was empty. That did not make a lot of sense and other cases were working because the explicit list of unlocks was getting "artificially filled". Now we consider a package unlocked when explicitly requested (`bundle update <package>`), or when everything is being unlocked (`bundle install` with no lockfile or `bundle update`). This makes things simpler and gets the edge case added as a test case working as expected.
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-conservativeness-when-lockfile-has-incorrect-deps
branch
from
93c7c96 to
b8e5508
Compare
deivid-rodriguez
deleted the
deivid-rodriguez/fix-conservativeness-when-lockfile-has-incorrect-deps
branch
deivid-rodriguez
added a commit
that referenced
this pull request
Feb 17, 2025
…iveness-when-lockfile-has-incorrect-deps Fix dependency locking when Bundler finds incorrect lockfile dependencies (cherry picked from commit 9efc31a)
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was the end-user or developer problem that led to this PR?
Recently ruby 3.4.1 shipped with an incorrect gemspec file for net-smtp 0.5.0, where it would be missing its net-protocol dependency.
If Bundler is run in local mode in this situation, it will include a entry for net-smtp missing that dependency in the lockfile, since that's the only information about that net-smtp version it has available.
Later, if Bundler is run in remote mode with that lockfile, it will detect the incorrect dependencies and try to correct the lockfile.
However, in order to do that, it will discard the incorrect local specification and completely unlock the dependency, resolving to net-smtp 0.5.1 instead. Bundler should correct the lockfile, but resolve using the same version that's already there.
What is your fix for the problem, implemented in this PR?
Bundler has logic to prioritize locked versions when considering versions for resolving. However, turns out the behavior was disabled by default. See:
https://github.com/rubygems/rubygems/blob/126cfe2f861ead7185507c5eef3adce29755ad56/bundler/lib/bundler/gem_version_promoter.rb#L134-L142
It's suprising that lockfile versions were still generally respected even with this behavior disabled, but that's because locked versions are still put in the front here:
https://github.com/rubygems/rubygems/blob/126cfe2f861ead7185507c5eef3adce29755ad56/bundler/lib/bundler/resolver.rb#L257
The obvious fix is to enable the disabled behavior. However, the unlocking behavior expected by this code did not match what the code was actually doing. Up until now, a package was considered unlocked if explicitly passed in the unlock array, or if the unlocked array was empty. That did not play nice with this situation, where the package was considered unlocked and thus locked version not put at the front.
So I made sure to refactor things so that package are considered locked by default, unless there's an explicit request to unlock all packages, or the package is explicitly requested to be unlocked.
This PR is built on top of #8486, so that should be merged first.
Fixes #8476.
Make sure the following tasks are checked