Skip to content

Commit 3d5dfa9

Browse files
Include the missing dependency's version in resolver errors & test and document Bundler-aligned resolver behaviors
1 parent 8f2cd72 commit 3d5dfa9

3 files changed

Lines changed: 85 additions & 6 deletions

File tree

lib/rubygems/resolver.rb

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,14 @@ def initialize(needed, set = nil)
127127
# Proceed with resolution! Returns an array of ActivationRequest objects.
128128

129129
def resolve
130-
# Pre-check: raise UnsatisfiableDependencyError for root deps with no matches
130+
# Pre-check: raise UnsatisfiableDependencyError for root deps with no
131+
# platform match. We filter by platform ONLY here (not required_ruby_version
132+
# / required_rubygems_version): a foreign-platform gem is genuinely "not
133+
# found", but a gem that exists yet is incompatible with the running Ruby
134+
# should flow through the solver to a DependencyResolutionError that names
135+
# the Ruby requirement. That matches Bundler (which models Ruby as a
136+
# synthetic dependency, so this surfaces as a solve failure) and gives a
137+
# clearer message than the platform-oriented UnsatisfiableDependencyError.
131138
@needed.each do |dep|
132139
next if @soft_missing
133140
dep_request = DependencyRequest.new(dep, nil)
@@ -175,6 +182,21 @@ def all_versions_for(package)
175182
name = package.to_s
176183

177184
if (skip_dep_gems = skip_gems[name]) && !skip_dep_gems.empty?
185+
# Conservative mode: float the already-installed (skip) versions to the
186+
# front so the solver prefers them. This sets *preference* only (it feeds
187+
# the strategy's version-index map); it does not restrict availability, so
188+
# every version stays selectable via versions_for. When an installed
189+
# version is made impossible by a downstream conflict, the solver
190+
# backtracks to a newer version instead of failing. Molinillo instead
191+
# hard-restricted the candidate set to skip versions and raised.
192+
#
193+
# This reaches the same outcome as Bundler (upgrade-over-raise) for the
194+
# common single-blocked-gem case, though the mechanism differs: Bundler
195+
# hard-pins locked gems and selectively unlocks + re-solves on conflict,
196+
# whereas we float as a preference and let PubGrub backtrack in one solve.
197+
# The float can therefore over-upgrade when several installed gems are
198+
# jointly involved in a conflict; that outcome-level divergence is
199+
# accepted (see test_conservative_upgrades_when_installed_blocked).
178200
skip_versions = skip_dep_gems.map(&:version)
179201
preferred, rest = versions.partition {|v| skip_versions.include?(v) }
180202
preferred + rest
@@ -270,9 +292,16 @@ def incompatibilities_for(package, version)
270292
# self_constraint lets clean sibling versions still resolve via backtracking.
271293
if @unfiltered_specs[dep_package_name].empty?
272294
cause = Gem::PubGrub::Incompatibility::InvalidDependency.new(dep_package, dep_constraint)
295+
self_term = Gem::PubGrub::Term.new(self_constraint, true)
296+
# PubGrub's default InvalidDependency rendering drops the version
297+
# requirement ("depends on unknown package bar"). Supply a custom
298+
# explanation so the missing dependency's constraint is preserved
299+
# ("depends on bar = 0.5 which could not be found in any repository"),
300+
# matching Molinillo's diagnostics.
273301
return [Gem::PubGrub::Incompatibility.new(
274-
[Gem::PubGrub::Term.new(self_constraint, true)],
275-
cause: cause
302+
[self_term],
303+
cause: cause,
304+
custom_explanation: "#{self_term.to_s(allow_every: true)} depends on #{dep_constraint} which could not be found in any repository"
276305
)]
277306
end
278307

test/rubygems/test_gem_dependency_installer.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ def test_install_domain_local
817817
inst.install "b"
818818
end
819819

820-
assert_match(/depends on unknown package a/, e.message)
820+
assert_match(/depends on a >= 0 which could not be found in any repository/, e.message)
821821
end
822822

823823
assert_equal [], inst.installed_gems.map(&:full_name)

test/rubygems/test_gem_resolver.rb

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,34 @@ def test_resolve_conservative
140140
assert_resolves_to [a2_spec, b2_spec, c1_spec, d2_spec, e1_spec], res
141141
end
142142

143+
def test_conservative_upgrades_when_installed_blocked
144+
# Conservative mode floats the installed (skip) version to the front but
145+
# keeps newer versions selectable. When the installed version cannot be
146+
# used because its own dependency is unsatisfiable, the solver backtracks
147+
# to a newer version instead of failing. This intentionally diverges from
148+
# Molinillo (which hard-restricted to skip versions and raised) and reaches
149+
# Bundler's upgrade-over-raise outcome. See the comment in
150+
# Gem::Resolver#all_versions_for.
151+
a1_spec = util_spec "a", 1 do |s|
152+
s.add_dependency "b", ">= 2"
153+
end
154+
a2_spec = util_spec "a", 2 do |s|
155+
s.add_dependency "b", ">= 1"
156+
end
157+
b1_spec = util_spec "b", 1
158+
159+
# b-2 is intentionally absent, so a-1's `b >= 2` cannot be satisfied.
160+
deps = [make_dep("a", ">= 1")]
161+
s = set a1_spec, a2_spec, b1_spec
162+
163+
res = Gem::Resolver.new deps, s
164+
# a-1 is already installed and satisfies `a >= 1`, so conservative mode
165+
# prefers it - but it is blocked by the missing b-2, forcing an upgrade.
166+
res.skip_gems = { "a" => [a1_spec] }
167+
168+
assert_resolves_to [a2_spec, b1_spec], res
169+
end
170+
143171
def test_resolve_development
144172
a_spec = util_spec "a", 1 do |s|
145173
s.add_development_dependency "b"
@@ -516,7 +544,7 @@ def test_raises_and_reports_an_implicit_request_properly
516544
r.resolve
517545
end
518546

519-
assert_match(/depends on unknown package b/, e.message)
547+
assert_match(/depends on b = 2 which could not be found in any repository/, e.message)
520548
end
521549

522550
def test_raises_when_possibles_are_exhausted
@@ -945,6 +973,28 @@ def test_error_includes_ruby_version_hint_when_filtered
945973
assert_match(/you have/, e.message)
946974
end
947975

976+
def test_root_gem_incompatible_ruby_version_names_ruby_requirement
977+
# A requested (root) gem available only for an incompatible Ruby version
978+
# flows through the solver to a DependencyResolutionError whose message
979+
# names the Ruby requirement. This matches Bundler (which models Ruby as a
980+
# synthetic dependency and reports a solve failure) and is clearer than the
981+
# platform-oriented UnsatisfiableDependencyError. Contrast the foreign-
982+
# *platform* case (test_raises_and_explains_when_platform_prevents_install),
983+
# which is genuinely "not found" and does raise UnsatisfiableDependencyError.
984+
a = util_spec "a", "1.0" do |s|
985+
s.required_ruby_version = ">= 999.0"
986+
end
987+
988+
ad = make_dep "a", "= 1.0"
989+
r = Gem::Resolver.new([ad], set(a))
990+
991+
e = assert_raise Gem::DependencyResolutionError do
992+
r.resolve
993+
end
994+
995+
assert_match(/requires Ruby >= 999.0/, e.message)
996+
end
997+
948998
def test_self_dependency_does_not_crash
949999
a = util_spec "a", "1.0" do |s|
9501000
s.add_dependency "a"
@@ -1090,7 +1140,7 @@ def test_fails_when_every_version_depends_on_missing_package
10901140
r.resolve
10911141
end
10921142

1093-
assert_match(/every version of a depends on unknown package zzz/, e.message)
1143+
assert_match(/every version of a depends on zzz >= 1 which could not be found in any repository/, e.message)
10941144
end
10951145

10961146
def test_resolves_when_only_lowest_version_has_missing_dep

0 commit comments

Comments
 (0)