Access __file__ lazily rather than at module level

[William Schwartz]

So-called frozen Python environments (such as those mentioned in #30950) that do not set all modules' ​__file__ variable, which ​need not be defined, cannot even import Django (without some workarounds) because a small number of Django modules use __file__ at the module level, in a class defined at the module level, or in a function that is called automatically upon import.

Five modules that use __file__ like this are likely to be imported when using Django and thereby cause a frozen Python to crash with a NameError or similar exception.

• Importing django.forms.renderers can be avoided only by avoiding both forms and the ORM altogether as it's imported from django.db.models.
• Importing django.views.debug might be avoidable if DEBUG=False or by avoiding all of the views and URLs APIs.
• django.utils.version's get_git_changeset is called when django is imported in pre-alpha development versions.
• Importing django.contrib.auth.password_validation is only avoidable by not using the Auth app.
• django.utils.translation.trans_real uses __file__ to find Django's localization files upon activation; this avoidable only by setting USE_I18N=False. Dealing with trans_real is sufficiently thorny (and, being an English speaker with English-speaking clients, I can avoid it for now) that I will not address it further here except to say that it might need to be part of the larger discussion at #30950.

What this ticket is not

I am not proposing removing use of __file__ at this time. That would require a longer discussion of intended semantics such as #30950. This ticket is only about removing use of __file__ at the module (or class definition) level in Django application code (not test code). Further I am not proposing banning use of __file__ at the module level at this time, hence minimal new tests and no update to the Django coding style documentation. That too would require a longer conversation.

Proposed fixes

I have pushed ​PR GH-13841 to address the four of those modules other than trans_real. I dealt with each module's use of __file__ in separate commits to make them easier to discuss and separate/cherry-pick if needed. Below I link to the individual commits as I discuss each of the four modules. These first two are fairly easy, but the second two may require further consideration.

django.forms.renders (​54d539c)

Remove the undocumented module constant ROOT and replace its single use.

django.utils.version (​f4edc6e)

Treat the lack of module-global __file__ the same as a failure of git log by returning None from get_git_changeset.

django.views.debug (​07f46b7)

The module-level constant CURRENT_DIR is used only in the module itself and is undocumented, so I'm assuming it's an obscure private symbol that no one will miss. I've replaced it with a module-level private function _builtin_template_path that refactors and centralizes finding built-in templates for the entire module.

The one tricky part is that #32105 added the html_template_path and text_template_path attributes django.views.debug.ExceptionReporter. I didn't want to disturb #32105's goal of making the template paths easily override-able, so I avoided calling _builtin_template_path in the class definition by making detecting the presence of the attributes in __init__ and setting defaults there. Alternatives include making the attributes properties with setters or cached properties without setters.

django.contrib.auth.password_validation (​24aa80b)

The CommonPasswordValidator-class constant DEFAULT_PASSWORD_LIST_PATH is used only in one place, the class's instance constructor. While the nature of DEFAULT_PASSWORD_LIST_PATH is not documented, its existence is inside the docs for the ​constructor's signature. I've changed DEFAULT_PASSWORD_LIST_PATH from a class constant into an instance attribute. Another possibility is making DEFAULT_PASSWORD_LIST_PATH be a django.utils.functional.classproperty.

[Tom Forbes]

I think this makes sense as __file__ is indeed documented as being optional (which is somewhat surprising). If we do go forward with this, how do we catch future uses of __file__ at the module level?

[William Schwartz]

If we do go forward with this, how do we catch future uses of __file__ at the module level?

Great question, and here's my terrible answer: I propose not to—for now. This is part of what I meant in my original post about "not proposing banning use of __file__ at the module level at this time."

The main reason is that I think testing for module-level use of __file__—as well as all the other subtleties involved in using Python freezers—will be easiest and most thorough if we now and then (or regularly on CI...) run the entire Django test suite from inside, say, ​PyOxidizer or one of the other full-featured freezers. But doing that only makes sense in the context of solving all the impediments to using Django with freezers. As discussed elsewhere, such as at #30950 and on ​django-developers prior to #32302, that's too much to solve in one go.

I view this ticket just as a way to nudge Django 3.2 toward more freezer friendliness before beta testing season. My (small) team then plans to dogfood the 3.2 betas this spring while developing an app we'll be running from PyOxidizer, so I'm hoping we'll notice if __file__ sneaks back in somewhere that would cause a frozen Python to crash.

[Carlton Gibson]

Hey William.

OK, let's accept this, but can I ask you to (briefly) write up a "Roadmap to Freezable Django", perhaps just for the mailing list, rather than a full DEP (but do use the DEP template if that helps)?

You've obviously got an end goal in mind — and that's probably worthwhile and something we should have in mind as a target — but these sequence of little adjustments feel a bit like death-by-a-thousand-cuts (if that makes sense). No problem with them, but the overview, and the obstacles currently foreseeable would be good to discuss, and to get wise-folk input on.

Make sense? 🙂

Thanks.

[William Schwartz]

Yes, I will absolute write a broader memo to the mailing list. It is not my intention to wear down y'all maintainers!

My plan all along has been to get this and a couple other tickets (#32302 and #32317 in particular) taken care of before the 3.2 feature freeze on the 14th, experiment with the results for awhile and consult my colleagues, and then write up something more big-picture for the mailing list. (I'll check out the DEP template—thanks for the idea.) While I have an idea of what I want to propose, getting some practical experience with a frozen Django project seems prudent before moving beyond minor cleanups. Besides, my time is extremely constrained at the moment, so it'll be several weeks before I can write up anything anyway.

Thanks for continuing to work with me on these incremental changes in the meantime!

[Carlton Gibson]

Patch needs minor adjustments and then it should be good to go.

Should wait until after #32355 so as to be able to use functools.cached_property
(assuming we'll deprecate Django's own once 3.8 is the minimum supported version. Mailing list thread on that ​https://groups.google.com/g/django-developers/c/HBWKl42nYTE)

[Mariusz Felisiak <felisiak.mariusz@…>]

In 7248afe1:

Refs #32105 -- Moved ExceptionReporter template paths to properties.

Refs #32316.

[Mariusz Felisiak <felisiak.mariusz@…>]

In a118564a:

[3.2.x] Refs #32105 -- Moved ExceptionReporter template paths to properties.

Refs #32316.

Backport of 7248afe12f40361870388ecdd7e0038eb0d58e47 from main

[Mariusz Felisiak <felisiak.mariusz@…>]

In 9ee693bd:

Fixed #32316 -- Deferred accessing file.

Deferred accessing the module-global variable file because the
Python import API does not guarantee it always exists—in particular, it
does not exist in certain "frozen" environments. The following changes
advanced this goal.

Thanks to Carlton Gibson, Tom Forbes, Mariusz Felisiak, and Shreyas
Ravi for review and feedback.

[William Schwartz]

I'm cleaning up random files on my desktop and I came across these notes I left for myself while working on this bug. I'm leaving them here for posterity.

Appendix: Other modules that use __file__.

From master at b41d38ae26b1da9519a6cd765bc2f2ce7d355007:

$ git grep --files-with-matches -e "__file__" b41d38a -- django | wc -l
b41d38a:django/apps/config.py
b41d38a:django/conf/__init__.py
b41d38a:django/conf/project_template/project_name/settings.py-tpl
b41d38a:django/contrib/auth/password_validation.py
b41d38a:django/core/management/commands/makemessages.py
b41d38a:django/db/migrations/loader.py
b41d38a:django/db/migrations/questioner.py
b41d38a:django/forms/renderers.py
b41d38a:django/utils/autoreload.py
b41d38a:django/utils/module_loading.py
b41d38a:django/utils/translation/trans_real.py
b41d38a:django/utils/version.py
b41d38a:django/views/debug.py

The remainder of the files listed in the git grep output above do not need to be addressed in this ticket at this time. This section explains why.

The following modules access __file__ only through proper getattr or hasattr guards:

• django.apps.config to compute AppConfig.path from its module
• django.db.migrations.loader (see #32302)
• django.db.migrations.questioner in MigrationQuestioner.ask_initial
• django.utils.module_loading in module_dir

django.utils.autoreload doesn't access __file__ at the module level. iter_modules_and_files has a proper hasattr guard but is_django_path would need different semantics. #32314 addresses get_child_arguments.

django.conf.LazySettings.PASSWORD_RESET_TIMEOUT_DAYS uses __file__ only to avoid warning about this deprecated setting. Frozen applications can avoid crashes simply by upgrading away from PASSWORD_RESET_TIMEOUT_DAYS.

django/conf/project_template/project_name/settings.py-tpl is the template settings module. Django projects are supposed to edit this anyway.

Finally the makemessages command has no use for projects with USE_I18N=False, which is necessary for frozen projects to work around the issue with trans_real described above.