Improve support for binary assets #326
Conversation
|
The whole assets pipeline is using unicode since Python 3 support was added. The only filter that needed binary support was gzip, and that was removed. If we want to support binary, there are two options:
|
|
I guess a full binary pipeline would be the "best" option (then users can switch image compression filters easily), but I can see that a "write" phase might be an easy workaround for the main problem in this case. So I'd support either option, whichever you have time for :) |
|
Or alternatively if there was a method on Bundle to create a FilterTool and a MemoryHunk, instead of hard-coding the class names, then we could replace them with binary-safe versions just by subclassing Bundle. |
|
I don't think I will have time to work on either for now, but if you want to have a go, I think even the second option I mentioned shouldn't be to hard; mainly moving the encode/decode calls to the right layer. |
Filters that expect binary input will need to declare a `binary_input` attribute. Otherwise they will be passed data in Unicode strings. Binary outputs will be converted to Unicode strings before each filter that doesn't declare itself as supporting binary input. That includes input files read from disk. Output files written to disk will be encoded in utf-8 if the last filter output Unicode, otherwise the binary data output by the last filter.
| @@ -125,7 +125,7 @@ def _apply_sass(self, _in, out, cd=None): | |||
| # shell: necessary on windows to execute | |||
| # ruby files, but doesn't work on linux. | |||
| shell=(os.name == 'nt')) | |||
| stdout, stderr = proc.communicate(_in.read().encode('utf-8')) | |||
| stdout, stderr = proc.communicate(_in.read()) | |||
There was a problem hiding this comment.
Do we still need these two lines? In fact, why do they work at all? Since the sass filter doesn't set the binary flags, aren't we trying to write bytes to a StringIO here?
|
Apologies for not noticing earlier. I like it a lot! |
|
The build failures seem pertinent here, in particular on Python 3. |
| @@ -492,7 +492,7 @@ def created(self): | |||
|
|
|||
| try: | |||
| data = (data.read() if hasattr(data, 'read') else data) | |||
| if data is not None: | |||
| if isinstance(data, unicode): | |||
There was a problem hiding this comment.
This change is very similar to my PR about supporting empty files #360, if not even more accurate.
|
As far as I understand, we can't have a bundle of binary files, each binary file must have 1 bundle and 1 output, right? We don't have a way to have |
|
Was this ever resolved? I'd like webassets to manage my fonts. |
|
What's the status of this? |
You may be interested in a new filter that we've developed: Django-Assets SVG. It generates optimised PNG files from SVG sources.
We had a lot of difficulty in processing binary files through Django-Assets, mainly because it's assuming in several places that files should be opened in Unicode and written out in UTF-8. Of course a PNG file does not take kindly to this transformation, let alone not being validly encoded in UTF-8 in the first place :)
The main issues that I saw were:
encoding='utf-8'strstrings.encoding='utf-8'Please could you look into how we can replace these functions when defining our bundles, so that we can process binary files properly? Thanks!