Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When less file exists in a different folder depth than output, staticfiles dies looking for images #84

Closed
estebistec opened this issue Mar 11, 2012 · 27 comments
Labels

Comments

@estebistec
Copy link
Contributor

I could swear this worked in 1.1.27...

When any of my less files specify background: url(''), the url is relative assuming that all compressed CSS will end up in css/*.css, and therefore the image urls being with '../img/'.

However, it seems that pipeline has passed paths off to django staticfiles that are all relative to their original less files, which leads to the image not being found:

Traceback (most recent call last):
...
  File "/Users/steven/virtualenvs/simple-pipeline/lib/python2.7/site-packages/staticfiles/management/commands/collectstatic.py", line 163, in handle_noargs
    collected = self.collect()
  File "/Users/steven/virtualenvs/simple-pipeline/lib/python2.7/site-packages/staticfiles/management/commands/collectstatic.py", line 120, in collect
    for original_path, processed_path, processed in processor:
  File "/Users/steven/virtualenvs/simple-pipeline/lib/python2.7/site-packages/staticfiles/storage.py", line 270, in post_process
    content = pattern.sub(converter, content)
  File "/Users/steven/virtualenvs/simple-pipeline/lib/python2.7/site-packages/staticfiles/storage.py", line 213, in converter
    hashed_url = self.url(unquote(joined_result), force=True)
  File "/Users/steven/virtualenvs/simple-pipeline/lib/python2.7/site-packages/staticfiles/storage.py", line 160, in url
    hashed_name = self.hashed_name(clean_name).replace('\\', '/')
  File "/Users/steven/virtualenvs/simple-pipeline/lib/python2.7/site-packages/staticfiles/storage.py", line 123, in hashed_name
    (clean_name, self))
ValueError: The file 'less/img/wilford-brimley-diabeetus-cat.jpg' could not be found with <simple_pipeline.myapp.pipeline_storage.S3PipelineStorage object at 0x1027f2a90>.

No to go figure out if/where this ever worked...

[1] http://pastebin.com/MGtcj9rA (full stacktrace here)

@estebistec
Copy link
Contributor Author

Looked in the code a while. This goes back to your comment from stack overflow: that you thought pipeline 1.1.X was dumb for needing to do things all in the same place. Now I see that processing each file in it's original folder is going to be a problem because of the way staticfiles processes what you give it. Given all of the moving parts, I'm having trouble deciding if the bug is here, or in staticfiles. Kind of hard to fault staticfiles for treating a css file that you give it like a CSS file that will live where pipeline has put it...

@estebistec
Copy link
Contributor Author

This issue is going to cause me to have to roll back to 1.1.27 for now and manually upload compressed files with boto :(

I'll look at the sources some more tomorrow, see if I can think of a good way to fix this... seems like pipeline might need a staging area. I really liked how in 1.1.X your static root only had the few resulting output files. Granted, it didn't have the img folder :/, but still.

@cyberdelia
Copy link
Member

I don't think this is related to only to less, url rewriting is still buggy, the switch from absolute url to relative url seems incomplete. Will look into it again, but if you can build up some test case, it would be wonderful.

@estebistec
Copy link
Contributor Author

Sure. It's just a slight modification to my test project... I'll have an update pushed in a couple of minutes...

@estebistec
Copy link
Contributor Author

My simple_pipeline project has been updated with the changes demonstrating the problem: https://github.com/estebistec/simple_pipeline

@estebistec
Copy link
Contributor Author

It looks to me like the URLs are actually be re-written here [1], in staticfiles. That's why I assumed the problem was placing the less compiled output in the same folder in the app where the source is. It doesn't look like you have any other way to modify how this re-writing is done except to place the compiled CSS in a folder at the same depth as the eventual output file, or better, not place any of the intermediate CSS files from less, just the concatenated one.

Does that make sense? I'm still feeling my way around these two code bases and where stuff really happens.

[1] https://github.com/jezdez/django-staticfiles/blob/develop/staticfiles/storage.py#L184

@estebistec
Copy link
Contributor Author

So, having looked at the code a while longer, here's how I see it:

  1. URL rewriting is a problem because we allow pipeline.compilers.Compiler write files in the same place it reads sources from
  2. we give staticfiles all of these paths to save before compressing, which affects the context that URLs were coded in

So I think I've confirmed my suspicion above. I think Compiler needs to change to create a temp folder for outputs, or we should move everything to a temp folder before even invoking Compiler so that we can compress stuff to it's intended folder depth before we let the staticfiles url converter at it (which assumes urls should be re-written according to current folder depth).

A pipeline has stages after all :) and we seemingly need a staging area for the intermediates.

@estebistec
Copy link
Contributor Author

'absolute_asset_paths': False, seems to save my paths from being destroyed in 1.1.27.

I noticed in the 1.2 changelog that it's been renamed to absolute_paths, but it seems to not have the desired affect, where I still get the above exception.

@cyberdelia
Copy link
Member

I've just reproduced your bug, your image should be referred as :

url('../../img/wilford-brimley-diabeetus-cat.jpg')

It should match the level your less file is (not the package concat/compressed output), this is for ease of use in DEBUG mode, pipeline/staticfiles will rewrite correctly the image url when running collectstatic.

Here is the output of collectstatic in your project :

Post-processed 'img/wilford-brimley-diabeetus-cat.jpg' as 'img/wilford-brimley-diabeetus-cat.b8a8a7b86e59.jpg

And the image is correctly referenced in test.*.css :

background:url("/static/img/wilford-brimley-diabeetus-cat.b8a8a7b86e59.jpg")

@estebistec
Copy link
Contributor Author

I see. I think that means using pipeline for dev is required vs. boostrap's less.js, if you use pipeline at all. They behave differently in this respect. My team has a sizeable reusable less project, with sources at different folder levels. It means I can't change just one project over to this :/

@cyberdelia
Copy link
Member

I don't see why less.js would require to not use to the same path as pipeline ?

@estebistec
Copy link
Contributor Author

It works with us specifying all of our paths as relative to the ultimate folder, not the less source. At this point I can't say one is more correct over the other, that just explains my expectation going into this.

@estebistec
Copy link
Contributor Author

One more important question though, why is pipeline even trying to read those files out of the less/css? If I set the "don't use absolute paths" settings, and roll along with the scheme I described above, it's simply my responsibility to have those images in place for later. What is pipeline trying to read them for?

@cyberdelia
Copy link
Member

absolute_paths don't exists anymore in 1.2, all paths are relative.
Paths are only rewritten when compressed, by pipeline and staticfiles.

@estebistec
Copy link
Contributor Author

Is there a way we can turn that off? Or have some more control over it (e.g., relative_to)?

And I still wonder why the image in a background:url even needs to be read, however, now that I look at the stacktrace again it appears that it's staticfiles trying to do this... so I guess I should ask that over there.

@estebistec
Copy link
Contributor Author

Or, are those paths that you're giving to staticfiles to save? Why cant we have a simple option, like just assume all images are under STATIC_ROOT/img and copy that tree?

@estebistec
Copy link
Contributor Author

BUMP I'm going to be looking at this again with Django 1.4 pretty soon...

@cyberdelia
Copy link
Member

@estebistec Any news so ?

@estebistec
Copy link
Contributor Author

No :/ I got delayed with some other things. I really do want to get back to this though as I'd rather be using this than some custom code we have. I should be able to try it again, latest, this coming weekend (US/Central).

@estebistec
Copy link
Contributor Author

Looks to be the same issue, relative img paths are calculated from less source instead of destination css, and so if they don't match pipeline barfs :/

I might just need to write a pipeline stage that re-writes those URLs so that it can pass...

@estebistec
Copy link
Contributor Author

Rebased my fork from upstream... I'll throw together a pull request soon...

@estebistec
Copy link
Contributor Author

Hey, just out of curiosity, when the doc says "If the source CSS contains relative URLs (i.e. relative to current file), those URLs will be converted to full relative path.".

  1. should that say "absolute path" (or else, what is a "full relative path")?
  2. when should it be doing this, because with my ongoing error here, django-staticfiles appears to be barfing on a relative path that's simply relative from the original less source location.

@estebistec
Copy link
Contributor Author

Next comment (you've got to be tired of me by now :)): I'm noticing I get the error when staticfiles storage is pulling an individual file in. Can I tell pipeline to NOT copy source files but only the compressed output files, so that staticfiles isn't trying to resolve relative URLs of source files? From that perspective, maybe this isn't a bug in URL re-writing at all, but that we should be able to neglect the source files in collectstatic. Thoughts?

@cyberdelia
Copy link
Member

staticfiless doesn't rewrite url if the file is not ending with .css.
You should try to catch me on freenode#django-pipeline.

@estebistec
Copy link
Contributor Author

Yeah. I'll see if I can match your hours sometime soon to sync up. I
haven't been able to dive as far as I'd like into how pipeline and
staticfiles interacts. I don't believe staticfiles to be re-writing URLs
itself, but I observe it trying to load files based on urls in CSS that I'm
fairly certain have been re-written by pipeline. My comment about
staticfiles was more that maybe I could get it to not try to load files
based on those URLs, but just by what is present in static dirs (I couldn't
find a way, and I don't believe it possible based on the only conditions I
found for that not being exposed outside of it's internals).

staticfiless doesn't rewrite url if the file is not ending with
.css.
You should try to catch me on freenode#django-pipeline.


Reply to this email directly or view it on GitHub:

#84 (comment)

@estebistec
Copy link
Contributor Author

This has become a non-issue for me for the time being. We've simplified our less file layout so that it doesn't have the extra folder depth.

I still believe there's a bug in here, I wasn't easily able to determine where there should be the option the option to not re-write URLs. It might be that pipeline should provide that option, since the doc states that it does re-write CSS URLs. But for now, it's not going to be blocking me. I may still poke at it sometime and see how that option would work. If I do I'll submit a new pull request.

@cyberdelia
Copy link
Member

Ok, I'll close this. Feel free to reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants