From 6ebb5669a89d3b8b0c15fe9f93c2573d51d02925 Mon Sep 17 00:00:00 2001 From: Samuel Giddins Date: Wed, 21 Dec 2016 01:10:34 +0100 Subject: [PATCH 1/3] [ParallelInstaller] Allow installing with corrupted lockfiles --- lib/bundler/installer/parallel_installer.rb | 28 ++++++++++++++------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/bundler/installer/parallel_installer.rb b/lib/bundler/installer/parallel_installer.rb index c973233d99d..4ed5a5c9b93 100644 --- a/lib/bundler/installer/parallel_installer.rb +++ b/lib/bundler/installer/parallel_installer.rb @@ -47,23 +47,26 @@ def ignorable_dependency?(dep) # sure needed dependencies have been installed. def dependencies_installed?(all_specs) installed_specs = all_specs.select(&:installed?).map(&:name) - dependencies(all_specs.map(&:name)).all? {|d| installed_specs.include? d.name } + dependencies.all? {|d| installed_specs.include? d.name } end # Represents only the non-development dependencies, the ones that are # itself and are in the total list. - def dependencies(all_spec_names) + def dependencies @dependencies ||= begin - deps = all_dependencies.reject {|dep| ignorable_dependency? dep } - missing = deps.reject {|dep| all_spec_names.include? dep.name } - unless missing.empty? - 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 + all_dependencies.reject {|dep| ignorable_dependency? dep } end end + def corrupt_lockfile?(all_spec_names) + deps = all_dependencies.reject {|dep| ignorable_dependency? dep } + missing = deps.reject {|dep| all_spec_names.include? dep.name } + return false if missing.empty? + Bundler.ui.warn "Your Gemfile.lock is corrupt. The following #{missing.size > 1 ? "gems are" : "gem is"} missing " \ + "from the DEPENDENCIES section for #{@spec.full_name}: '#{missing.map(&:name).join("', '")}'" + true + end + # Represents all dependencies def all_dependencies @spec.dependencies @@ -92,6 +95,7 @@ def call # TODO: remove in bundler 2.0 require "bundler/gem_remote_fetcher" if RUBY_VERSION < "1.9" + check_for_corrupt_lockfile enqueue_specs process_specs until @specs.all?(&:installed?) || @specs.any?(&:failed?) handle_error if @specs.any?(&:failed?) @@ -135,6 +139,12 @@ def handle_error raise Bundler::InstallError, errors.map(&:to_s).join("\n\n") end + def check_for_corrupt_lockfile + return unless @specs.any? {|s| s.corrupt_lockfile?(@specs) } + Bundler.ui.warn "Using 1 thread to install specs instead of #{@size} due to lockfile corruption" unless @size == 1 + @size = 1 + end + # Keys in the remains hash represent uninstalled gems specs. # We enqueue all gem specs that do not have any dependencies. # Later we call this lambda again to install specs that depended on From edf6cab6b83a3528b71609c33131a88611ca38e4 Mon Sep 17 00:00:00 2001 From: Samuel Giddins Date: Mon, 26 Dec 2016 16:32:08 -0600 Subject: [PATCH 2/3] [ParallelInstaller] Only print 1 warning for missing dependencies --- lib/bundler/installer/parallel_installer.rb | 33 +++++++++++++++------ 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/lib/bundler/installer/parallel_installer.rb b/lib/bundler/installer/parallel_installer.rb index 4ed5a5c9b93..9e3a8d51f74 100644 --- a/lib/bundler/installer/parallel_installer.rb +++ b/lib/bundler/installer/parallel_installer.rb @@ -58,13 +58,9 @@ def dependencies end end - def corrupt_lockfile?(all_spec_names) + def missing_lockfile_dependencies(all_spec_names) deps = all_dependencies.reject {|dep| ignorable_dependency? dep } - missing = deps.reject {|dep| all_spec_names.include? dep.name } - return false if missing.empty? - Bundler.ui.warn "Your Gemfile.lock is corrupt. The following #{missing.size > 1 ? "gems are" : "gem is"} missing " \ - "from the DEPENDENCIES section for #{@spec.full_name}: '#{missing.map(&:name).join("', '")}'" - true + deps.reject {|dep| all_spec_names.include? dep.name } end # Represents all dependencies @@ -140,9 +136,28 @@ def handle_error end def check_for_corrupt_lockfile - return unless @specs.any? {|s| s.corrupt_lockfile?(@specs) } - Bundler.ui.warn "Using 1 thread to install specs instead of #{@size} due to lockfile corruption" unless @size == 1 - @size = 1 + missing_dependencies = @specs.map do |s| + [ + s, + s.missing_lockfile_dependencies(@specs.map(&:name)), + ] + end.reject { |a| a.last.empty? } + return if missing_dependencies.empty? + + warning = [] + warning << "Your lockfile was created by an old Bundler that left some things out." + if @size != 1 + warning << "Because of the missing DEPENDENCIES, we can only install gems one at a time, instead of installing #{@size} at a time." + @size = 1 + end + warning << "You can fix this by adding the missing gems to your Gemfile, running bundle install, and then removing the gems from your Gemfile." + warning << "The missing gems are:" + + missing_dependencies.each do |spec, missing| + warning << "* #{missing.map(&:name).join(", ")} depended upon by #{spec.name}" + end + + Bundler.ui.warn(warning.join("\n")) end # Keys in the remains hash represent uninstalled gems specs. From e86631c1d2b0b5073c4f29ed7cc262f61932aa01 Mon Sep 17 00:00:00 2001 From: Samuel Giddins Date: Mon, 26 Dec 2016 16:47:45 -0600 Subject: [PATCH 3/3] Add specs for falling back to 1 thread when the lockfile is corrupt --- lib/bundler/installer/parallel_installer.rb | 2 + .../installer/parallel_installer_spec.rb | 47 +++++++++++++++++++ .../installer}/spec_installation_spec.rb | 15 ------ 3 files changed, 49 insertions(+), 15 deletions(-) create mode 100644 spec/bundler/installer/parallel_installer_spec.rb rename spec/{install/parallel => bundler/installer}/spec_installation_spec.rb (67%) diff --git a/lib/bundler/installer/parallel_installer.rb b/lib/bundler/installer/parallel_installer.rb index 9e3a8d51f74..70939ceeb70 100644 --- a/lib/bundler/installer/parallel_installer.rb +++ b/lib/bundler/installer/parallel_installer.rb @@ -78,6 +78,8 @@ def self.max_threads [Bundler.settings[:jobs].to_i - 1, 1].max end + attr_reader :size + def initialize(installer, all_specs, size, standalone, force) @installer = installer @size = size diff --git a/spec/bundler/installer/parallel_installer_spec.rb b/spec/bundler/installer/parallel_installer_spec.rb new file mode 100644 index 00000000000..0f4e7dba9ab --- /dev/null +++ b/spec/bundler/installer/parallel_installer_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true +require "spec_helper" +require "bundler/installer/parallel_installer" + +describe Bundler::ParallelInstaller do + let(:installer) { instance_double("Installer") } + let(:all_specs) { [] } + let(:size) { 1 } + let(:standalone) { false } + let(:force) { false } + + subject { described_class.new(installer, all_specs, size, standalone, force) } + + context "when dependencies that are not on the overall installation list are the only ones not installed" do + let(:all_specs) do + [ + build_spec("alpha", "1.0") {|s| s.runtime "a", "1" }, + ].flatten + end + + it "prints a warning" do + expect(Bundler.ui).to receive(:warn).with(<<-W.strip) +Your lockfile was created by an old Bundler that left some things out. +You can fix this by adding the missing gems to your Gemfile, running bundle install, and then removing the gems from your Gemfile. +The missing gems are: +* a depended upon by alpha + W + subject.check_for_corrupt_lockfile + end + + context "when size > 1" do + let(:size) { 500 } + + it "prints a warning and sets size to 1" do + expect(Bundler.ui).to receive(:warn).with(<<-W.strip) +Your lockfile was created by an old Bundler that left some things out. +Because of the missing DEPENDENCIES, we can only install gems one at a time, instead of installing 500 at a time. +You can fix this by adding the missing gems to your Gemfile, running bundle install, and then removing the gems from your Gemfile. +The missing gems are: +* a depended upon by alpha + W + subject.check_for_corrupt_lockfile + expect(subject.size).to eq(1) + end + end + end +end diff --git a/spec/install/parallel/spec_installation_spec.rb b/spec/bundler/installer/spec_installation_spec.rb similarity index 67% rename from spec/install/parallel/spec_installation_spec.rb rename to spec/bundler/installer/spec_installation_spec.rb index 7bbd2bd0e69..b18695bf268 100644 --- a/spec/install/parallel/spec_installation_spec.rb +++ b/spec/bundler/installer/spec_installation_spec.rb @@ -58,20 +58,5 @@ def a_spec.name 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 = described_class.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