Skip to content

Fixed #21581 -- Fixed a number of issues with collectstatic. #2057

Closed
wants to merge 1 commit into from

2 participants

@loic
Django member
loic commented Dec 9, 2013

No description provided.

@timgraham timgraham commented on the diff Dec 28, 2013
django/conf/global_settings.py
@@ -290,7 +290,7 @@
# Absolute path to the directory static files should be collected to.
# Example: "/var/www/example.com/static/"
-STATIC_ROOT = ''
+STATIC_ROOT = None
@timgraham
Django member
timgraham added a note Dec 28, 2013

setting documentation should be updated for the new default value

@loic
Django member
loic added a note Dec 29, 2013

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Dec 28, 2013
tests/staticfiles_tests/tests.py
@@ -225,6 +225,15 @@ def test_all_files_less_verbose(self):
self.assertIn('apps', force_text(lines[1]))
+class TestConfiguration(StaticFilesTestCase):
+ def test_location_empty(self):
+ err = six.StringIO()
+ for root in ['', None]:
+ with override_settings(STATIC_ROOT=root):
+ with self.assertRaises(ImproperlyConfigured):
@timgraham
Django member
timgraham added a note Dec 28, 2013

should we verify exception message as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on the diff Dec 28, 2013
...trib/staticfiles/management/commands/collectstatic.py
@@ -137,32 +137,38 @@ def collect(self):
def handle_noargs(self, **options):
self.set_options(**options)
- # Warn before doing anything more.
- if (isinstance(self.storage, FileSystemStorage) and
+
+ message = ['\n']
+ if self.dry_run:
+ message.append(
+ 'You have activated the --dry-run option so no files will be modified.\n\n'
+ )
+
+ message.append(
+ 'You have requested to collect static files at the destination\n'
+ 'location as specified in your settings'
+ )
+
+ if (isinstance(self.storage._wrapped, FileSystemStorage) and
@timgraham
Django member
timgraham added a note Dec 28, 2013

could we add a test for this bug by verifying the output?

@loic
Django member
loic added a note Dec 29, 2013

I don't know how to test this since it's displayed when run in interactive mode. Any idea?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham
Django member

How can I reproduce the data loss issue? With a default project setup and running collectstatic I just seem to get an "admin" directory in directory that contains manage.py. I guess if I already had an admin directory with other stuff it would get nuked?

If we're going to backport this could you add it to the 1.6.2 release notes as well?

@loic
Django member
loic commented Dec 29, 2013

@timgraham did you use the --clear option? i.e. ./manage.py collectstatic --clear.

@loic
Django member
loic commented Dec 29, 2013

@timgraham, reading your comment again, I probably should specify that this wouldn't just nuke from the project root and below, but rather from the current cwd and below. So if you were to run /absolute/path/to/manage.py collectstatic --clear --noinput from your home directory, you would nuke your whole home directory.

@loic loic Fixed #21581 -- Fixed a number of issues with collectstatic.
When STATIC_ROOT wasn't set, collectstatic --clear would delete
every files within the current directory and its descendants.

This patch makes the following changes:

Prevent collectstatic from running if STATIC_ROOT isn't set.

Fixed an issue that prevented collectstatic from displaying the
destination directory.

Changed the warning header to notify when the command is run
in dry-run mode.
0374eda
@timgraham
Django member

merged in 4befb30.

@timgraham timgraham closed this Dec 31, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.