Make cssrewrite work with staticfiles. Closes #72.

cssrewrite filter operated on filesystem level, not in the URL space. This is
because historically, they have been the same.

This required some adjustments in the filter API, so we can pass the
non-rewritten file path to filters. Official policy now is that filters must
accept **kw.

On the place side, the CSSUrlReplace base class became a lot simpler, no longer
needing to (badly) reimplement absurl().

Will hopefully also fix (or help fix) the equivalent issue with Flask
blueprints.

docs/custom_filters.rst

@@ -116,7 +116,7 @@ a potential ``Filter`` suffix is removed.
 The ``input`` method will be called for every source file, the ``output``
 method will be applied once after a bundle's contents have been concated.
 
-The ``kwargs`` you currently receive are:
+Among the ``kwargs`` you currently receive are:
 
 - ``source_path`` (only for ``input()``): The filename behind the ``in``
   stream, though note that other input filters may already have transformed
@@ -125,6 +125,11 @@ The ``kwargs`` you currently receive are:
 - ``output_path``: The final output path that your filters work will
   ultimatily end up in.
 
+.. note::
+
+   Always make your filters accept arbitrary ``**kwargs``. The API does allow
+   for additional values to be passed along in the future.
+
 Registering
 ~~~~~~~~~~~
 
@@ -285,4 +290,4 @@ More?
 -----
 
 You can have a look inside the ``django_assets.filter`` module source
-code to see a large number of example filters.
+code to see a large number of example filters.

docs/upgrading.rst

@@ -49,9 +49,11 @@ Other changes:
 - ``PythonLoader.load_bundles()`` now returns a dict with the bundle names
   as keys, rather than a list.
 
-- The ``Filter.output()`` method now receives keyword arguments. All builtin
-  filters have defined this method with ``**kwargs`` for a while now, though
-  so far this was not used. Your custom filters may need updating.
+- Filters now receive new keyword arguments. The API now officially requires
+  filters to accept arbitrary ``**kwargs`` for compatibility with future
+  versions. While the documentation has always suggested ``**kwargs`` be used,
+  not all builtin filters followed this rule. Your custom filters may need
+  updating as well.
 
 
 In 0.6

src/django_assets/filter.py

@@ -20,7 +20,7 @@ def __init__(self, context=None):
         super(TemplateFilter, self).__init__()
         self.context = context
 
-    def input(self, _in, out, source_path, output_path):
+    def input(self, _in, out, source_path, output_path, **kw):
         t = Template(_in.read(), origin='django-assets', name=source_path)
         out.write(t.render(Context(self.context if self.context else {}) ))
 

src/webassets/bundle.py

@@ -277,7 +277,7 @@ def _get_env(self, env):
             raise BundleError('Bundle is not connected to an environment')
         return env
 
-    def _merge_and_apply(self, env, output_path, force, parent_debug=None,
+    def _merge_and_apply(self, env, output, force, parent_debug=None,
                          parent_filters=[], extra_filters=[],
                          disable_cache=None):
         """Internal recursive build method.
@@ -301,6 +301,9 @@ def _merge_and_apply(self, env, output_path, force, parent_debug=None,
         cache, since the cache key is not taking into account changes
         in those dependencies (for now).
         """
+
+        assert not path.isabs(output)
+
         # Determine the debug option to work, which will tell us what
         # building the bundle entails. The reduce chooses the first
         # non-None value.
@@ -357,38 +360,46 @@ def _merge_and_apply(self, env, output_path, force, parent_debug=None,
 
         filtertool = FilterTool(
             env.cache, no_cache_read=actually_skip_cache_here,
-            kwargs={'output_path': output_path})
+            kwargs={'output': output,
+                    'output_path': env.abspath(output)})
 
         # Apply input filters to all the contents. Note that we use both this
         # bundle's filters as well as those given to us by the parent. We ONLY
         # do those this for the input filters, because we need them to be
         # applied before the apply our own output filters.
         combined_filters = merge_filters(filters, parent_filters)
         hunks = []
-        for _, c in resolved_contents:
-            if isinstance(c, Bundle):
-                hunk = c._merge_and_apply(
-                    env, output_path, force, debug,
+        for rel_name, item in resolved_contents:
+            if isinstance(item, Bundle):
+                hunk = item._merge_and_apply(
+                    env, output, force, debug,
                     combined_filters, disable_cache=disable_cache)
                 hunks.append(hunk)
             else:
-                # Give a filter the change to open his file.
+                # Pass along the original relative path, as specified by the
+                # user. This may differ from the actual filesystem path, if
+                # extensions provide a virtualized filesystem (e.g. Flask
+                # blueprints, Django staticfiles).
+                kwargs = {'source': rel_name}
+
+                # Give a filter the chance to open his file.
                 try:
-                    hunk = filtertool.apply_func(combined_filters, 'open', [c])
+                    hunk = filtertool.apply_func(
+                        combined_filters, 'open', [item], kwargs=kwargs)
                 except MoreThanOneFilterError, e:
                     raise BuildError(e)
 
                 if not hunk:
-                    if is_url(c):
-                        hunk = UrlHunk(c)
+                    if is_url(item):
+                        hunk = UrlHunk(item)
                     else:
-                        hunk = FileHunk(c)
+                        hunk = FileHunk(item)
 
                 if no_filters:
                     hunks.append(hunk)
                 else:
                     hunks.append(filtertool.apply(
-                        hunk, combined_filters, 'input'))
+                        hunk, combined_filters, 'input', kwargs=kwargs))
 
         # Merge the individual files together.
         try:

src/webassets/filter/__init__.py

@@ -280,7 +280,7 @@ def output(self, _in, out, **kw):
         This will be called for every output file.
         """
 
-    def open(self, out, source, **kw):
+    def open(self, out, source_path, **kw):
         """Implement your actual filter here.
 
         This is like input(), but only one filter may provide this.

src/webassets/filter/coffeescript.py

@@ -28,7 +28,7 @@ class CoffeeScriptFilter(Filter):
         'no_bare': 'COFFEE_NO_BARE',
     }
 
-    def input(self, _in, out, source_path, output_path):
+    def input(self, _in, out, source_path, output_path, **kw):
         old_dir = os.getcwd()
         os.chdir(os.path.dirname(source_path))
         try:

src/webassets/filter/compass.py

@@ -69,7 +69,7 @@ class CompassFilter(Filter):
         'plugins': option('COMPASS_PLUGINS', type=list)
     }
 
-    def open(self, out, source, **kw):
+    def open(self, out, source_path, **kw):
         """Compass currently doesn't take data from stdin, and doesn't allow
         us accessing the result from stdout either.
 
@@ -108,8 +108,8 @@ def open(self, out, source, **kw):
             # compass' simplistic path handling, where it just assumes
             # source_path is within sassdir, and cuts off the length of
             # sassdir from the input file.
-            sassdir = path.normpath(path.dirname(source))
-            source = path.normpath(source)
+            sassdir = path.normpath(path.dirname(source_path))
+            source_path = path.normpath(source_path)
 
             # Compass offers some helpers like image-url(), which need
             # information about the urls under which media files will be
@@ -158,7 +158,7 @@ def open(self, out, source, **kw):
                             '--quiet',
                             '--boring',
                             '--output-style', 'expanded',
-                            source])
+                            source_path])
 
             proc = subprocess.Popen(command,
                                     stdout=subprocess.PIPE,
@@ -177,7 +177,7 @@ def open(self, out, source, **kw):
 
 
             guessed_outputfile = \
-                path.join(tempout, path.splitext(path.basename(source))[0])
+                path.join(tempout, path.splitext(path.basename(source_path))[0])
             f = open("%s.css" % guessed_outputfile)
             try:
                 out.write(f.read())

src/webassets/filter/cssrewrite/base.py

@@ -73,24 +73,13 @@ class CSSUrlRewriter(PatternRewriter):
     }
 
     def input(self, _in, out, **kw):
-        source_path, output_path = kw['source_path'], kw['output_path']
-
-        # Get source and output path relative to media directory (they are
-        # probably absolute paths, we need to work with them as env.url
-        # based urls (e.g. the following code will consider them absolute
-        # within a filesystem chrooted into env.url).
-        root = addsep(self.env.directory)
-        # To make common_path_prefix() work properly in all cases, make sure
-        # we remove stuff like ../ from all paths.
-        output_path = normpath(join(root, output_path))
-        source_path = normpath(join(root, source_path))
-        root = normpath(root)
-
-        cpp = common_path_prefix
+        source, source_path, output, output_path = \
+            kw['source'], kw['source_path'], kw['output'], kw['output_path']
+
         self.source_path = source_path
         self.output_path = output_path
-        self.output_url = path2url(output_path[len(cpp([root, output_path])):])
-        self.source_url = path2url(source_path[len(cpp([root, source_path])):])
+        self.source_url = self.env.absurl(source)
+        self.output_url = self.env.absurl(output)
 
         return super(CSSUrlRewriter, self).input(_in, out, **kw)
 

src/webassets/filter/handlebars.py

@@ -31,10 +31,10 @@ class HandlebarsFilter(JSTemplateFilter):
     # XXX Due to the way this filter works, any other filters applied
     # WILL BE IGNORED. Maybe this method should be allowed to return True
     # to indicate that the input() processor is not supported.
-    def open(self, out, source, **kw):
-        self.templates.append(source)
+    def open(self, out, source_path, **kw):
+        self.templates.append(source_path)
         # Write back or the cache would not detect changes
-        out.write(FileHunk(source).data())
+        out.write(FileHunk(source_path).data())
 
     def output(self, _in, out, **kw):
         if self.root is True:

src/webassets/filter/jst.py

@@ -47,7 +47,7 @@ def setup(self):
         self.include_jst_script = \
             (self.template_function == 'template') or not self.template_function
 
-    def input(self, _in, out, source_path, output_path):
+    def input(self, _in, out, source_path, output_path, **kw):
         data = _in.read()
         self.templates.append((source_path, data))
 

src/webassets/filter/less.py

@@ -16,9 +16,9 @@ class LessFilter(Filter):
         'less': ('binary', 'LESS_BIN')
     }
 
-    def open(self, out, source, **kw):
+    def open(self, out, source_path, **kw):
         proc = subprocess.Popen(
-            [self.less or 'lessc', source],
+            [self.less or 'lessc', source_path],
             stdout = subprocess.PIPE,
             stderr = subprocess.PIPE
         )

src/webassets/filter/less_ruby.py

@@ -39,7 +39,7 @@ class LessFilter(Filter):
         'less': ('binary', 'LESS_RUBY_PATH')
     }
 
-    def open(self, out, source, **kw):
+    def open(self, out, sourcePath, **kw):
         """Less currently doesn't take data from stdin, and doesn't allow
         us from stdout either. Neither does it return a proper non-0 error
         code when an error occurs, or even write to stderr (stdout instead)!
@@ -53,7 +53,7 @@ def open(self, out, source, **kw):
                                     'assets_temp_%d.css' % int(time.time()))
 
         proc = subprocess.Popen(
-            [self.less or 'lessc', source, outtemp_name],
+            [self.less or 'lessc', sourcePath, outtemp_name],
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             # shell: necessary on windows to execute

src/webassets/filter/pyscss.py

@@ -53,12 +53,14 @@ def setup(self):
         scss.ASSETS_ROOT = self.assets_url or self.env.url
         scss.ASSETS_URL = self.assets_root or self.env.directory
 
-    def input(self, _in, out, source_path, output_path):
+    def input(self, _in, out, **kw):
         """Like the original sass filter, this also needs to work as
         an input filter, so that relative @imports can be properly
         resolved.
         """
 
+        source_path = kw['source_path']
+
         # Because PyScss always puts the current working dir at first
         # place of the load path, this is what we need to use to make
         # relative references work.

src/webassets/filter/sass.py

@@ -139,7 +139,7 @@ def _apply_sass(self, _in, out, cd=None):
             if cd:
                 os.chdir(old_dir)
 
-    def input(self, _in, out, source_path, output_path):
+    def input(self, _in, out, source_path, output_path, **kw):
         if self.as_output:
             out.write(_in.read())
         else:

tests/test_ext/test_django/test_django.py

@@ -242,7 +242,7 @@ def setup(self):
             raise SkipTest()
 
         # Configure a staticfiles-using project.
-        settings.STATIC_ROOT = settings.MEDIA_ROOT
+        settings.STATIC_ROOT = settings.MEDIA_ROOT   # /media via baseclass
         settings.MEDIA_ROOT = self.path('needs_to_differ_from_static_root')
         settings.STATIC_URL = '/media/'
         settings.INSTALLED_APPS += ('django.contrib.staticfiles',)
@@ -295,6 +295,21 @@ def test_serve_built_files(self):
         from django_assets.finders import AssetsFinder
         assert AssetsFinder().find('out') == self.path("media/out")
 
+    def test_css_rewrite(self):
+        """Test that the cssrewrite filter can deal with staticfiles.
+        """
+        # file1 is in ./foo, file2 is in ./bar, the output will be
+        # STATIC_ROOT = ./media
+        self.create_files(
+                {'foo/css': 'h1{background: url("file1"), url("file2")}'})
+        self.mkbundle('css', filters='cssrewrite', output="out").build()
+        print self.get('media/out')
+        # The urls are NOT rewritte to foo/file1, but because all three
+        # directories are essentially mapped into the same url space, they
+        # remain as is.
+        assert self.get('media/out') == \
+                '''h1{background: url("file1"), url("file2")}'''
+
 
 class TestFilter(TempEnvironmentHelper):