diff options
-rw-r--r-- | railties/CHANGELOG.md | 9 | ||||
-rw-r--r-- | railties/lib/rails/engine.rb | 12 | ||||
-rw-r--r-- | railties/lib/rails/engine/configuration.rb | 24 | ||||
-rw-r--r-- | railties/lib/rails/paths.rb | 44 | ||||
-rw-r--r-- | railties/test/application/initializers/load_path_test.rb | 2 | ||||
-rw-r--r-- | railties/test/application/paths_test.rb | 10 | ||||
-rw-r--r-- | railties/test/paths_test.rb | 56 |
7 files changed, 91 insertions, 66 deletions
diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 3b0f282143..ab30962aad 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,5 +1,14 @@ ## Rails 4.0.0 (unreleased) ## +* Deprecate the `eager_load_paths` configuration and alias it to `autoload_paths`. + Since the default in Rails 4.0 is to run in 'threadsafe' mode we need to eager + load all of the paths in `autoload_paths`. This may have unintended consequences + if you have added 'lib' to `autoload_paths` such as loading unneeded code or + code intended only for development and/or test environments. If this applies to + your application you should thoroughly check what is being eager loaded. + + *Andrew White* + * Restore Rails::Engine::Railties#engines with deprecation to ensure compatibility with gems such as Thinking Sphinx Fix #8551 diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 46a6485c44..8a6d1f34aa 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -34,9 +34,8 @@ module Rails # == Configuration # # Besides the +Railtie+ configuration which is shared across the application, in a - # <tt>Rails::Engine</tt> you can access <tt>autoload_paths</tt>, <tt>eager_load_paths</tt> - # and <tt>autoload_once_paths</tt>, which, differently from a <tt>Railtie</tt>, are scoped to - # the current engine. + # <tt>Rails::Engine</tt> you can access <tt>autoload_paths</tt> and <tt>autoload_once_paths</tt>, + # which, differently from a <tt>Railtie</tt>, are scoped to the current engine. # # class MyEngine < Rails::Engine # # Add a load path for this specific Engine @@ -456,9 +455,9 @@ module Rails end # Eager load the application by loading all ruby - # files inside eager_load paths. + # files inside autoload_paths. def eager_load! - config.eager_load_paths.each do |load_path| + config.autoload_paths.each do |load_path| matcher = /\A#{Regexp.escape(load_path)}\/(.*)\.rb\Z/ Dir.glob("#{load_path}/**/*.rb").sort.each do |file| require_dependency file.sub(matcher, '\1') @@ -558,7 +557,6 @@ module Rails # Freeze so future modifications will fail rather than do nothing mysteriously config.autoload_paths.freeze - config.eager_load_paths.freeze config.autoload_once_paths.freeze end @@ -671,7 +669,7 @@ module Rails end def _all_autoload_paths #:nodoc: - @_all_autoload_paths ||= (config.autoload_paths + config.eager_load_paths + config.autoload_once_paths).uniq + @_all_autoload_paths ||= (config.autoload_paths + config.autoload_once_paths).uniq end def _all_load_paths #:nodoc: diff --git a/railties/lib/rails/engine/configuration.rb b/railties/lib/rails/engine/configuration.rb index 10d1821709..2b23d8be38 100644 --- a/railties/lib/rails/engine/configuration.rb +++ b/railties/lib/rails/engine/configuration.rb @@ -4,7 +4,7 @@ module Rails class Engine class Configuration < ::Rails::Railtie::Configuration attr_reader :root - attr_writer :middleware, :eager_load_paths, :autoload_once_paths, :autoload_paths + attr_writer :middleware, :autoload_once_paths, :autoload_paths def initialize(root=nil) super() @@ -39,16 +39,16 @@ module Rails @paths ||= begin paths = Rails::Paths::Root.new(@root) - paths.add "app", eager_load: true, glob: "*" + paths.add "app", autoload: true, glob: "*" paths.add "app/assets", glob: "*" - paths.add "app/controllers", eager_load: true - paths.add "app/helpers", eager_load: true - paths.add "app/models", eager_load: true - paths.add "app/mailers", eager_load: true + paths.add "app/controllers", autoload: true + paths.add "app/helpers", autoload: true + paths.add "app/models", autoload: true + paths.add "app/mailers", autoload: true paths.add "app/views" - paths.add "app/controllers/concerns", eager_load: true - paths.add "app/models/concerns", eager_load: true + paths.add "app/controllers/concerns", autoload: true + paths.add "app/models/concerns", autoload: true paths.add "lib", load_path: true paths.add "lib/assets", glob: "*" @@ -76,7 +76,13 @@ module Rails end def eager_load_paths - @eager_load_paths ||= paths.eager_load + ActiveSupport::Deprecation.warn "eager_load_paths is deprecated and all autoload_paths are now eagerly loaded." + autoload_paths + end + + def eager_load_paths=(paths) + ActiveSupport::Deprecation.warn "eager_load_paths is deprecated and all autoload_paths are now eagerly loaded." + self.autoload_paths = paths end def autoload_once_paths diff --git a/railties/lib/rails/paths.rb b/railties/lib/rails/paths.rb index de6795eda2..80ba144441 100644 --- a/railties/lib/rails/paths.rb +++ b/railties/lib/rails/paths.rb @@ -5,13 +5,13 @@ module Rails # paths by a Hash like API. It requires you to give a physical path on initialization. # # root = Root.new "/rails" - # root.add "app/controllers", eager_load: true + # root.add "app/controllers", autoload: true # # The command above creates a new root object and add "app/controllers" as a path. # This means we can get a <tt>Rails::Paths::Path</tt> object back like below: # # path = root["app/controllers"] - # path.eager_load? # => true + # path.autoload? # => true # path.is_a?(Rails::Paths::Path) # => true # # The +Path+ object is simply an enumerable and allows you to easily add extra paths: @@ -30,7 +30,7 @@ module Rails # root["config/routes"].inspect # => ["config/routes.rb"] # # The +add+ method accepts the following options as arguments: - # eager_load, autoload, autoload_once and glob. + # autoload, autoload_once and glob. # # Finally, the +Path+ object also provides a few helpers: # @@ -85,7 +85,8 @@ module Rails end def eager_load - filter_by(:eager_load?) + ActiveSupport::Deprecation.warn "eager_load is deprecated and all autoload_paths are now eagerly loaded." + filter_by(:autoload?) end def autoload_paths @@ -124,9 +125,13 @@ module Rails @glob = options[:glob] options[:autoload_once] ? autoload_once! : skip_autoload_once! - options[:eager_load] ? eager_load! : skip_eager_load! options[:autoload] ? autoload! : skip_autoload! options[:load_path] ? load_path! : skip_load_path! + + if !options.key?(:autoload) && options.key?(:eager_load) + ActiveSupport::Deprecation.warn "the :eager_load option is deprecated and all :autoload paths are now eagerly loaded." + options[:eager_load] ? autoload! : skip_autoload! + end end def children @@ -143,22 +148,37 @@ module Rails expanded.last end - %w(autoload_once eager_load autoload load_path).each do |m| + %w(autoload_once autoload load_path).each do |m| class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{m}! # def eager_load! - @#{m} = true # @eager_load = true + def #{m}! # def autoload! + @#{m} = true # @autoload = true end # end # - def skip_#{m}! # def skip_eager_load! - @#{m} = false # @eager_load = false + def skip_#{m}! # def skip_autoload! + @#{m} = false # @autoload = false end # end # - def #{m}? # def eager_load? - @#{m} # @eager_load + def #{m}? # def autoload? + @#{m} # @autoload end # end RUBY end + def eager_load! + ActiveSupport::Deprecation.warn "eager_load paths are deprecated and all autoload paths are now eagerly loaded." + autoload! + end + + def skip_eager_load! + ActiveSupport::Deprecation.warn "eager_load paths are deprecated and all autoload paths are now eagerly loaded." + skip_autoload! + end + + def eager_load? + ActiveSupport::Deprecation.warn "eager_load paths are deprecated and all autoload paths are now eagerly loaded." + autoload? + end + def each(&block) @paths.each(&block) end diff --git a/railties/test/application/initializers/load_path_test.rb b/railties/test/application/initializers/load_path_test.rb index 31811e7f92..595f58bd91 100644 --- a/railties/test/application/initializers/load_path_test.rb +++ b/railties/test/application/initializers/load_path_test.rb @@ -64,7 +64,7 @@ module ApplicationTests add_to_config <<-RUBY config.root = "#{app_path}" - config.eager_load_paths << "#{app_path}/lib" + config.autoload_paths << "#{app_path}/lib" RUBY require "#{app_path}/config/environment" diff --git a/railties/test/application/paths_test.rb b/railties/test/application/paths_test.rb index 4029984ce9..f6a77a0d17 100644 --- a/railties/test/application/paths_test.rb +++ b/railties/test/application/paths_test.rb @@ -54,11 +54,11 @@ module ApplicationTests assert_equal root("app", "controllers"), @paths["app/controllers"].expanded.first end - test "booting up Rails yields a list of paths that are eager" do - eager_load = @paths.eager_load - assert eager_load.include?(root("app/controllers")) - assert eager_load.include?(root("app/helpers")) - assert eager_load.include?(root("app/models")) + test "booting up Rails yields a list of paths that will be eager loaded" do + autoload_paths = @paths.autoload_paths + assert autoload_paths.include?(root("app/controllers")) + assert autoload_paths.include?(root("app/helpers")) + assert autoload_paths.include?(root("app/models")) end test "environments has a glob equal to the current environment" do diff --git a/railties/test/paths_test.rb b/railties/test/paths_test.rb index 12f18b9dbf..6f860469fd 100644 --- a/railties/test/paths_test.rb +++ b/railties/test/paths_test.rb @@ -135,56 +135,48 @@ class PathsTest < ActiveSupport::TestCase assert_equal 2, @root.autoload_once.size end - test "it is possible to mark a path as eager loaded" do + test "marking a path as eager loaded is deprecated" do @root["app"] = "/app" - @root["app"].eager_load! - assert @root["app"].eager_load? - assert @root.eager_load.include?(@root["app"].to_a.first) + assert_deprecated{ @root["app"].eager_load! } + assert_deprecated{ assert @root["app"].eager_load? } + assert_deprecated{ assert @root.eager_load.include?(@root["app"].to_a.first) } end - test "it is possible to skip a path from eager loading" do + test "skipping a path from eager loading is deprecated" do @root["app"] = "/app" - @root["app"].eager_load! - assert @root["app"].eager_load? + assert_deprecated{ @root["app"].eager_load! } + assert_deprecated{ assert @root["app"].eager_load? } - @root["app"].skip_eager_load! - assert !@root["app"].eager_load? - assert !@root.eager_load.include?(@root["app"].to_a.first) + assert_deprecated{ @root["app"].skip_eager_load! } + assert_deprecated{ assert !@root["app"].eager_load? } + assert_deprecated{ assert !@root.eager_load.include?(@root["app"].to_a.first) } end - test "it is possible to add a path without assignment and mark it as eager" do - @root.add "app", with: "/app", eager_load: true - assert @root["app"].eager_load? - assert @root.eager_load.include?("/app") + test "adding a path with eager_load option is deprecated" do + assert_deprecated{ @root.add "app", with: "/app", eager_load: true } + assert_deprecated{ assert @root["app"].eager_load? } + assert_deprecated{ assert @root.eager_load.include?("/app") } end - test "it is possible to add multiple paths without assignment and mark them as eager" do - @root.add "app", with: ["/app", "/app2"], eager_load: true - assert @root["app"].eager_load? - assert @root.eager_load.include?("/app") - assert @root.eager_load.include?("/app2") - end - - test "it is possible to create a path without assignment and mark it both as eager and load once" do - @root.add "app", with: "/app", eager_load: true, autoload_once: true - assert @root["app"].eager_load? - assert @root["app"].autoload_once? - assert @root.eager_load.include?("/app") - assert @root.autoload_once.include?("/app") + test "adding multiple paths with eager_load option is deprecated" do + assert_deprecated{ @root.add "app", with: ["/app", "/app2"], eager_load: true } + assert_deprecated{ assert @root["app"].eager_load? } + assert_deprecated{ assert @root.eager_load.include?("/app") } + assert_deprecated{ assert @root.eager_load.include?("/app2") } end test "making a path eager more than once only includes it once in @root.eager_paths" do @root["app"] = "/app" - @root["app"].eager_load! - @root["app"].eager_load! - assert_equal 1, @root.eager_load.select {|p| p == @root["app"].expanded.first }.size + assert_deprecated{ @root["app"].eager_load! } + assert_deprecated{ @root["app"].eager_load! } + assert_deprecated{ assert_equal 1, @root.eager_load.select {|p| p == @root["app"].expanded.first }.size } end test "paths added to a eager_load path should be added to the eager_load collection" do @root["app"] = "/app" - @root["app"].eager_load! + assert_deprecated{ @root["app"].eager_load! } @root["app"] << "/app2" - assert_equal 2, @root.eager_load.size + assert_deprecated{ assert_equal 2, @root.eager_load.size } end test "it should be possible to add a path's default glob" do |