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 #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.

.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:

lib/bundler/definition.rb

@@ -448,7 +448,7 @@ def ensure_equivalent_gemfile_and_lockfile(explicit_flag = false)
       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 @@ def converge_locked_specs
       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

lib/bundler/endpoint_specification.rb

@@ -5,8 +5,8 @@ class EndpointSpecification < Gem::Specification
     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 @@ def _local_specification
     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
 

lib/bundler/lazy_specification.rb

@@ -79,7 +79,7 @@ def __materialize__
             "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

lib/bundler/remote_specification.rb

@@ -51,13 +51,7 @@ def <=>(other)
     # 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
 

lib/bundler/runtime.rb

@@ -11,6 +11,8 @@ def initialize(root, definition)
     end
 
     def setup(*groups)
+      @definition.ensure_equivalent_gemfile_and_lockfile if Bundler.settings[:frozen]
+
       groups.map!(&:to_sym)
 
       # Has to happen first

lib/bundler/shared_helpers.rb

@@ -152,6 +152,24 @@ def trap(signal, override = false, &block)
       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

lib/bundler/spec_set.rb

@@ -14,7 +14,7 @@ def initialize(specs)
       @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 @@ def for(dependencies, skip = [], check = false, match_current_platform = false)
           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 @@ def to_hash
     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)

spec/bundler/remote_specification_spec.rb

@@ -129,7 +129,7 @@
 
   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) }
 

spec/install/deploy_spec.rb

@@ -258,7 +258,17 @@
         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
 

spec/install/gems/compact_index_spec.rb

@@ -764,9 +764,9 @@ def start
             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

spec/lock/lockfile_spec.rb

@@ -1258,6 +1258,29 @@
     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)