From f93a2fe83dea6ea9994f9ea610ccddf29574129d Mon Sep 17 00:00:00 2001 From: The Bundler Bot Date: Sat, 3 Feb 2018 21:44:31 +0000 Subject: [PATCH] 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 81854e0758b807a647ca9ce94bd6c2e325a62302) --- lib/bundler/definition.rb | 8 ++++++-- lib/bundler/spec_set.rb | 5 ++++- spec/runtime/platform_spec.rb | 32 ++++++++++++++++++++++++++++++++ spec/support/builders.rb | 8 ++++++++ spec/support/helpers.rb | 4 ++-- 5 files changed, 52 insertions(+), 5 deletions(-) diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb index dddb0aeec27..79cdaebba56 100644 --- a/lib/bundler/definition.rb +++ b/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 diff --git a/lib/bundler/spec_set.rb b/lib/bundler/spec_set.rb index 7cd30219978..5003b2cbeca 100644 --- a/lib/bundler/spec_set.rb +++ b/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 diff --git a/spec/runtime/platform_spec.rb b/spec/runtime/platform_spec.rb index f38f7338459..eecf162427c 100644 --- a/spec/runtime/platform_spec.rb +++ b/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 diff --git a/spec/support/builders.rb b/spec/support/builders.rb index 377ca35523c..a90260a80e2 100644 --- a/spec/support/builders.rb +++ b/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'" diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 62016310111..1b52ed52580 100644 --- a/spec/support/helpers.rb +++ b/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