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

Fix the path to the Gemfile during evaluation. #3349

Closed
wants to merge 2 commits into from

Conversation

cstrahan
Copy link

Bundler.default_gemfile was being used instead of the explicitly passed Gemfile path.

This prevents errors like the following when gemfile argument isn't "$PWD/Gemfile":

/nix/store/na27qjv9g32rg2nh98m51jd7ihrg7w7n-ruby-2.1.3-p0-bundler-1.7.9/lib/ruby/gems/2.1.3/gems/bundler-1.7.9/lib/bundler/shared_helpers.rb:26:in `default_gemfile': Could not locate Gemfile (Bundler::GemfileNotFound)
        from /nix/store/na27qjv9g32rg2nh98m51jd7ihrg7w7n-ruby-2.1.3-p0-bundler-1.7.9/lib/ruby/gems/2.1.3/gems/bundler-1.7.9/lib/bundler.rb:262:in `default_gemfile'
        from /nix/store/na27qjv9g32rg2nh98m51jd7ihrg7w7n-ruby-2.1.3-p0-bundler-1.7.9/lib/ruby/gems/2.1.3/gems/bundler-1.7.9/lib/bundler/dsl.rb:27:in `initialize'
        from /nix/store/na27qjv9g32rg2nh98m51jd7ihrg7w7n-ruby-2.1.3-p0-bundler-1.7.9/lib/ruby/gems/2.1.3/gems/bundler-1.7.9/lib/bundler/dsl.rb:9:in `new'
        from /nix/store/na27qjv9g32rg2nh98m51jd7ihrg7w7n-ruby-2.1.3-p0-bundler-1.7.9/lib/ruby/gems/2.1.3/gems/bundler-1.7.9/lib/bundler/dsl.rb:9:in `evaluate'
        from /nix/store/na27qjv9g32rg2nh98m51jd7ihrg7w7n-ruby-2.1.3-p0-bundler-1.7.9/lib/ruby/gems/2.1.3/gems/bundler-1.7.9/lib/bundler/definition.rb:25:in `build'
        from /nix/store/yzpjmgpc5fdhll3rbacq5r5ay9kjxxym-ruby-2.1.3-p0-bundix-0.1.0/lib/ruby/gems/2.1.3/gems/bundix-0.1.0/lib/bundix/cli.rb:55:in `expr'
        from /nix/store/hkqpgmrq1x2c8icjhl433d2yhkzy33rn-ruby-2.1.3-p0-thor-0.19.1/lib/ruby/gems/2.1.3/gems/thor-0.19.1/lib/thor/command.rb:27:in `run'
        from /nix/store/hkqpgmrq1x2c8icjhl433d2yhkzy33rn-ruby-2.1.3-p0-thor-0.19.1/lib/ruby/gems/2.1.3/gems/thor-0.19.1/lib/thor/invocation.rb:126:in `invoke_command'
        from /nix/store/hkqpgmrq1x2c8icjhl433d2yhkzy33rn-ruby-2.1.3-p0-thor-0.19.1/lib/ruby/gems/2.1.3/gems/thor-0.19.1/lib/thor.rb:359:in `dispatch'
        from /nix/store/hkqpgmrq1x2c8icjhl433d2yhkzy33rn-ruby-2.1.3-p0-thor-0.19.1/lib/ruby/gems/2.1.3/gems/thor-0.19.1/lib/thor/base.rb:440:in `start'
        from /nix/store/yzpjmgpc5fdhll3rbacq5r5ay9kjxxym-ruby-2.1.3-p0-bundix-0.1.0/lib/ruby/gems/2.1.3/gems/bundix-0.1.0/bin/bundix:4:in `<main>'

`Bundler.default_gemfile` was being used instead of the explicitly
passed Gemfile path.
@cstrahan
Copy link
Author

One thing ea3ded9 doesn't address is when you do something like:

gem "pg", :path => "../pg"

gemspec looks for gemspecs relative to the Gemfile, so I believe the above path should also be relative to the Gemfile.

@cstrahan
Copy link
Author

The last commit (b233205) implements what I suggested earlier: all paths during Gemfile evaluation will be expanded relative to the Gemfile.

@cstrahan
Copy link
Author

One problem with b233205 - the path sources will be full paths, which leaks into the lock files and breaks the tests.

I'm not sure what the best way to fix this is. If we want to support gemfiles outside of $PWD (which, for a couple upcoming PRs, I believe we do), we need to handle the fact that paths relative to the Gemfile won't be the same as paths relative to $PWD. Using full paths fixes this, but then we need a way to drop back to paths relative to the Gemfile when we create the Gemfile.lock.

Could someone lend me a hand with this? I'm on IRC (as cstrahan) if someone would like to discuss this.

@indirect
Copy link
Member

Thanks for the patch! The change seems like an improvement in general, but it definitely needs to handle paths carefully. We've had a lot of issues with relative paths in the past, and it is absolutely required that relative paths in the Gemfile stay relative paths in the lock. They should be expanded for use while Bundler is running, but they should always be stored in their original form so that they stay compatible across machines. We can't merge a pull request that expands path sources unless it maintains the relative paths inside the lock file.

@indirect
Copy link
Member

indirect commented Apr 1, 2015

To follow up here, if you can fix the tests, this is mergable, but if you can't it's not.

homu added a commit that referenced this pull request Jan 31, 2016
Fix the path to the Gemfile during evaluation.

This is a continuation of #3349 Following up per: #3349 (comment)

The issue here is that paths used with `Bundler::Dsl#gemspec` will only work when the Gemfile being evaluated is `Bundler.default_gemfile`.

Passing a Gemfile other than `Bundler.default_gemfile` to `Bundler::Dsl.evaluate` will break uses of `path:` options in the Gemfile.

These changes update `Bundler::Dsl` to remember the Gemfile passed into `eval_gemfile` and use it to resolve relative paths.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants