Opened 10 years ago
Closed 10 years ago
#23436 closed Cleanup/optimization (fixed)
Should use abspath for default settings.BASE_DIR
Reported by: | Harry Percival | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | 1.6 |
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 )
ref the default project template's settings.py, BASE_DIR is defined as:
BASE_DIR = os.path.dirname(os.path.dirname(__file__))
Because there's no abspath, you can get different results for BASE_DIR depending on how settings.py is run. This is an edge case, since settings.py is almost always imported by django in the right context, but still -- compare:
python manage.py shell >>> from django.conf import settings print(BASE_DIR)
With:
python -i myproject/settings.py >>> print(BASE_DIR)
As an aside, in general, in any path wrangling you should always call abspath first. Try running this script, saved to a file:
import os print(os.path.abspath(os.path.dirname(os.path.dirname(__file__)))) print(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
As to tests, the only one I can think of would be a pretty horrible one involving actually callling out using subprocess.Popen to a python -i of the project template settings file, so maybe ok not to bother?
credit to @CleanCut for turning me onto this weirdness!
Change History (9)
comment:1 by , 10 years ago
Description: | modified (diff) |
---|
comment:2 by , 10 years ago
Description: | modified (diff) |
---|
comment:3 by , 10 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
comment:4 by , 10 years ago
Sorry, should have spotted that. I did do a search for BASE_DIR before submitting, but it didn't show up...
comment:5 by , 10 years ago
Resolution: | duplicate |
---|---|
Status: | closed → new |
with due respect, I'm going to tentatively re-open this ticket. #21409 seems to be about changing BASE_DIR to being a different directory, whereas this ticket asks for BASE_DIR to be an absolute path.
#21409 was closed by @aaugustin, appropriately I think, because BASE_DIR is exactly where it should be.
But after that, actually, the discussion in #21409 looks like it started to be about whether the path should be absolute or relative. I couldn't see any arguments for keeping it relative, and there's definitely an argument against it (ie, it can cause confusion as per my initial submission above, and also, it's the default in Python3.4 anyway, so if we don't do it then that introduces different behaviours for django under python2 vs 3)
One person did make a case for using joins of os.pardir
instead of double path.dirname, which I guess would be fine (although it feels like the "more semantic" benefit is outweighed by the "harder to read" price). But in the meantime, I feel like BASE_DIR should definitely be an absolute path...
comment:6 by , 10 years ago
I'm still not sure what problems this would cause in practice. In the example you give, python -i myproject/settings.py
, it seems like __file__
doesn't return an absolute on Python 3.4 since it's covered by: "with the sole exception of __main__.__file__
when a script has been executed directly using a relative path".
comment:7 by , 10 years ago
Component: | Uncategorized → Core (Other) |
---|---|
Triage Stage: | Unreviewed → Accepted |
I don't see what purpose would be served by executing a Django settings module with python path/to/settings.py
. Is there a use case for which this is a proper solution? As Tim explained, we can't cover that case on Python 3.4+ anyway, so I'm going to ignore it.
For consistency between Python 3.4+ and earlier versions, I'm going to add the abspath
call, even thought I'm still convinced it doesn't change anything, except maybe in contrived made-up examples.
comment:8 by , 10 years ago
I don't want to backport this to 1.7.x because the default settings are often reproducted in training materials, especially books, and even small differences can unsettle beginners.
comment:9 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Duplicate of #21409