BenjjinF / django-fontawesome-5

A utility for using icons in models, forms, and templates.
BSD 2-Clause "Simplified" License
21 stars 18 forks source link

Circular dependency in finding static files urls (for manifested files) #15

Open thclark opened 4 years ago

thclark commented 4 years ago

I think django-fontawesome-5 is incompatible with manifested static files collection (e.g. using whitenoise to efficiently cache and serve).

This is because the css_admin is not lazily evaluated at widgets.py, line 9:

css_admin = get_css_admin()

This gets called even when invoking management commands, like collectstatic. Of course, if you are compressing and/or manifesting your static files (e.g. like using a CDN or whitenoise) the call to static() (which gets their new, hashed, name) will check for the presence of the file in the manifest... which isn't there, because you haven't run collectstatic yet!

Without such manifesting, it's not a problem because django won't run the check for the presence of the modified filename.

Basically, you can't call get_css_admin() until such a time as you can reasonably expect that collectstatic will have completed. I think the fix is as simple as moving the call to the Media class below.

Here's a traceback:


-----> $ python manage.py collectstatic --noinput
       Traceback (most recent call last):
         File "manage.py", line 12, in <module>
           execute_from_command_line(sys.argv)
         File "/app/.heroku/python/lib/python3.6/site-packages/django/core/management/__init__.py", line 381, in execute_from_command_line
           utility.execute()
         File "/app/.heroku/python/lib/python3.6/site-packages/django/core/management/__init__.py", line 357, in execute
           django.setup()
         File "/app/.heroku/python/lib/python3.6/site-packages/django/__init__.py", line 24, in setup
           apps.populate(settings.INSTALLED_APPS)
         File "/app/.heroku/python/lib/python3.6/site-packages/django/apps/registry.py", line 114, in populate
           app_config.import_models()
         File "/app/.heroku/python/lib/python3.6/site-packages/django/apps/config.py", line 211, in import_models
           self.models_module = import_module(models_module_name)
         File "/app/.heroku/python/lib/python3.6/importlib/__init__.py", line 126, in import_module
           return _bootstrap._gcd_import(name[level:], package, level)
         File "<frozen importlib._bootstrap>", line 994, in _gcd_import
         File "<frozen importlib._bootstrap>", line 971, in _find_and_load
         File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
         File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
         File "<frozen importlib._bootstrap_external>", line 678, in exec_module
         File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
         File "/tmp/build_46af82c36d4c7b74c2828a2c3960e015/planex/cms_site/models/__init__.py", line 1, in <module>
           from .pages import PageSectionsOrderable, SitePage  # noqa: F401
         File "/tmp/build_46af82c36d4c7b74c2828a2c3960e015/planex/cms_site/models/pages.py", line 3, in <module>
           from fontawesome_5.fields import IconField
         File "/app/.heroku/python/lib/python3.6/site-packages/fontawesome_5/fields.py", line 8, in <module>
           from .forms import IconFormField
         File "/app/.heroku/python/lib/python3.6/site-packages/fontawesome_5/forms.py", line 7, in <module>
           from .widgets import IconWidget
         File "/app/.heroku/python/lib/python3.6/site-packages/fontawesome_5/widgets.py", line 9, in <module>
           css_admin = get_css_admin()
         File "/app/.heroku/python/lib/python3.6/site-packages/fontawesome_5/app_settings.py", line 31, in get_css_admin
           css = [static('fontawesome_5/css/django-fontawesome.css'), static('fontawesome_5/css/all.min.css')]
         File "/app/.heroku/python/lib/python3.6/site-packages/django/templatetags/static.py", line 167, in static
           return StaticNode.handle_simple(path)
         File "/app/.heroku/python/lib/python3.6/site-packages/django/templatetags/static.py", line 118, in handle_simple
           return staticfiles_storage.url(path)
         File "/app/.heroku/python/lib/python3.6/site-packages/django/contrib/staticfiles/storage.py", line 153, in url
           return self._url(self.stored_name, name, force)
         File "/app/.heroku/python/lib/python3.6/site-packages/django/contrib/staticfiles/storage.py", line 132, in _url
           hashed_name = hashed_name_func(*args)
         File "/app/.heroku/python/lib/python3.6/site-packages/django/contrib/staticfiles/storage.py", line 420, in stored_name
           raise ValueError("Missing staticfiles manifest entry for '%s'" % clean_name)
       ValueError: Missing staticfiles manifest entry for 'fontawesome_5/css/django-fontawesome.css'
 !     Error while running '$ python manage.py collectstatic --noinput'.
thclark commented 4 years ago

Some further discussion of the same issue in https://code.djangoproject.com/ticket/21221

mcrot commented 3 years ago

Thanks @thclark for the analysis, I think have the same issue, which makes it impossible to use the icons in my project together with whitenoise. A workaround is simply not possible, right? Since I need to use sth. like whitenoise in my setup with Django 3, I cannot use this package.

The fix seems to be simple, but I'm not sure how to write a unit test for it. How can I force manifested static files collection in a test? Could this be done by just setting

STATICFILES_STORAGE = 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage'

in the settings in the test and then calling 'collectstatic'?

thclark commented 3 years ago

Thanks @thclark for the analysis, I think have the same issue, which makes it impossible to use the icons in my project together with whitenoise. A workaround is simply not possible, right? Since I need to use sth. like whitenoise in my setup with Django 3, I cannot use this package.

The fix seems to be simple, but I'm not sure how to write a unit test for it. How can I force manifested static files collection in a test? Could this be done by just setting

STATICFILES_STORAGE = 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage'

in the settings in the test and then calling 'collectstatic'?

Hmm. I don't know actually. Yes, I guess so! Even if untested in this particular scenario it'd be a helpful fix to have (I'm still in the dark ages on font awesome 4!)

thclark commented 3 years ago

@mcrot this might be of help; use call_command to do the static collection within a unit test... https://adamj.eu/tech/2020/09/07/how-to-unit-test-a-django-management-command/

mcrot commented 3 years ago

There seem to be no tests so far, so maybe I should not be holier than the pope.

I've created a PR with your fix, but I haven't tested it yet. Is it easy for you to do a manual test? For me it's a bit difficult in my setup.

However, I will also try the package django-icons as an alternative which is using a CDN.

mcrot commented 3 years ago

Thanks for the pointer to call_command, I would definitely need it for a test.