[Django] #24890: Add system check for static files that will always be overwritten on collectstatic
Django
collectstatic
-------------------------------------+--------------------
Reporter: giuliettamasina | Owner: nobody
Type: New feature | Status: new
Component: contrib.staticfiles | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+--------------------
If you have a project with several apps, some of which have static files
directly in their `static` folder named identically, one of them will
always be overwritten by the other when running `collectstatic`. The
recommendation, and what is noted in the
[https://docs.djangoproject.com/en/1.8/howto/static-files/ documentation],
is to namespace by keeping all static files belonging to an app in an
additional subdirectory in the `static` folder named like the app name.
But if you're not doing that, the result can be very confusing. I've seen
many developers run into this and spending quite some time before figuring
it out through the documentation or otherwise.
I think there should be a default system check that looks for clashing
static file names as described above, and warn if there are files that
will overwrite each other when running `collectstatic`.
--
Ticket URL: <https://code.djangoproject.com/ticket/24890>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+------------------------------------
Reporter: giuliettamasina | Owner: nobody
Type: New feature | Status: new
Component: contrib.staticfiles | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_docs: => 0
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
Comment:
I'm not sure a system check is a good idea since it seems such a check
would involve a lot of disk reading and it could be expensive to do that
each time the dev server is restarted (currently all system checks are
simply static code analysis). Perhaps a warning when running collecstatic
would be a better approach?
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:1>
Django
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Component: contrib.staticfiles | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by giuliettamasina):
* owner: nobody => giuliettamasina
* status: new => assigned
Comment:
That sounds reasonable. Would it make sense to make such check as part of
the management command itself? I'll probably work on this during the
DjangoCon Europe sprints, so I'm assigning this ticket to myself for now
:)
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Yes, I envisioned the warning as part of collectstatic's logging output.
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by giuliettamasina):
* has_patch: 0 => 1
Comment:
I've open a pull request for this:
https://github.com/django/django/pull/4740
A couple of thoughts:
* Would it make more sense to use `warnings.warn()` instead of logging
with level 0?
* Any pointers on how to write tests for this change?
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by giuliettamasina):
I did another commit to change the log level from 0 to 1, since it would
show up in e.g. test output otherwise.
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_tests: 0 => 1
Comment:
You can grep for `stdout=out` in Django's test suit to see how to capture
output from management commands.
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by giuliettamasina):
Great, thanks a lot! Will have a look at that and add tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by giuliettamasina):
I've added tests to the pull request now, would love some review of it.
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Reviewed the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by giuliettamasina):
I've updated the pull request according to code review. Should I squash
all of the commits into a single one?
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by giuliettamasina):
* cc: markus.magnuson@… (added)
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: giuliettamasina | Owner:
| giuliettamasina
Component: contrib.staticfiles | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"5304494585c58b0c9245ea9896a6d6122a8673a2" 53044945]:
{{{
#!CommitTicketReference repository=""
revision="5304494585c58b0c9245ea9896a6d6122a8673a2"
Fixed #24890 -- Added warning to collectstatic when static files have
clashing names
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24890#comment:13>