Merge pull request #5812 from rubygems/fix-yank-unlocks

Fix regression where yanked gems are now unintentionally updated when other gems are unlocked

bundler/lib/bundler/definition.rb

@@ -739,16 +739,24 @@ def converge_specs(specs)
         specs[dep].any? {|s| s.satisfies?(dep) && (!dep.source || s.source.include?(dep.source)) }
       end
 
+      @specs_that_changed_sources = []
+
       specs.each do |s|
-        # Replace the locked dependency's source with the equivalent source from the Gemfile
         dep = @dependencies.find {|d| s.satisfies?(d) }
 
-        s.source = (dep && dep.source) || sources.get(s.source) || sources.default_source
+        # Replace the locked dependency's source with the equivalent source from the Gemfile
+        s.source = if dep && dep.source
+          gemfile_source = dep.source
+          lockfile_source = s.source
+
+          @specs_that_changed_sources << s if gemfile_source != lockfile_source
 
-        next if @unlock[:sources].include?(s.source.name)
+          gemfile_source
+        else
+          sources.get_with_fallback(s.source)
+        end
 
-        # If the spec is from a path source and it doesn't exist anymore
-        # then we unlock it.
+        next if @unlock[:sources].include?(s.source.name)
 
         # Path sources have special logic
         if s.source.instance_of?(Source::Path) || s.source.instance_of?(Source::Gemspec)
@@ -824,9 +832,18 @@ def source_requirements
       end
       source_requirements[:default_bundler] = source_requirements["bundler"] || sources.default_source
       source_requirements["bundler"] = sources.metadata_source # needs to come last to override
+      verify_changed_sources!
       source_requirements
     end
 
+    def verify_changed_sources!
+      @specs_that_changed_sources.each do |s|
+        if s.source.specs.search(s.name).empty?
+          raise GemNotFound, "Could not find gem '#{s.name}' in #{s.source}"
+        end
+      end
+    end
+
     def requested_groups
       values = groups - Bundler.settings[:without] - @optional_groups + Bundler.settings[:with]
       values &= Bundler.settings[:only] unless Bundler.settings[:only].empty?

bundler/lib/bundler/lazy_specification.rb

@@ -93,14 +93,6 @@ def materialize_for_installation
       __materialize__(candidates)
     end
 
-    def materialize_for_resolution
-      return self unless Gem::Platform.match_spec?(self)
-
-      candidates = source.specs.search(self)
-
-      __materialize__(candidates)
-    end
-
     def __materialize__(candidates)
       @specification = begin
         search = candidates.reverse.find do |spec|

bundler/lib/bundler/resolver.rb

@@ -28,10 +28,11 @@ def self.resolve(requirements, source_requirements = {}, base = [], gem_version_
     def initialize(source_requirements, base, gem_version_promoter, additional_base_requirements, platforms, metadata_requirements)
       @source_requirements = source_requirements
       @metadata_requirements = metadata_requirements
+      @base = base
       @resolver = Molinillo::Resolver.new(self, self)
       @search_for = {}
       @base_dg = Molinillo::DependencyGraph.new
-      @base = base.materialized_for_resolution do |ls|
+      base.each do |ls|
         dep = Dependency.new(ls.name, ls.version)
         @base_dg.add_vertex(ls.name, DepProxy.get_proxy(dep, ls.platform), true)
       end

bundler/lib/bundler/source_list.rb

@@ -101,6 +101,10 @@ def get(source)
       source_list_for(source).find {|s| equivalent_source?(source, s) }
     end
 
+    def get_with_fallback(source)
+      get(source) || default_source
+    end
+
     def lock_sources
       lock_other_sources + lock_rubygems_sources
     end

bundler/lib/bundler/spec_set.rb

@@ -82,15 +82,6 @@ def materialized_for_all_platforms
       end
     end
 
-    def materialized_for_resolution
-      materialized = @specs.map do |s|
-        spec = s.materialize_for_resolution
-        yield spec if spec
-        spec
-      end.compact
-      SpecSet.new(materialized)
-    end
-
     def incomplete_ruby_specs?(deps)
       self.class.new(self.for(deps, true, [Gem::Platform::RUBY])).incomplete_specs.any?
     end

bundler/spec/install/yanked_spec.rb

@@ -43,6 +43,63 @@
   end
 end
 
+RSpec.context "when resolving a bundle that includes yanked gems, but unlocking an unrelated gem" do
+  before(:each) do
+    build_repo4 do
+      build_gem "foo", "10.0.0"
+
+      build_gem "bar", "1.0.0"
+      build_gem "bar", "2.0.0"
+    end
+
+    lockfile <<-L
+      GEM
+        remote: #{file_uri_for(gem_repo4)}
+        specs:
+          foo (9.0.0)
+          bar (1.0.0)
+
+      PLATFORMS
+        #{lockfile_platforms}
+
+      DEPENDENCIES
+        foo
+        bar
+
+      BUNDLED WITH
+         #{Bundler::VERSION}
+    L
+
+    gemfile <<-G
+      source "#{file_uri_for(gem_repo4)}"
+      gem "foo"
+      gem "bar"
+    G
+  end
+
+  it "does not update the yanked gem" do
+    bundle "lock --update bar"
+
+    expect(lockfile).to eq <<~L
+      GEM
+        remote: #{file_uri_for(gem_repo4)}/
+        specs:
+          bar (2.0.0)
+          foo (9.0.0)
+
+      PLATFORMS
+        #{lockfile_platforms}
+
+      DEPENDENCIES
+        bar
+        foo
+
+      BUNDLED WITH
+         #{Bundler::VERSION}
+    L
+  end
+end
+
 RSpec.context "when using gem before installing" do
   it "does not suggest the author has yanked the gem" do
     gemfile <<-G

bundler/spec/resolver/basic_spec.rb

@@ -233,7 +233,7 @@
     it "resolves foo only to latest patch - changing dependency declared case" do
       # bar is locked AND a declared dependency in the Gemfile, so it will not move, and therefore
       # foo can only move up to 1.4.4.
-      @base << Bundler::LazySpecification.new("bar", "2.0.3", nil)
+      @base << build_spec("bar", "2.0.3").first
       should_conservative_resolve_and_include :patch, ["foo"], %w[foo-1.4.4 bar-2.0.3]
     end