Skip to content

Commit

Permalink
Avoid crashing with a corrupted lockfile
Browse files Browse the repository at this point in the history
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 #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
* #<Thread:0x000000010833b130 sleep_forever>
   rb_thread_t:0x00007fa6b6704660 native:0x0000000108985600 int:0

* #<Thread:0x0000000108dea630@Parallel Installer Worker #0 tmp/1/gems/system/gems/bundler-2.5.0.dev/lib/bundler/worker.rb:90 sleep_forever>
   rb_thread_t:0x00007fa6b67f67c0 native:0x0000700009a62000 int:0

* #<Thread:0x0000000108dea4a0@Parallel Installer Worker #1 tmp/1/gems/system/gems/bundler-2.5.0.dev/lib/bundler/worker.rb:90 sleep_forever>
   rb_thread_t:0x00007fa6b67f63c0 native:0x0000700009c65000 int:0

<internal:thread_sync>: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.
  • Loading branch information
composerinteralia committed Feb 7, 2023
1 parent c0d1521 commit f8e5007
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 4 deletions.
18 changes: 16 additions & 2 deletions bundler/lib/bundler/installer/parallel_installer.rb
Expand Up @@ -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
Expand Down Expand Up @@ -110,12 +117,17 @@ def check_for_unmet_dependencies

warning = []
warning << "Your lockfile doesn't include a valid resolution."
warning << "You can fix this by regenerating your lockfile or trying to manually editing the bad locked gems to a version that satisfies all dependencies."
warning << "You can fix this by regenerating your lockfile or manually editing the bad locked gems to a version that satisfies all dependencies."
warning << "The unmet dependencies are:"

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 by #{spec.full_name}, unsatisfied by #{found.full_name}"
else
warning << "* #{unmet_spec_dependency}, depended upon by #{spec.full_name} but missing from lockfile"
end
end
end

Expand Down Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions bundler/spec/lock/lockfile_spec.rb
Expand Up @@ -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|
Expand Down
4 changes: 2 additions & 2 deletions bundler/spec/support/helpers.rb
Expand Up @@ -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
Expand Down

0 comments on commit f8e5007

Please sign in to comment.