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

Remove ruby version check from "bundle exec"? #2952

Closed
nirvdrum opened this issue Mar 29, 2014 · 4 comments
Closed

Remove ruby version check from "bundle exec"? #2952

nirvdrum opened this issue Mar 29, 2014 · 4 comments

Comments

@nirvdrum
Copy link

This is a continuation to #2951. Is there any reason to do a Ruby version check when running "bundle exec" at all? As far as I can tell there's two situations:

  • Running a command that does load the Gemfile, such as "bundle exec rails c"
  • Running a command that doesn't load the Gemfile at all, such as "bundle exec ruby -v"

I realize I'm stating the obvious there. But in the former case, the Ruby version check will be performed by the subcommand anyway. As of today, it's effectively being run twice. In the latter case, where the Gemfile isn't being loaded, you wouldn't see the error you see today, but does that really matter? Bundler isn't being used for anything other than a command dispatcher in that case.

@indirect
Copy link
Member

Bundle exec has to load the bundled environment to know what PATH to use for commands that it’s going to exec. I think the Ruby version check is an incidental part of that, but it shouldn’t be required.

On Mar 29, 2014, at 9:17 AM, Kevin Menard notifications@github.com wrote:

This is a continuation to #2951. Is there any reason to do a Ruby version check when running "bundle exec" at all? As far as I can tell there's two situations:

Running a command that does load the Gemfile, such as "bundle exec rails c"
Running a command that doesn't load the Gemfile at all, such as "bundle exec ruby -v"
I realize I'm stating the obvious there. But in the former case, the Ruby version check will be performed by the subcommand anyway. As of today, it's effectively being run twice. In the latter case, where the Gemfile isn't being loaded, you wouldn't see the error you see today, but does that really matter? Bundler isn't being used for anything other than a command dispatcher in that case.


Reply to this email directly or view it on GitHub.

@nirvdrum
Copy link
Author

Got it. Those are two separate lines. The version check seems to be deliberate. If I remove that, the environment is still loaded. I'll pull together a PR for review.

@segiddins
Copy link
Member

Removing that Bundler.definition.validate_ruby! line yields:

  2) bundle platform bundle exec fails when the ruby version doesn't match
     Failure/Error: expect(exitstatus).to eq(18) if opts[:exitstatus]

       expected: 18
            got: 1

       (compared using ==)
     # ./spec/other/platform_spec.rb:209:in `should_be_ruby_version_incorrect'
     # ./spec/other/platform_spec.rb:830:in `block (3 levels) in <top (required)>'

  3) bundle platform bundle exec fails when the engine doesn't match
     Failure/Error: expect(exitstatus).to eq(18) if opts[:exitstatus]

       expected: 18
            got: 1

       (compared using ==)
     # ./spec/other/platform_spec.rb:214:in `should_be_engine_incorrect'
     # ./spec/other/platform_spec.rb:841:in `block (3 levels) in <top (required)>'

  4) bundle platform bundle exec fails when the engine version doesn't match
     Failure/Error: expect(exitstatus).to eq(18) if opts[:exitstatus]

       expected: 18
            got: 1

       (compared using ==)
     # ./spec/other/platform_spec.rb:219:in `should_be_engine_version_incorrect'
     # ./spec/other/platform_spec.rb:853:in `block (4 levels) in <top (required)>'
     # ./spec/support/helpers.rb:327:in `simulate_ruby_engine'
     # ./spec/other/platform_spec.rb:845:in `block (3 levels) in <top (required)>'

  5) bundle platform bundle exec fails when patchlevel doesn't match
     Failure/Error: expect(exitstatus).to eq(18) if opts[:exitstatus]

       expected: 18
            got: 1

       (compared using ==)
     # ./spec/other/platform_spec.rb:224:in `should_be_patchlevel_incorrect'
     # ./spec/other/platform_spec.rb:866:in `block (3 levels) in <top (required)>'

@segiddins
Copy link
Member

@indirect are those spec failures OK?

homu added a commit that referenced this issue Feb 4, 2016
[Exec] Avoid loading the definition before exec-ing

Closes #2951, #2952.

@indirect it feels like I'm missing something?
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants