Add bundler Dependency to gemspec

[sonalkr132]

bundle exec was added to acceptance test cause otherwise it was giving Gem::LoadError for conflicting bundler versions.

An alternate solution would have been to add it here:

 Bundler.with_clean_env do
   Process.spawn(
     env,
     command.to_s,   # may be change this line to "bundle exec #{command}"
     out:   stdout.last,
     err:   stderr.last,
     in:    :close,
     chdir: root.to_s,
   )
 end

I felt it was non-intuitive. Additionally, above code block doesn't get called only from Spring::Test::AcceptanceTest, so it would need changes on many different files.

EDIT:
Resolves: #275

[rails-bot]

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @rafaelfranca (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[grosser]

we always advertise to not use bundle exec ... so with_clean_env would be better :(

[sonalkr132]

I think error on travis (check this) can be on user side as well (?). Looking at the test won't help him a lot cause it won't be apparent that command passing in test is not actually:

bin/spring

but

bundle exec bin/spring

Also, some of the commands being passed to with_clean_env are:

# gives syntax error near unexpected token `('
# can be solved by just removing '('
(gem list bundler | grep bundler) || gem install bundler
# not sure from where does this gets passed but some jugglery will be needed    
bundle check || bundle update --retry=2 

## some gem install command

## more commands from other files calling the same app.run

[sonalkr132]

@grosser Do you really want me to change it to with_clean_env?
Actually, it needs just 2 lines of change.

[grosser]

I'm confused now :D
Do you think this is better or with_clean_env is better ?
I think either would work, I was just sad to see bundle exec being added all over the place + that it uses a pattern we try to discourage ...

[sonalkr132]

with_clean_env feels like we are secretly adding bundle exec to every command.
However, with_clean_env is lot cleaner and gets the job done. Also, issue of conflicting bundler version seems to be only specific for travis runs. Locally, I have bundler 1.10.6 and I don't get any error.

[sonalkr132]

@grosser I changed it to with_clean_env version. Can you please have a look?
Order of application.bundle and build_and_install_gems is changed cause to bundle and install gems we will need bundler first. Bundler is install by application.bundle.

[grosser]

looks good to me ... but could break all kinds of stuff ... @jonleighton want to have a look ?

[sonalkr132]

but could break all kinds of stuff

Are you referring to change of order of application.bundle and build_and_install_gems? Those things are only used for tests and has nothing to do with working of gem.

[grosser]

ahhh you are right! :D
so you don't need a new release after this ,right ?

[sonalkr132]

so you don't need a new release after this ,right?

TBH, I am not sure. It does add bundler as hard dependency.

[sonalkr132]

Shit! I just noticed it. I did reset hard and forgot to add the most important line -_-

[grosser]

:D

[sonalkr132]

Fixed it!

so you don't need a new release after this ,right?

I think it will need changelog that now spring has bundler as hard dependency.

[grosser]

👍

@jonleighton / @rafaelfranca looks good ?

[jonleighton]

Why change this? It makes it less readable IMO

[sonalkr132]

bundle exec (gem list bundler | grep bundler) || gem install bundler gives following error:

bash: syntax error near unexpected token `('

[sonalkr132]

Can someone please suggest tests so that #456 doesn't happen again? I can't think of anything reasonable.

[sonalkr132]

closed in favor of #458