Merge pull request #7558 from rubygems/update-until-really-updates

Make sure `bundle update <specific_gems>` can always update to the latest resolvable version of each requested gem

(cherry picked from commit d95430f)

Author: deivid-rodriguez

bundler/lib/bundler/definition.rb

@@ -132,7 +132,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
         @sources.merged_gem_lockfile_sections!(locked_gem_sources.first)
       end
 
-      @unlock[:sources] ||= []
+      @sources_to_unlock = @unlock.delete(:sources) || []
       @unlock[:ruby] ||= if @ruby_version && locked_ruby_version_object
         @ruby_version.diff(locked_ruby_version_object)
       end
@@ -144,11 +144,13 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
       @path_changes = converge_paths
       @source_changes = converge_sources
 
+      @explicit_unlocks = @unlock.delete(:gems) || []
+
       if @unlock[:conservative]
-        @unlock[:gems] ||= @dependencies.map(&:name)
+        @gems_to_unlock = @explicit_unlocks.any? ? @explicit_unlocks : @dependencies.map(&:name)
       else
-        eager_unlock = (@unlock[:gems] || []).map {|name| Dependency.new(name, ">= 0") }
-        @unlock[:gems] = @locked_specs.for(eager_unlock, false, platforms).map(&:name).uniq
+        eager_unlock = @explicit_unlocks.map {|name| Dependency.new(name, ">= 0") }
+        @gems_to_unlock = @locked_specs.for(eager_unlock, false, platforms).map(&:name).uniq
       end
 
       @dependency_changes = converge_dependencies
@@ -227,7 +229,6 @@ def missing_specs?
       @resolver = nil
       @resolution_packages = nil
       @specs = nil
-      @gem_version_promoter = nil
 
       Bundler.ui.debug "The definition is missing dependencies, failed to resolve & materialize locally (#{e})"
       true
@@ -568,8 +569,10 @@ def resolution_packages
       @resolution_packages ||= begin
         last_resolve = converge_locked_specs
         remove_invalid_platforms!(current_dependencies)
-        packages = Resolver::Base.new(source_requirements, expanded_dependencies, last_resolve, @platforms, locked_specs: @originally_locked_specs, unlock: @unlock[:gems], prerelease: gem_version_promoter.pre?)
-        additional_base_requirements_for_resolve(packages, last_resolve)
+        packages = Resolver::Base.new(source_requirements, expanded_dependencies, last_resolve, @platforms, locked_specs: @originally_locked_specs, unlock: @gems_to_unlock, prerelease: gem_version_promoter.pre?)
+        packages = additional_base_requirements_to_prevent_downgrades(packages, last_resolve)
+        packages = additional_base_requirements_to_force_updates(packages)
+        packages
       end
     end
 
@@ -673,14 +676,18 @@ def add_current_platform
 
     def change_reason
       if unlocking?
-        unlock_reason = @unlock.reject {|_k, v| Array(v).empty? }.map do |k, v|
-          if v == true
-            k.to_s
-          else
-            v = Array(v)
-            "#{k}: (#{v.join(", ")})"
-          end
-        end.join(", ")
+        unlock_targets = if @gems_to_unlock.any?
+          ["gems", @gems_to_unlock]
+        elsif @sources_to_unlock.any?
+          ["sources", @sources_to_unlock]
+        end
+
+        unlock_reason = if unlock_targets
+          "#{unlock_targets.first}: (#{unlock_targets.last.join(", ")})"
+        else
+          @unlock[:ruby] ? "ruby" : ""
+        end
+
         return "bundler is unlocking #{unlock_reason}"
       end
       [
@@ -735,15 +742,15 @@ def converge_locals
         spec   = @dependencies.find {|s| s.name == k }
         source = spec&.source
         if source&.respond_to?(:local_override!)
-          source.unlock! if @unlock[:gems].include?(spec.name)
+          source.unlock! if @gems_to_unlock.include?(spec.name)
           locals << [source, source.local_override!(v)]
         end
       end
 
       sources_with_changes = locals.select do |source, changed|
         changed || specs_changed?(source)
       end.map(&:first)
-      !sources_with_changes.each {|source| @unlock[:sources] << source.name }.empty?
+      !sources_with_changes.each {|source| @sources_to_unlock << source.name }.empty?
     end
 
     def check_lockfile
@@ -820,7 +827,7 @@ def converge_sources
         # gem), unlock it. For git sources, this means to unlock the revision, which
         # will cause the `ref` used to be the most recent for the branch (or master) if
         # an explicit `ref` is not used.
-        if source.respond_to?(:unlock!) && @unlock[:sources].include?(source.name)
+        if source.respond_to?(:unlock!) && @sources_to_unlock.include?(source.name)
           source.unlock!
           changes = true
         end
@@ -864,7 +871,7 @@ def converge_dependencies
     def converge_locked_specs
       converged = converge_specs(@locked_specs)
 
-      resolve = SpecSet.new(converged.reject {|s| @unlock[:gems].include?(s.name) })
+      resolve = SpecSet.new(converged.reject {|s| @gems_to_unlock.include?(s.name) })
 
       diff = nil
 
@@ -897,7 +904,7 @@ def converge_specs(specs)
 
           @specs_that_changed_sources << s if gemfile_source != lockfile_source
           deps << dep if !dep.source || lockfile_source.include?(dep.source)
-          @unlock[:gems] << name if lockfile_source.include?(dep.source) && lockfile_source != gemfile_source
+          @gems_to_unlock << name if lockfile_source.include?(dep.source) && lockfile_source != gemfile_source
 
           # Replace the locked dependency's source with the equivalent source from the Gemfile
           s.source = gemfile_source
@@ -906,7 +913,7 @@ def converge_specs(specs)
           s.source = default_source unless sources.get(lockfile_source)
         end
 
-        next if @unlock[:sources].include?(s.source.name)
+        next if @sources_to_unlock.include?(s.source.name)
 
         # Path sources have special logic
         if s.source.instance_of?(Source::Path) || s.source.instance_of?(Source::Gemspec)
@@ -928,12 +935,12 @@ def converge_specs(specs)
           else
             # If the spec is no longer in the path source, unlock it. This
             # commonly happens if the version changed in the gemspec
-            @unlock[:gems] << name
+            @gems_to_unlock << name
           end
         end
 
         if dep.nil? && requested_dependencies.find {|d| name == d.name }
-          @unlock[:gems] << s.name
+          @gems_to_unlock << s.name
         else
           converged << s
         end
@@ -1010,7 +1017,7 @@ def lockfiles_equal?(current, proposed, preserve_unknown_sections)
       current == proposed
     end
 
-    def additional_base_requirements_for_resolve(resolution_packages, last_resolve)
+    def additional_base_requirements_to_prevent_downgrades(resolution_packages, last_resolve)
       return resolution_packages unless @locked_gems && !sources.expired_sources?(@locked_gems.sources)
       converge_specs(@originally_locked_specs - last_resolve).each do |locked_spec|
         next if locked_spec.source.is_a?(Source::Path)
@@ -1019,6 +1026,28 @@ def additional_base_requirements_for_resolve(resolution_packages, last_resolve)
       resolution_packages
     end
 
+    def additional_base_requirements_to_force_updates(resolution_packages)
+      return resolution_packages if @explicit_unlocks.empty?
+      full_update = dup_for_full_unlock.resolve
+      @explicit_unlocks.each do |name|
+        version = full_update[name].first&.version
+        resolution_packages.base_requirements[name] = Gem::Requirement.new("= #{version}") if version
+      end
+      resolution_packages
+    end
+
+    def dup_for_full_unlock
+      unlocked_definition = self.class.new(@lockfile, @dependencies, @sources, true, @ruby_version, @optional_groups, @gemfiles)
+      unlocked_definition.resolution_mode = { "local" => !@remote }
+      unlocked_definition.setup_sources_for_resolve
+      unlocked_definition.gem_version_promoter.tap do |gvp|
+        gvp.level = gem_version_promoter.level
+        gvp.strict = gem_version_promoter.strict
+        gvp.pre = gem_version_promoter.pre
+      end
+      unlocked_definition
+    end
+
     def remove_invalid_platforms!(dependencies)
       return if Bundler.frozen_bundle?
 

bundler/spec/commands/lock_spec.rb

@@ -252,6 +252,128 @@
     expect(read_lockfile).to eq(remove_checksums_from_lockfile(@lockfile, "(2.3.2)", "(#{rake_version})"))
   end
 
+  it "updates specific gems using --update, even if that requires unlocking other top level gems" do
+    build_repo4 do
+      build_gem "prism", "0.15.1"
+      build_gem "prism", "0.24.0"
+
+      build_gem "ruby-lsp", "0.12.0" do |s|
+        s.add_dependency "prism", "< 0.24.0"
+      end
+
+      build_gem "ruby-lsp", "0.16.1" do |s|
+        s.add_dependency "prism", ">= 0.24.0"
+      end
+
+      build_gem "tapioca", "0.11.10" do |s|
+        s.add_dependency "prism", "< 0.24.0"
+      end
+
+      build_gem "tapioca", "0.13.1" do |s|
+        s.add_dependency "prism", ">= 0.24.0"
+      end
+    end
+
+    gemfile <<~G
+      source "#{file_uri_for(gem_repo4)}"
+
+      gem "tapioca"
+      gem "ruby-lsp"
+    G
+
+    lockfile <<~L
+      GEM
+        remote: #{file_uri_for(gem_repo4)}
+        specs:
+          prism (0.15.1)
+          ruby-lsp (0.12.0)
+            prism (< 0.24.0)
+          tapioca (0.11.10)
+            prism (< 0.24.0)
+
+      PLATFORMS
+        #{lockfile_platforms}
+
+      DEPENDENCIES
+        ruby-lsp
+        tapioca
+
+      BUNDLED WITH
+         #{Bundler::VERSION}
+    L
+
+    bundle "lock --update tapioca --verbose"
+
+    expect(lockfile).to include("tapioca (0.13.1)")
+  end
+
+  it "updates specific gems using --update, even if that requires unlocking other top level gems, but only as few as possible" do
+    build_repo4 do
+      build_gem "prism", "0.15.1"
+      build_gem "prism", "0.24.0"
+
+      build_gem "ruby-lsp", "0.12.0" do |s|
+        s.add_dependency "prism", "< 0.24.0"
+      end
+
+      build_gem "ruby-lsp", "0.16.1" do |s|
+        s.add_dependency "prism", ">= 0.24.0"
+      end
+
+      build_gem "tapioca", "0.11.10" do |s|
+        s.add_dependency "prism", "< 0.24.0"
+      end
+
+      build_gem "tapioca", "0.13.1" do |s|
+        s.add_dependency "prism", ">= 0.24.0"
+      end
+
+      build_gem "other-prism-dependent", "1.0.0" do |s|
+        s.add_dependency "prism", ">= 0.15.1"
+      end
+
+      build_gem "other-prism-dependent", "1.1.0" do |s|
+        s.add_dependency "prism", ">= 0.15.1"
+      end
+    end
+
+    gemfile <<~G
+      source "#{file_uri_for(gem_repo4)}"
+
+      gem "tapioca"
+      gem "ruby-lsp"
+      gem "other-prism-dependent"
+    G
+
+    lockfile <<~L
+      GEM
+        remote: #{file_uri_for(gem_repo4)}
+        specs:
+          other-prism-dependent (1.0.0)
+            prism (>= 0.15.1)
+          prism (0.15.1)
+          ruby-lsp (0.12.0)
+            prism (< 0.24.0)
+          tapioca (0.11.10)
+            prism (< 0.24.0)
+
+      PLATFORMS
+        #{lockfile_platforms}
+
+      DEPENDENCIES
+        ruby-lsp
+        tapioca
+
+      BUNDLED WITH
+         #{Bundler::VERSION}
+    L
+
+    bundle "lock --update tapioca"
+
+    expect(lockfile).to include("tapioca (0.13.1)")
+    expect(lockfile).to include("other-prism-dependent (1.0.0)")
+  end
+
   it "preserves unknown checksum algorithms" do
     lockfile @lockfile.gsub(/(sha256=[a-f0-9]+)$/, "constant=true,\\1,xyz=123")