Merge pull request #560 from michaelglass/michaelglass/warning-on-multiple-manifests

Warn if there are multiple manifest files

Author: schneems

CHANGELOG.md

@@ -6,6 +6,7 @@ Get upgrade notes from Sprockets 3.x to 4.x at https://github.com/rails/sprocket
 
 - Minimum Ruby version for Sprockets 4 is now 2.5+ which matches minimum ruby verision of Rails [#604]
 - Fix threading bug introduced in Sprockets 4 [#603]
+- Warn when two potential manifest files exist. [#560]
 
 ## 4.0.0.beta8
 
@@ -54,4 +55,3 @@ Get upgrade notes from Sprockets 3.x to 4.x at https://github.com/rails/sprocket
 - Initial release of Sprockets 4
 
 Please upgrade to the latest Sprockets 3 version before upgrading to Sprockets 4. Check the 3.x branch for previous changes https://github.com/rails/sprockets/blob/3.x/CHANGELOG.md.
-

lib/sprockets/manifest.rb

@@ -53,7 +53,7 @@ def initialize(*args)
 
       # If directory is given w/o filename, pick a random manifest location
       if @directory && @filename.nil?
-        @filename = find_directory_manifest(@directory)
+        @filename = find_directory_manifest(@directory, logger)
       end
 
       unless @directory && @filename

lib/sprockets/manifest_utils.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 require 'securerandom'
+require 'logger'
 
 module Sprockets
   # Public: Manifest utilities.
@@ -33,10 +34,13 @@ def generate_manifest_path
     #     # => "/app/public/assets/.sprockets-manifest-abc123.json"
     #
     # Returns String filename.
-    def find_directory_manifest(dirname)
+    def find_directory_manifest(dirname, logger = Logger.new($stderr))
       entries = File.directory?(dirname) ? Dir.entries(dirname) : []
-      entry = entries.find { |e| e =~ MANIFEST_RE } ||
-        generate_manifest_path
+      manifest_entries = entries.select { |e| e =~ MANIFEST_RE }
+      if manifest_entries.length > 1
+        logger.warn("Found multiple manifests: #{manifest_entries}. Choosing the first alphabetically: #{manifest_entries.first}")
+      end
+      entry = manifest_entries.first || generate_manifest_path
       File.join(dirname, entry)
     end
   end

test/fixtures/manifest_utils/with_two_manifests/.sprockets-manifest-00000000000000000000000000000000.json

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 require 'minitest/autorun'
 require 'sprockets/manifest_utils'
+require 'logger'
 
 class TestManifestUtils < MiniTest::Test
   include Sprockets::ManifestUtils
@@ -16,5 +17,21 @@ def test_find_directory_manifest
 
     assert_equal "#{root}/default/.sprockets-manifest-f4bf345974645583d284686ddfb7625e.json",
       find_directory_manifest("#{root}/default")
+
+  end
+
+  def test_warn_on_two
+    root = File.expand_path("../fixtures/manifest_utils", __FILE__)
+
+    assert_match MANIFEST_RE, File.basename(find_directory_manifest(root))
+
+    r, w = IO.pipe
+    logger = Logger.new(w)
+    # finds the first one alphabetically
+    assert_equal "#{root}/with_two_manifests/.sprockets-manifest-00000000000000000000000000000000.json",
+      find_directory_manifest("#{root}/with_two_manifests", logger)
+    output = r.gets
+
+    assert_match(/W, \[[^\]]+\]  WARN -- : Found multiple manifests: .+ Choosing the first alphabetically: \.sprockets-manifest-00000000000000000000000000000000\.json/, output)
   end
 end