Merge pull request #6495 from rubygems/dont-remove-ruby-platform-when-healing

Fix incorrect removal of ruby platform when auto-healing corrupted lockfiles

(cherry picked from commit 9eeb6a6)

Author: deivid-rodriguez

Manifest.txt

@@ -80,6 +80,7 @@ bundler/lib/bundler/gem_helpers.rb
 bundler/lib/bundler/gem_tasks.rb
 bundler/lib/bundler/gem_version_promoter.rb
 bundler/lib/bundler/graph.rb
+bundler/lib/bundler/incomplete_specification.rb
 bundler/lib/bundler/index.rb
 bundler/lib/bundler/injector.rb
 bundler/lib/bundler/inline.rb

bundler/lib/bundler.rb

@@ -62,6 +62,7 @@ module Bundler
   autoload :GemHelpers,             File.expand_path("bundler/gem_helpers", __dir__)
   autoload :GemVersionPromoter,     File.expand_path("bundler/gem_version_promoter", __dir__)
   autoload :Graph,                  File.expand_path("bundler/graph", __dir__)
+  autoload :IncompleteSpecification, File.expand_path("bundler/incomplete_specification", __dir__)
   autoload :Index,                  File.expand_path("bundler/index", __dir__)
   autoload :Injector,               File.expand_path("bundler/injector", __dir__)
   autoload :Installer,              File.expand_path("bundler/installer", __dir__)

bundler/lib/bundler/incomplete_specification.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module Bundler
+  #
+  # Represents a package name that was found to be incomplete when trying to
+  # materialize a fresh resolution or the lockfile.
+  #
+  # Holds the actual partially complete set of specifications for the name.
+  # These are used so that they can be unlocked in a future resolution, and fix
+  # the situation.
+  #
+  class IncompleteSpecification
+    attr_reader :name, :partially_complete_specs
+
+    def initialize(name, partially_complete_specs = [])
+      @name = name
+      @partially_complete_specs = partially_complete_specs
+    end
+
+    def ==(other)
+      partially_complete_specs == other.partially_complete_specs
+    end
+  end
+end

bundler/lib/bundler/resolver/base.rb

@@ -34,9 +34,11 @@ def [](name)
         @base[name]
       end
 
-      def delete(specs)
-        specs.each do |spec|
-          @base.delete(spec)
+      def delete(incomplete_specs)
+        incomplete_specs.each do |incomplete_spec|
+          incomplete_spec.partially_complete_specs.each do |spec|
+            @base.delete(spec)
+          end
         end
       end
 

bundler/lib/bundler/spec_set.rb

@@ -7,11 +7,8 @@ class SpecSet
     include Enumerable
     include TSort
 
-    attr_reader :incomplete_specs
-
-    def initialize(specs, incomplete_specs = [])
+    def initialize(specs)
       @specs = specs
-      @incomplete_specs = incomplete_specs
     end
 
     def for(dependencies, check = false, platforms = [nil])
@@ -45,7 +42,7 @@ def for(dependencies, check = false, platforms = [nil])
         end
 
         if incomplete && check
-          @incomplete_specs += lookup[name].any? ? lookup[name] : [LazySpecification.new(name, nil, nil)]
+          specs << IncompleteSpecification.new(name, lookup[name])
         end
       end
 
@@ -81,10 +78,10 @@ def to_hash
       lookup.dup
     end
 
-    def materialize(deps)
-      materialized = self.for(deps, true)
+    def materialize(deps, platforms = [nil])
+      materialized = self.for(deps, true, platforms)
 
-      SpecSet.new(materialized, incomplete_specs)
+      SpecSet.new(materialized)
     end
 
     # Materialize for all the specs in the spec set, regardless of what platform they're for
@@ -101,15 +98,19 @@ def materialized_for_all_platforms
     end
 
     def incomplete_ruby_specs?(deps)
-      self.for(deps, true, [Gem::Platform::RUBY])
+      return false if @specs.empty?
 
-      @incomplete_specs.any?
+      materialize(deps, [Gem::Platform::RUBY]).incomplete_specs.any?
     end
 
     def missing_specs
       @specs.select {|s| s.is_a?(LazySpecification) }
     end
 
+    def incomplete_specs
+      @specs.select {|s| s.is_a?(IncompleteSpecification) }
+    end
+
     def merge(set)
       arr = sorted.dup
       set.each do |set_spec|

bundler/spec/lock/lockfile_spec.rb

@@ -1262,7 +1262,7 @@
           indirect_dependency (1.2.3)
 
       PLATFORMS
-        #{lockfile_platforms}
+        #{formatted_lockfile_platforms(*["ruby", generic_local_platform].uniq)}
 
       DEPENDENCIES
         direct_dependency
@@ -1272,57 +1272,71 @@
     G
   end
 
-  it "auto-heals when the lockfile is missing dependent specs" do
-    build_repo4 do
-      build_gem "minitest-bisect", "1.6.0" do |s|
-        s.add_dependency "path_expander", "~> 1.1"
+  shared_examples_for "a lockfile missing dependent specs" do
+    it "auto-heals" do
+      build_repo4 do
+        build_gem "minitest-bisect", "1.6.0" do |s|
+          s.add_dependency "path_expander", "~> 1.1"
+        end
+
+        build_gem "path_expander", "1.1.1"
       end
 
-      build_gem "path_expander", "1.1.1"
-    end
+      gemfile <<~G
+        source "#{file_uri_for(gem_repo4)}"
+        gem "minitest-bisect"
+      G
 
-    gemfile <<~G
-      source "#{file_uri_for(gem_repo4)}"
-      gem "minitest-bisect"
-    G
+      # Corrupt lockfile (completely missing path_expander)
+      lockfile <<~L
+        GEM
+          remote: #{file_uri_for(gem_repo4)}/
+          specs:
+            minitest-bisect (1.6.0)
 
-    # Corrupt lockfile (completely missing path_expander)
-    lockfile <<~L
-      GEM
-        remote: #{file_uri_for(gem_repo4)}/
-        specs:
-          minitest-bisect (1.6.0)
+        PLATFORMS
+          #{platforms}
 
-      PLATFORMS
-        #{lockfile_platforms}
+        DEPENDENCIES
+          minitest-bisect
 
-      DEPENDENCIES
-        minitest-bisect
+        BUNDLED WITH
+           #{Bundler::VERSION}
+      L
 
-      BUNDLED WITH
-         #{Bundler::VERSION}
-    L
+      cache_gems "minitest-bisect-1.6.0", "path_expander-1.1.1", :gem_repo => gem_repo4
+      bundle :install
 
-    cache_gems "minitest-bisect-1.6.0", "path_expander-1.1.1", :gem_repo => gem_repo4
-    bundle :install
+      expect(lockfile).to eq <<~L
+        GEM
+          remote: #{file_uri_for(gem_repo4)}/
+          specs:
+            minitest-bisect (1.6.0)
+              path_expander (~> 1.1)
+            path_expander (1.1.1)
 
-    expect(lockfile).to eq <<~L
-      GEM
-        remote: #{file_uri_for(gem_repo4)}/
-        specs:
-          minitest-bisect (1.6.0)
-            path_expander (~> 1.1)
-          path_expander (1.1.1)
+        PLATFORMS
+          #{platforms}
 
-      PLATFORMS
-        #{lockfile_platforms}
+        DEPENDENCIES
+          minitest-bisect
 
-      DEPENDENCIES
-        minitest-bisect
+        BUNDLED WITH
+           #{Bundler::VERSION}
+      L
+    end
+  end
 
-      BUNDLED WITH
-         #{Bundler::VERSION}
-    L
+  context "with just specific platform" do
+    let(:platforms) { lockfile_platforms }
+
+    it_behaves_like "a lockfile missing dependent specs"
+  end
+
+  context "with both ruby and specific platform" do
+    let(:platforms) { lockfile_platforms("ruby") }
+
+    it_behaves_like "a lockfile missing dependent specs"
   end
 
   it "auto-heals when the lockfile is missing specs" do

bundler/spec/support/platforms.rb

@@ -96,7 +96,11 @@ def not_local_patchlevel
     end
 
     def lockfile_platforms(*extra)
-      [local_platform, *extra].map(&:to_s).sort.join("\n  ")
+      formatted_lockfile_platforms(local_platform, *extra)
+    end
+
+    def formatted_lockfile_platforms(*platforms)
+      platforms.map(&:to_s).sort.join("\n  ")
     end
   end
 end