Use a helper function

python · miss-islington · Oct 7, 2022 · Oct 3, 2022 · Oct 3, 2022 · Oct 4, 2022
commit 1066b9facc64accfe53b057dc639045ab38d29db
@@ -854,6 +854,61 @@ def spec_from_file_location(name, location=None, *, loader=None,
     return spec
 
 
+def _bless_my_loader(module):
+    """Helper function implementing the current module loader policy.
+
+    In Python 3.14, the end state is to require and use the module's
+    __spec__.loader and ignore any __loader__ attribute on the
+    module.
+
+   * If you have a __loader__ and a __spec__.loader but they are not the
+     same, in Python 3.12 we issue a DeprecationWarning and fall back to
+     __loader__ for backward compatibility.  In Python 3.14, we'll flip
+     this case to ignoring __loader__ entirely, without error.
+
+   * If you do not have a __spec__ or __spec__.loader, we also issue a
+     DeprecationWarning and fall back to __loader__.  In Python 3.14,
+     we'll make this case fail with an AttributeError, and ignore
+     __loader__.
+
+   * In Python 3.12 and beyond, if you do not have a __loader__, we don't
+     care as long as you still have a __spec__.loader, otherwise you get
+     an AttributeError, telling you to add a __spec__.loader.
+
+    See GH#97850 for details.
+    """
+    missing = object()
+    loader = getattr(module, '__loader__', None)
+    spec = getattr(module, '__spec__', missing)
+
+    if loader is None and spec is missing:
+        raise AttributeError(f'{module!r} is missing a __spec__')
+    if loader is None and spec is None:
+        raise ValueError(f'{module!r} is missing a __spec__.loader')
+
+    spec_loader = getattr(spec, 'loader', None)
+
+    if spec_loader is None:
+        if loader is None:
+            raise ValueError(f'{module!r} is missing a __spec__.loader')
+        _warnings.warn(
+            f'{module!r} is missing a __spec__.loader',
+            DeprecationWarning)
+        spec_loader = loader
+
+    assert spec_loader is not None
+    if loader is not None and loader != spec_loader:
+        _warnings.warn(
+            f'{module!r}; __loader__ != __spec__.loader',
+            DeprecationWarning)
+        preferred_loader = loader
+    if loader is None:
+        preferred_loader = spec_loader
+
+    assert preferred_loader is not None
+    return preferred_loader
+
+
 # Loaders #####################################################################
 
 class WindowsRegistryFinder:

diff --git a/Lib/test/test_warnings/__init__.py b/Lib/test/test_warnings/__init__.py
@@ -886,26 +886,122 @@ def test_issue31566(self):
                  support.swap_item(globals(), '__file__', None):
                 self.assertRaises(UserWarning, self.module.warn, 'bar')
 
+    # GH#86298 is part of the migration away from module attributes and toward
+    # __spec__ attributes.  There are several cases to test here.  This will
+    # have to change in Python 3.14 when we actually remove/ignore __loader__
+    # in favor of requiring __spec__.loader.
+
     @support.cpython_only
-    def test_gh86298(self):
-        # warn_explicit() itself issues an ImportWarning when a module and
-        # module_globals are both given, and the named module has a __loader__
-        # != its __spec__.loader.
+    def test_gh86298_no_loader_and_no_spec(self):
+        bar = ModuleType('bar')
+        with original_warnings.catch_warnings():
+            self.assertRaises(
+                AttributeError, self.module.warn_explicit,
+                'warning!', RuntimeWarning,
+                'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
+
+    @support.cpython_only
+    def test_gh86298_loader_is_none_and_no_spec(self):
+        bar = ModuleType('bar')
+        bar.__loader__ = None
+        with original_warnings.catch_warnings():
+            self.assertRaises(
+                AttributeError, self.module.warn_explicit,
+                'warning!', RuntimeWarning,
+                'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
+
+    @support.cpython_only
+    def test_gh86298_no_loader_and_spec_is_none(self):
+        bar = ModuleType('bar')
+        bar.__spec__ = None
+        with original_warnings.catch_warnings():
+            self.assertRaises(
+                ValueError, self.module.warn_explicit,
+                'warning!', RuntimeWarning,
+                'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
+
+    @support.cpython_only
+    def test_gh86298_loader_is_none_and_spec_is_none(self):
+        bar = ModuleType('bar')
+        bar.__loader__ = None
+        bar.__spec__ = None
+        with original_warnings.catch_warnings():
+            self.assertRaises(
+                ValueError, self.module.warn_explicit,
+                'warning!', RuntimeWarning,
+                'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
+
+    @support.cpython_only
+    def test_gh86298_loader_is_none_and_spec_loader_is_none(self):
+        bar = ModuleType('bar')
+        bar.__loader__ = None
+        bar.__spec__ = SimpleNamespace(loader=None)
+        with original_warnings.catch_warnings():
+            self.assertRaises(
+                ValueError, self.module.warn_explicit,
+                'warning!', RuntimeWarning,
+                'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
+
+    @support.cpython_only
+    def test_gh86298_no_spec(self):
+        bar = ModuleType('bar')
+        bar.__loader__ = object()
+        with original_warnings.catch_warnings():
+            self.assertWarns(
+                DeprecationWarning, self.module.warn_explicit,
+                'warning!', RuntimeWarning,
+                'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
+
+    @support.cpython_only
+    def test_gh86298_spec_is_none(self):
+        bar = ModuleType('bar')
+        bar.__loader__ = object()
+        bar.__spec__ = None
+        with original_warnings.catch_warnings():
+            self.assertWarns(
+                DeprecationWarning, self.module.warn_explicit,
+                'warning!', RuntimeWarning,
+                'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
+
+    @support.cpython_only
+    def test_gh86298_no_spec_loader(self):
+        bar = ModuleType('bar')
+        bar.__loader__ = object()
+        bar.__spec__ = SimpleNamespace()
+        with original_warnings.catch_warnings():
+            self.assertWarns(
+                DeprecationWarning, self.module.warn_explicit,
+                'warning!', RuntimeWarning,
+                'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
+
+    @support.cpython_only
+    def test_gh86298_loader_and_spec_loader_disagree(self):
         bar = ModuleType('bar')
-        # First, make the __loader__ != the __spec__.loader.  This should
-        # trigger the ImportWarning inside warn_explicit().
         bar.__loader__ = object()
         bar.__spec__ = SimpleNamespace(loader=object())
         with original_warnings.catch_warnings():
             self.assertWarns(
-                ImportWarning, self.module.warn_explicit,
+                DeprecationWarning, self.module.warn_explicit,
+                'warning!', RuntimeWarning,
+                'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
+
+    @support.cpython_only
+    def test_gh86298_no_loader_and_no_spec_loader(self):
+        bar = ModuleType('bar')
+        bar.__spec__ = SimpleNamespace()
+        with original_warnings.catch_warnings():
+            self.assertRaises(
+                AttributeError, self.module.warn_explicit,
                 'warning!', RuntimeWarning,
                 'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
-        # Now make the __loader__ == __spec__.loader.  This will not raise the exception.
-        bar.__spec__.loader = bar.__loader__
+
+    @support.cpython_only
+    def test_gh86298_no_loader_with_spec_loader_okay(self):
+        bar = ModuleType('bar')
+        bar.__spec__ = SimpleNamespace(loader=object())
         with original_warnings.catch_warnings():
-            self.module.filterwarnings('error', category=ImportWarning)
-            self.module.warn_explicit(
+            self.assertWarn(
+                RuntimeWarning, self.module.warn_explicit,
                 'warning!', RuntimeWarning,
                 'bar.py', 2, module='bar knee', module_globals=bar.__dict__)
 

diff --git a/Python/_warnings.c b/Python/_warnings.c
@@ -975,61 +975,33 @@ warnings_warn_impl(PyObject *module, PyObject *message, PyObject *category,
 }
 
 static PyObject *
-get_source_line(PyInterpreterState *interp, PyObject *module_globals, int lineno)
+get_source_line(PyInterpreterState *interp, PyObject* mod, PyObject *module_globals, int lineno)
 {
-    PyObject *loader, *spec_loader;
-    PyObject *spec;
+    PyObject *external;
+    PyObject *loader;
     PyObject *module_name;
     PyObject *get_source;
     PyObject *source;
     PyObject *source_list;
     PyObject *source_line;
 
-    module_name = _PyDict_GetItemWithError(module_globals, &_Py_ID(__name__));
-    if (!module_name) {
+    /* stolen from import.c */
+    external = PyObject_GetAttrString(interp->importlib, "_bootstrap_external");
+    if (external == NULL) {
         return NULL;
     }
-    Py_INCREF(module_name);
 
-    /* Check/get the requisite pieces needed for the loader.  Get both the
-       __spec__.loader and __loader__ attributes and warn if they are not the
-       same.
-    */
-    spec = _PyDict_GetItemWithError(module_globals, &_Py_ID(__spec__));
-    if (spec == NULL) {
-        Py_DECREF(module_name);
-        return NULL;
-    }
-    Py_INCREF(spec);
-
-    spec_loader = PyObject_GetAttrString(spec, "loader");
-    Py_DECREF(spec);
-    if (spec_loader == NULL) {
-        Py_DECREF(module_name);
+    loader = PyObject_CallMethodObjArgs(external, "_bless_my_loader", mod, NULL);
+    Py_DECREF(external);
+    if (loader == NULL) {
         return NULL;
     }
 
-    loader = _PyDict_GetItemWithError(module_globals, &_Py_ID(__loader__));
-    if (loader == NULL) {
-        Py_DECREF(spec_loader);
-        Py_DECREF(module_name);
+    module_name = _PyDict_GetItemWithError(module_globals, &_Py_ID(__name__));
+    if (!module_name) {
         return NULL;
     }
-    Py_INCREF(loader);
-
-    if (spec_loader != loader) {
-        int error = PyErr_WarnFormat(
-            PyExc_ImportWarning, 2,
-            "Module %U; __loader__ != __spec__.loader (%R != %R)",
-            modu
1E79
le_name, spec_loader, loader);
-        if (error < 0) {
-            Py_DECREF(module_name);
-            Py_DECREF(spec_loader);
-            Py_DECREF(loader);
-            return NULL;
-        }
-    }
-    Py_DECREF(spec_loader);
+    Py_INCREF(module_name);
 
     /* Make sure the loader implements the optional get_source() method. */
     (void)_PyObject_LookupAttr(loader, &_Py_ID(get_source), &get_source);
@@ -1102,7 +1074,7 @@ warnings_warn_explicit_impl(PyObject *module, PyObject *message,
             return NULL;
         }
 
-        source_line = get_source_line(tstate->interp, module_globals, lineno);
+        source_line = get_source_line(tstate->interp, mod, module_globals, lineno);
         if (source_line == NULL && PyErr_Occurred()) {
             return NULL;
         }