Sprockets::FileOutsidePaths error seems to be caused by fix to #59 #96
Comments
If you're using an asset that isn't inside of the root of where you're running sprockets, an absolute path will be used to generate the cache key. Can you give me an example project that shows the problem? |
|
I haven't yet tried in a new project but can give you repro directions:
|
|
so this is actually very easy to repro with a new project
|
|
with v3.2.0 there's no error, but only because the cache is not used due to absolute paths with v2.12.4 there's no error and the cache is used |
|
There's a lot of grey area between 1 and 10. I'll attempt trying this tomorrow, |
the original project does not have the asset cache (assets:clobber in step 5) |
|
I tried that and couldn't reproduce I'm pointing at my copy's assets and cache: Using sprockets 3.3.0 It works: If you can repro the problem, can you wrap up the two repos you're using in another git repo and throw em' on github for me? What are you doing IRL to cause this issue? Do you have multiple rails app deploys that are simlinked to the same asset directory is this only to show a repro case? |
|
Don't link public/assets. The presence of precompiled assets bypasses the cache entirely I believe (right?). Also this is dev mode (on demand compilation is on) The whole point is to have the original project use the cache generated in the copy. Also, I am linking just the tmp/cache/assets not the whole tmp (this probably does not matter) In capistrano deploys the assets cache is shared between releases. I am dealing with a project in which on demand compilation is on in prod (that will change but not immediately ). On demand compilation tries to use shared cache entries generated by and pointing to previous releases and produces the error (this is exactly what this test demonstrates). So please try without public/assets linking, if that does not work, I'll upload. |
|
Still working for me: Started with Result |
Not quite. First sprockets tries to see if there is a valid asset by first using mtime with the cache to make sure the source file in
This is a huge performance penalty to your site, I hope you've got some really good CDN in front of your site so each asset only has to be compiled once, otherwise you're in for a world of pain. I would highly recommend against leaving this option on, I would be curious hearing why it's needed but perhaps in a different conversation to keep this thread focused. problemFor your use case, can you add the physical location of the assets to sprockets load paths? Sprockets can use multiple paths, though i'm not sure how to set this through rails, that might alleviate the issue. Another option could be symlinking the public/assets directory so that you're pointing the cache and the assets directory to the same project/path-structure. |
|
you are doing a HEAD on application.js and it's responding with method not allowed. You are not hitting the asset cache with that. I tested doing just a GET on application.js and it works. So, as I directed, you need to invoke an action that does a javascript_include_tag or css. I'll upload, unless you can retry quickly. |
|
That was the wrong paste. It renders correctly when I visit the page and when I curl without |
|
created a repo Please follow these directions exactly:
|
As I mentioned, sprockets 2 works fine, so that's the workaround we are using. sprockets 2 cache does not use absolute paths for it's keys or anything else (I think there is nothing else in 2.x), which facilitates cache sharing between releases. Sprockets 3 should not be using absolute paths at all either. 3.3 started along this path, but didn't finish. |
|
Got the same error in production this morning, the way we do the deploy is by compiling locally and checking the manifest into the repo and uploading to cloudfront (via asset_sync). Here's the backtrace in case it's useful: Looks like relative paths are being confused by the changed root (from 20150814095620 to 20150814102012) which is stored in the cache for some reason. Also I tried the steps above and I can reproduce. |
|
I hit the same error when putting some images in That |
|
Thanks for the report, I got the example @bughit sent over to reproduce the problem.
Right now if an asset is not inside of your root, then we cache the absolute path. sprockets/lib/sprockets/loader.rb Lines 154 to 161 in bae6aa7 My original assumption was all your assets would either be relative to your project root or at a static path such as I'm not sure what other alternatives we have. If we cache the relative path to the asset |
I don't understand what you are saying there. There are only two assets, application.js and application.css and both are inside the project (app/assets). I am assuming that's what you mean by "inside the root". The cache is generated in the asset_cache_test_copy by precompile. So you appear to be saying there should not be any absolute paths in the cache files, but clearly there are. |
|
Here's the issue. When we try to load the asset via sprockets/lib/sprockets/loader.rb Lines 201 to 202 in de057b4 The sprockets/test/test_performance.rb Lines 109 to 134 in de057b4 |
Right, and they shouldn't. There is no good reason to store absolute paths in the cache entry (unless, as you pointed out, the assets is outside the project) |
|
yes |
The patch in #92 was incomplete, it converted all cache keys to use relative paths, but didn't fully remove all absolute paths from cache contents. The test case was accidentally passing due to the copying of files instead of moving files - [x] dependency paths - [x] asset uris - [x] "included" paths (no idea what these are) - [ ] load paths This is a WIP
|
Here's my WIP #101 needs some major cleanup. Not sure about using presence of a |
|
For anyone having that error with vendor/assets (which worked before), I was able to resolve production deploy by adding to Not sure if it's a correct fix, but it helped me to deploy my app. |
The patch in rails#92 was incomplete, it converted all cache keys to use relative paths, but didn't fully remove all absolute paths from cache contents. The test case was accidentally passing due to the copying of files instead of moving files - [x] dependency paths - [x] filename - [x] asset uris - [x] "included" paths (no idea what these are) - [x] load paths - [x] update docs - [ ] test broken behavior This is a WIP
|
#101 is now (what I believe) to be a working fix. I'm trying to write a failing test case in 3.x so we don't have to worry about a future regression. |
The patch in #92 was incomplete, it converted all cache keys to use relative paths, but didn't fully remove all absolute paths from cache contents. The test case was accidentally passing since we didn't check to make sure any of the paths from cache were different from the original ones stored. This patch eliminates these absolute paths stored in the cache: - [x] dependency paths - [x] filename - [x] asset uris - [x] "included" paths (no idea what these are) - [x] load paths ### URITar This patch works by introducing a utility class URITar which can "compress" or "expand" a given uri. A "compressed" uri can either be one that is relative to the root, or an absolute path if it is outside of the root. An expanded uri will always be an absolute path. A uri that is relative to the root will be compressed with no beginning slash file://relative/to/root/file.js A uri that is outside of the root will be compressed with a beginning slash file:///absolute/path/to/file.js Even though I'm using "uri" here the URITar class can also operate on file paths without a valid URI scheme name. Like: relative/to/root/file.js and /absolute/path/to/file.js The UnloadedAsset class was moved to it's own file and refactored to use the new URITar class. ## Fix Before putting anything in the cache, we will "compress" all uris and paths so that no absolute paths are in the cache (unless they're not relative to the root which would indicate they're somewhere global e.g. from a gem or shared directory). Upon loading an asset in memory, we "expand" all uris since sprockets relies on absolute paths for just about everything. Almost all the business logic is limited to the loader, so the rest of sprockets has no clue if relative or absolute paths were used to build the asset. We are also compressing the "environment-paths" so that dependencies in different paths will differ. I think this is needed, but the tests don't fail when it's taken out. ATP
|
|
The comment is being inserted by https://github.com/sass/sass/blob/1421c1fb38f56d947d1e0271ab5ed12dee510d48/lib/sass/tree/visitors/to_css.rb#L307 and that other absolute paths come from sass cache. Sass uses marshal dump: https://github.com/sass/sass/blob/stable/lib/sass/cache_stores/base.rb#L51 The nodes it dumps contain a "source_range" All the absolute paths are coming from sass. If there's no absolute paths coming from sprockets (that we can find for now) I vote we merge and cut a release. I'll then work with sass to remove the absolute paths there. Unfortunately sprockets doesn't have a dependency on sass since you can use sprockets without sass, so we can't rev the sass dependency to guarantee no absolute paths. We could check the version and emit a warning perhaps? |
|
I'm testing #96 (comment) right now and couldn't experience the error again. Looks like as if it is fixed for me. |
|
@joker-777 thanks for testing! I opened an issue with sass sass/sass#1802 hopefully I can get some help there while I work on a patch. |
|
@schneems You are welcome. Unfortunately we just discovered that it somehow doesn't recompile js files which were changed. Investigating further. |
|
@joker-777 let me know. I just tested with @bughit's app and it seems to work |
|
Ok, we definitely have this problem that changes js files don't get precompiled. I will clobber the current assets and try it again. |
|
Can you give me an example app that reproduces the problem? I'll be happy to take a look. |
|
Ok, now it works. We keep and eye on it. |
|
The clobber might have been needed for the cache changes to take affect if the old cache had absolute paths pointing at assets that were still on disk. Thanks again for testing. |
|
Unfortunately I have tell you that we still experience that js files don't get precompiled even though they should contain changes. In the beginning it worked but now happened again. |
|
Can you give me an example app that reproduces the problem? I'll be happy to take a look. |
|
I'm chiming in, just in case I can be of any help. I tried a deploy this morning after updating sprockets, and it failed with a I did get a successful deploy by forcing the version to 3.3.0. Using versions 3.3.2, or 3.3.3 results in the deploy attempting to use the old release path during precompile (below, notice that the css file it's referring to is in
The order I went in testing versions of sprockets was:
So for me this doesn't seem to be caused solely by a particular version of sprockets. Whether or not a previous deploy was successful also seems to play a role. Whatever that means. :-) It seems that version 3.3.0 is the newest version that is capable of completing a deploy, so I'm currently pinned there. Let me know if you'd like more information or have any tests I could run for you. |
|
@JamesChevalier Version 3.3.0 introduced changes to the cache, so you won't see any issue at all unless you are using the cache. Sprockets invalidates the cache on every version change, so every time you upgrade or downgrade sprockets you theoretically should be getting a fresh cache. Since that's the case, you should be getting a problem the second time you deploy, not the first. So deploying with 3.3.0 or 3.3.1 or 3.3.2 might work the first time but probably shouldn't work the second time as there's known bugs in them. If you can give me a 3.3.3 example app that reproduces this error, I would be grateful. |
|
We had the same issue here and solved it by changing into the old directory used (20150821195208 in your case) and deleting the whole tmp/cache/assets/production dir. Afterwards the deployment worked again - reproducible. |
|
When using sprockets v 3.3.0, I am able to deploy more than once. I tested simply running What are the specific files/details you'd like to receive, for the reproduced error? I want to be sure I don't waste your time. |
|
3.3.0 works because works because it has bugs in it, 3.3.3 fixed all known bugs so if you are getting errors with 3.3.3 then we need to reproduce them and fix them. If you can give me a 3.3.3 example app on github that reproduces this error, and instructions on how to run it to reproduce the error then I can probably fix whatever issue you're seeing. |
|
OK, sounds good. I've got this app back up to 3.3.3, and it's working fine (something about that 3.3.0 deploy I mentioned above as step 3 in my testing has fixed it for now). Thanks for your help & hard work! |
|
@themilkman @schneems |
|
@karlingen same here, worked like charm! |
|
Upgrading to 3.3 and running on the server before deploying worked. |
|
Awesome, going to close this issue. If you see any other weirdness please open up a new one with information on how to reproduce. Thanks for everyone's help ❤️ |
|
@schneems I have no idea what has changed, but I am getting the same issue running |
|
I vote for reopening this issue. |
|
@schneems @itay-grudev yes, me also - downgrading to before starting deployment. |
|
Seeing the same issue as well on |
|
I am seeing this same issue within the test suite on I stumbled on another bug which I think I have fixed, but now when I run the test suite, I get this error: I am still trying to figure out how to tackle this properly, but I don't fully understand the issue yet. Edit 1 Hey @schneems could you help me understanding the flow of some of this code and how this works. I think I have isolated the problem to these lines. Here are the first 3 variables in action - Then if we look at the 2nd 3 variables: That But I can't quite grok what's happening here and why it creates that funky URI in the Thoughts? |
Not a 100% sure, but what seems to be happening is even though cache key generation now uses relative paths, some or all depedency references are absolute.
so if you are sharing the asset cache folder among multiple release, sprockets ends up finding the right cache entry but then tries to load dependencies from an older release absolute path (where the cache was originally constructed) and fails with Sprockets::FileOutsidePaths
The text was updated successfully, but these errors were encountered: