Skip to content

Replace Molinillo with PubGrub for dependency resolution#9402

Draft
mlarraz wants to merge 22 commits intoruby:masterfrom
mlarraz:replace-molinillo-with-pubgrub
Draft

Replace Molinillo with PubGrub for dependency resolution#9402
mlarraz wants to merge 22 commits intoruby:masterfrom
mlarraz:replace-molinillo-with-pubgrub

Conversation

@mlarraz
Copy link
Copy Markdown
Contributor

@mlarraz mlarraz commented Mar 16, 2026

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?

  • Vendor PubGrub from Bundler's existing copy
  • Re-namespace it under Gem::PubGrub
  • Rewrite Gem::Resolver to implement PubGrub's source interface
  • Delete Molinillo

We should probably to de-duplicate the copies at some point but I figured that can wait.

Gem::Resolver now implements PubGrub's source interface (all_versions_for, versions_for, incompatibilities_for, no_versions_incompatibility_for) instead of Molinillo's interfaces.

Some notes:

  • Version preference: all_versions_for returns highest-first, with already-installed versions preferred to avoid unnecessary upgrades. The skip_gems mechanism continues to work for conservative updates.
  • Prereleases: Excluded from the preference index by default, but still available to the solver when a constraint requires them (e.g., = 2.a).
  • Platform specificity: spec_for prefers InstalledSpecification when available, then selects by
    Gem::Platform.platform_specificity_match
  • Error handling: A pre-check still raises UnsatisfiableDependencyError for root deps with zero matches. Everything else flows through PubGrub's SolveFailure -> DependencyResolutionError with its full explanation chain.
  • Compatibility fix for InstallerSet: PubGrub needs all available versions upfront, but Source::Local#find_gem only returns the highest matching version. Added Source::Local#find_all_gems (returns all matches) and updated InstallerSet#find_all to use it.

Make sure the following tasks are checked

@mlarraz mlarraz force-pushed the replace-molinillo-with-pubgrub branch from e8b7fb1 to 32a966d Compare March 16, 2026 23:30
@colby-swandale
Copy link
Copy Markdown
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.

@mlarraz
Copy link
Copy Markdown
Contributor Author

mlarraz commented Mar 17, 2026

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.

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Apr 1, 2026

@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.

mlarraz and others added 7 commits April 12, 2026 12:57
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>
@colby-swandale colby-swandale force-pushed the replace-molinillo-with-pubgrub branch from 666a38a to 38b0aa6 Compare April 12, 2026 02:57
@colby-swandale colby-swandale marked this pull request as draft April 13, 2026 02:54
@colby-swandale
Copy link
Copy Markdown
Member

colby-swandale commented Apr 13, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants