Skip to content
Permalink
Browse files
Install binstubs by default
  • Loading branch information
wycats committed Dec 22, 2012
1 parent cba0588 commit f34c27a452418d8aa17f92bb0fd7ae97b5f7e252
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
@@ -261,7 +261,7 @@ def bundle_command(command)
end

def run_bundle
bundle_command('install') unless options[:skip_gemfile] || options[:skip_bundle] || options[:pretend]
bundle_command('install --binstubs') unless options[:skip_gemfile] || options[:skip_bundle] || options[:pretend]
end

def empty_directory_with_keep_file(destination, config = {})
@@ -27,7 +27,7 @@ def test_skeleton_is_created
end

def test_generation_runs_bundle_install
generator([destination_root]).expects(:bundle_command).with('install').once
generator([destination_root]).expects(:bundle_command).with('install --binstubs').once
quietly { generator.invoke_all }
end

@@ -101,14 +101,14 @@ def test_template_is_executed_when_supplied_an_https_path
end

def test_dev_option
generator([destination_root], dev: true).expects(:bundle_command).with('install').once
generator([destination_root], dev: true).expects(:bundle_command).with('install --binstubs').once
quietly { generator.invoke_all }
rails_path = File.expand_path('../../..', Rails.root)
assert_file 'Gemfile', /^gem\s+["']rails["'],\s+path:\s+["']#{Regexp.escape(rails_path)}["']$/
end

def test_edge_option
generator([destination_root], edge: true).expects(:bundle_command).with('install').once
generator([destination_root], edge: true).expects(:bundle_command).with('install --binstubs').once
quietly { generator.invoke_all }
assert_file 'Gemfile', %r{^gem\s+["']rails["'],\s+github:\s+["']#{Regexp.escape("rails/rails")}["']$}
end

11 comments on commit f34c27a

@steveklabnik
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@bensie
Copy link
Contributor

@bensie bensie commented on f34c27a Dec 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered making this the default in Bundler instead of the initial Rails app generator? This may cause unexpected behavior for those new to Rails - where they have binstubs initially but subsequent calls to bundle install/update doesn't refresh them or add new ones.

@indirect
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bensie that's not how bundler works. The flag passed to install will be remembered, and later calls to install or update will update the binstubs.

@wycats
Copy link
Member Author

@wycats wycats commented on f34c27a Dec 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bensie we can't make it the default in bundler because some project (especially gems) have their own bin directories, which would be clobbered.

@bensie
Copy link
Contributor

@bensie bensie commented on f34c27a Dec 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@indirect @wycats Makes sense. Remembering the flag passed to install sounds great. Is that part a Rails or Bundler thing?

@guilleiguaran
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bensie is part of Bundler.

Also we can update docs to tell to developers that they may want to use bundle install --binstubs instead of bundle install the first time they checkout a Rails project from VCS.

@dhh
Copy link
Member

@dhh dhh commented on f34c27a Dec 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most projects should just checkin the bin stubs.

@bensie
Copy link
Contributor

@bensie bensie commented on f34c27a Dec 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhh Commit 35c554f encourages the opposite.

@wycats
Copy link
Member Author

@wycats wycats commented on f34c27a Dec 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with checking in binstubs in general is that the shebang line is system specific (special for windows, rbenv) so your team would need to have a very consistent setup to avoid problems.

@dhh
Copy link
Member

@dhh dhh commented on f34c27a Dec 23, 2012 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhh
Copy link
Member

@dhh dhh commented on f34c27a Dec 26, 2012 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.