From 04170b6dd7fdd1ee58291a4a38eaa9b443501e7c Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Tue, 7 Feb 2023 13:13:33 -0500 Subject: [PATCH] Avoid crashing with a corrupted lockfile I did a bad thing (script that edits the Gemfile.lock directly) and ended up with a Gemfile.lock that was completely missing some indirect dependencies. While this is my fault and an error is reasonable, I noticed that the error got progressively less friendly in recent versions of bundler. Something similar came up in https://github.com/rubygems/rubygems/issues/6210, and this commit would have helped with that case as well (although we've already handled this a different way with #6219). Details: --- Back on Bundler 2.2.23, a corrupt lockfile like this would cause a helpful error: ``` Unable to find a spec satisfying minitest (>= 5.1) in the set. Perhaps the lockfile is corrupted? ``` Bundler 2.3.26 gave a helpful warning: ``` Warning: 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 16 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: * minitest depended upon by activesupport ``` But then continued on and crashed while trying to report the unmet dependency: ``` --- ERROR REPORT TEMPLATE ------------------------------------------------------- NoMethodError: undefined method `full_name' for nil:NilClass lib/bundler/installer/parallel_installer.rb:127:in `block (2 levels) in check_for_unmet_dependencies' ... ``` Bundler 2.4.0 and up crash as above when jobs=1, but crash even harder when run in parallel: ``` --- ERROR REPORT TEMPLATE ------------------------------------------------------- fatal: No live threads left. Deadlock? 3 threads, 3 sleeps current:0x00007fa6b6704660 main thread:0x00007fa6b6704660 * # rb_thread_t:0x00007fa6b6704660 native:0x0000000108985600 int:0 * # rb_thread_t:0x00007fa6b67f67c0 native:0x0000700009a62000 int:0 * # rb_thread_t:0x00007fa6b67f63c0 native:0x0000700009c65000 int:0 :18:in `pop' tmp/1/gems/system/gems/bundler-2.5.0.dev/lib/bundler/worker.rb:42:in `deq' ... ``` Changes --- This commit fixes the confusing thread deadlock crash by detecting if dependencies are missing such that we'll never be able to enqueue. When that happens we treat it as a failure so the install can finish. That gets us back to the `NoMethodError`, which this commit fixes by using a different warning in the case where no spec is found. --- .../bundler/installer/parallel_installer.rb | 16 +++++++- bundler/spec/lock/lockfile_spec.rb | 38 +++++++++++++++++++ bundler/spec/support/helpers.rb | 4 +- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/bundler/lib/bundler/installer/parallel_installer.rb b/bundler/lib/bundler/installer/parallel_installer.rb index 0bbc0f256e16..9213c8902dbe 100644 --- a/bundler/lib/bundler/installer/parallel_installer.rb +++ b/bundler/lib/bundler/installer/parallel_installer.rb @@ -47,6 +47,13 @@ def dependencies_installed?(all_specs) 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 @@ -115,7 +122,12 @@ def check_for_unmet_dependencies unmet_dependencies.each do |spec, unmet_spec_dependencies| unmet_spec_dependencies.each do |unmet_spec_dependency| - warning << "* #{unmet_spec_dependency}, depended upon #{spec.full_name}, unsatisfied by #{@specs.find {|s| s.name == unmet_spec_dependency.name && !unmet_spec_dependency.matches_spec?(s.spec) }.full_name}" + found = @specs.find {|s| s.name == unmet_spec_dependency.name && !unmet_spec_dependency.matches_spec?(s.spec) } + if found + warning << "* #{unmet_spec_dependency}, depended upon #{spec.full_name}, unsatisfied by #{found.full_name}" + else + warning << "* #{unmet_spec_dependency}, dependency of #{spec.full_name} but missing from lockfile" + end end end @@ -212,6 +224,8 @@ def enqueue_specs 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/bundler/spec/lock/lockfile_spec.rb b/bundler/spec/lock/lockfile_spec.rb index cd603e336a22..0543e642fe70 100644 --- a/bundler/spec/lock/lockfile_spec.rb +++ b/bundler/spec/lock/lockfile_spec.rb @@ -1221,6 +1221,44 @@ 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 + build_repo4 do + build_gem "minitest-bisect", "1.6.0" do |s| + s.add_dependency "path_expander", "~> 1.1" + end + + build_gem "path_expander", "1.1.1" + end + + gemfile <<~G + source "#{file_uri_for(gem_repo4)}" + gem "minitest-bisect" + G + + # Corrupt lockfile (completely missing path_expander) + lockfile <<~L + GEM + remote: #{file_uri_for(gem_repo4)}/ + specs: + minitest-bisect (1.6.0) + + PLATFORMS + #{lockfile_platforms} + + DEPENDENCIES + minitest-bisect + + BUNDLED WITH + #{Bundler::VERSION} + L + + cache_gems "minitest-bisect-1.6.0", "path_expander-1.1.1", :gem_repo => gem_repo4 + bundle :install, :raise_on_error => false + + expect(err).not_to include("ERROR REPORT TEMPLATE") + expect(err).to include("path_expander (~> 1.1), depended upon by minitest-bisect-1.6.0 but missing from lockfile") + end + it "auto-heals when the lockfile is missing specs" do build_repo4 do build_gem "minitest-bisect", "1.6.0" do |s| diff --git a/bundler/spec/support/helpers.rb b/bundler/spec/support/helpers.rb index 703c98eca31f..8202ee808048 100644 --- a/bundler/spec/support/helpers.rb +++ b/bundler/spec/support/helpers.rb @@ -414,14 +414,14 @@ def realworld_system_gems(*gems) end end - def cache_gems(*gems) + def cache_gems(*gems, gem_repo: gem_repo1) gems = gems.flatten FileUtils.rm_rf("#{bundled_app}/vendor/cache") FileUtils.mkdir_p("#{bundled_app}/vendor/cache") gems.each do |g| - path = "#{gem_repo1}/gems/#{g}.gem" + path = "#{gem_repo}/gems/#{g}.gem" raise "OMG `#{path}` does not exist!" unless File.exist?(path) FileUtils.cp(path, "#{bundled_app}/vendor/cache") end