-
-
Notifications
You must be signed in to change notification settings - Fork 529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Faster, more correct bundler gem and Gemfile install #497
Conversation
@@ -7,8 +7,8 @@ | |||
set -e | |||
|
|||
# Set up Ruby dependencies via Bundler | |||
gem list bundler --installed > /dev/null || gem install bundler | |||
bundle install | |||
command -v bundle > /dev/null || gem install bundler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
command -v
is insufficient if you are an rbenv user. It will pass if you have rbenv installed in any ruby, as the shim for bundler will be in the path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good to know. I cargo-culted it from lower down in the script.
I can change the PR to then
- add
bundle check
- apply the regex fix to
gem list
@jferris suggested gem install bundler --conservative
but I think that is unrelated to this PR, which was originally just about bundle check
'ing "all the things" :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is related.
The gem list bundler
line being changed here is to make sure Bundler gets installed. It looks like RubyGems has that functionality built-in (install a gem, only if it's missing), and the flag for it is --conservative
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use gem install --conservative bundler
in Paperback's bin/setup
script. Maybe @trevororeilly has thoughts on it.
On Thu, Jan 22, 2015 at 2:10 PM, Joe Ferris [email protected]
wrote:
In templates/bin_setup.erb
#497 (comment):@@ -7,8 +7,8 @@
set -eSet up Ruby dependencies via Bundler
-gem list bundler --installed > /dev/null || gem install bundler
-bundle install
+command -v bundle > /dev/null || gem install bundlerI think it is related.
The gem list bundler line being changed here is to make sure Bundler gets
installed. It looks like RubyGems has that functionality built-in (install
a gem, only if it's missing), and the flag for it is --conservative.—
Reply to this email directly or view it on GitHub
https://github.com/thoughtbot/suspenders/pull/497/files#r23414815.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I accidentally the gem docs.
|
My local numbers are much faster for each case, and I agree with @derekprior that it's more important to be correct than half a second faster in this case. One other option we haven't explored is the
|
- About 3x faster when gems are available - Does not return false positive when other gems on system have 'bundler' in name Tested on my 2015 macbook pro, Mavericks, with RVM and Ruby 2.1.5 running via bash See details in thoughtbot#497
Updated per discussion. Also removed the now incorrect and wordy comments from the commit message. |
I first noticed the Looks like the same approach as what's proposed here. |
The current version looks pretty good to me. @croaky any thoughts? |
- About 3x faster when gems are available - Does not return false positive when other gems on system have 'bundler' in name Tested on my 2015 Macbook Pro, Mavericks, with RVM and Ruby 2.1.5 running via bash See details in #497
Turn out, |
I'm surprised |
I think that, when generating a brand new Rails project, we probably want to fetch the gems index so that we install with the latest gems. |
Indeed, I thought we were talking about suspenders' CI. |
Hmm, yeah, it looks like generating a Suspenders app does its own Using |
- About 3x faster when gems are available - Does not return false positive when other gems on system have 'bundler' in name Tested on my 2015 Macbook Pro, Mavericks, with RVM and Ruby 2.1.5 running via bash See details in thoughtbot/suspenders#497
Updated: per discussion, to use
gem install bundler --conservative
as it works with rbenv and will only install the gem if not installed.Tested on my 2015 macbook pro, Mavericks, with RVM and Ruby 2.1.5 running via bash
Regarding the
gem uninstall -aIx gem_name
command used belowBundler not installed
gem list bundler --installed, 0.619*
but note it is falsely returning 'true' because I have other gems on the system with 'bundler' in the name
gem list ^bundler$ --installed, 0.604
command -v bundle, 0.000
Bundler not installed, installing bundler
gem list bundler --installed || gem install bundler, 0.621*
again, it is falsely only executing the LHS because I have other gems on the system with 'bundler' in the name
gem list ^bundler$ --installed || gem install bundler, 4.695
command -v bundle || gem install bundler, 4.051
Bundler installed, Gemfile containing "gem 'dotenv-rails', '1.0.2'"
Gems Not Installed
Without Gemfile.lock
bundle, 2.706
bundle check || bundle install, 2.423
With Gemfile.lock
bundle, 3.176
bundle check || bundle install, 2.880
Gems Installed
As the time to resolve and install gems increases,
bundle check || bundle
's speedadvantage becomes more impressive. As a bonus,
bundle check
will update theGemfile.lock
per changes in the Gemfile and all dependent gem versions are available locally.
Without Gemfile.lock
bundle, 1.519
bundle check || bundle install, 0.612
With Gemfile.lock
bundle, 0.669
``bash
bundle
time bundle; echo $?
bundle check || bundle install, 0.596