From 4790d0accdb745f9d8e605fd42eab712e4ebf834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Thu, 18 Aug 2022 23:28:26 +0200 Subject: [PATCH] [rubygems/rubygems] Fix conservative update downgrading top level gems When `--conservative` is passed, explicit unlocks are set for top level gems via `@unlock[:gems]`, so that only those particular gems are allowed to be updated. When we compute the "base resolve" from the lockfile (the set of gems whose versions should be kept pinned by the resolver), we always exclude gems explicitly unlocked through `@unlock[:gems]` from it. This is done by the `converge_specs` method. However, the `converge_specs` method is also used for figuring out additional lower bound requirements from the lockfile. But in this case, even if gems are explicitly unlock in `@unlock[:gems]`, we still want to add the additional requirement, so that gems are not downgraded by the resolver. So the solution is to move the line filtering out gems in `@unlock[:gems]` from the `converged_specs` method out of that method, so that it only applies for computing the "base resolve", but not the addtional lower bound requirements. https://github.com/rubygems/rubygems/commit/405119bd7b --- lib/bundler/definition.rb | 6 ++- spec/bundler/commands/update_spec.rb | 60 ++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb index 0ab0451695..7d28375bb5 100644 --- a/lib/bundler/definition.rb +++ b/lib/bundler/definition.rb @@ -715,7 +715,9 @@ module Bundler # commonly happen if the Gemfile has changed since the lockfile was last # generated def converge_locked_specs - resolve = converge_specs(@locked_specs) + converged = converge_specs(@locked_specs) + + resolve = SpecSet.new(converged.reject {|s| @unlock[:gems].include?(s.name) }) diff = nil @@ -788,7 +790,7 @@ module Bundler end end - SpecSet.new(filter_specs(converged, deps).reject {|s| @unlock[:gems].include?(s.name) }) + filter_specs(converged, deps) end def metadata_dependencies diff --git a/spec/bundler/commands/update_spec.rb b/spec/bundler/commands/update_spec.rb index 8ca537ac10..11ff49bf89 100644 --- a/spec/bundler/commands/update_spec.rb +++ b/spec/bundler/commands/update_spec.rb @@ -301,6 +301,66 @@ RSpec.describe "bundle update" do expect(lockfile).to eq(previous_lockfile) end + it "does not downgrade direct dependencies when run with --conservative" do + build_repo4 do + build_gem "oauth2", "2.0.6" do |s| + s.add_dependency "faraday", ">= 0.17.3", "< 3.0" + end + + build_gem "oauth2", "1.4.10" do |s| + s.add_dependency "faraday", ">= 0.17.3", "< 3.0" + s.add_dependency "multi_json", "~> 1.3" + end + + build_gem "faraday", "2.5.2" + + build_gem "multi_json", "1.15.0" + + build_gem "quickbooks-ruby", "1.0.19" do |s| + s.add_dependency "oauth2", "~> 1.4" + end + + build_gem "quickbooks-ruby", "0.1.9" do |s| + s.add_dependency "oauth2" + end + end + + gemfile <<-G + source "#{file_uri_for(gem_repo4)}" + + gem "oauth2" + gem "quickbooks-ruby" + G + + lockfile <<~L + GEM + remote: #{file_uri_for(gem_repo4)}/ + specs: + faraday (2.5.2) + multi_json (1.15.0) + oauth2 (1.4.10) + faraday (>= 0.17.3, < 3.0) + multi_json (~> 1.3) + quickbooks-ruby (1.0.19) + oauth2 (~> 1.4) + + PLATFORMS + #{lockfile_platforms} + + DEPENDENCIES + oauth2 + quickbooks-ruby + + BUNDLED WITH + #{Bundler::VERSION} + L + + bundle "update --conservative --verbose" + + expect(out).not_to include("Installing quickbooks-ruby 0.1.9") + expect(out).to include("Installing quickbooks-ruby 1.0.19").and include("Installing oauth2 1.4.10") + end + it "does not downgrade indirect dependencies unnecessarily" do build_repo4 do build_gem "a" do |s|