search

documentcloud / jammit

Industrial Strength Asset Packaging for Rails
http://documentcloud.github.com/jammit/
MIT License
1.16k stars 197 forks source link

Bug: Jammit::Packager#cache doesn't make sure the asset's path's dirname exists. #135

Closed yfeldblum closed 13 years ago

yfeldblum commented 13 years ago

Bug: Jammit::Packager#cache doesn't make sure the asset's path's dirname exists. It only makes sure that the output directory exists. But if the asset name has a slash (subdir/asset-name), then Jammit fails to create a subdirectory for that asset before trying to write to a file in a subdirectory that doesn't exist. Instead, Jammit should get the full pathname of the asset, get the dirname, and then mkpath or mkdir_p on that.

Cheers!

jashkenas commented 13 years ago

I'm a little fuzzy on this bug ... can you provide an example of an assets.yml file that demonstrates the problem?

yfeldblum commented 13 years ago

In this example, I have configured Jammit to write to tmp/assets.

# config/assets.yml
javascripts:
  admin/main:
    - public/javascripts/admin/models.js
    - public/javascripts/admin/views.js

Jammit will try to create the tmp/assets directory. But it won't try to create the tmp/assets/admin directory.

(What's the context for this example? Suppose I have set up a Rack::Static middleware to serve static files from tmp/assets, and have set up another custom initializer/middleware that updates all packages. So I have "dynamic" Jammitting on every HTML request and "dynamic" serving from tmp/assets.)

Cheers!

jashkenas commented 13 years ago

Sorry, but I'm afraid that a package name is supposed to just be a simple name, and not potentially a fragment of a directory path. There isn't anything in the code that's configured to handle it that way.

admin-main should work well enough.

yfeldblum commented 13 years ago

For the most part, Jammit would treat a package name as just a name that may have slashes in it; and anytime Jammit needs to write to the file with that name, it just makes sure to mkpath before writing to the file. There doesn't need to be much code to handle it beyond that. It should be a one-line fix in Jammit::Package#cache, where Jammit makes the path before writing the file. Otherwise, someone can easily alias_method_chain-patch Jammit::Package#cache to stick the feature in. Works on my machine just fine. Beyond that, what else would Jammit need to do to support this type of scenario?

Cheers!

jashkenas commented 13 years ago

Great -- want to link me to the patch that fixes it?