Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#33253 closed Bug (fixed)

Collectstatic fails when using ManifestStaticFilesStorage and specific Javascript file

Reported by: Hervé Le Roy Owned by: Carlton Gibson
Component: contrib.staticfiles Version: 4.0
Severity: Release blocker Keywords: ManifestStaticFilesStorage
Cc: gilmrjc Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

python manage.py collectstatic --no-input
Post-processing 'rest_framework/docs/js/highlight.pack.js' failed!

Steps to reproduce the issue

Start a new project
django-admin startproject mysite

Install the following packages

Django==4.0b1
djangorestframework~=3.12

Add rest_framework to installed apps

Update settings.py to use ManifestStaticFilesStorage

STATIC_ROOT = BASE_DIR / 'static'
STATICFILES_STORAGE = 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage'

Run collecstatic

python manage.py collectstatic --no-input
Post-processing 'rest_framework/docs/js/highlight.pack.js' failed!

Traceback (most recent call last):
  File "/home/Devs/Sandbox/django4/mysite/manage.py", line 22, in <module>
    main()
  File "/home/Devs/Sandbox/django4/mysite/manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/core/management/__init__.py", line 425, in execute_from_command_line
    utility.execute()
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/core/management/__init__.py", line 419, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/core/management/base.py", line 373, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/core/management/base.py", line 417, in execute
    output = self.handle(*args, **options)
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/contrib/staticfiles/management/commands/collectstatic.py", line 187, in handle
    collected = self.collect()
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/contrib/staticfiles/management/commands/collectstatic.py", line 134, in collect
    raise processed
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/contrib/staticfiles/storage.py", line 321, in _post_process
    content = pattern.sub(converter, content)
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/contrib/staticfiles/storage.py", line 207, in converter
    hashed_url = self._url(
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/contrib/staticfiles/storage.py", line 144, in _url
    hashed_name = hashed_name_func(*args)
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/contrib/staticfiles/storage.py", line 371, in _stored_name
    cache_name = self.clean_name(self.hashed_name(name))
  File "/home/Devs/Sandbox/django4/venv/lib/python3.9/site-packages/django/contrib/staticfiles/storage.py", line 106, in hashed_name
    raise ValueError("The file '%s' could not be found with %r." % (filename, self))
ValueError: The file 'rest_framework/docs/js/,constant:' could not be found with <django.contrib.staticfiles.storage.ManifestStaticFilesStorage object at 0x7f3aa1b7ebe0>.

It used to work with previous versions of ManifestStaticFilesStorage. Maybe a regression with the recent additions of ES Module support and Javascript source map support ?

Change History (8)

comment:1 by Keryn Knight, 3 years ago

Triage Stage: UnreviewedAccepted

Tentatively marking as accepted, and indeed relates to ESM support via ticket #32319 (commit 91e21836f667c784a8a63ab1f18d81f553e679cb)

Looks like it's this regex:

r"""(?P<matched>import\s+(?s:(?P<imports>.*?))\s*from\s*["'](?P<url>.*?)["'])"""

which is being used like so:

(Pdb) matchobj
<re.Match object; span=(15745, 125465), match='import from as",c:[e.ASM,e.QSM]},{cN:"class",bK:">
(Pdb) matchobj.groupdict().keys()
dict_keys(['matched', 'imports', 'url'])
(Pdb) len(matches['matched'])
109720
(Pdb) matches['url']
',constant:'
Version 2, edited 3 years ago by Keryn Knight (previous) (next) (diff)

comment:2 by Keryn Knight, 3 years ago

Severity: NormalRelease blocker

Better to be safe than sorry, worst that can happen is that these states are reverted.

comment:3 by Mariusz Felisiak, 3 years ago

Cc: gilmrjc added

comment:4 by Hervé Le Roy, 3 years ago

The following regex restricts characters for identifiers:

<matched>import\s+(?:(?P<imports>[\*\{\}\,\w ]*?))\s*from\s*[\"'](?P<url>.*?)[\"'])

It works with several import syntaxes: Regex101
It doesn't fail with the example provided in the ticket (highlight.pack.js).

Probably needs further testing and feedback.

comment:5 by Hervé Le Roy, 3 years ago

For both import and export:

r"""(?P<matched>import\s+(?:(?P<imports>[\*\{\}\,\w ]*?))\s*from\s*[\"'](?P<url>.*?)[\"'])"""
r"""(?P<matched>export\s+(?:(?P<exports>[\*\{\}\,\w ]*?))\s*from\s*[\"'](?P<url>.*?)[\"'])"""

I have also:

  • removed the 's' in (?s: (Regex101 was reporting it as incorrect in Python regex syntax, and, anyway, Javascript import statement can be multi-line)
  • escaped double-quotes

comment:6 by Carlton Gibson, 3 years ago

Has patch: set
Owner: changed from nobody to Carlton Gibson
Status: newassigned

I've opened a draft PR for this that tightens the regex to ≈valid identifiers plus {},\s due to appearing in the import/export.
I'm sure it can be improved. Comments welcome there.

removed the 's' in (?s: (Regex101 was reporting it as incorrect in Python regex syntax, and, anyway, Javascript import statement can be multi-line

(?s) is `re.DOTALL` allowing . to match newlines, which matters for the multi-line case.

comment:7 by GitHub <noreply@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In ba9ced3e:

Fixed #33253 -- Reverted "Fixed #32319 -- Added ES module support to ManifestStaticFilesStorage."

This reverts commit 91e21836f667c784a8a63ab1f18d81f553e679cb.

export and import directives have several syntax variants and not
all of them were properly covered.

Thanks Hervé Le Roy for the report.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In b7b3bbc8:

[4.0.x] Fixed #33253 -- Reverted "Fixed #32319 -- Added ES module support to ManifestStaticFilesStorage."

This reverts commit 91e21836f667c784a8a63ab1f18d81f553e679cb.

export and import directives have several syntax variants and not
all of them were properly covered.

Thanks Hervé Le Roy for the report.
Backport of ba9ced3e9a643a05bc521f0a2e6d02e3569de374 from main

Note: See TracTickets for help on using tickets.
Back to Top