jazzband / django-pipeline

Pipeline is an asset packaging library for Django.
https://django-pipeline.readthedocs.io/
MIT License
1.52k stars 372 forks source link

Glob syntax does not appear to work #155

Open mikeknoop opened 12 years ago

mikeknoop commented 12 years ago

I have a directory with two JS files in it. The following config outputs no content:

PIPELINE_JS = {
    'external': {
        'source_filenames': (
          'javascripts/lib/backbone/*.js',
        ),
        'output_filename': 'build/external.js',
    },
}

however the following config outputs the concatted, minified contents:

PIPELINE_JS = {
    'external': {
        'source_filenames': (
          'javascripts/lib/backbone/backbone-edge.js',
          'javascripts/lib/backbone/localstorage.js',
        ),
        'output_filename': 'build/external.js',
    },
}

Any ideas? Did not see any reason to suspect the first config would not work as expected.

mikeknoop commented 12 years ago

To clarify, the contents of external.js after running python manage.py collectstatic

mikeknoop commented 12 years ago

I found the root cause. I was using named STATICFILES_DIRS:

ASSETS_STATIC = os.path.join(ROOT_DIR, 'assets')
STATICFILES_DIRS = (
    ("images", os.path.join(ROOT_DIR, 'assets/images')),
    ("stylesheets", os.path.join(ROOT_DIR, 'assets/stylesheets')),
    ("javascripts", os.path.join(ROOT_DIR, 'assets/javascripts')),
)

GIven my origin example config, this caused pipeline/storage.py:listdir to try each path:

  1. /.../assets/images/javascripts/lib/backbone (EXCEPTION: doesn't exist)
  2. /.../assets/stylesheets/javascripts/lib/backbone (EXCEPTION: doesn't exist)
  3. /.../assets/javascripts/javascripts/lib/backbone (EXCEPTION: doesn't exist)

The real path is: /.../assets/javascripts/lib/backbone. Because all the finders excepted out, the glob lookup failed with an exception and was silently handled as a no-op in pipeline/glob.py:glob1

The solution is to drop back to not using named STATICFILES_DIRS:

ASSETS_STATIC = os.path.join(ROOT_DIR, 'assets')
STATICFILES_DIRS = (
    ASSETS_STATIC,
)

It is not perfect, I am now including extra static folders in /assets that I did not originally want to. However it fixed the glob lookup problem.

By the way, if I try and trick Pipeline by using this config:

PIPELINE_JS = {
    'external': {
        'source_filenames': (
          'lib/backbone/*.js',
        ),
        'output_filename': 'build/external.js',
    },
}

then pipeline/storage.py:listdir successfully finds the directory and returns it, but I get another exception elsewhere:

  File "/.../pipeline/storage.py", line 125, in find_storage
    raise ValueError("The file '%s' could not be found with %r." % (name, self))
ValueError: The file 'lib/backbone/backbone-0.9.2.js' could not be found with <pipeline.storage.PipelineFinderStorage object at 0x2616a50>.

It appears that the pipeline/storage.py:find_storage does not take into account the possible named STATICFILES_DIRS like the pipeline/storage.py:listdir method does.

Is this a case of Pipeline mishandling something? Is there a way to use Pipeline with named STATICFILES_DIRS?

cyberdelia commented 12 years ago

You seem to have found a loophole with how we deal with staticfiles prefixes. I will give a try, but I would love to have some input from @chipx86 about this before.

chipx86 commented 12 years ago

Hey guys,

Mike, you and I have very similar configurations, except I'm not using globbing and instead am explicitly listing each file (which I find beneficial, as I can be certain about the ordering and can be sure no in-progress or temporary files, patch-provided files, or non-minified copies of otherwise minified files are included unless specifically listed).

In my setup, I have:

STATICFILES_DIRS = (
    ('lib', os.path.join(REVIEWBOARD_ROOT, 'static', 'lib')),
    ('rb', os.path.join(REVIEWBOARD_ROOT, 'static', 'rb')),
    ('djblets', os.path.join(os.path.dirname(djblets.__file__), 'media')),
)

STATICFILES_FINDERS = (
    'django.contrib.staticfiles.finders.FileSystemFinder',
    'django.contrib.staticfiles.finders.AppDirectoriesFinder',
)

STATICFILES_STORAGE = 'pipeline.storage.PipelineCachedStorage'

...

PIPELINE_JS = {
    '3rdparty': {
        'source_filenames': (
            'lib/js/underscore-1.3.3.min.js',
            'lib/js/backbone-0.9.2.min.js',
            'lib/js/jquery.form.js',
            'lib/js/jquery.timesince.js',
            'lib/js/ui.autocomplete.js',
        ),
        'output_filename': 'lib/js/3rdparty.min.js',
    },
    'common': {
        'source_filenames': (
            'rb/js/utils/backboneUtils.js',
            'rb/js/common.js',
            'rb/js/datastore.js',
        ),
        'output_filename': 'rb/js/base.min.js',
    },
    ...
}

PIPELINE = True

I wouldn't know off-hand how to make it properly support globbing for source filenames.

Now, for your second problem, I've hit that before in general, though not with a globbing attempt. I think that was fixed in a release of pipeline at one point, along with the configuration I use above for the finders and storage.

For reference, my full settings file with this information is at:

https://github.com/reviewboard/reviewboard/blob/master/reviewboard/settings.py

mikeknoop commented 12 years ago

I can verify that explicit file paths work with named STATICFILES_DIRS. When using explicit paths, pipeline/glob.py will use the glob0 method instead of the glob1 method to find the file.

So it would seem to be a small problem with pipeline/storage.py:listdir trying each source_filename appended to each STATICFILES_DIRS path. It should instead only append each source_filename to STATIC_ROOT when looking up globs in glob1

cyberdelia commented 12 years ago

Ok, looks like it's confirmed, I will try to fix it as soon as possible.

yuvadm commented 10 years ago

+1'ing this issue, there's no documentation of globbing not working when using named static dirs.