Hey Andy, thanks for looking at this. Can I propose a different solution though?

In Rails 3, all your application paths are configurable using the config.paths method. So, the proper fix to this issue is indeed deprecate Rails.public_path, but however it should point to config.paths.public.

This is the method which defines the paths:

http://github.com/rails/rails/blob/master/railties/lib/rails/applic...

As you can see, it calls super, which gets the paths definitions from the engine:

http://github.com/rails/rails/blob/master/railties/lib/rails/engine...

All you need to do is define paths.public, deprecate Rails.public_path as you already did and ensure tests pass. Are you in mood for trying a new patch then?

Thanks!

Sure José, I want to start contributing to Rails so I'll definitely try it the new way. I'll have a go at whipping something up today. Thanks for the hints.

You're welcome! If you need any help, post here and I will reply as soon as possible.

I've tried to rework the patch, but I fail. The tests all pass except for a couple of metal ones (where it seems to return the index.html content - not sure that's me).

However, when I try to use stylesheet_link_tag (to test that asset_tag_helper is working) it throws a Nil exception (because Rails.public returns nil at that point).

I'm a bit stuck for why now, as this is my first real look in to Rails and I'm not sure of the startup order.

If you could have a look at my patch and make any suggestions as to where I'm going wrong, I'd appreciate it.

Andy, you get it right. The problem is with the load order. What you will need to do is to create a new initializer in "actionpack/lib/action_controller/railtie.rb" which sets the @@page_cache_directory. Something like:

initializer "action_controller.set_page_cache_directory" do |app|
  ActionController::Base.page_cache_directory = app.config.paths.public.to_a.first
end

Instead "app.config.paths.public.to_a.first", you may also do "Rails.public" as well. Good work so far!

It appears the metal failures (which now it becomes important I'll list them - test_multiple_metal_endpoints and test_single_metal_endpoint) are me as a vanilla master checkout passes them.

However this simple metal works fine:

Allow the metal piece to run in isolation

require File.expand_path('../../../config/environment', FILE) unless defined?(Rails)

class Test
def self.call(env)

if env["PATH_INFO"] =~ /^\/test\-metal/
  [200, {"Content-Type" => "text/html"}, ["Hello, World!"]]
else
  [404, {"Content-Type" => "text/html", "X-Cascade" => "pass"}, ["Not Found"]]
end

end end

José Thanks for that, I guess it's a hint rather than the whole solution, but I can't tell how to apply it to AssetTagHelper?

The helpers are all autoloaded, so they should only be defined on first access. That shouldn't happen until after the Rails module is defined, right?

Imagine that I have a gem that extends AssetTagHelper. I will load it before the application is loaded. In other words, AssetTagHelper can be loaded before the application is defined.

The best solution is to refactor AssertTagHelper to use something like public_path or page_cache_directory as well, that we can also be set in the Railtie.

Oh man alive, this is turning out in to quite a job for my first little patch to dip my toes in!

I'll have a bit of a deeper look at this problem next week then.

Sure! Fell free to make different patches too. You can have one to make AssetTagHelper more modular and then have a second patch which changes Rails.public_path.

If you're willing to offer help to a new developer like this, it might be better to take it to email/MSN rather than fill up lighthouse with it. My email address is andy@andyjeffries.co.uk and MSN is passport@andyjeffries.co.uk

The pros of having it in Lighthouse is that it may help other people as well to understand how Rails works. Anyway, I send you an e-mail in case you want to continue by e-mail. Both ways work for me.

I guess it might be a good idea to continue the discussion here. I find that I learn a lot just by following on the discussion.

It seems that Carl was faster and already refactored the whole configuration stuff. What's left is just to deprecate the public_path method (no need to add a Rails.public method).

OK, never mind - he's probably done a better job than me anyway (mine still wasn't working).

Did you ever speak to the Core guys to see if they wanted public_path deprecating?

AJ