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?- http://pastie.caboo.se/28668

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