Merge pull request #7915 from rubygems/deivid-rodriguez/fix-incorrect-source-on-gem-unlock

Fix `bundle update <indirect_dep>` failing to upgrade when versions present in two different sources

Author: deivid-rodriguez

bundler/lib/bundler/definition.rb

@@ -214,6 +214,7 @@ def missing_specs?
       @resolve = nil
       @resolver = nil
       @resolution_packages = nil
+      @source_requirements = nil
       @specs = nil
 
       Bundler.ui.debug "The definition is missing dependencies, failed to resolve & materialize locally (#{e})"
@@ -476,9 +477,6 @@ def most_specific_locked_platform
       end
     end
 
-    attr_reader :sources
-    private :sources
-
     def nothing_changed?
       return false unless lockfile_exists?
 
@@ -502,8 +500,12 @@ def unlocking?
       @unlocking
     end
 
+    attr_writer :source_requirements
+
     private
 
+    attr_reader :sources
+
     def should_add_extra_platforms?
       !lockfile_exists? && generic_local_platform_is_ruby? && !Bundler.settings[:force_ruby_platform]
     end
@@ -972,6 +974,10 @@ def metadata_dependencies
     end
 
     def source_requirements
+      @source_requirements ||= find_source_requirements
+    end
+
+    def find_source_requirements
       # Record the specs available in each gem's source, so that those
       # specs will be available later when the resolver knows where to
       # look for that gemspec (or its dependencies)
@@ -1053,6 +1059,7 @@ def additional_base_requirements_to_force_updates(resolution_packages)
 
     def dup_for_full_unlock
       unlocked_definition = self.class.new(@lockfile, @dependencies, @sources, true, @ruby_version, @optional_groups, @gemfiles)
+      unlocked_definition.source_requirements = source_requirements
       unlocked_definition.gem_version_promoter.tap do |gvp|
         gvp.level = gem_version_promoter.level
         gvp.strict = gem_version_promoter.strict

bundler/spec/commands/outdated_spec.rb

@@ -162,7 +162,7 @@
         build_gem "vcr", "6.0.0"
       end
 
-      build_repo gem_repo3 do
+      build_repo3 do
         build_gem "pkg-gem-flowbyte-with-dep", "1.0.0" do |s|
           s.add_dependency "oj"
         end

bundler/spec/commands/remove_spec.rb

@@ -409,7 +409,7 @@
 
   context "with sources" do
     before do
-      build_repo gem_repo3 do
+      build_repo3 do
         build_gem "rspec"
       end
     end

bundler/spec/commands/update_spec.rb

@@ -956,7 +956,7 @@
         build_gem "vcr", "6.0.0"
       end
 
-      build_repo gem_repo3 do
+      build_repo3 do
         build_gem "pkg-gem-flowbyte-with-dep", "1.0.0" do |s|
           s.add_dependency "oj"
         end

bundler/spec/install/gemfile/sources_spec.rb

@@ -8,7 +8,7 @@
     before do
       # Oh no! Someone evil is trying to hijack myrack :(
       # need this to be broken to check for correct source ordering
-      build_repo gem_repo3 do
+      build_repo3 do
         build_gem "myrack", repo3_myrack_version do |s|
           s.write "lib/myrack.rb", "MYRACK = 'FAIL'"
         end
@@ -156,7 +156,7 @@
       before do
         # Oh no! Someone evil is trying to hijack myrack :(
         # need this to be broken to check for correct source ordering
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "myrack", "1.0.0" do |s|
             s.write "lib/myrack.rb", "MYRACK = 'FAIL'"
           end
@@ -200,7 +200,7 @@
       before do
         # Oh no! Someone evil is trying to hijack myrack :(
         # need this to be broken to check for correct source ordering
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "myrack", "1.0.0" do |s|
             s.write "lib/myrack.rb", "MYRACK = 'FAIL'"
           end
@@ -225,7 +225,7 @@
 
     context "when a pinned gem has an indirect dependency in the pinned source" do
       before do
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "depends_on_myrack", "1.0.1" do |s|
             s.add_dependency "myrack"
           end
@@ -287,7 +287,7 @@
       before do
         # In these tests, we need a working myrack gem in repo2 and not repo3
 
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "depends_on_myrack", "1.0.1" do |s|
             s.add_dependency "myrack"
           end
@@ -502,7 +502,7 @@
       before do
         build_repo2
 
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "private_gem_1", "1.0.0"
           build_gem "private_gem_2", "1.0.0"
         end
@@ -528,7 +528,7 @@
       before do
         build_repo2
 
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "depends_on_missing", "1.0.1" do |s|
             s.add_dependency "missing"
           end
@@ -565,7 +565,7 @@
           end
         end
 
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "unrelated_gem", "1.0.0"
         end
 
@@ -645,7 +645,7 @@
 
     context "when a scoped gem has a deeply nested indirect dependency" do
       before do
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "depends_on_depends_on_myrack", "1.0.1" do |s|
             s.add_dependency "depends_on_myrack"
           end
@@ -764,7 +764,7 @@
           build_gem "zeitwerk", "2.4.2"
         end
 
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "sidekiq-pro", "5.2.1" do |s|
             s.add_dependency "connection_pool", ">= 2.2.3"
             s.add_dependency "sidekiq", ">= 6.1.0"
@@ -1080,7 +1080,7 @@
 
     context "when a pinned gem has an indirect dependency with more than one level of indirection in the default source " do
       before do
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "handsoap", "0.2.5.5" do |s|
             s.add_dependency "nokogiri", ">= 1.2.3"
           end
@@ -1157,7 +1157,7 @@
 
     context "with a gem that is only found in the wrong source" do
       before do
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "not_in_repo1", "1.0.0"
         end
 
@@ -1250,7 +1250,7 @@
       end
 
       before do
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "myrack", "0.9.1"
         end
 
@@ -1393,7 +1393,7 @@
   context "re-resolving" do
     context "when there is a mix of sources in the gemfile" do
       before do
-        build_repo gem_repo3 do
+        build_repo3 do
           build_gem "myrack"
         end
 
@@ -1921,4 +1921,70 @@
       expect(err).to include("Could not find gem 'example' in rubygems repository https://gem.repo4/")
     end
   end
+
+  context "when a gem has versions in two sources, but only the locked one has updates" do
+    let(:original_lockfile) do
+      <<~L
+        GEM
+          remote: https://main.source/
+          specs:
+            activesupport (1.0)
+              bigdecimal
+            bigdecimal (1.0.0)
+
+        GEM
+          remote: https://main.source/extra/
+          specs:
+            foo (1.0)
+              bigdecimal
+
+        PLATFORMS
+          #{lockfile_platforms}
+
+        DEPENDENCIES
+          activesupport
+          foo!
+
+        BUNDLED WITH
+           #{Bundler::VERSION}
+      L
+    end
+
+    before do
+      build_repo3 do
+        build_gem "activesupport" do |s|
+          s.add_dependency "bigdecimal"
+        end
+
+        build_gem "bigdecimal", "1.0.0"
+        build_gem "bigdecimal", "3.3.1"
+      end
+
+      build_repo4 do
+        build_gem "foo" do |s|
+          s.add_dependency "bigdecimal"
+        end
+
+        build_gem "bigdecimal", "1.0.0"
+      end
+
+      gemfile <<~G
+        source "https://main.source"
+
+        gem "activesupport"
+
+        source "https://main.source/extra" do
+          gem "foo"
+        end
+      G
+
+      lockfile original_lockfile
+    end
+
+    it "properly upgrades the lockfile when updating that specific gem" do
+      bundle "update bigdecimal --conservative", artifice: "compact_index_extra_api", env: { "BUNDLER_SPEC_GEM_REPO" => gem_repo3.to_s }
+
+      expect(lockfile).to eq original_lockfile.gsub("bigdecimal (1.0.0)", "bigdecimal (3.3.1)")
+    end
+  end
 end

bundler/spec/install/prereleases_spec.rb

@@ -38,7 +38,7 @@
 
   describe "when prerelease gems are not available" do
     it "still works" do
-      build_repo gem_repo3 do
+      build_repo3 do
         build_gem "myrack"
       end
       FileUtils.rm_rf Dir[gem_repo3("prerelease*")]

bundler/spec/other/major_deprecation_spec.rb

@@ -454,7 +454,7 @@
 
   context "bundle install in frozen mode with a lockfile with a single rubygems section with multiple remotes" do
     before do
-      build_repo gem_repo3 do
+      build_repo3 do
         build_gem "myrack", "0.9.1"
       end
 

bundler/spec/support/builders.rb

@@ -188,9 +188,15 @@ def build_repo2(**kwargs, &blk)
 
     # A repo that has no pre-installed gems included. (The caller completely
     # determines the contents with the block.)
+    def build_repo3(**kwargs, &blk)
+      build_empty_repo gem_repo3, **kwargs, &blk
+    end
+
+    # Like build_repo3, this is a repo that has no pre-installed gems included.
+    # We have two different methods for situations where two different empty
+    # sources are needed.
     def build_repo4(**kwargs, &blk)
-      FileUtils.rm_rf gem_repo4
-      build_repo(gem_repo4, **kwargs, &blk)
+      build_empty_repo gem_repo4, **kwargs, &blk
     end
 
     def update_repo4(&blk)
@@ -307,6 +313,11 @@ def build_plugin(name, *args, &blk)
 
     private
 
+    def build_empty_repo(gem_repo, **kwargs, &blk)
+      FileUtils.rm_rf gem_repo
+      build_repo(gem_repo, **kwargs, &blk)
+    end
+
     def build_with(builder, name, args, &blk)
       @_build_path ||= nil
       @_build_repo ||= nil