Fix config.assets.manifest Rake task and Railtie

[johnnyshields]

Currently, config.assets.manifest is not functioning as it should be. However, currently there no problem if config.assets.manifest is nil.

• Fix assets:precompile Rake task. The Sprockets::Manifest object used in the Rake task is separate from the one created in the Railtie.
• Fix config.assets.compile = false case. It is necessary to initialize the Sprockets::Manifest object with both the path to the assets, and the manifest path as parameters.
• In Railtie initializer, rename internal variable manifest_path to manifest_assets_path for clarity, and revert its definition to what it was before PR Restore config.assets.manifest option #122
• Add additional test cases related to config.assets.manifest option.

Proper functionality of the config.assets.manifest also requires this PR: sstephenson/sprockets#560. (Again, there is no issue if config.assets.manifest is left nil)

[johnnyshields]

Tests are failing pending next sprockets gem release, specifically this commit: sstephenson/sprockets@d52c9df

[matthewd]

I think "full path [..] for [..] file" pretty much covers "must include filename". I think the bigger issue here remains: it doesn't actually default to that at all... it defaults to a randomly-generated filename inside the config.assets.prefix directory within the public folder.

[johnnyshields]

Shall I use the wording:

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.

[matthewd]

While it's not up to me.. I do feel that better addresses any suggestion that the option can point to a directory.

[rafaelfranca]

@johnnyshields we will need to explicitly as @josh to backport that commit to 2.11.x release or we will not able to use in sprockets-rails 2.x. sprockets master is already sprockets 3.

[johnnyshields]

@rafaelfranca the issue in Sprockets is that when user specifies a manifest file in a directory that does not yet exist, Sprockets raises an error (rather than creating the directory). I've raised a PR for the backport here: sstephenson/sprockets#561

Sprocket-Rails could be released without this backport (I can change the test case) but it may cause some users to scratch their heads if they haven't already created their manifest dir.

[johnnyshields]

@rafaelfranca issue is resolved in Sprockets, @josh has pushed a new version, and tests are passing after pointing Sprockets-Rails gemspec to the new Sprockets version.

[rafaelfranca]

New version is 2.12.1 and Rails applications are not able to use 2.12.x 😢 I can merge this one but people using sass-rails will still have to provide the full path with directories already existing.

[johnnyshields]

OK I undid the sprockets dependency bump. CI tests are passing because existing gemspec references sprockets ~> 2.8, which evaluates to 2.12.1 for Travis purposes. Green-light to merge, no change for SASS users.

[johnnyshields]

@rafaelfranca this PR is ready to merge.

[ryanjohns]

+1

@rafaelfranca, can this please be merged? I have a couple of projects that need this before I can upgrade to rails 4.1. Thanks

[seantanly]

+1

Same here. This is the main blocker that's preventing the projects that I have from adopting/upgrading to Rails 4.x

[johnnyshields]

@rafaelfranca thank you for merging.

To anyone upgrading, as a friendly reminder you'll need to set config.assets.manifest to include the manifest filename, e.g. Rails.root.join('config/manifest.json'). It cannot be a directory as done in Rails 3.2.

[seantanly]

Thank you @johnnyshields for creating the PR and @rafaelfranca for merging it.

For those who are using asset_sync gem, and uses the config.manifest = true option, Rails.application.config.assets.manifest being a file now will break asset_sync. You can check out a quick fix that I made. https://github.com/seantan/asset_sync/tree/filedetect-manifest

[johnnyshields]

FYI doc update for Rails Guides related to this is in rails/rails#16375