Loading pattern for connection adapters that plays nicely with bundler --standalone #7755

Closed
wants to merge 1 commit into
from

Projects

None yet

8 participants

@saimonmoore
Contributor

Hi

I'm using the relatively new --standalone feature/option of bundler so as to avoid extracting a part of my rails project into a separate project. My only requirement is to not have to load rails entirely:

By creating a custom gemfile (subset of your typical gemfile) and executing:

bundle install --gemfile=Gemfile.my_subset --standalone

bundler will create a ruby script which requires all the gems in your gemfile subset using the
original gems locations.

e.g. :

path = File.expand_path('..', __FILE__)
$:.unshift File.expand_path("#{path}/../ruby/1.9.1/gems/rake-0.9.2.2/lib")
$:.unshift File.expand_path("#{path}/../ruby/1.9.1/gems/multi_json-1.0.4/lib")
$:.unshift File.expand_path("#{path}/../ruby/1.9.1/gems/activesupport-3.1.6/lib")
$:.unshift File.expand_path("#{path}/../ruby/1.9.1/gems/builder-3.0.3/lib")
$:.unshift File.expand_path("#{path}/../ruby/1.9.1/gems/i18n-0.6.0/lib")
$:.unshift File.expand_path("#{path}/../ruby/1.9.1/gems/activemodel-3.1.6/lib")
$:.unshift File.expand_path("#{path}/../ruby/1.9.1/gems/arel-2.2.3/lib")
$:.unshift File.expand_path("#{path}/../ruby/1.9.1/gems/tzinfo-0.3.33/lib")
$:.unshift File.expand_path("#{path}/../ruby/1.9.1/gems/activerecord-3.1.6/lib")
...

This allows scripts to e.g. load just activerecord without setting up a separate
gemfile.

The issue here is that with this setup script, you aren't using bundler (to avoid loading rails and
the gems in your typical gemfile.). Additionally, the gems the script requires are expanded gems
and have not been installed via gem install.

Activerecord connection adapters have this code:

gem 'mysql2', '~> 0.3.10'
require 'mysql2'

This code assumes:

  • You're running the code within bundler (recommended)
  • You have the mysql2 gem installed as a gem in your system

When using bundler standalone neither of these cases are true so the app breaks with:

Please install the mysql2 adapter: gem install activerecord-mysql2-adapter (no such file to load -- active_record/connection_adapters/mysql2_adapter)
because it hits the 'gem' call before it attempts to require 'mysql2'

IMHO a better approach would be:

begin
  require 'mysql2'
rescue LoadError
  gem 'mysql2', '~> 0.3.10'
  require 'mysql2'
end

This code assumes you already have mysql2 on your load path somehow and only if it raises an exception do we attempt to issue the gem call.

Is this reasonable?

I've coded up the changes against 3-1-stable (which is what we're using in production) but I can easily backport it to master (elsewhere) if people see this as a good idea.

I had the same test failures before and after my changes: https://gist.github.com/3782040 (I tested sqlite3 and mysql/2)

Regards,

Saimon

Note: See issue #7753 for original discussion.

@steveklabnik
Member

Are there any other places in Rails that we use gem? Why are we using gem in the first place?

@steveklabnik
Member

Hey @indirect, do you have any thoughts on this patch?

@indirect
Member

Hoooly shit please don't use gem, ever. That method exists so you can make application-level demands of Rubygems. Gems should only ever require 'foo', and let the code that required them in the first place handle setting up the load path for those other dependencies, etc.

Good patch. :)

@indirect
Member

Aaron said pretty much exactly what I said, from the opposite direction -- the adapter code has hidden dependencies on Rubygems (the library) to provide the gem method, and on a specific version of the mysql itself. Bundler can obviously handle this hidden dependency externally, so the PR (or the suggested patch in the mailing list discussion) seems like it would work.

@steveklabnik
Member

Oh, this pull request is for 3-1-stable, so it's not going to get accepted anyway. That said, I am willing to port it to master if @saimonmoore doesn't want to, once we determine exactly what we need. I'll keep it open till we figure that out.

@saimonmoore
Contributor

I'm willing to port it to master if it has a chance of getting in :)

@parndt
Contributor
parndt commented Mar 5, 2013

This should totally get ported/merged to master, if it hasn't been already.

@wfarr
Contributor
wfarr commented Mar 28, 2013

The concerns @tenderlove mentioned are still relevant even if this changeset was directly copied to master. The failure case of "well, gem couldn't find it so let's just load it and hope" isn't a particularly good one.

I'm not sure what core's opinion on this would be, but I'd like to propose making the activerecord-adapter-* libraries into proper gems and letting the dependencies live in the gemspecs for each particular adapter. This would be a breaking change in that the adapter would have to be specified directly in the Gemfile which may make it a candidate for Rails 4 inclusion still (I'm unsure on that). This would allow Rubygems or Bundler to worry about the version dependencies of these libraries rather than doing it in Rails itself.

If this is an avenue that the core team is interested in pursuing for inclusion into Rails 4, I'd be happy to help.

@laurocaetano
Contributor

I'm closing this PR since Rails 3.1 is not under maintenance anymore.
You can can open a new PR targeting master if it's still a problem.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment