Replace Molinillo with PubGrub for dependency resolution#9402
Open
mlarraz wants to merge 3 commits intoruby:masterfrom
Open
Replace Molinillo with PubGrub for dependency resolution#9402mlarraz wants to merge 3 commits intoruby:masterfrom
mlarraz wants to merge 3 commits intoruby:masterfrom
Conversation
Molinillo (a backtracking resolver) is replaced by PubGrub (a CDCL SAT-based solver) which provides better conflict explanations, smarter backjumping, and provable completeness. PubGrub is already used by Bundler; this vendors the same library re-namespaced under Gem::PubGrub. Key changes: - Vendor PubGrub from bundler/lib/bundler/vendor/pub_grub/, re-namespaced Bundler::PubGrub -> Gem::PubGrub - Rewrite Gem::Resolver to implement PubGrub's source interface (all_versions_for, versions_for, incompatibilities_for, etc.) - Add Gem::Resolver::PubGrubFailure for error reporting via PubGrub's superior conflict explanation format - Add Source::Local#find_all_gems to expose all local gem versions (PubGrub needs complete version information upfront) - Prefer installed specs in version ordering to avoid unnecessary upgrades - Delete Molinillo (21 files, ~2400 lines) and resolver/stats.rb Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Preserve the original dependency requirement from root deps when building ActivationRequests, so lockfiles correctly record constraints like "a (>= 1)" instead of bare "a". Update the orphaned dependencies test: PubGrub correctly backtracks from b-2 (missing c-2) to b-1 (has c-1), finding a valid solution that Molinillo's simpler backtracking missed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mlarraz
force-pushed
the
replace-molinillo-with-pubgrub
branch
from
e8b7fb1 to
32a966d
Compare
Member
|
I appreciate the effort here, but I should flag that I was already actively working on this. I opened the issue, I'm assigned to it, and I've had a WIP implementation going for several weeks now. That said, I don't want to be a gatekeeper, but I'm not exactly sure what the path forward here is. |
- Create pub_grub-rubygems.patch for custom Logger and Strategy changes - Remove molinillo from vendor_gems.rb and its lockfile - Remove molinillo-master.patch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
Makes sense. Not sure what your version looks like but feel free to use any ideas from mine if it helps. I would just like the change to get done. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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?
My attempt at fixing #9365.
What is your fix for the problem, implemented in this PR?
Gem::PubGrubGem::Resolverto implement PubGrub's source interfaceWe should probably to de-duplicate the copies at some point but I figured that can wait.
Gem::Resolvernow implements PubGrub's source interface (all_versions_for,versions_for,incompatibilities_for,no_versions_incompatibility_for) instead of Molinillo's interfaces.Some notes:
all_versions_forreturns highest-first, with already-installed versions preferred to avoid unnecessary upgrades. Theskip_gemsmechanism continues to work for conservative updates.= 2.a).spec_forprefersInstalledSpecificationwhen available, then selects byGem::Platform.platform_specificity_matchUnsatisfiableDependencyErrorfor root deps with zero matches. Everything else flows through PubGrub'sSolveFailure->DependencyResolutionErrorwith its full explanation chain.InstallerSet: PubGrub needs all available versions upfront, butSource::Local#find_gemonly returns the highest matching version. AddedSource::Local#find_all_gems(returns all matches) and updatedInstallerSet#find_allto use it.Make sure the following tasks are checked