Replace Molinillo with PubGrub for dependency resolution#9402
Replace Molinillo with PubGrub for dependency resolution#9402mlarraz wants to merge 22 commits intoruby:masterfrom
Conversation
e8b7fb1 to
32a966d
Compare
|
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. |
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. |
|
@colby-swandale Could you open a pull request with your current WIP branch? Even if it's still in progress, having it visible will help us coordinate. Once your PR is up, we can compare both implementations and incorporate any useful differences from this PR into yours. The goal is to combine the best of both approaches into the final PubGrub integration. |
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>
- 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>
666a38a to
38b0aa6
Compare
…sions_for PubGrub requires both methods to agree on the version universe.
|
Lots of progress over the weekend. Performance is noticeably much much improved, which is encouraging. That said, there's still a fair amount of resolver behaviour to verify and work through before this is ready. Will block out some more time this week before RubyKaigi to get this ready for review. |
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