Changeset #5738 Breaks plugins loading (Still)

Continuing from http://groups.google.com/group/rubyonrails-core/browse_frm/thread/f6a655492e571aa2

This is the second time this plugin loading patch was committed and is still broken. I can only reproduce the error running in production mode.

Can changeset #5738 please be revert (again).

What's the error you receive? The previous one no longer occurs for me in any environment, including production.

I get this error locally and on my production server. Yes, I run a production server on edge, :slight_smile:

http://pastie.caboo.se/28648

Can you provide production.rb too?

Was something new added to the default production config?

http://pastie.caboo.se/28654

Nope: http://dev.rubyonrails.org/svn/rails/trunk/railties/environments/production.rb

Have you require'd your paypal library at _the top_ of your environment.rb?

-- tim

Removing "Paypal::Notification.ipn_url = .." removes the error.

Has the order of loading plugins changed? Do they load after all my custom config?

It looks like "/vendor/plugins/*/lib/" has been remove from the path.

Every works fine the way I have it on #5737.

Ah, this is the problem here.

Unit tests still pass and environment has access to plugins.

Should I submit the patch?- Parked at Loopia

We actually just had a patch that removed that line. Why is it necessary for your plugin to operate? Plugin lib directories are added to the load path(s) as each plugin is initialized (see Rails::Initializer#load_plugin), and the line you highlight actually adds ALL plugin lib directories to the load path (*again*).

If you could figure out why your application/plugin NEEDS all plugin lib directories to be in the load path twice, that would explain what the real problem is.

For what it's worth, please don't revert the whole changeset, as it contains other useful functionality that isn't relevant to the problem here. If we can't figure out why there are some dependencies issues, then adding the removed set of duplicate load paths should be sufficient (i.e. reverting the patch from ticket #6842)

James Adam wrote:

If you could figure out why your application/plugin NEEDS all plugin lib directories to be in the load path twice, that would explain what the real problem is.

I don't need everything to be load twice. I need the plugins to be loaded before it reads the bottom of my environment file. This was the behavior before the patch.

Ah. I see the problem. Josh is referring to a Constant in his production.rb environment file which is only available in a plugin. However, because the environment/*.rb files are loaded BEFORE plugins are loaded, none of these constants will be available.

This presents a bit of a chicken-and-egg problem, because it makes sense (and certainly seems to be good practice) to add environment-specific configuration in this files, but without effectively ignoring config.plugins while they load, we'll always have errors like this.

Hmm.

For 1.2, it seems better to re-add ALL plugins to the load path than try and resolve this tangle. It seems more likely that this could be cleaned up in a release where things are expected to break. I've added a patch to Ticket #6856 which re-adds this plugin lib directories to the load path; I'd suggest committing this until a "better" solution to this problem can be provided.

Sounds good.

I have nothing against all the features but everything submited to 1.2 should be backward compatible (unless noted on rails blog).

Can't this be solved with after_initialize? I do this for any non-rails specifics within environment configs:

config.after_initialize do    SystemPaths.smtp_conf_file = "/etc/conf/smtp.conf" end

-- tim

Can't this be solved with after_initialize? I do this for any non- rails specifics within environment configs:

config.after_initialize do    SystemPaths.smtp_conf_file = "/etc/conf/smtp.conf" end

That's exactly right. config.after_initialize is there for this reason. Even before the load paths were changed, init.rb still wasn't fired until after the initializer block had completely run. So if you had a plugin which 'monkeypatched' some core files in init.rb, those changes weren't visible in production.rb.

Awesome.

My problem is solved but do we still want to include it with 1.2? You could leave it in edge but remove revert it in the release candidate.

Awesome.

My problem is solved but do we still want to include it with 1.2? You could leave it in edge but remove revert it in the release candidate.

I'm not convinced it's worth pulling from the release. When I added config.after_initialize it was because a plugin I was using defined some constants in init.rb which I was trying to use in production.rb.

So anything which is triggered by init.rb wasn't previously available, your case worked because all it had to do was require a file, more complicated stuff wouldn't have.

What are other people's take on this?

Can't this be solved with after_initialize? I do this for any non- rails specifics within environment configs:

config.after_initialize do    SystemPaths.smtp_conf_file = "/etc/conf/smtp.conf" end

How about config.to_prepare for things that need to happen each time the dispatcher is reloaded, such as a call to Liquid::Template.register_tag?

This is what bit me, same thing, the Liquid plugin isn't loaded at the time this runs.

I've written about this further, for those that find the Rails initialisation process mysterious: http://toolmantim.com/article/2006/12/27/environments_and_the_rails_initialisation_process

I hope the rdocs for Rails::Initializer and Rails::Configuration are included in api.rubyonrails.org for 1.2, and their rdocs make sense and that are referenced from the rdoc README and/or generated environment templates.

-- tim