Hey all,
I’d like to propose a small cleanup (ticket-34778): when the startproject command checks for name clashes with other modules, it could just use importlib’s find_spec() (docs), which checks for the presence of a module without importing it. Right now the command tries to import modules and depends on an exception being raised in the absence of one.
Why?
Importing modules can execute code, e.g. dumping to the terminal or importing other further modules. (Of course, the user has trusted any hypothetical same-named modules by having installed them already, but it just seems polite to stop at checking for existence when that’s all we need for the name clash check.)
Who might this help?
My team is wrapping the startproject command to help people create instances of our software, let’s call it presto. A user might do something like use our command to create a project called presto-education. But what if they’ve installed someone else’s module called presto-education before there was a cool startpresto-way to get everything they need to use presto for education? The kinds of users we’re making this command for are not full-time Django developers. We write explicit instructions helping them accomplish their upgrades. I noticed this in code review and saw that we were just extending Django’s behavior. (I opened a ticket and Natalia helpfully suggested raising a forum post.)
The diff would look like:
diff --git a/django/core/management/templates.py b/django/core/management/templates.py
index 8c2232ee7a..ec4a4b4214 100644
--- a/django/core/management/templates.py
+++ b/django/core/management/templates.py
@@ -5,7 +5,7 @@ import posixpath
import shutil
import stat
import tempfile
-from importlib import import_module
+from importlib.util import find_spec
from urllib.request import build_opener
import django
@@ -277,11 +277,7 @@ class TemplateCommand(BaseCommand):
)
)
# Check it cannot be imported.
- try:
- import_module(name)
- except ImportError:
- pass
- else:
+ if "." in name or find_spec(name) is not None:
raise CommandError(
"'{name}' conflicts with the name of an existing Python "
"module and cannot be used as {an} {app} {type}. Please try "
Of course, this is a judgment call. Maybe not worth bothering. We haven’t even shipped our wrapped command yet. But this came up in code review and I wanted to “offer it back upstream”.
Your thoughts welcome!
) Thanks!