Skip to content

gh-127651: Use __file__ in diagnostics if origin is missing #127660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Dec 10, 2024
48 changes: 46 additions & 2 deletions Lib/test/test_import/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,50 @@ def test_issue105979(self):
self.assertIn("Frozen object named 'x' is invalid",
str(cm.exception))

def test_frozen_module_from_import_error(self):
with self.assertRaises(ImportError) as cm:
from os import this_will_never_exist
self.assertIn(
f"cannot import name 'this_will_never_exist' from 'os' ({os.__file__})",
str(cm.exception),
)
with self.assertRaises(ImportError) as cm:
from sys import this_will_never_exist
self.assertIn(
"cannot import name 'this_will_never_exist' from 'sys' (unknown location)",
str(cm.exception),
)

scripts = [
"""
import os
os.__spec__.has_location = False
os.__file__ = []
from os import this_will_never_exist
""",
"""
import os
os.__spec__.has_location = False
del os.__file__
from os import this_will_never_exist
""",
"""
import os
os.__spec__.origin = []
os.__file__ = []
from os import this_will_never_exist
"""
]
for script in scripts:
with self.subTest(script=script):
expected_error = (
b"cannot import name 'this_will_never_exist' "
b"from 'os' (unknown location)"
)
popen = script_helper.spawn_python("-c", script)
stdout, stderr = popen.communicate()
self.assertIn(expected_error, stdout)

def test_script_shadowing_stdlib(self):
script_errors = [
(
Expand Down Expand Up @@ -1068,7 +1112,7 @@ class substr(str):
except AttributeError as e:
print(str(e))

fractions.__spec__.origin = 0
fractions.__spec__.origin = []
try:
fractions.Fraction
except AttributeError as e:
Expand All @@ -1092,7 +1136,7 @@ class substr(str):
except ImportError as e:
print(str(e))

fractions.__spec__.origin = 0
fractions.__spec__.origin = []
try:
from fractions import Fraction
except ImportError as e:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
When raising :exc:`ImportError` for missing symbols in ``from`` imports, use ``__file__`` in the error message if ``__spec__.origin`` is not a location
22 changes: 19 additions & 3 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -2859,6 +2859,20 @@ _PyEval_ImportFrom(PyThreadState *tstate, PyObject *v, PyObject *name)
}
}

if (origin == NULL) {
// Fall back to __file__ for diagnostics if we don't have
// an origin that is a location
origin = PyModule_GetFilenameObject(v);
if (origin == NULL) {
if (!PyErr_ExceptionMatches(PyExc_SystemError)) {
goto done;
}
// PyModule_GetFilenameObject raised "module filename missing"
_PyErr_Clear(tstate);
}
assert(origin == NULL || PyUnicode_Check(origin));
}

if (is_possibly_shadowing_stdlib) {
assert(origin);
errmsg = PyUnicode_FromFormat(
Expand Down Expand Up @@ -2919,9 +2933,11 @@ _PyEval_ImportFrom(PyThreadState *tstate, PyObject *v, PyObject *name)
}

done_with_errmsg:
/* NULL checks for errmsg, mod_name, origin done by PyErr_SetImportError. */
_PyErr_SetImportErrorWithNameFrom(errmsg, mod_name, origin, name);
Py_DECREF(errmsg);
if (errmsg != NULL) {
/* NULL checks for mod_name and origin done by _PyErr_SetImportErrorWithNameFrom */
_PyErr_SetImportErrorWithNameFrom(errmsg, mod_name, origin, name);
Py_DECREF(errmsg);
}

done:
Py_XDECREF(origin);
Expand Down
Loading