Opened 13 years ago

Last modified 13 years ago

#16161 closed Bug

If extra static files are added into STATIC_ROOT, they are transformed to symlinks in the next run of ``collectstatic -l`` — at Version 12

Reported by: George Notaras Owned by: Aymeric Augustin
Component: contrib.staticfiles Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Aymeric Augustin)

Here are the related settings:

MEDIA_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), 'media', '')).replace('\\','/')
MEDIA_URL = '/media/'
STATIC_ROOT = os.path.join(MEDIA_ROOT, 'site')
STATIC_URL = MEDIA_URL + 'site/'
ADMIN_MEDIA_PREFIX = STATIC_URL + 'admin/'
STATICFILES_DIRS = ()
STATICFILES_FINDERS = (
    'django.contrib.staticfiles.finders.FileSystemFinder',
    'django.contrib.staticfiles.finders.AppDirectoriesFinder',
#    'django.contrib.staticfiles.finders.DefaultStorageFinder',
    'compressor.finders.CompressorFinder',
)

How to re-produce:

  1. Run : python manage.py collectstatic -l --noinput
  2. Put some extra static files in STATIC_ROOT
  3. Re-run python manage.py collectstatic -l --noinput
  4. The extra files have been replaced by symlinks pointing to themselves.

Change History (12)

comment:1 by George Notaras, 13 years ago

Summary: If extra static files are added into STATIC_ROOT are transformed to symlinks in the next run of ``collectstatic -l``If extra static files are added into STATIC_ROOT, they are transformed to symlinks in the next run of ``collectstatic -l``

comment:2 by Aymeric Augustin, 13 years ago

Component: contrib.staticfilesDocumentation
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

You shouldn't put some extra static files in STATIC_ROOT.

I expected to find a big warning in the docs: "Don't put your static files in STATIC_ROOT", but I didn't. We should add it.

comment:3 by Aymeric Augustin, 13 years ago

Summary: If extra static files are added into STATIC_ROOT, they are transformed to symlinks in the next run of ``collectstatic -l``Warn that extra static files shouldn't be added directly into STATIC_ROOT

in reply to:  2 ; comment:4 by George Notaras, 13 years ago

Replying to aaugustin:

You shouldn't put some extra static files in STATIC_ROOT.

So, what you say is that the transformation of a file to a symlink to itself is expected behavior from a software. What if I didn't have a backup of that file? I think this issue requires more clarification before changing the subject and the ticket type.

Why would putting extra files in STATIC_ROOT manually be a problem? These extra files are still files required for proper display of the web site, like a custom logo or a custom image map and the place they belong to is STATIC_ROOT.

Suppose that STATIC_URL points to a separate domain dedicated to serving web site media. Why shouldn't I be able to put the logo or a custom CSS file in there? Where would be the right place to put the custom logo?

The change of the summary line is unacceptable as it practically buries the reported issue. Please change it back to the original.

in reply to:  4 comment:5 by George Notaras, 13 years ago

STATICFILES_DIRS can still be utilized so as to avoid adding the extra files manually, but the original bug still remains.

in reply to:  2 ; comment:6 by Ramiro Morales, 13 years ago

Component: Documentationcontrib.staticfiles
Summary: Warn that extra static files shouldn't be added directly into STATIC_ROOTIf extra static files are added into STATIC_ROOT, they are transformed to symlinks in the next run of ``collectstatic -l``
Type: Cleanup/optimizationBug

Replying to aaugustin:

You shouldn't put some extra static files in STATIC_ROOT.

I expected to find a big warning in the docs: "Don't put your static files in STATIC_ROOT", but I didn't. We should add it.

It is just there, in the STATIC_ROOT setting documentation:

Warning

This should be an (initially empty) destination directory for collecting your static files from their permanent locations into one directory for ease of deployment; it is not a place to store your static files permanently. You should do that in directories that will be found by staticfiles's finders, which by default, are 'static/' app sub-directories and any directories you include in STATICFILES_DIRS).

Replying to gnotaras:

Why would putting extra files in STATIC_ROOT manually be a problem? These extra files are still files required for proper display of the web site, like a custom logo or a custom image map and the place they belong to is STATIC_ROOT.

No, as per above they belong to a directory listed in STATICFILES_DIRS

But I agree replacing fiels with links to self isn't right.

in reply to:  6 comment:7 by George Notaras, 13 years ago

Why would putting extra files in STATIC_ROOT manually be a problem? These extra files are still files required for proper display of the web site, like a custom logo or a custom image map and the place they belong to is STATIC_ROOT.

No, as per above they belong to a directory listed in STATICFILES_DIRS

Hello. There are times that static files can be automatically generated. An example case is the dynamic concatenation of CSS and JS files. The resulting compressed files normally have to be stored within STATIC_ROOT. Storing them elsewhere does not make sense. I think that the requirement of an initially empty STATIC_ROOT should not exist. Django should update any files within STATIC_ROOT only if a file with the same name is found by the static finders.

comment:8 by Aymeric Augustin, 13 years ago

STATIC_ROOT is a build target. It's managed by the build system. We're not supposed to alter it.

collectstatic says that it will overwrite everything inside:

You have requested to collect static files at the destination
location as specified in your settings file.

This will overwrite existing files.
Are you sure you want to do this?

Type 'yes' to continue, or 'no' to cancel: 

If it must be altered, that is a step of the deployment processs that must happen after collectstatic. For your example, JS / CSS minification, see django-compressor: compress runs after collectstatic.


Still, it's true that the current behavior of collectstatic -l when someone adds files to STATIC_ROOT does not make sense.

I think the most predictable behavior is to delete any file in STATIC_ROOT that is not found in one of STATICFILES_DIRS.

If we don't do this, developers may believe that they can add stuff to STATIC_ROOT ("Hey dude! It just works!"), and that creates a data-loss problem. Suppose I add manually a "logo.png" file in STATIC_ROOT on the production server, and later on, someone else creates "logo.png" in one of STATICFILES_DIRS. During the next collectstatic, the version from STATIC_ROOT will be overwritten silently and lost!

Short version : if it breaks (ie. a file is overwritten or deleted), it'd better break now and loudly than at some point in the future and silently.

in reply to:  8 ; comment:9 by George Notaras, 13 years ago

Replying to aaugustin:

collectstatic says that it will overwrite everything inside:

Thanks for your detailed reply. Overwriting or forcing the creation of new symlinks for the those files that are found by the finders in the app directories or in the STATICFILES_DIRS, is the expected behavior (at least for me).

The cleanup of STATIC_ROOT, removing any files or symlinks that are not found by the finders, could be useful, but it does not take place right now. There could be a command line switch for collectstatic that adds cleanup functionality.

Still, it's true that the current behavior of collectstatic -l when someone adds files to STATIC_ROOT does not make sense.

I think the most predictable behavior is to delete any file in STATIC_ROOT that is not found in one of STATICFILES_DIRS.

If we don't do this, developers may believe that they can add stuff to STATIC_ROOT ("Hey dude! It just works!"), and that creates a data-loss problem. Suppose I add manually a "logo.png" file in STATIC_ROOT on the production server, and later on, someone else creates "logo.png" in one of STATICFILES_DIRS. During the next collectstatic, the version from STATIC_ROOT will be overwritten silently and lost!

Short version : if it breaks (ie. a file is overwritten or deleted), it'd better break now and loudly than at some point in the future and silently.

You are right about this, but I would prefer deletions inside STATIC_ROOT to take place only if the user used a command line switch and not by default.

---

All in all, I'd like to clarify that I did not mean to complain about collectstatic. The way it currently works is fantastic. The only issue I encountered occurs only if the -l switch is used, in which case any files inside STATIC_ROOT that are not found by the static-finders are replaced by symlinks-to-self. If -l is not used, then everything works fine. I am sorry if I did not clarify it from the beginning.

in reply to:  9 comment:10 by Aymeric Augustin, 13 years ago

Replying to gnotaras:

All in all, I'd like to clarify that I did not mean to complain about collectstatic.

Sorry if my comments sounded too harsh. We're all trying to make Django as useful as possible.


The way it currently works is fantastic. The only issue I encountered occurs only if the -l switch is used, in which case any files inside STATIC_ROOT that are not found by the static-finders are replaced by symlinks-to-self. If -l is not used, then everything works fine. I am sorry if I did not clarify it from the beginning.

Yes, I understand this.

I think we have to choose between the following three options:

  • 1) just fix the problem with the "-l" switch; it's an improvement, but we still have an underdefined behavior when STATIC_ROOT contains extra files (either added manually to STATIC_ROOT, or removed from STATICFILES_DIRS), and we're exposed to the data-loss problem I explained;
  • 2) normalize the behavior of collectstatic by removing any extra files; but that's backwards incompatible for people that rely on the current, undocumented behavior;
  • 3) same as 2), but with a command-line option to control whether collectstatic will clean up unused files; the default value of this option is debatable (backward compatibility vs. predictability).

comment:11 by Luke Plant, 13 years ago

I think we should get collectstatic to properly clean out all existing files. There is an additional reason for this:

Existing files, possibly from a previous version of the project, can cause bugs. For example, suppose there is a javascript file that is loaded by some pages and alters some behaviour. Suppose the javascript file is deliberately removed, to remove that behaviour, but not all of the templates it is used in are corrected.

In development, the developer will test the pages, and they will have changed their behaviour as required, because even when the file is requested by the browser, the static files view will not find the js file, and not return it. However, in production, since STATIC_ROOT is not cleaned, the old file is still there, and the old behaviour will occur.

So, we should fix it aaugustin suggested.

However, since we've never explicitly said that the process will delete existing files, we should: 1) allow a command line switch to *disable* it (it should be on by default) 2) put a note in the release notes about the new default behaviour.

comment:12 by Aymeric Augustin, 13 years ago

Description: modified (diff)
Owner: changed from nobody to Aymeric Augustin

Thanks Luke for making the decision. I'll work on a patch.

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