Skip to content

Commit d79be25

Browse files
committed
Fix a regression in determining when to collect static files.
A commit made in 1.6.10 (ae6b3f8) changed a storage lookup to use the prefixed_path variable instead of the path variable when looking up modification times in order to determine if an existing collected file should be deleted and re-collected. By using the wrong path variable, the storage lookups always failed, resulting in every single file being re-collected every single time. This reduces performance considerably, particularly with large codebases. Since every file ends up copied, they appear as new and are recompiled. This can result in very long page reloads on a development server, especially if working on an older machine, in a VM, on a shared filesystem, or on the Linux subsystem for Windows. This is a simple change that fixes the lookup to use the correct variable. Unit tests were added to ensure this continues to work as expected and does not regress.
1 parent dbf66e6 commit d79be25

File tree

2 files changed

+24
-1
lines changed

2 files changed

+24
-1
lines changed

pipeline/collector.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def delete_file(self, path, prefixed_path, source_storage):
7878
else:
7979
try:
8080
# When was the source file modified last time?
81-
source_last_modified = self._get_modified_time(source_storage, prefixed_path)
81+
source_last_modified = self._get_modified_time(source_storage, path)
8282
except (OSError, NotImplementedError, AttributeError):
8383
pass
8484
else:

tests/tests/test_collector.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,17 @@
33
import os
44

55
from django.contrib.staticfiles import finders
6+
from django.core.files.storage import FileSystemStorage
67
from django.test import TestCase
78

89
from pipeline.collector import default_collector
910
from pipeline.finders import PipelineFinder
1011

1112

13+
def local_path(path):
14+
return os.path.abspath(os.path.join(os.path.dirname(__file__), '..', path))
15+
16+
1217
class CollectorTest(TestCase):
1318
def tearDown(self):
1419
super(CollectorTest, self).tearDown()
@@ -31,6 +36,24 @@ def test_collect_with_files(self):
3136
'pipeline/js/second.js',
3237
]))
3338

39+
def test_delete_file_with_modified(self):
40+
list(default_collector.collect())
41+
42+
storage = FileSystemStorage(local_path('assets'))
43+
new_mtime = os.path.getmtime(storage.path('js/first.js')) - 1000
44+
os.utime(default_collector.storage.path('pipeline/js/first.js'),
45+
(new_mtime, new_mtime))
46+
47+
self.assertTrue(default_collector.delete_file(
48+
'js/first.js', 'pipeline/js/first.js', storage))
49+
50+
def test_delete_file_with_unmodified(self):
51+
list(default_collector.collect(files=['pipeline/js/first.js']))
52+
53+
self.assertFalse(default_collector.delete_file(
54+
'js/first.js', 'pipeline/js/first.js',
55+
FileSystemStorage(local_path('assets'))))
56+
3457
def _get_collectable_files(self):
3558
for finder in finders.get_finders():
3659
if not isinstance(finder, PipelineFinder):

0 commit comments

Comments
 (0)