summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThe Bundler Bot <bot@bundler.io>2017-07-05 14:17:41 +0000
committerThe Bundler Bot <bot@bundler.io>2017-07-05 14:17:41 +0000
commitfb3384649d866f08cb59683be8a711290aa75c07 (patch)
tree87facd1dcb37f3c12c84056eccc918b7656694de
parentb0883924ee284d2d2f6e8848ac2994085c95035a (diff)
parent3db89cafa715527ff0066efb38fdeec1f07119f1 (diff)
downloadbundler-fb3384649d866f08cb59683be8a711290aa75c07.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
-rw-r--r--lib/bundler/source/path.rb6
-rw-r--r--spec/bundler/source/path_spec.rb31
-rw-r--r--spec/install/gemfile/eval_gemfile_spec.rb7
3 files changed, 43 insertions, 1 deletions
diff --git a/lib/bundler/source/path.rb b/lib/bundler/source/path.rb
index 84a98420a9..323e60e83e 100644
--- a/lib/bundler/source/path.rb
+++ b/lib/bundler/source/path.rb
@@ -65,7 +65,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
@@ -119,6 +119,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 b76fae02dc..242b746e01 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