Skip to content

Commit a657aff

Browse files
Merge pull request #8491 from rubygems/deivid-rodriguez/fix-unintended-downgrades
Fix regression when running `bundle update <foo>` would sometimes downgrade a top level dependency (cherry picked from commit 8fe5bae)
1 parent abbea0c commit a657aff

File tree

3 files changed

+101
-7
lines changed

3 files changed

+101
-7
lines changed

bundler/lib/bundler/definition.rb

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,7 @@ def resolution_packages
626626
last_resolve = converge_locked_specs
627627
remove_invalid_platforms!
628628
packages = Resolver::Base.new(source_requirements, expanded_dependencies, last_resolve, @platforms, locked_specs: @originally_locked_specs, unlock: @unlocking_all || @gems_to_unlock, prerelease: gem_version_promoter.pre?, prefer_local: @prefer_local, new_platforms: @new_platforms)
629-
packages = additional_base_requirements_to_prevent_downgrades(packages, last_resolve)
629+
packages = additional_base_requirements_to_prevent_downgrades(packages)
630630
packages = additional_base_requirements_to_force_updates(packages)
631631
packages
632632
end
@@ -938,7 +938,7 @@ def converge_sources
938938

939939
def converge_dependencies
940940
@missing_lockfile_dep = nil
941-
changes = false
941+
@changed_dependencies = []
942942

943943
current_dependencies.each do |dep|
944944
if dep.source
@@ -960,10 +960,10 @@ def converge_dependencies
960960
end
961961
end
962962

963-
changes ||= dep_changed
963+
@changed_dependencies << name if dep_changed
964964
end
965965

966-
changes
966+
@changed_dependencies.any?
967967
end
968968

969969
# Remove elements from the locked specs that are expired. This will most
@@ -1093,11 +1093,15 @@ def lockfiles_equal?(current, proposed, preserve_unknown_sections)
10931093
current == proposed
10941094
end
10951095

1096-
def additional_base_requirements_to_prevent_downgrades(resolution_packages, last_resolve)
1096+
def additional_base_requirements_to_prevent_downgrades(resolution_packages)
10971097
return resolution_packages unless @locked_gems && !sources.expired_sources?(@locked_gems.sources)
1098-
converge_specs(@originally_locked_specs - last_resolve).each do |locked_spec|
1098+
@originally_locked_specs.each do |locked_spec|
10991099
next if locked_spec.source.is_a?(Source::Path)
1100-
resolution_packages.base_requirements[locked_spec.name] = Gem::Requirement.new(">= #{locked_spec.version}")
1100+
1101+
name = locked_spec.name
1102+
next if @changed_dependencies.include?(name)
1103+
1104+
resolution_packages.base_requirements[name] = Gem::Requirement.new(">= #{locked_spec.version}")
11011105
end
11021106
resolution_packages
11031107
end

bundler/spec/commands/update_spec.rb

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,72 @@
428428
expect(out).to include("Installing sneakers 2.11.0").and include("Installing rake 13.0.6")
429429
end
430430

431+
it "does not downgrade direct dependencies unnecessarily" do
432+
build_repo4 do
433+
build_gem "redis", "4.8.1"
434+
build_gem "redis", "5.3.0"
435+
436+
build_gem "sidekiq", "6.5.5" do |s|
437+
s.add_dependency "redis", ">= 4.5.0"
438+
end
439+
440+
build_gem "sidekiq", "6.5.12" do |s|
441+
s.add_dependency "redis", ">= 4.5.0", "< 5"
442+
end
443+
444+
# one version of sidekiq above Gemfile's range is needed to make the
445+
# resolver choose `redis` first and trying to upgrade it, reproducing
446+
# the accidental sidekiq downgrade as a result
447+
build_gem "sidekiq", "7.0.0 " do |s|
448+
s.add_dependency "redis", ">= 4.2.0"
449+
end
450+
451+
build_gem "sentry-sidekiq", "5.22.0" do |s|
452+
s.add_dependency "sidekiq", ">= 3.0"
453+
end
454+
455+
build_gem "sentry-sidekiq", "5.22.4" do |s|
456+
s.add_dependency "sidekiq", ">= 3.0"
457+
end
458+
end
459+
460+
gemfile <<~G
461+
source "https://gem.repo4"
462+
463+
gem "redis"
464+
gem "sidekiq", "~> 6.5"
465+
gem "sentry-sidekiq"
466+
G
467+
468+
original_lockfile = <<~L
469+
GEM
470+
remote: https://gem.repo4/
471+
specs:
472+
redis (4.8.1)
473+
sentry-sidekiq (5.22.0)
474+
sidekiq (>= 3.0)
475+
sidekiq (6.5.12)
476+
redis (>= 4.5.0, < 5)
477+
478+
PLATFORMS
479+
#{lockfile_platforms}
480+
481+
DEPENDENCIES
482+
redis
483+
sentry-sidekiq
484+
sidekiq (~> 6.5)
485+
486+
BUNDLED WITH
487+
#{Bundler::VERSION}
488+
L
489+
490+
lockfile original_lockfile
491+
492+
bundle "lock --update sentry-sidekiq"
493+
494+
expect(lockfile).to eq(original_lockfile.sub("sentry-sidekiq (5.22.0)", "sentry-sidekiq (5.22.4)"))
495+
end
496+
431497
it "does not downgrade indirect dependencies unnecessarily" do
432498
build_repo4 do
433499
build_gem "a" do |s|

bundler/spec/install/gems/flex_spec.rb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,30 @@
295295

296296
it "should work when you update" do
297297
bundle "update myrack"
298+
299+
checksums = checksums_section_when_enabled do |c|
300+
c.checksum gem_repo1, "myrack", "0.9.1"
301+
c.checksum gem_repo1, "myrack-obama", "1.0"
302+
end
303+
304+
expect(lockfile).to eq <<~L
305+
GEM
306+
remote: https://gem.repo1/
307+
specs:
308+
myrack (0.9.1)
309+
myrack-obama (1.0)
310+
myrack
311+
312+
PLATFORMS
313+
#{lockfile_platforms}
314+
315+
DEPENDENCIES
316+
myrack (= 0.9.1)
317+
myrack-obama
318+
#{checksums}
319+
BUNDLED WITH
320+
#{Bundler::VERSION}
321+
L
298322
end
299323
end
300324

0 commit comments

Comments
 (0)