Trivial cleanups following bpo-31370#3649
Conversation
ericsnowcurrently
left a comment
There was a problem hiding this comment.
The importlib changes look good to me.
| builtin_module = sys.modules[builtin_name] | ||
| setattr(self_module, builtin_name, builtin_module) | ||
|
|
||
| # Directly load the _thread module (needed during bootstrap). |
There was a problem hiding this comment.
Please look also at _bootstrap_external.py. It contains almost duplicated code.
There was a problem hiding this comment.
Looks like you're right. @ericsnowcurrently, out of curiosity, why the two "bootstrap" modules?
There was a problem hiding this comment.
A while back we split the fundamental import machinery from the file-based parts. The "external" part encapsulates the file-based finders and loaders (e.g. .py, extensions) and related functionality. In addition to maintenance benefits, during runtime startup it helps to have a hard separation between the two. Furthermore, the split also supports some planned work (encapsulating the import state).
There was a problem hiding this comment.
In particular, with the multi-phase bootstrap refactoring, the builtin and frozen module support is initialised as one of the last steps in the core runtime initialisation (https://github.com/python/cpython/blob/master/Python/pylifecycle.c#L717), while setting up filesystem imports doesn't happen until the main interpreter is being configured (https://github.com/python/cpython/blob/master/Python/pylifecycle.c#L803)
There was a problem hiding this comment.
Thank you both for the explanation. I think if those files had been named something else (such as _bootstrap_core.py and _bootstrap_importers.py) the situation would have been less confusing to me ("externals" sounded it was something else entirely). Also, you could consider improving the top-level module docstrings and/or comments to describe the split accurately.
https://bugs.python.org/issue31370