diff options
author | The Bundler Bot <bot@bundler.io> | 2017-07-05 14:17:41 +0000 |
---|---|---|
committer | Samuel Giddins <segiddins@segiddins.me> | 2017-07-17 12:03:18 -0500 |
commit | 94d2057d39238e1adc1939f2788fcb9c09a1f8f7 (patch) | |
tree | b165d45272c0ae9dd334d9b385a672280d770718 | |
parent | 89aeafd19750e61e9e456f3aa0282287875e57ec (diff) | |
download | bundler-94d2057d39238e1adc1939f2788fcb9c09a1f8f7.tar.gz |
Auto merge of #5817 - NickLaMuro:bug_with_path_gem_source_equivalency, r=segiddins
Compare sources using source's root
### What was the end-user problem that led to this PR?
Given a setup where:
1. A project's Gemfile makes use of another project's Gemfile and the `eval_gemfile` method to share the dependencies. Something like:
```ruby
# project_plugin's Gemfile
eval_gemfile(File.expand_path("../../main_project/Gemfile", __FILE__))
```
2. The main project includes a path gem in it that is nested in the main project:
```ruby
# main_project's Gemfile
gem "foo_gem", :path => "vendor/foo_gem"
```
3. A `bundle install` is followed by a `bundle install --deployment`, the second of which triggers a comparison of the `lockfile.sources` and the `Bundler.definition`
A error will occur when comparing the specs, saying the the "foo_gem" source has been deleted:
```console
$ bundle install
...
$ bundle install --deployment
You are trying to install in deployment mode after changing
your Gemfile. Run `bundle install` elsewhere and add the
updated Gemfile.lock to version control.
the gemspecs for path gems changed
You have deleted from the Gemfile:
* source: source at `../main_project/vendor/foo_gem`
```
### What was your diagnosis of the problem?
(extracted from the commit message)
When doing the following:
expand(other.original_path)
inside a `Bundler::Source::Path` instance, the `@root_path` from the instance that is having `eq?` called on it, the the `other` instance's `root_path`. This, in obscure cases, can cause a bug when you are doing an nested eval_gemfile or other calls when comparing the lockfile's locked path sources to the `Bundler.definition`'s path sources.
### What is your fix for the problem, implemented in this PR?
Use a new public method, `Bundler::Source::Path#expanded_original_path`, in the `eq?` method instead of using's the instance's `#expand` method.
### Why did you choose this fix out of the possible options?
(extracted from the commit message)
Creating the `expanded_original_path` method allows a public interface to be made available (and doesn't mess with any exiting functionality) that allows comparing the source path of one `Source::Path`'s `expand_path` to another, where each uses their own `root_path` to resolve their full path, instead of sharing the base one and causing edge case bugs
(cherry picked from commit fb3384649d866f08cb59683be8a711290aa75c07)
-rw-r--r-- | lib/bundler/source/path.rb | 6 | ||||
-rw-r--r-- | spec/bundler/source/path_spec.rb | 31 | ||||
-rw-r--r-- | spec/install/gemfile/eval_gemfile_spec.rb | 7 |
3 files changed, 43 insertions, 1 deletions
diff --git a/lib/bundler/source/path.rb b/lib/bundler/source/path.rb index 0ad0a029f0..8dd0763cc1 100644 --- a/lib/bundler/source/path.rb +++ b/lib/bundler/source/path.rb @@ -63,7 +63,7 @@ module Bundler def eql?(other) return unless other.class == self.class - expand(@original_path) == expand(other.original_path) && + expanded_original_path == other.expanded_original_path && version == other.version end @@ -117,6 +117,10 @@ module Bundler instance_of?(Path) end + def expanded_original_path + @expanded_original_path ||= expand(original_path) + end + private def expanded_path diff --git a/spec/bundler/source/path_spec.rb b/spec/bundler/source/path_spec.rb new file mode 100644 index 0000000000..1d13e03ec1 --- /dev/null +++ b/spec/bundler/source/path_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +RSpec.describe Bundler::Source::Path do + before do + allow(Bundler).to receive(:root) { Pathname.new("root") } + end + + describe "#eql?" do + subject { described_class.new("path" => "gems/a") } + + context "with two equivalent relative paths from different roots" do + let(:a_gem_opts) { { "path" => "../gems/a", "root_path" => Bundler.root.join("nested") } } + let(:a_gem) { described_class.new a_gem_opts } + + it "returns true" do + expect(subject).to eq a_gem + end + end + + context "with the same (but not equivalent) relative path from different roots" do + subject { described_class.new("path" => "gems/a") } + + let(:a_gem_opts) { { "path" => "gems/a", "root_path" => Bundler.root.join("nested") } } + let(:a_gem) { described_class.new a_gem_opts } + + it "returns false" do + expect(subject).to_not eq a_gem + end + end + end +end diff --git a/spec/install/gemfile/eval_gemfile_spec.rb b/spec/install/gemfile/eval_gemfile_spec.rb index 4f5a109e32..f02223d34d 100644 --- a/spec/install/gemfile/eval_gemfile_spec.rb +++ b/spec/install/gemfile/eval_gemfile_spec.rb @@ -42,6 +42,13 @@ RSpec.describe "bundle install with gemfile that uses eval_gemfile" do bundle! :install expect(the_bundle).to include_gem("a 1.0") end + + # Make sure that we are properly comparing path based gems between the + # parsed lockfile and the evaluated gemfile. + it "bundles with --deployment" do + bundle! :install + bundle! "install --deployment" + end end context "Gemfile uses gemspec paths after eval-ing a Gemfile" do |