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 Harry Percival)

ref the default project template's, 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 is run. This is an edge case, since is almost always imported by django in the right context, but still -- compare:

python shell
>>> from django.conf import settings


python -i myproject/
>>> 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

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 Harry Percival, 10 years ago

Description: modified (diff)

comment:2 by Harry Percival, 10 years ago

Description: modified (diff)

comment:3 by Tim Graham, 10 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #21409

comment:4 by Harry Percival, 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 Harry Percival, 10 years ago

Resolution: duplicate
Status: closednew

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 Tim Graham, 10 years ago

I'm still not sure what problems this would cause in practice. In the example you give, python -i myproject/, 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 Aymeric Augustin, 10 years ago

Component: UncategorizedCore (Other)
Triage Stage: UnreviewedAccepted

I don't see what purpose would be served by executing a Django settings module with python path/to/ 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 Aymeric Augustin, 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 Aymeric Augustin <aymeric.augustin@…>, 10 years ago

Resolution: fixed
Status: newclosed

In caf5cd7ba7c9d73194e394a26c81f1a677d54a6c:

Fixed #23436 -- Made BASE_DIR absolute in settings template.

This ensures consistency between Python 3.4+ and earlier versions.

Thanks Harry Percival for the report.

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