From 6ebb5669a89d3b8b0c15fe9f93c2573d51d02925 Mon Sep 17 00:00:00 2001 From: Samuel Giddins Date: Wed, 21 Dec 2016 01:10:34 +0100 Subject: [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 c973233d99..4ed5a5c9b9 100644 --- a/lib/bundler/installer/parallel_installer.rb +++ b/lib/bundler/installer/parallel_installer.rb @@ -47,23 +47,26 @@ module Bundler # 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 @@ module Bundler # 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 @@ module Bundler 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 -- cgit v1.2.1 From edf6cab6b83a3528b71609c33131a88611ca38e4 Mon Sep 17 00:00:00 2001 From: Samuel Giddins Date: Mon, 26 Dec 2016 16:32:08 -0600 Subject: [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 4ed5a5c9b9..9e3a8d51f7 100644 --- a/lib/bundler/installer/parallel_installer.rb +++ b/lib/bundler/installer/parallel_installer.rb @@ -58,13 +58,9 @@ module Bundler 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 @@ module Bundler 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. -- cgit v1.2.1 From e86631c1d2b0b5073c4f29ed7cc262f61932aa01 Mon Sep 17 00:00:00 2001 From: Samuel Giddins Date: Mon, 26 Dec 2016 16:47:45 -0600 Subject: Add specs for falling back to 1 thread when the lockfile is corrupt --- lib/bundler/installer/parallel_installer.rb | 2 + spec/bundler/installer/parallel_installer_spec.rb | 47 ++++++++++++++ spec/bundler/installer/spec_installation_spec.rb | 62 ++++++++++++++++++ spec/install/parallel/spec_installation_spec.rb | 77 ----------------------- 4 files changed, 111 insertions(+), 77 deletions(-) create mode 100644 spec/bundler/installer/parallel_installer_spec.rb create mode 100644 spec/bundler/installer/spec_installation_spec.rb delete mode 100644 spec/install/parallel/spec_installation_spec.rb diff --git a/lib/bundler/installer/parallel_installer.rb b/lib/bundler/installer/parallel_installer.rb index 9e3a8d51f7..70939ceeb7 100644 --- a/lib/bundler/installer/parallel_installer.rb +++ b/lib/bundler/installer/parallel_installer.rb @@ -78,6 +78,8 @@ module Bundler [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 0000000000..0f4e7dba9a --- /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/bundler/installer/spec_installation_spec.rb b/spec/bundler/installer/spec_installation_spec.rb new file mode 100644 index 0000000000..b18695bf26 --- /dev/null +++ b/spec/bundler/installer/spec_installation_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true +require "spec_helper" +require "bundler/installer/parallel_installer" + +describe Bundler::ParallelInstaller::SpecInstallation do + let!(:dep) do + a_spec = Object.new + def a_spec.name + "I like tests" + end + a_spec + end + + describe "#ready_to_enqueue?" do + context "when in enqueued state" do + it "is falsey" do + spec = described_class.new(dep) + spec.state = :enqueued + expect(spec.ready_to_enqueue?).to be_falsey + end + end + + context "when in installed state" do + it "returns falsey" do + spec = described_class.new(dep) + spec.state = :installed + expect(spec.ready_to_enqueue?).to be_falsey + end + end + + it "returns truthy" do + spec = described_class.new(dep) + expect(spec.ready_to_enqueue?).to be_truthy + end + end + + describe "#dependencies_installed?" do + context "when all dependencies are installed" do + it "returns true" do + dependencies = [] + dependencies << instance_double("SpecInstallation", :spec => "alpha", :name => "alpha", :installed? => true, :all_dependencies => [], :type => :production) + dependencies << instance_double("SpecInstallation", :spec => "beta", :name => "beta", :installed? => true, :all_dependencies => [], :type => :production) + all_specs = dependencies + [instance_double("SpecInstallation", :spec => "gamma", :name => "gamma", :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 be_truthy + end + end + + context "when all dependencies are not installed" do + it "returns false" do + dependencies = [] + dependencies << instance_double("SpecInstallation", :spec => "alpha", :name => "alpha", :installed? => false, :all_dependencies => [], :type => :production) + dependencies << instance_double("SpecInstallation", :spec => "beta", :name => "beta", :installed? => true, :all_dependencies => [], :type => :production) + all_specs = dependencies + [instance_double("SpecInstallation", :spec => "gamma", :name => "gamma", :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 be_falsey + end + end + end +end diff --git a/spec/install/parallel/spec_installation_spec.rb b/spec/install/parallel/spec_installation_spec.rb deleted file mode 100644 index 7bbd2bd0e6..0000000000 --- a/spec/install/parallel/spec_installation_spec.rb +++ /dev/null @@ -1,77 +0,0 @@ -# frozen_string_literal: true -require "spec_helper" -require "bundler/installer/parallel_installer" - -describe Bundler::ParallelInstaller::SpecInstallation do - let!(:dep) do - a_spec = Object.new - def a_spec.name - "I like tests" - end - a_spec - end - - describe "#ready_to_enqueue?" do - context "when in enqueued state" do - it "is falsey" do - spec = described_class.new(dep) - spec.state = :enqueued - expect(spec.ready_to_enqueue?).to be_falsey - end - end - - context "when in installed state" do - it "returns falsey" do - spec = described_class.new(dep) - spec.state = :installed - expect(spec.ready_to_enqueue?).to be_falsey - end - end - - it "returns truthy" do - spec = described_class.new(dep) - expect(spec.ready_to_enqueue?).to be_truthy - end - end - - describe "#dependencies_installed?" do - context "when all dependencies are installed" do - it "returns true" do - dependencies = [] - dependencies << instance_double("SpecInstallation", :spec => "alpha", :name => "alpha", :installed? => true, :all_dependencies => [], :type => :production) - dependencies << instance_double("SpecInstallation", :spec => "beta", :name => "beta", :installed? => true, :all_dependencies => [], :type => :production) - all_specs = dependencies + [instance_double("SpecInstallation", :spec => "gamma", :name => "gamma", :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 be_truthy - end - end - - context "when all dependencies are not installed" do - it "returns false" do - dependencies = [] - dependencies << instance_double("SpecInstallation", :spec => "alpha", :name => "alpha", :installed? => false, :all_dependencies => [], :type => :production) - dependencies << instance_double("SpecInstallation", :spec => "beta", :name => "beta", :installed? => true, :all_dependencies => [], :type => :production) - all_specs = dependencies + [instance_double("SpecInstallation", :spec => "gamma", :name => "gamma", :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 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 -- cgit v1.2.1