Skip to content

Commit 13d4489

Browse files
authored
gh-86298: Ensure that __loader__ and __spec__.loader agree in warnings.warn_explicit() (GH-97803)
In `_warnings.c`, in the C equivalent of `warnings.warn_explicit()`, if the module globals are given (and not None), the warning will attempt to get the source line for the issued warning. To do this, it needs the module's loader. Previously, it would only look up `__loader__` in the module globals. In #86298 we want to defer to the `__spec__.loader` if available. The first step on this journey is to check that `loader == __spec__.loader` and issue another warning if it is not. This commit does that. Since this is a PoC, only manual testing for now. ```python # /tmp/foo.py import warnings import bar warnings.warn_explicit( 'warning!', RuntimeWarning, 'bar.py', 2, module='bar knee', module_globals=bar.__dict__, ) ``` ```python # /tmp/bar.py import sys import os import pathlib # __loader__ = pathlib.Path() ``` Then running this: `./python.exe -Wdefault /tmp/foo.py` Produces: ``` bar.py:2: RuntimeWarning: warning! import os ``` Uncomment the `__loader__ = ` line in `bar.py` and try it again: ``` sys:1: ImportWarning: Module bar; __loader__ != __spec__.loader (<_frozen_importlib_external.SourceFileLoader object at 0x109f7dfa0> != PosixPath('.')) bar.py:2: RuntimeWarning: warning! import os ``` Automerge-Triggered-By: GH:warsaw
1 parent 27369ef commit 13d4489

File tree

5 files changed

+182
-3
lines changed

5 files changed

+182
-3
lines changed

Doc/reference/import.rst

+8
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,11 @@ listed below.
558558
It is **strongly** recommended that you rely on :attr:`__spec__`
559559
instead instead of this attribute.
560560

561+
.. versionchanged:: 3.12
562+
The value of ``__loader__`` is expected to be the same as
563+
``__spec__.loader``. The use of ``__loader__`` is deprecated and slated
564+
for removal in Python 3.14.
565+
561566
.. attribute:: __package__
562567

563568
The module's ``__package__`` attribute may be set. Its value must
@@ -568,6 +573,9 @@ listed below.
568573
submodules, to the parent package's name. See :pep:`366` for further
569574
details.
570575

576+
This attribute is used instead of ``__name__`` to calculate explicit
577+
relative imports for main modules, as defined in :pep:`366`.
578+
571579
It is **strongly** recommended that you rely on :attr:`__spec__`
572580
instead instead of this attribute.
573581

Lib/importlib/_bootstrap_external.py

+48
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,54 @@ def spec_from_file_location(name, location=None, *, loader=None,
862862
return spec
863863

864864

865+
def _bless_my_loader(module_globals):
866+
"""Helper function for _warnings.c
867+
868+
See GH#97850 for details.
869+
"""
870+
# 2022-10-06(warsaw): For now, this helper is only used in _warnings.c and
871+
# that use case only has the module globals. This function could be
872+
# extended to accept either that or a module object. However, in the
873+
# latter case, it would be better to raise certain exceptions when looking
874+
# at a module, which should have either a __loader__ or __spec__.loader.
875+
# For backward compatibility, it is possible that we'll get an empty
876+
# dictionary for the module globals, and that cannot raise an exception.
877+
if not isinstance(module_globals, dict):
878+
return None
879+
880+
missing = object()
881+
loader = module_globals.get('__loader__', None)
882+
spec = module_globals.get('__spec__', missing)
883+
884+
if loader is None:
885+
if spec is missing:
886+
# If working with a module:
887+
# raise AttributeError('Module globals is missing a __spec__')
888+
return None
889+
elif spec is None:
890+
raise ValueError('Module globals is missing a __spec__.loader')
891+
892+
spec_loader = getattr(spec, 'loader', missing)
893+
894+
if spec_loader in (missing, None):
895+
if loader is None:
896+
exc = AttributeError if spec_loader is missing else ValueError
897+
raise exc('Module globals is missing a __spec__.loader')
898+
_warnings.warn(
899+
'Module globals is missing a __spec__.loader',
900+
DeprecationWarning)
901+
spec_loader = loader
902+
903+
assert spec_loader is not None
904+
if loader is not None and loader != spec_loader:
905+
_warnings.warn(
906+
'Module globals; __loader__ != __spec__.loader',
907+
DeprecationWarning)
908+
return loader
909+
910+
return spec_loader
911+
912+
865913
# Loaders #####################################################################
866914

867915
class WindowsRegistryFinder:

Lib/test/test_importlib/import_/test_helpers.py

+113
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
from importlib import _bootstrap_external, machinery
44
import os.path
5+
from types import ModuleType, SimpleNamespace
56
import unittest
7+
import warnings
68

79
from .. import util
810

@@ -67,5 +69,116 @@ def test_no_loader_no_spec_but_source(self):
6769

6870
FrozenFixUpModuleTests, SourceFixUpModuleTests = util.test_both(FixUpModuleTests)
6971

72+
73+
class TestBlessMyLoader(unittest.TestCase):
74+
# GH#86298 is part of the migration away from module attributes and toward
75+
# __spec__ attributes. There are several cases to test here. This will
76+
# have to change in Python 3.14 when we actually remove/ignore __loader__
77+
# in favor of requiring __spec__.loader.
78+
79+
def test_gh86298_no_loader_and_no_spec(self):
80+
bar = ModuleType('bar')
81+
del bar.__loader__
82+
del bar.__spec__
83+
# 2022-10-06(warsaw): For backward compatibility with the
84+
# implementation in _warnings.c, this can't raise an
85+
# AttributeError. See _bless_my_loader() in _bootstrap_external.py
86+
# If working with a module:
87+
## self.assertRaises(
88+
## AttributeError, _bootstrap_external._bless_my_loader,
89+
## bar.__dict__)
90+
self.assertIsNone(_bootstrap_external._bless_my_loader(bar.__dict__))
91+
92+
def test_gh86298_loader_is_none_and_no_spec(self):
93+
bar = ModuleType('bar')
94+
bar.__loader__ = None
95+
del bar.__spec__
96+
# 2022-10-06(warsaw): For backward compatibility with the
97+
# implementation in _warnings.c, this can't raise an
98+
# AttributeError. See _bless_my_loader() in _bootstrap_external.py
99+
# If working with a module:
100+
## self.assertRaises(
101+
## AttributeError, _bootstrap_external._bless_my_loader,
102+
## bar.__dict__)
103+
self.assertIsNone(_bootstrap_external._bless_my_loader(bar.__dict__))
104+
105+
def test_gh86298_no_loader_and_spec_is_none(self):
106+
bar = ModuleType('bar')
107+
del bar.__loader__
108+
bar.__spec__ = None
109+
self.assertRaises(
110+
ValueError,
111+
_bootstrap_external._bless_my_loader, bar.__dict__)
112+
113+
def test_gh86298_loader_is_none_and_spec_is_none(self):
114+
bar = ModuleType('bar')
115+
bar.__loader__ = None
116+
bar.__spec__ = None
117+
self.assertRaises(
118+
ValueError,
119+
_bootstrap_external._bless_my_loader, bar.__dict__)
120+
121+
def test_gh86298_loader_is_none_and_spec_loader_is_none(self):
122+
bar = ModuleType('bar')
123+
bar.__loader__ = None
124+
bar.__spec__ = SimpleNamespace(loader=None)
125+
self.assertRaises(
126+
ValueError,
127+
_bootstrap_external._bless_my_loader, bar.__dict__)
128+
129+
def test_gh86298_no_spec(self):
130+
bar = ModuleType('bar')
131+
bar.__loader__ = object()
132+
del bar.__spec__
133+
with warnings.catch_warnings():
134+
self.assertWarns(
135+
DeprecationWarning,
136+
_bootstrap_external._bless_my_loader, bar.__dict__)
137+
138+
def test_gh86298_spec_is_none(self):
139+
bar = ModuleType('bar')
140+
bar.__loader__ = object()
141+
bar.__spec__ = None
142+
with warnings.catch_warnings():
143+
self.assertWarns(
144+
DeprecationWarning,
145+
_bootstrap_external._bless_my_loader, bar.__dict__)
146+
147+
def test_gh86298_no_spec_loader(self):
148+
bar = ModuleType('bar')
149+
bar.__loader__ = object()
150+
bar.__spec__ = SimpleNamespace()
151+
with warnings.catch_warnings():
152+
self.assertWarns(
153+
DeprecationWarning,
154+
_bootstrap_external._bless_my_loader, bar.__dict__)
155+
156+
def test_gh86298_loader_and_spec_loader_disagree(self):
157+
bar = ModuleType('bar')
158+
bar.__loader__ = object()
159+
bar.__spec__ = SimpleNamespace(loader=object())
160+
with warnings.catch_warnings():
161+
self.assertWarns(
162+
DeprecationWarning,
163+
_bootstrap_external._bless_my_loader, bar.__dict__)
164+
165+
def test_gh86298_no_loader_and_no_spec_loader(self):
166+
bar = ModuleType('bar')
167+
del bar.__loader__
168+
bar.__spec__ = SimpleNamespace()
169+
self.assertRaises(
170+
AttributeError,
171+
_bootstrap_external._bless_my_loader, bar.__dict__)
172+
173+
def test_gh86298_no_loader_with_spec_loader_okay(self):
174+
bar = ModuleType('bar')
175+
del bar.__loader__
176+
loader = object()
177+
bar.__spec__ = SimpleNamespace(loader=loader)
178+
self.assertEqual(
179+
_bootstrap_external._bless_my_loader(bar.__dict__),
180+
loader)
181+
182+
70183
if __name__ == "__main__":
71184
unittest.main()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
In cases where ``warnings.warn_explicit()`` consults the module's loader, an
2+
``DeprecationWarning`` is issued when ``m.__loader__`` differs from
3+
``m.__spec__.loader``.

Python/_warnings.c

+10-3
Original file line numberDiff line numberDiff line change
@@ -977,19 +977,26 @@ warnings_warn_impl(PyObject *module, PyObject *message, PyObject *category,
977977
static PyObject *
978978
get_source_line(PyInterpreterState *interp, PyObject *module_globals, int lineno)
979979
{
980+
PyObject *external;
980981
PyObject *loader;
981982
PyObject *module_name;
982983
PyObject *get_source;
983984
PyObject *source;
984985
PyObject *source_list;
985986
PyObject *source_line;
986987

987-
/* Check/get the requisite pieces needed for the loader. */
988-
loader = _PyDict_GetItemWithError(module_globals, &_Py_ID(__loader__));
988+
/* stolen from import.c */
989+
external = PyObject_GetAttrString(interp->importlib, "_bootstrap_external");
990+
if (external == NULL) {
991+
return NULL;
992+
}
993+
994+
loader = PyObject_CallMethod(external, "_bless_my_loader", "O", module_globals, NULL);
995+
Py_DECREF(external);
989996
if (loader == NULL) {
990997
return NULL;
991998
}
992-
Py_INCREF(loader);
999+
9931000
module_name = _PyDict_GetItemWithError(module_globals, &_Py_ID(__name__));
9941001
if (!module_name) {
9951002
Py_DECREF(loader);

0 commit comments

Comments
 (0)