Auto merge of #4020 - simplybusiness:error_on_missing_dependency, r=segiddins

Error on missing dependency - partial fix for #3692 (alternative)

This pull request implements the strategy suggested in the comments for https://github.com/bundler/bundler/pull/4012, namely to raise an error when dependencies are not present (due to a corrupted `Gemfile.lock`).

This also removes the `install_sequentially` option.

I wasn't totally sure about the error message formatting.

3 files changed, 33 insertions, 22 deletions

diff --git a/lib/bundler/installer.rb b/lib/bundler/installer.rb
index 29f2a30fb0..923fbd1faf 100644
--- a/lib/bundler/installer.rb
+++ b/lib/bundler/installer.rb
@@ -1,6 +1,7 @@
 require "erb"
 require "rubygems/dependency_installer"
 require "bundler/worker"
+require "bundler/installer/parallel_installer"
 
 module Bundler
   class Installer < Environment
@@ -176,13 +177,9 @@ module Bundler
     # installation is just SO MUCH FASTER. so we let people opt in.
     def install(options)
       force = options["force"]
-      jobs = [Bundler.settings[:jobs].to_i - 1, 1].max
-      if jobs > 1 && can_install_in_parallel?
-        require "bundler/installer/parallel_installer"
-        install_in_parallel jobs, options[:standalone], force
-      else
-        install_sequentially options[:standalone], force
-      end
+      jobs = 1
+      jobs = [Bundler.settings[:jobs].to_i - 1, 1].max if can_install_in_parallel?
+      install_in_parallel jobs, options[:standalone], force
     end
 
     def can_install_in_parallel?
@@ -190,7 +187,7 @@ module Bundler
         true
       else
         Bundler.ui.warn "Rubygems #{Gem::VERSION} is not threadsafe, so your "\
-          "gems must be installed one at a time. Upgrade to Rubygems 2.1.0 " \
+          "gems will be installed one at a time. Upgrade to Rubygems 2.1.0 " \
           "or higher to enable parallel gem installation."
         false
       end
@@ -250,15 +247,6 @@ module Bundler
       end
     end
 
-    def install_sequentially(standalone, force = false)
-      specs.each do |spec|
-        message = install_gem_from_spec spec, standalone, 0, force
-        if message
-          Installer.post_install_messages[spec.name] = message
-        end
-      end
-    end
-
     def install_in_parallel(size, standalone, force = false)
       ParallelInstaller.call(self, specs, size, standalone, force)
     end
diff --git a/lib/bundler/installer/parallel_installer.rb b/lib/bundler/installer/parallel_installer.rb
index 84bdb425fa..c1a8065a09 100644
--- a/lib/bundler/installer/parallel_installer.rb
+++ b/lib/bundler/installer/parallel_installer.rb
@@ -35,13 +35,21 @@ class ParallelInstaller
     # sure needed dependencies have been installed.
     def dependencies_installed?(all_specs)
       installed_specs = all_specs.select(&:installed?).map(&:name)
-      dependencies.all? {|d| installed_specs.include? d.name }
+      dependencies(all_specs.map(&:name)).all? {|d| installed_specs.include? d.name }
     end
 
-    # Represents only the non-development dependencies and the ones that
-    # are itself.
-    def dependencies
-      @dependencies ||= all_dependencies.reject {|dep| ignorable_dependency? dep }
+    # Represents only the non-development dependencies, the ones that are
+    # itself and are in the total list.
+    def dependencies(all_spec_names)
+      @dependencies ||= begin
+        deps = all_dependencies.reject {|dep| ignorable_dependency? dep }
+        missing = deps.reject {|dep| all_spec_names.include? dep.name }
+        if missing.size > 0
+          raise Bundler::LockfileError, "Your Gemfile.lock is corrupt. The following #{missing.size > 1 ? "gems are" : "gem is"} missing " \
+                              "from the DEPENDENCIES section: '#{missing.map(&:name).join('\' \'')}'"
+        end
+        deps
+      end
     end
 
     # Represents all dependencies
diff --git a/spec/install/parallel/spec_installation_spec.rb b/spec/install/parallel/spec_installation_spec.rb
index 2d741a15ff..6e92bb4cb8 100644
--- a/spec/install/parallel/spec_installation_spec.rb
+++ b/spec/install/parallel/spec_installation_spec.rb
@@ -57,5 +57,20 @@ describe ParallelInstaller::SpecInstallation do
         expect(spec.dependencies_installed?(all_specs)).to be_falsey
       end
     end
+
+    context "when dependencies that are not on the overall installation list are the only ones not installed" do
+      it "raises an error" do
+        dependencies = []
+        dependencies << instance_double("SpecInstallation", :spec => "alpha", :name => "alpha", :installed? => true, :all_dependencies => [], :type => :production)
+        all_specs = dependencies + [instance_double("SpecInstallation", :spec => "gamma", :name => "gamma", :installed? => false, :all_dependencies => [], :type => :production)]
+        # Add dependency which is not in all_specs
+        dependencies << instance_double("SpecInstallation", :spec => "beta", :name => "beta", :installed? => false, :all_dependencies => [], :type => :production)
+        dependencies << instance_double("SpecInstallation", :spec => "delta", :name => "delta", :installed? => false, :all_dependencies => [], :type => :production)
+        spec = ParallelInstaller::SpecInstallation.new(dep)
+        allow(spec).to receive(:all_dependencies).and_return(dependencies)
+        expect { spec.dependencies_installed?(all_specs) }.
+          to raise_error(Bundler::LockfileError, /Your Gemfile.lock is corrupt\. The following.*'beta' 'delta'/)
+      end
+    end
   end
 end