Skip to content

Commit 1e8ff18

Browse files
gh-157757: Fix to make lazy import a.b as c import the module a.b (#158092)
* Import the module a lazy `import a.b as c` names `import a.b as c` compiles to `IMPORT_NAME a.b` followed by `IMPORT_FROM b`. Lazily, IMPORT_NAME leaves a placeholder holding "a.b", and IMPORT_FROM rewrote it into the placeholder `lazy from a import b` produces. Reification then imported `a` alone and read `b` off it, so the module `a.b` was never imported under its own name: an attribute of the package shadowing it answered instead, and `math.pi`, which no module backs, bound the float where the eager statement raises ModuleNotFoundError. Mark the dotted import on the placeholder and keep the whole name on it. Reification imports that name and then walks its components with IMPORT_FROM, which is what the eager statement does. The test pinning `lazy import math.pi as pi` as working is inverted, since the eager statement raises. * Stop excluding test_trace from the lazy-imports-all run It passes now that a lazy `import a.b as c` imports the module: the KeyError on 'test.tracedmodules.testmod' came from the submodule never being imported under its own name. * Rename lz_submodule to lz_dotted_as and trim the comments The flag means "bind the whole dotted name, not the root", which the old name did not say, and import.c already has unrelated lazy_pending_submodules machinery to be confused with. * Chain lazy IMPORT_FROM placeholders instead of flagging dotted imports Each deferred IMPORT_FROM off a placeholder without a fromlist now keeps the previous placeholder in lz_from and the attribute name in lz_attr. Reification walks back to the placeholder IMPORT_NAME left, runs that import, and replays the lookups in order with _PyEval_ImportFrom, which is what the eager bytecode does. This drops the lz_dotted_as flag and also follows a custom __lazy_import__ that returns a placeholder for a different module name. * Chain every deferred IMPORT_FROM onto the previous placeholder `lazy from a import b` now records its lookup the same way as `import a.b as c`, so a placeholder holds either the module name and fromlist or the previous placeholder and an attribute name, and reification has a single path. The import passes only the name being resolved as the fromlist, so accessing b still does not import the other names' submodules. * Treat an empty fromlist on a lazy import placeholder as no fromlist __import__("a.b", fromlist=()) returns the top-level package `a`, the same as fromlist=None, but the placeholder kept the empty tuple. Every consumer of lz_attr then read it as a real fromlist: reification narrowed it to the chained attribute and replayed the lookups on `a.b`, _PyEval_LazyImportFrom took the attribute off sys.modules["a.b"], and the repr named `a.b.attr`. _PyLazyImport_New already collapses None to NULL for exactly this reason, so collapse an empty tuple there too and every site follows. * gh-157757: Preserve empty fromlists for custom import hooks --------- Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
1 parent 972cfaa commit 1e8ff18

7 files changed

Lines changed: 234 additions & 63 deletions

File tree

‎Lib/test/lazy_imports_all_exclude.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,5 @@ test_pyrepl
3535
test_subprocess
3636
test_symtable
3737
test_tools
38-
test_trace
3938
test_type_annotations
4039
test_unittest

‎Lib/test/test_lazy_import/__init__.py‎

Lines changed: 141 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -724,10 +724,17 @@ def test_non_package_lazily_imported(self):
724724
assert_python_ok("-c", code)
725725

726726
def test_non_package_lazily_imported_as(self):
727-
"""Doing a dotted lazy import as still works"""
727+
"""A dotted lazy import as raises when the name is not a module."""
728+
# gh-157757: the eager statement raises, so the lazy one raises too.
728729
code = textwrap.dedent("""
729730
lazy import math.pi as pi
730-
pi
731+
732+
try:
733+
pi
734+
except ModuleNotFoundError:
735+
pass
736+
else:
737+
raise AssertionError("ModuleNotFoundError was not raised")
731738
""")
732739
assert_python_ok("-c", code)
733740

@@ -1170,6 +1177,22 @@ def test_accessing_one_name_leaves_others_as_proxies(self):
11701177
self.assertEqual(result.returncode, 0, f"stdout: {result.stdout}, stderr: {result.stderr}")
11711178
self.assertIn("OK", result.stdout)
11721179

1180+
def test_accessing_one_name_imports_only_its_submodule(self):
1181+
"""Accessing one name should not import the other names' submodules."""
1182+
code = textwrap.dedent("""
1183+
import sys
1184+
1185+
lazy from test.test_lazy_import.data.pkg import b, bar, broken
1186+
1187+
# Importing bar prints, and importing broken raises.
1188+
b.foo()
1189+
1190+
assert "test.test_lazy_import.data.pkg.bar" not in sys.modules
1191+
assert "test.test_lazy_import.data.pkg.broken" not in sys.modules
1192+
""")
1193+
rc, out, err = assert_python_ok("-c", code)
1194+
self.assertEqual(out, b"")
1195+
11731196
def test_all_names_reified_after_all_accessed(self):
11741197
"""All names should be reified after each is accessed."""
11751198
code = textwrap.dedent("""
@@ -2209,6 +2232,122 @@ def test_import_after_variable_wins(self):
22092232
]
22102233
self.assertIs(module_same_name_var_order2.bar, bar_mod)
22112234

2235+
def test_lazy_import_as_wins_over_variable(self):
2236+
"""A dotted lazy import as imports the submodule the variable hides."""
2237+
# gh-157757: importing pkg.b rebinds pkg.b from the variable to the
2238+
# module, eagerly and lazily alike.
2239+
code = textwrap.dedent("""
2240+
import sys
2241+
import test.test_lazy_import.data.pkg as pkg
2242+
pkg.b = "hides the b submodule"
2243+
2244+
lazy import test.test_lazy_import.data.pkg.b as b
2245+
lazy import test.test_lazy_import.data.metasyntactic.foo.bar as bar
2246+
2247+
assert b is sys.modules["test.test_lazy_import.data.pkg.b"], b
2248+
assert bar is sys.modules[
2249+
"test.test_lazy_import.data.metasyntactic.foo.bar"], bar
2250+
""")
2251+
assert_python_ok("-c", code)
2252+
2253+
def test_dotted_as_of_loaded_module(self):
2254+
"""A dotted lazy import as binds the module, not a same-named attribute."""
2255+
# importlib.metadata is already loaded and has a `metadata` attribute.
2256+
code = textwrap.dedent("""
2257+
import importlib.metadata
2258+
import importlib.metadata as eager
2259+
2260+
lazy import importlib.metadata as lazily
2261+
2262+
assert lazily is eager, lazily
2263+
""")
2264+
assert_python_ok("-c", code)
2265+
2266+
def test_dotted_as_replays_lookups_on_custom_placeholder(self):
2267+
"""A dotted lazy import as looks up its names on what the hook returned."""
2268+
code = textwrap.dedent("""
2269+
import builtins
2270+
import xml.dom
2271+
2272+
# In a list, so the hook reading it does not resolve it.
2273+
placeholder = [__lazy_import__("xml")]
2274+
default = builtins.__lazy_import__
2275+
builtins.__lazy_import__ = lambda *args: placeholder[0]
2276+
lazy import fake.dom as dom
2277+
builtins.__lazy_import__ = default
2278+
2279+
assert dom is xml.dom, dom
2280+
""")
2281+
assert_python_ok("-c", code)
2282+
2283+
def test_empty_fromlist_placeholder_matches_no_fromlist(self):
2284+
"""An empty fromlist behaves like None."""
2285+
code = textwrap.dedent("""
2286+
expected = "<lazy_import 'xml.dom'>"
2287+
# In lists, so reading them does not resolve them.
2288+
for fromlist in (None, ()):
2289+
same = [__lazy_import__("xml.dom", fromlist=fromlist)]
2290+
assert repr(same[0]) == expected, (fromlist, repr(same[0]))
2291+
bare = [__lazy_import__("xml.dom")]
2292+
assert repr(bare[0]) == expected, repr(bare[0])
2293+
""")
2294+
assert_python_ok("-c", code)
2295+
2296+
def test_empty_fromlist_preserved_for_custom_import(self):
2297+
code = textwrap.dedent("""
2298+
import builtins
2299+
import types
2300+
2301+
value = object()
2302+
module = types.SimpleNamespace(dom=value)
2303+
placeholder = [__lazy_import__("xml.dom", fromlist=())]
2304+
default_import = builtins.__import__
2305+
default_lazy_import = builtins.__lazy_import__
2306+
calls = []
2307+
2308+
def import_hook(name, globals, locals, fromlist, level):
2309+
assert name == "xml.dom", name
2310+
assert fromlist == (), fromlist
2311+
calls.append(fromlist)
2312+
return module
2313+
2314+
builtins.__import__ = import_hook
2315+
assert placeholder[0].resolve() is module
2316+
builtins.__lazy_import__ = lambda *args: placeholder[0]
2317+
lazy import fake.dom as dom
2318+
assert dom is value
2319+
builtins.__import__ = default_import
2320+
builtins.__lazy_import__ = default_lazy_import
2321+
2322+
assert calls == [(), ()], calls
2323+
""")
2324+
assert_python_ok("-c", code)
2325+
2326+
def test_dotted_as_replays_lookups_on_dotted_placeholder(self):
2327+
"""A dotted lazy import as replays its names on the hook's package."""
2328+
# importlib.metadata has a `metadata` attribute of its own, which the
2329+
# placeholder for importlib must not answer with.
2330+
for target in ("xml.dom", "importlib.metadata"):
2331+
with self.subTest(target=target):
2332+
leaf = target.rpartition(".")[2]
2333+
code = textwrap.dedent(f"""
2334+
import builtins
2335+
import sys
2336+
import {target}
2337+
2338+
# In a list, so the hook reading it does not resolve it.
2339+
placeholder = [__lazy_import__("{target}", fromlist=())]
2340+
default = builtins.__lazy_import__
2341+
builtins.__lazy_import__ = lambda *args: placeholder[0]
2342+
lazy import fake.{leaf} as {leaf}
2343+
builtins.__lazy_import__ = default
2344+
2345+
name = repr(globals()["{leaf}"])
2346+
assert name == "<lazy_import '{target}'>", name
2347+
assert {leaf} is sys.modules["{target}"], {leaf}
2348+
""")
2349+
assert_python_ok("-c", code)
2350+
22122351

22132352
class DeletedModuleReimportTests(unittest.TestCase):
22142353
"""Tests for reimporting after module deletion from sys.modules."""
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# Submodule that raises an error during import
2+
raise ValueError("This module always fails to import")
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a lazy ``import a.b as c`` reading ``b`` off ``a`` instead of importing
2+
the module ``a.b``. It now binds the submodule, and raises
3+
:exc:`ModuleNotFoundError` when no module backs the name.

‎Objects/lazyimportobject.c‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ PyObject *
1414
_PyLazyImport_New(_PyInterpreterFrame *frame, PyObject *builtins, PyObject *name, PyObject *fromlist)
1515
{
1616
PyLazyImportObject *m;
17-
if (!name || !PyUnicode_Check(name)) {
18-
PyErr_SetString(PyExc_TypeError, "expected str for name");
17+
if (!name || !(PyUnicode_Check(name) || PyLazyImport_CheckExact(name))) {
18+
PyErr_SetString(PyExc_TypeError, "expected str or lazy_import for name");
1919
return NULL;
2020
}
2121
if (fromlist == Py_None || fromlist == NULL) {
@@ -104,16 +104,45 @@ lazy_import_getattro(PyObject *op, PyObject *name)
104104
return value;
105105
}
106106

107+
// The dotted name of the object that resolving the placeholder returns.
107108
static PyObject *
108-
lazy_import_name(PyLazyImportObject *m)
109+
lazy_import_path(PyLazyImportObject *m)
109110
{
110-
if (m->lz_attr != NULL) {
111-
if (PyUnicode_Check(m->lz_attr)) {
112-
return PyUnicode_FromFormat("%U.%U", m->lz_from, m->lz_attr);
113-
}
114-
else {
115-
return PyUnicode_FromFormat("%U...", m->lz_from);
111+
if (PyLazyImport_CheckExact(m->lz_from)) {
112+
PyObject *base = lazy_import_path((PyLazyImportObject *)m->lz_from);
113+
if (base == NULL) {
114+
return NULL;
116115
}
116+
PyObject *res = PyUnicode_FromFormat("%U.%U", base, m->lz_attr);
117+
Py_DECREF(base);
118+
return res;
119+
}
120+
if (m->lz_attr != NULL &&
121+
(!PyTuple_Check(m->lz_attr) || PyTuple_GET_SIZE(m->lz_attr) > 0)) {
122+
return Py_NewRef(m->lz_from);
123+
}
124+
// __import__("a.b") returns the top-level package `a`.
125+
Py_ssize_t dot = PyUnicode_FindChar(
126+
m->lz_from, '.', 0, PyUnicode_GET_LENGTH(m->lz_from), 1
127+
);
128+
if (dot == -2) {
129+
return NULL;
130+
}
131+
if (dot < 0) {
132+
return Py_NewRef(m->lz_from);
133+
}
134+
return PyUnicode_Substring(m->lz_from, 0, dot);
135+
}
136+
137+
static PyObject *
138+
lazy_import_name(PyLazyImportObject *m)
139+
{
140+
if (PyLazyImport_CheckExact(m->lz_from)) {
141+
return lazy_import_path(m);
142+
}
143+
if (m->lz_attr != NULL &&
144+
(!PyTuple_Check(m->lz_attr) || PyTuple_GET_SIZE(m->lz_attr) > 0)) {
145+
return PyUnicode_FromFormat("%U...", m->lz_from);
117146
}
118147
return Py_NewRef(m->lz_from);
119148
}

‎Python/ceval.c‎

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3333,7 +3333,13 @@ _PyEval_LazyImportFrom(PyThreadState *tstate, _PyInterpreterFrame *frame, PyObje
33333333
assert(PyUnicode_Check(name));
33343334
PyObject *ret;
33353335
PyLazyImportObject *d = (PyLazyImportObject *)v;
3336-
PyObject *mod = PyImport_GetModule(d->lz_from);
3336+
PyObject *mod = NULL;
3337+
// Only `from a import b` can take b off an already imported a;
3338+
// `import a.b as c` has to import a.b first.
3339+
if (d->lz_attr != NULL && PyTuple_Check(d->lz_attr) &&
3340+
PyTuple_GET_SIZE(d->lz_attr) > 0) {
3341+
mod = PyImport_GetModule(d->lz_from);
3342+
}
33373343
if (mod != NULL) {
33383344
// Check if the module already has the attribute, if so, resolve it
33393345
// eagerly.
@@ -3353,34 +3359,7 @@ _PyEval_LazyImportFrom(PyThreadState *tstate, _PyInterpreterFrame *frame, PyObje
33533359
Py_DECREF(mod);
33543360
}
33553361

3356-
if (d->lz_attr != NULL) {
3357-
if (PyUnicode_Check(d->lz_attr)) {
3358-
PyObject *from = PyUnicode_FromFormat(
3359-
"%U.%U", d->lz_from, d->lz_attr);
3360-
if (from == NULL) {
3361-
return NULL;
3362-
}
3363-
ret = _PyLazyImport_New(frame, d->lz_builtins, from, name);
3364-
Py_DECREF(from);
3365-
return ret;
3366-
}
3367-
}
3368-
else {
3369-
Py_ssize_t dot = PyUnicode_FindChar(
3370-
d->lz_from, '.', 0, PyUnicode_GET_LENGTH(d->lz_from), 1
3371-
);
3372-
if (dot >= 0) {
3373-
PyObject *from = PyUnicode_Substring(d->lz_from, 0, dot);
3374-
if (from == NULL) {
3375-
return NULL;
3376-
}
3377-
ret = _PyLazyImport_New(frame, d->lz_builtins, from, name);
3378-
Py_DECREF(from);
3379-
return ret;
3380-
}
3381-
}
3382-
ret = _PyLazyImport_New(frame, d->lz_builtins, d->lz_from, name);
3383-
return ret;
3362+
return _PyLazyImport_New(frame, d->lz_builtins, v, name);
33843363
}
33853364

33863365
#define CANNOT_CATCH_MSG "catching classes that do not inherit from "\

0 commit comments

Comments
 (0)