Auto merge of #6288 - bundler:seg-lockfile-missing-platform-specific-…

…specs, r=indirect

Gracefully handle when the lockfile is missing spec entries for the current platform

### What was the end-user problem that led to this PR?

The problem was users could get `Unable to find a spec satisfying ... perhaps the lockfile is corrupted?` error messages, particularly when they use multiple platforms.

Fixes #6079.

### What was your diagnosis of the problem?

My diagnosis was the lockfile _was_ indeed corrupted, because it was missing `spec_name (version)` entries, but resolution could still be skipped (preventing those gems from being added back in).

### What is your fix for the problem, implemented in this PR?

My fix checks whether all specs are present _in the lockfile_ (e.g. not locally) for the current platform, and considers that a "change" that forces the resolver to run, allowing those missing specs to be added back to the bundle.

### Why did you choose this fix out of the possible options?

I chose this fix because it was a way to force re-resolution in a way that ties into our existing `#change_reason` infrastructure. Additionally, it shouldn't have much of a performance overhead, since the calculation is only made when we're converging locked specs anyways.

(cherry picked from commit 81854e0)

lib/bundler/definition.rb

@@ -77,6 +77,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
       @lockfile_contents      = String.new
       @locked_bundler_version = nil
       @locked_ruby_version    = nil
+      @locked_specs_incomplete_for_platform = false
 
       if lockfile && File.exist?(lockfile)
         @lockfile_contents = Bundler.read_file(lockfile)
@@ -530,7 +531,7 @@ def find_indexed_specs(current_spec)
     private :sources
 
     def nothing_changed?
-      !@source_changes && !@dependency_changes && !@new_platform && !@path_changes && !@local_changes
+      !@source_changes && !@dependency_changes && !@new_platform && !@path_changes && !@local_changes && !@locked_specs_incomplete_for_platform
     end
 
     def unlocking?
@@ -557,6 +558,7 @@ def change_reason
         [@new_platform, "you added a new platform to your gemfile"],
         [@path_changes, "the gemspecs for path gems changed"],
         [@local_changes, "the gemspecs for git local gems changed"],
+        [@locked_specs_incomplete_for_platform, "the lockfile does not have all gems needed for the current platform"],
       ].select(&:first).map(&:last).join(", ")
     end
 
@@ -803,7 +805,9 @@ def converge_locked_specs
       end
 
       resolve = SpecSet.new(converged)
-      resolve = resolve.for(expand_dependencies(deps, true), @unlock[:gems], false, false, false)
+      expanded_deps = expand_dependencies(deps, true)
+      @locked_specs_incomplete_for_platform = !resolve.for(expanded_deps, @unlock[:gems], true, true)
+      resolve = resolve.for(expanded_deps, @unlock[:gems], false, false, false)
       diff    = nil
 
       # Now, we unlock any sources that do not have anymore gems pinned to it

lib/bundler/spec_set.rb

@@ -37,7 +37,10 @@ def for(dependencies, skip = [], check = false, match_current_platform = false,
         elsif check
           return false
         elsif raise_on_missing
-          raise "Unable to find a spec satisfying #{dep} in the set. Perhaps the lockfile is corrupted?"
+          others = lookup[dep.name] if match_current_platform
+          message = "Unable to find a spec satisfying #{dep} in the set. Perhaps the lockfile is corrupted?"
+          message += " Found #{others.join(", ")} that did not match the current platform." if others && !others.empty?
+          raise GemNotFound, message
         end
       end
 

spec/runtime/platform_spec.rb

@@ -115,4 +115,36 @@
       expect(the_bundle).to include_gems "platform_specific 1.0 RUBY"
     end
   end
+
+  it "recovers when the lockfile is missing a platform-specific gem" do
+    build_repo2 do
+      build_gem "requires_platform_specific" do |s|
+        s.add_dependency "platform_specific"
+      end
+    end
+    simulate_windows x64_mingw do
+      lockfile <<-L
+        GEM
+          remote: file:#{gem_repo2}/
+          specs:
+            platform_specific (1.0-x86-mingw32)
+            requires_platform_specific (1.0)
+              platform_specific
+
+        PLATFORMS
+          x64-mingw32
+          x86-mingw32
+
+        DEPENDENCIES
+          requires_platform_specific
+      L
+
+      install_gemfile! <<-G, :verbose => true
+        source "file://#{gem_repo2}"
+        gem "requires_platform_specific"
+      G
+
+      expect(the_bundle).to include_gem "platform_specific 1.0 x64-mingw32"
+    end
+  end
 end

spec/support/builders.rb

@@ -100,6 +100,14 @@ def build_repo1
           s.write "lib/platform_specific.rb", "PLATFORM_SPECIFIC = '1.0.0 MSWIN'"
         end
 
+        build_gem "platform_specific" do |s|
+          s.platform = "x86-mingw32"
+        end
+
+        build_gem "platform_specific" do |s|
+          s.platform = "x64-mingw32"
+        end
+
         build_gem "platform_specific" do |s|
           s.platform = "x86-darwin-100"
           s.write "lib/platform_specific.rb", "PLATFORM_SPECIFIC = '1.0.0 x86-darwin-100'"

spec/support/helpers.rb

@@ -489,10 +489,10 @@ def simulate_rubygems_version(version)
       ENV["BUNDLER_SPEC_RUBYGEMS_VERSION"] = old if block_given?
     end
 
-    def simulate_windows
+    def simulate_windows(platform = mswin)
       old = ENV["BUNDLER_SPEC_WINDOWS"]
       ENV["BUNDLER_SPEC_WINDOWS"] = "true"
-      simulate_platform mswin do
+      simulate_platform platform do
         yield
       end
     ensure