Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Commit

Permalink
Merge #7149
Browse files Browse the repository at this point in the history
7149: Fix bundle update crash r=deivid-rodriguez a=deivid-rodriguez

### What was the end-user problem that led to this PR?

The problem was that I introduced a regression with #6329. When running `bundle update <gem>`, where `<gem>` is included in the Gemfile, but it's for another platform from the current one, `bundle` would crash with the following error:

```
NoMethodError: undefined method `source' for nil:NilClass
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/cli/update.rb:86:in `block in run'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/cli/update.rb:78:in `each'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/cli/update.rb:78:in `run'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/cli.rb:280:in `block in update'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/settings.rb:129:in `temporary'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/cli.rb:279:in `update'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/cli.rb:26:in `dispatch'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/vendor/thor/lib/thor/base.rb:466:in `start'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/cli.rb:17:in `start'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/exe/bundle:21:in `block in <top (required)>'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/lib/bundler/friendly_errors.rb:123:in `with_friendly_errors'
  /home/deivid/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/bundler-2.1.0.pre.1/exe/bundle:13:in `<top (required)>'
  /home/deivid/.rbenv/versions/2.6.3/bin/bundle:23:in `load'
  /home/deivid/.rbenv/versions/2.6.3/bin/bundle:23:in `<main>'
```


### What was your diagnosis of the problem?

My diagnosis was that we can't always rely on the spec being updated not being `nil` in the definition. It can be `nil` for gems in excluded groups, or for gems included only for platforms different from the current one.

### What is your fix for the problem, implemented in this PR?

My fix is to handle this case and show a proper error.

### Why did you choose this fix out of the possible options?

I chose this fix because it's user friendly.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
  • Loading branch information
bundlerbot and deivid-rodriguez committed May 3, 2019
2 parents 8d2c4e8 + f143b37 commit 9cc693b
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 10 deletions.
29 changes: 19 additions & 10 deletions lib/bundler/cli/update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ def run
Bundler.definition.validate_runtime!

if locked_gems = Bundler.definition.locked_gems
previous_locked_specs = locked_gems.specs.reduce({}) do |h, s|
h[s.name] = { :version => s.version, :source => s.source.to_s }
previous_locked_info = locked_gems.specs.reduce({}) do |h, s|
h[s.name] = { :spec => s, :version => s.version, :source => s.source.to_s }
h
end
end
Expand All @@ -76,17 +76,26 @@ def run

if locked_gems
gems.each do |name|
locked_spec = previous_locked_specs[name]
next unless locked_spec
locked_source = locked_spec[:source]
locked_version = locked_spec[:version]
locked_info = previous_locked_info[name]
next unless locked_info

locked_spec = locked_info[:spec]
new_spec = Bundler.definition.specs[name].first
unless new_spec
if Bundler.rubygems.platforms.none? {|p| locked_spec.match_platform(p) }
Bundler.ui.warn "Bundler attempted to update #{name} but it was not considered because it is for a different platform from the current one"
end

next
end

locked_source = locked_info[:source]
new_source = new_spec.source.to_s
new_version = new_spec.version
next if locked_source != new_source
if !new_version
Bundler.ui.warn "Bundler attempted to update #{name} but it was removed from the bundle"
elsif new_version < locked_version

new_version = new_spec.version
locked_version = locked_info[:version]
if new_version < locked_version
Bundler.ui.warn "Note: #{name} version regressed from #{locked_version} to #{new_version}"
elsif new_version == locked_version
Bundler.ui.warn "Bundler attempted to update #{name} but its version stayed the same"
Expand Down
35 changes: 35 additions & 0 deletions spec/commands/update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,41 @@
expect(the_bundle).to include_gem "a 1.1"
end
end

context "when the dependency is for a different platform" do
before do
build_repo4 do
build_gem("a", "0.9") {|s| s.platform = "java" }
build_gem("a", "1.1") {|s| s.platform = "java" }
end

gemfile <<-G
source "file://#{gem_repo4}"
gem "a", platform: :jruby
G

lockfile <<-L
GEM
remote: file://#{gem_repo4}
specs:
a (0.9-java)
PLATFORMS
java
DEPENDENCIES
a
L

simulate_platform linux
end

it "is not updated because it is not actually included in the bundle" do
bundle! "update a"
expect(last_command.stdboth).to include "Bundler attempted to update a but it was not considered because it is for a different platform from the current one"
expect(the_bundle).to_not include_gem "a"
end
end
end

RSpec.describe "bundle update without a Gemfile.lock" do
Expand Down

0 comments on commit 9cc693b

Please sign in to comment.