[Django] #27658: collectstatic overwrites newer files in remote storage
Django
-------------------------------------+-------------------------------------
Reporter: pgcd | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.10
contrib.staticfiles |
Severity: Normal | Keywords: staticfiles
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
The change to
django/contrib/staticfiles/management/commands/collectstatic.py in commit
2cd2d188516475ddf256e6267cd82c495fb5c430 causes the command to overwrite
all static files in remote storage (eg. S3), instead of only updatable
ones.
A quick workaround is to override the command and restore the previous
Command.delete_file() in the subclass, but I suppose this should be fixed.
--
Ticket URL: <https://code.djangoproject.com/ticket/27658>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: pgcd | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: staticfiles | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):
* easy: 1 => 0
Comment:
Could you be more specific about the steps to reproduce the issue (ideally
without S3 involved since Django's test suite cannot rely on that)? If you
could provide a test case for `tests/staticfiles_tests` that would be
great.
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: pgcd | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: staticfiles | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Rebkavets Vitali):
This line
[https://github.com/django/django/commit/2cd2d188516475ddf256e6267cd82c495fb5c430
#diff-5e526cd393e2652c1d7f96704889a434R263 Github link]
full_path for remote storages such as S3 is always None
The clause was improperly refactored
It should look like
{{{
...
if (target_last_modified.replace(microsecond=0) >=
source_last_modified.replace(microsecond=0) and
if not full_path or full_path and not
(self.symlink ^ os.path.islink(full_path))):
...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 1.10
Severity: Normal | Resolution:
Keywords: staticfiles | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Description changed by Paolo Dente:
Old description:
> The change to
> django/contrib/staticfiles/management/commands/collectstatic.py in commit
> 2cd2d188516475ddf256e6267cd82c495fb5c430 causes the command to overwrite
> all static files in remote storage (eg. S3), instead of only updatable
> ones.
>
> A quick workaround is to override the command and restore the previous
> Command.delete_file() in the subclass, but I suppose this should be
> fixed.
New description:
@Tim: as far as I can tell, any non-local storage should be affected so a
possible test would require mocking Storage or the management command so
that Command.local() is False; I can try and write one. On the other hand,
it looks like @Vitali spotted the problem with the refactoring, so I don't
know how to proceed from here.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 1.10
Severity: Normal | Resolution:
Keywords: staticfiles | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Old description:
> @Tim: as far as I can tell, any non-local storage should be affected so a
> possible test would require mocking Storage or the management command so
> that Command.local() is False; I can try and write one. On the other
> hand, it looks like @Vitali spotted the problem with the refactoring, so
> I don't know how to proceed from here.
New description:
The change to
django/contrib/staticfiles/management/commands/collectstatic.py in commit
2cd2d188516475ddf256e6267cd82c495fb5c430 causes the command to overwrite
all static files in remote storage (eg. S3), instead of only updatable
ones.
A quick workaround is to override the command and restore the previous
Command.delete_file() in the subclass, but I suppose this should be fixed.
--
Comment (by Paolo Dente):
@Tim: as far as I can tell, any non-local storage should be affected so a
possible test would require mocking Storage or the management command so
that Command.local() is False; I can try and write one. On the other hand,
it looks like @Vitali spotted the problem with the refactoring, so I don't
know how to proceed from here.
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:4>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 1.10
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+------------------------------------
Changes (by Claude Paroz):
* stage: Unreviewed => Accepted
Comment:
The next step is to write a test that demonstrates the current regression.
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:5>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 1.10
Severity: Normal | Resolution:
Keywords: staticfiles | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/7767 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:6>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 1.10
Severity: Normal | Resolution:
Keywords: staticfiles | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:7>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 1.10
Severity: Normal | Resolution:
Keywords: staticfiles | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by Rebkavets Vitali):
Here is a code snippet demonstrating the problem with the refactoring:
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:8>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 1.10
Severity: Release blocker | Resolution:
Keywords: staticfiles | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+------------------------------------
* severity: Normal => Release blocker
* needs_tests: 1 => 0
Comment:
I added a test to the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:9>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: closed
Component: contrib.staticfiles | Version: 1.10
Severity: Release blocker | Resolution: fixed
Keywords: staticfiles | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"c85831e4b7b5a7e4249df10327175b7251cb012d" c85831e4]:
{{{
#!CommitTicketReference repository=""
revision="c85831e4b7b5a7e4249df10327175b7251cb012d"
Fixed #27658 -- Prevented collectstatic from overwriting newer files in
remote storages.
Thanks revimi for the initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:10>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: closed
Component: contrib.staticfiles | Version: 1.10
Severity: Release blocker | Resolution: fixed
Keywords: staticfiles | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"da9b36c52d403a8de77682f4892d8890075f1289" da9b36c5]:
{{{
#!CommitTicketReference repository=""
revision="da9b36c52d403a8de77682f4892d8890075f1289"
[1.10.x] Fixed #27658 -- Prevented collectstatic from overwriting newer
files in remote storages.
Thanks revimi for the initial patch.
Backport of c85831e4b7b5a7e4249df10327175b7251cb012d from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27658#comment:11>