From 62b3bcba5ee422d7431f90567636155358234288 Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Tue, 21 Feb 2023 16:27:54 -0500 Subject: [PATCH] [rubygems/rubygems] Auto-heal on corrupted lockfile with missing deps Following up on https://github.com/rubygems/rubygems/pull/6355, which turned a crash into a nicer error message, this commit auto-heals the corrupt lockfile instead. In this particular case (a corrupt Gemfile.lock with missing dependencies) the LazySpecification will not have accurate dependency information, we have to materialize the SpecSet to determine there are missing dependencies. We've already got a way to handle this, via `SpecSet#incomplete_specs`, but it wasn't quite working for this case because we'd get to `@incomplete_specs += lookup[name]` and `lookup[name]` would be empty for the dependency. With this commit we catch it a bit earlier, marking the parent spec containing the missing dependency as incomplete. https://github.com/rubygems/rubygems/commit/486ecb8f20 --- lib/bundler/installer/parallel_installer.rb | 15 +------------- lib/bundler/spec_set.rb | 8 +++++-- spec/bundler/lock/lockfile_spec.rb | 23 +++++++++++++++++---- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/lib/bundler/installer/parallel_installer.rb b/lib/bundler/installer/parallel_installer.rb index 851b6c8d41..83a381f592 100644 --- a/lib/bundler/installer/parallel_installer.rb +++ b/lib/bundler/installer/parallel_installer.rb @@ -47,13 +47,6 @@ module Bundler dependencies.all? {|d| installed_specs.include? d.name } end - # Check whether spec's dependencies are missing, which can indicate a - # corrupted lockfile - def dependencies_missing?(all_specs) - spec_names = all_specs.map(&:name) - dependencies.any? {|d| !spec_names.include? d.name } - end - # Represents only the non-development dependencies, the ones that are # itself and are in the total list. def dependencies @@ -123,11 +116,7 @@ module Bundler unmet_dependencies.each do |spec, unmet_spec_dependencies| unmet_spec_dependencies.each do |unmet_spec_dependency| found = @specs.find {|s| s.name == unmet_spec_dependency.name && !unmet_spec_dependency.matches_spec?(s.spec) } - if found - warning << "* #{unmet_spec_dependency}, dependency of #{spec.full_name}, unsatisfied by #{found.full_name}" - else - warning << "* #{unmet_spec_dependency}, dependency of #{spec.full_name} but missing from lockfile" - end + warning << "* #{unmet_spec_dependency}, dependency of #{spec.full_name}, unsatisfied by #{found.full_name}" end end @@ -224,8 +213,6 @@ module Bundler if spec.dependencies_installed? @specs spec.state = :enqueued worker_pool.enq spec - elsif spec.dependencies_missing? @specs - spec.state = :failed end end end diff --git a/lib/bundler/spec_set.rb b/lib/bundler/spec_set.rb index 7478bd9ca2..79ef3738a9 100644 --- a/lib/bundler/spec_set.rb +++ b/lib/bundler/spec_set.rb @@ -24,6 +24,7 @@ module Bundler name = dep[0].name platform = dep[1] + incomplete = false key = [name, platform] next if handled.key?(key) @@ -36,11 +37,14 @@ module Bundler specs_for_dep.first.dependencies.each do |d| next if d.type == :development + incomplete = true if d.name != "bundler" && lookup[d.name].empty? deps << [d, dep[1]] end - elsif check - @incomplete_specs += lookup[name] + else + incomplete = true end + + @incomplete_specs += lookup[name] if incomplete && check end specs diff --git a/spec/bundler/lock/lockfile_spec.rb b/spec/bundler/lock/lockfile_spec.rb index 88016189e6..185e2a85fa 100644 --- a/spec/bundler/lock/lockfile_spec.rb +++ b/spec/bundler/lock/lockfile_spec.rb @@ -1221,7 +1221,7 @@ RSpec.describe "the lockfile format" do and include("Either installing with `--full-index` or running `bundle update rack_middleware` should fix the problem.") end - it "errors gracefully on a corrupt lockfile" do + it "auto-heals when the lockfile is missing dependent specs" do build_repo4 do build_gem "minitest-bisect", "1.6.0" do |s| s.add_dependency "path_expander", "~> 1.1" @@ -1253,10 +1253,25 @@ RSpec.describe "the lockfile format" do L cache_gems "minitest-bisect-1.6.0", "path_expander-1.1.1", :gem_repo => gem_repo4 - bundle :install, :raise_on_error => false + bundle :install - expect(err).not_to include("ERROR REPORT TEMPLATE") - expect(err).to include("path_expander (~> 1.1), dependency of minitest-bisect-1.6.0 but missing from lockfile") + expect(lockfile).to eq <<~L + GEM + remote: #{file_uri_for(gem_repo4)}/ + specs: + minitest-bisect (1.6.0) + path_expander (~> 1.1) + path_expander (1.1.1) + + PLATFORMS + #{lockfile_platforms} + + DEPENDENCIES + minitest-bisect + + BUNDLED WITH + #{Bundler::VERSION} + L end it "auto-heals when the lockfile is missing specs" do