Auto merge of #5571 - bundler:seg-deadlocking-is-bad, r=indirect

Avoid deadlocking when installing with a lockfile missing dependencies

Closes #5378.
Closes #5480.
Closes #5519.
Closes #5526.
Closes #5529.
Closes #5549.
Closes https://github.com/bundler/bundler/issues/5572.

Not the ideal fix (the error message could be more helpful in pointing out which issue is being faced), but overall better than deadlocking.

12 files changed, 79 insertions, 37 deletions

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index d03086eab2..7d16768980 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2017-03-30 23:48:11 +0900 using RuboCop version 0.48.0.
+# on 2017-04-08 17:26:10 -0500 using RuboCop version 0.48.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -72,12 +72,12 @@ Lint/UselessAssignment:
     - 'lib/bundler/index.rb'
     - 'lib/bundler/installer.rb'
 
-# Offense count: 444
+# Offense count: 451
 # Configuration parameters: CountComments, ExcludedMethods.
 Metrics/BlockLength:
-  Max: 980
+  Max: 988
 
-# Offense count: 1991
+# Offense count: 2035
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
 Metrics/LineLength:
@@ -141,7 +141,7 @@ Style/CaseEquality:
     - 'lib/bundler/match_platform.rb'
     - 'lib/bundler/rubygems_ext.rb'
 
-# Offense count: 24
+# Offense count: 25
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: nested, compact
 Style/ClassAndModuleChildren:
@@ -163,11 +163,11 @@ Style/ConditionalAssignment:
     - 'lib/bundler/source/git.rb'
     - 'lib/bundler/source/rubygems.rb'
 
-# Offense count: 154
+# Offense count: 158
 Style/Documentation:
   Enabled: false
 
-# Offense count: 300
+# Offense count: 304
 # Cop supports --auto-correct.
 Style/EmptyLineAfterMagicComment:
   Enabled: false
@@ -205,7 +205,7 @@ Style/GlobalVars:
     - 'lib/bundler/cli.rb'
     - 'spec/spec_helper.rb'
 
-# Offense count: 18
+# Offense count: 17
 # Configuration parameters: MinBodyLength.
 Style/GuardClause:
   Exclude:
@@ -217,7 +217,6 @@ Style/GuardClause:
     - 'lib/bundler/definition.rb'
     - 'lib/bundler/dsl.rb'
     - 'lib/bundler/installer.rb'
-    - 'lib/bundler/lockfile_parser.rb'
     - 'lib/bundler/runtime.rb'
     - 'lib/bundler/source/path/installer.rb'
     - 'lib/bundler/source_list.rb'
@@ -241,14 +240,14 @@ Style/IfUnlessModifierOfIfUnless:
 Style/IndentArray:
   EnforcedStyle: consistent
 
-# Offense count: 33
+# Offense count: 34
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: auto_detection, squiggly, active_support, powerpack, unindent
 Style/IndentHeredoc:
   Enabled: false
 
-# Offense count: 8
+# Offense count: 9
 # Cop supports --auto-correct.
 # Configuration parameters: InverseMethods, InverseBlocks.
 Style/InverseMethods:
diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index 5dca425c7e..6d3ad4f63a 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -448,7 +448,7 @@ module Bundler
       end
 
       reason = change_reason
-      msg << "\n\n#{reason.split(", ").join("\n")}\n" unless reason.strip.empty?
+      msg << "\n\n#{reason.split(", ").map(&:capitalize).join("\n")}" unless reason.strip.empty?
       msg << "\n\nYou have added to the Gemfile:\n" << added.join("\n") if added.any?
       msg << "\n\nYou have deleted from the Gemfile:\n" << deleted.join("\n") if deleted.any?
       msg << "\n\nYou have changed in the Gemfile:\n" << changed.join("\n") if changed.any?
@@ -775,7 +775,7 @@ module Bundler
       end
 
       resolve = SpecSet.new(converged)
-      resolve = resolve.for(expand_dependencies(deps, true), @unlock[:gems])
+      resolve = resolve.for(expand_dependencies(deps, true), @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/endpoint_specification.rb b/lib/bundler/endpoint_specification.rb
index 5e14f03265..5a1deeea47 100644
--- a/lib/bundler/endpoint_specification.rb
+++ b/lib/bundler/endpoint_specification.rb
@@ -5,8 +5,8 @@ module Bundler
     ILLFORMED_MESSAGE = 'Ill-formed requirement ["#<YAML::Syck::DefaultKey'.freeze
     include MatchPlatform
 
-    attr_reader :name, :version, :platform, :dependencies, :required_rubygems_version, :required_ruby_version, :checksum
-    attr_accessor :source, :remote
+    attr_reader :name, :version, :platform, :required_rubygems_version, :required_ruby_version, :checksum
+    attr_accessor :source, :remote, :dependencies
 
     def initialize(name, version, platform, dependencies, metadata = nil)
       @name         = name
@@ -91,13 +91,7 @@ module Bundler
     end
 
     def __swap__(spec)
-      without_type = proc {|d| Gem::Dependency.new(d.name, d.requirements_list.sort) }
-      if (extra_deps = spec.runtime_dependencies.map(&without_type).-(dependencies.map(&without_type))) && extra_deps.any?
-        Bundler.ui.debug "#{full_name} from #{remote} has corrupted API dependencies (API returned #{dependencies}, real spec has (#{spec.runtime_dependencies}))"
-        raise APIResponseMismatchError,
-          "Downloading #{full_name} revealed dependencies not in the API (#{extra_deps.map(&:to_s).join(", ")})." \
-          "\nInstalling with `--full-index` should fix the problem."
-      end
+      SharedHelpers.ensure_same_dependencies(self, dependencies, spec.dependencies)
       @remote_specification = spec
     end
 
diff --git a/lib/bundler/lazy_specification.rb b/lib/bundler/lazy_specification.rb
index 46f50844e1..9a8268a2d5 100644
--- a/lib/bundler/lazy_specification.rb
+++ b/lib/bundler/lazy_specification.rb
@@ -79,7 +79,7 @@ module Bundler
             "To use the platform-specific version of the gem, run `bundle config specific_platform true` and install again."
           search = source.specs.search(self).last
         end
-        search.dependencies = dependencies if search.is_a?(RemoteSpecification)
+        search.dependencies = dependencies if search.is_a?(RemoteSpecification) || search.is_a?(EndpointSpecification)
         search
       end
     end
diff --git a/lib/bundler/remote_specification.rb b/lib/bundler/remote_specification.rb
index 82b5cb8c6f..d89abd98ab 100644
--- a/lib/bundler/remote_specification.rb
+++ b/lib/bundler/remote_specification.rb
@@ -51,13 +51,7 @@ module Bundler
     # once the remote gem is downloaded, the backend specification will
     # be swapped out.
     def __swap__(spec)
-      without_type = proc {|d| Gem::Dependency.new(d.name, d.requirements_list) }
-      if (extra_deps = spec.runtime_dependencies.map(&without_type).-(dependencies.map(&without_type))) && extra_deps.any?
-        Bundler.ui.debug "#{full_name} from #{remote} has corrupted API dependencies (API returned #{dependencies}, real spec has (#{spec.runtime_dependencies}))"
-        raise APIResponseMismatchError,
-          "Downloading #{full_name} revealed dependencies not in the API (#{extra_deps.map(&without_type).map(&:to_s).join(", ")})." \
-          "\nInstalling with `--full-index` should fix the problem."
-      end
+      SharedHelpers.ensure_same_dependencies(self, dependencies, spec.dependencies)
       @_remote_specification = spec
     end
 
diff --git a/lib/bundler/runtime.rb b/lib/bundler/runtime.rb
index 6ccb0d89b5..5bdc9dfa63 100644
--- a/lib/bundler/runtime.rb
+++ b/lib/bundler/runtime.rb
@@ -11,6 +11,8 @@ module Bundler
     end
 
     def setup(*groups)
+      @definition.ensure_equivalent_gemfile_and_lockfile if Bundler.settings[:frozen]
+
       groups.map!(&:to_sym)
 
       # Has to happen first
diff --git a/lib/bundler/shared_helpers.rb b/lib/bundler/shared_helpers.rb
index 086624b383..74700cde35 100644
--- a/lib/bundler/shared_helpers.rb
+++ b/lib/bundler/shared_helpers.rb
@@ -152,6 +152,24 @@ module Bundler
       end
     end
 
+    def ensure_same_dependencies(spec, old_deps, new_deps)
+      new_deps = new_deps.reject {|d| d.type == :development }
+      old_deps = old_deps.reject {|d| d.type == :development }
+
+      without_type = proc {|d| Gem::Dependency.new(d.name, d.requirements_list.sort) }
+      new_deps.map!(&without_type)
+      old_deps.map!(&without_type)
+
+      extra_deps = new_deps - old_deps
+      return if extra_deps.empty?
+
+      Bundler.ui.debug "#{spec.full_name} from #{spec.remote} has either corrupted API or lockfile dependencies" \
+        " (was expecting #{old_deps.map(&:to_s)}, but the real spec has #{new_deps.map(&:to_s)})"
+      raise APIResponseMismatchError,
+        "Downloading #{spec.full_name} revealed dependencies not in the API or the lockfile (#{extra_deps.join(", ")})." \
+        "\nEither installing with `--full-index` or running `bundle update #{spec.name}` should fix the problem."
+    end
+
   private
 
     def find_gemfile
diff --git a/lib/bundler/spec_set.rb b/lib/bundler/spec_set.rb
index f3826beaa5..67bbe3060c 100644
--- a/lib/bundler/spec_set.rb
+++ b/lib/bundler/spec_set.rb
@@ -14,7 +14,7 @@ module Bundler
       @specs = specs.sort_by(&:name)
     end
 
-    def for(dependencies, skip = [], check = false, match_current_platform = false)
+    def for(dependencies, skip = [], check = false, match_current_platform = false, raise_on_missing = true)
       handled = {}
       deps = dependencies.dup
       specs = []
@@ -36,6 +36,8 @@ module Bundler
           end
         elsif check
           return false
+        elsif raise_on_missing
+          raise "Unable to find a spec satisfying #{dep} in the set. Perhaps the lockfile is corrupted?"
         end
       end
 
@@ -75,7 +77,7 @@ module Bundler
     end
 
     def materialize(deps, missing_specs = nil)
-      materialized = self.for(deps, [], false, true).to_a
+      materialized = self.for(deps, [], false, true, missing_specs).to_a
       deps = materialized.map(&:name).uniq
       materialized.map! do |s|
         next s unless s.is_a?(LazySpecification)
diff --git a/spec/bundler/remote_specification_spec.rb b/spec/bundler/remote_specification_spec.rb
index 69a2ab3d67..644814c563 100644
--- a/spec/bundler/remote_specification_spec.rb
+++ b/spec/bundler/remote_specification_spec.rb
@@ -129,7 +129,7 @@ RSpec.describe Bundler::RemoteSpecification do
 
   describe "#__swap__" do
     let(:spec) { double(:spec, :dependencies => []) }
-    let(:new_spec) { double(:new_spec, :runtime_dependencies => []) }
+    let(:new_spec) { double(:new_spec, :dependencies => [], :runtime_dependencies => []) }
 
     before { subject.instance_variable_set(:@_remote_specification, spec) }
 
diff --git a/spec/install/deploy_spec.rb b/spec/install/deploy_spec.rb
index 4a4f6e912f..bcbd57bc4f 100644
--- a/spec/install/deploy_spec.rb
+++ b/spec/install/deploy_spec.rb
@@ -258,7 +258,17 @@ RSpec.describe "install with --deployment or --frozen" do
         gem "rack-obama"
       G
 
-      expect(the_bundle).to include_gems "rack 1.0.0"
+      expect(the_bundle).not_to include_gems "rack 1.0.0"
+      expect(err).to include strip_whitespace(<<-E).strip
+The dependencies in your gemfile changed
+
+You have added to the Gemfile:
+* rack (= 1.0.0)
+* rack-obama
+
+You have deleted from the Gemfile:
+* rack
+      E
     end
   end
 
diff --git a/spec/install/gems/compact_index_spec.rb b/spec/install/gems/compact_index_spec.rb
index 3ba858ed08..87f59e32c8 100644
--- a/spec/install/gems/compact_index_spec.rb
+++ b/spec/install/gems/compact_index_spec.rb
@@ -764,9 +764,9 @@ The checksum of /versions does not match the checksum provided by the server! So
             Gem::Dependency.new("activerecord", "= 2.3.2"),
             Gem::Dependency.new("actionmailer", "= 2.3.2"),
             Gem::Dependency.new("activeresource", "= 2.3.2")]
-    expect(out).to include(<<-E.strip).and include("rails-2.3.2 from rubygems remote at #{source_uri}/ has corrupted API dependencies")
-Downloading rails-2.3.2 revealed dependencies not in the API (#{deps.join(", ")}).
-Installing with `--full-index` should fix the problem.
+    expect(out).to include(<<-E.strip).and include("rails-2.3.2 from rubygems remote at #{source_uri}/ has either corrupted API or lockfile dependencies")
+Bundler::APIResponseMismatchError: Downloading rails-2.3.2 revealed dependencies not in the API or the lockfile (#{deps.map(&:to_s).join(", ")}).
+Either installing with `--full-index` or running `bundle update rails` should fix the problem.
     E
   end
 end
diff --git a/spec/lock/lockfile_spec.rb b/spec/lock/lockfile_spec.rb
index 7d768e337c..968c969a55 100644
--- a/spec/lock/lockfile_spec.rb
+++ b/spec/lock/lockfile_spec.rb
@@ -1258,6 +1258,29 @@ RSpec.describe "the lockfile format" do
     L
   end
 
+  it "raises a helpful error message when the lockfile is missing deps" do
+    lockfile <<-L
+      GEM
+        remote: file:#{gem_repo1}/
+        specs:
+          rack_middleware (1.0)
+
+      PLATFORMS
+        #{local}
+
+      DEPENDENCIES
+        rack_middleware
+    L
+
+    install_gemfile <<-G
+      source "file:#{gem_repo1}"
+      gem "rack_middleware"
+    G
+
+    expect(out).to include("Downloading rack_middleware-1.0 revealed dependencies not in the API or the lockfile (#{Gem::Dependency.new("rack", "= 0.9.1")}).").
+      and include("Either installing with `--full-index` or running `bundle update rack_middleware` should fix the problem.")
+  end
+
   describe "a line ending" do
     def set_lockfile_mtime_to_known_value
       time = Time.local(2000, 1, 1, 0, 0, 0)