Merge pull request rails#136 from johnnyshields/fix-manifest-path

Fix config.assets.manifest Rake task and Railtie

Author: rafaelfranca

CHANGELOG.md

@@ -1,3 +1,10 @@
+### Unreleased
+
+*   Fix issues related `config.assets.manifest` option, including issues with `assets:precompile` Rake task.
+
+    *Johnny Shields*
+
+
 ### 2.1.3
 
 *   Correct NameError on Sprockets::Rails::VERSION.

README.md

@@ -85,7 +85,7 @@ Defaults to `/assets`. Changes the directory to compile assets to.
 
 **`config.assets.manifest`**
 
-Defines the full path to be used for the asset precompiler's manifest file. Defaults to using the `config.assets.prefix` directory within the public folder.
+Defines the full path to be used for the asset precompiler's manifest file. Defaults to a randomly-generated filename in the `config.assets.prefix` directory within the public folder.
 
 **`config.assets.digest`**
 
@@ -134,6 +134,7 @@ The following plugins provide some extras for the Sprockets Asset Pipeline.
 * Unmanaged asset paths and urls fallback to linking to public/. This should make it easier to work with both compiled assets and simple static assets. As a side effect, there will never be any "asset not precompiled errors" when linking to missing assets. They will just link to a public file which may or may not exist.
 * JS and CSS compressors must be explicitly set. Magic detection has been removed to avoid loading compressors in environments where you want to avoid loading any of the asset libraries. Assign `config.assets.js_compressor = :uglifier` or `config.assets.css_compressor = :sass` for the standard compressors.
 * The manifest file is now in a JSON format. Since it lives in public/ by default, the initial filename is also randomized to obfuscate public access to the resource.
+* `config.assets.manifest` (if used) must now include the manifest filename, e.g. `Rails.root.join('config/manifest.json')`. It cannot be a directory.
 * Two cleanup tasks. `rake assets:clean` is now a safe cleanup that only removes older assets that are no longer used. While `rake assets:clobber` nukes the entire `public/assets` directory and clears your filesystem cache. The clean task allows for rolling deploys that may still be linking to an old asset while the new assets are being built.
 
 

lib/sprockets/rails/task.rb

@@ -37,6 +37,14 @@ def assets
         end
       end
 
+      def manifest
+        if app
+          Sprockets::Manifest.new(index, output, app.config.assets.manifest)
+        else
+          super
+        end
+      end
+
       def cache_path
         if app
           "#{app.config.root}/tmp/cache/assets"

lib/sprockets/railtie.rb

@@ -66,7 +66,7 @@ def configure(&block)
     config.after_initialize do |app|
       config = app.config
 
-      manifest_path = config.assets.manifest || File.join(config.paths['public'].first, config.assets.prefix)
+      manifest_assets_path = File.join(config.paths['public'].first, config.assets.prefix)
 
       # Configuration options that should invalidate
       # the Sprockets cache when changed.
@@ -99,9 +99,9 @@ def configure(&block)
 
         if config.assets.compile
           self.assets_environment = app.assets
-          self.assets_manifest    = Sprockets::Manifest.new(app.assets, manifest_path)
+          self.assets_manifest    = Sprockets::Manifest.new(app.assets, manifest_assets_path, config.assets.manifest)
         else
-          self.assets_manifest = Sprockets::Manifest.new(manifest_path)
+          self.assets_manifest = Sprockets::Manifest.new(manifest_assets_path, config.assets.manifest)
         end
       end
 

test/test_railtie.rb

@@ -194,11 +194,12 @@ def test_sprockets_context_helper
 
   def test_manifest_path
     app.configure do
-      config.assets.manifest = Rails.root.join('config','foo')
+      config.assets.manifest = Rails.root.join('config','foo','bar.json')
     end
     app.initialize!
 
-    assert_match %r{config/foo/manifest-.*.json}, ActionView::Base.assets_manifest.path
+    assert_match %r{config/foo/bar\.json$}, ActionView::Base.assets_manifest.path
+    assert_match %r{public/assets$}, ActionView::Base.assets_manifest.dir
   end
 
   def test_manifest_path_respects_rails_public_path
@@ -207,6 +208,7 @@ def test_manifest_path_respects_rails_public_path
     end
     app.initialize!
 
-    assert_match %r{test_public/assets/manifest-.*.json}, ActionView::Base.assets_manifest.path
+    assert_match %r{test_public/assets/manifest-.*\.json$}, ActionView::Base.assets_manifest.path
+    assert_match %r{test_public/assets$}, ActionView::Base.assets_manifest.dir
   end
 end

test/test_task.rb

@@ -16,9 +16,10 @@ def setup
     @assets = Sprockets::Environment.new
     @assets.append_path FIXTURES_PATH
 
-    @dir = File.join(Dir::tmpdir, 'rails/task')
+    @dir = File.join(Dir::tmpdir, 'rails', 'task')
 
-    @manifest = Sprockets::Manifest.new(@assets, @dir)
+    @manifest_file = File.join(Dir::tmpdir, 'rails', 'manifest', 'custom-manifest.json')
+    @manifest = Sprockets::Manifest.new(@assets, @dir, @manifest_file)
 
     @environment_ran = false
     # Stub Rails environment task
@@ -39,6 +40,10 @@ def teardown
 
     FileUtils.rm_rf(@dir)
     assert Dir["#{@dir}/*"].empty?
+
+    manifest_dir = File.dirname(@manifest_file)
+    FileUtils.rm_rf(manifest_dir)
+    assert Dir["#{manifest_dir}/*"].empty?
   end
 
   def test_precompile
@@ -49,6 +54,26 @@ def test_precompile
 
     @rake['assets:precompile'].invoke
 
+    assert @environment_ran
+    assert File.exist?(@manifest_file)
+    assert File.exist?("#{@dir}/#{digest_path}")
+  end
+
+  def test_precompile_without_manifest
+    Sprockets::Rails::Task.new do |t|
+      t.environment = @assets
+      t.manifest    = Sprockets::Manifest.new(@assets, @dir, nil)
+      t.assets      = ['foo.js', 'foo-modified.js']
+      t.log_level   = :fatal
+    end
+
+    assert !@environment_ran
+
+    digest_path = @assets['foo.js'].digest_path
+    assert !File.exist?("#{@dir}/#{digest_path}")
+
+    @rake['assets:precompile'].invoke
+
     assert @environment_ran
     assert Dir["#{@dir}/manifest-*.json"].first
     assert File.exist?("#{@dir}/#{digest_path}")