Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
gh-86298: Ensure that __loader__ and __spec__.loader agree in warnings.warn_explicit()#97803
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
37b581a049910b317e253428d86306d686dfc50fe11066b9f4602366831f83237254cffe9d62e3bcdba9f5cef9c5600c9c8631e53b1b0d86823400cFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -862,6 +862,54 @@ def spec_from_file_location(name, location=None, *, loader=None, | ||
| return spec | ||
| def _bless_my_loader(module_globals): | ||
| """Helper function for _warnings.c | ||
| See GH#97850 for details. | ||
| """ | ||
| # 2022-10-06(warsaw): For now, this helper is only used in _warnings.c and | ||
| # that use case only has the module globals. This function could be | ||
| # extended to accept either that or a module object. However, in the | ||
| # latter case, it would be better to raise certain exceptions when looking | ||
| # at a module, which should have either a __loader__ or __spec__.loader. | ||
| # For backward compatibility, it is possible that we'll get an empty | ||
| # dictionary for the module globals, and that cannot raise an exception. | ||
| if not isinstance(module_globals, dict): | ||
| return None | ||
| missing = object() | ||
| loader = module_globals.get('__loader__', None) | ||
| spec = module_globals.get('__spec__', missing) | ||
| if loader is None: | ||
| if spec is missing: | ||
| # If working with a module: | ||
| # raise AttributeError('Module globals is missing a __spec__') | ||
| return None | ||
| elif spec is None: | ||
| raise ValueError('Module globals is missing a __spec__.loader') | ||
| spec_loader = getattr(spec, 'loader', missing) | ||
| if spec_loader in (missing, None): | ||
warsaw marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if loader is None: | ||
| exc = AttributeError if spec_loader is missing else ValueError | ||
| raise exc('Module globals is missing a __spec__.loader') | ||
| _warnings.warn( | ||
| 'Module globals 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( | ||
| 'Module globals; __loader__ != __spec__.loader', | ||
warsaw marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| DeprecationWarning) | ||
| return loader | ||
| return spec_loader | ||
| # Loaders ##################################################################### | ||
| class WindowsRegistryFinder: | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2,7 +2,9 @@ | ||
| from importlib import _bootstrap_external, machinery | ||
| import os.path | ||
| from types import ModuleType, SimpleNamespace | ||
| import unittest | ||
| import warnings | ||
| from .. import util | ||
| @@ -67,5 +69,116 @@ def test_no_loader_no_spec_but_source(self): | ||
| FrozenFixUpModuleTests, SourceFixUpModuleTests = util.test_both(FixUpModuleTests) | ||
| class TestBlessMyLoader(unittest.TestCase): | ||
| # 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. | ||
| def test_gh86298_no_loader_and_no_spec(self): | ||
| bar = ModuleType('bar') | ||
| del bar.__loader__ | ||
| del bar.__spec__ | ||
| # 2022-10-06(warsaw): For backward compatibility with the | ||
| # implementation in _warnings.c, this can't raise an | ||
| # AttributeError. See _bless_my_loader() in _bootstrap_external.py | ||
| # If working with a module: | ||
| ## self.assertRaises( | ||
warsaw marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| ## AttributeError, _bootstrap_external._bless_my_loader, | ||
| ## bar.__dict__) | ||
| self.assertIsNone(_bootstrap_external._bless_my_loader(bar.__dict__)) | ||
| def test_gh86298_loader_is_none_and_no_spec(self): | ||
| bar = ModuleType('bar') | ||
| bar.__loader__ = None | ||
| del bar.__spec__ | ||
| # 2022-10-06(warsaw): For backward compatibility with the | ||
| # implementation in _warnings.c, this can't raise an | ||
| # AttributeError. See _bless_my_loader() in _bootstrap_external.py | ||
| # If working with a module: | ||
| ## self.assertRaises( | ||
warsaw marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| ## AttributeError, _bootstrap_external._bless_my_loader, | ||
| ## bar.__dict__) | ||
| self.assertIsNone(_bootstrap_external._bless_my_loader(bar.__dict__)) | ||
| def test_gh86298_no_loader_and_spec_is_none(self): | ||
| bar = ModuleType('bar') | ||
| del bar.__loader__ | ||
| bar.__spec__ = None | ||
| self.assertRaises( | ||
| ValueError, | ||
| _bootstrap_external._bless_my_loader, bar.__dict__) | ||
| def test_gh86298_loader_is_none_and_spec_is_none(self): | ||
| bar = ModuleType('bar') | ||
| bar.__loader__ = None | ||
| bar.__spec__ = None | ||
| self.assertRaises( | ||
| ValueError, | ||
| _bootstrap_external._bless_my_loader, bar.__dict__) | ||
| def test_gh86298_loader_is_none_and_spec_loader_is_none(self): | ||
| bar = ModuleType('bar') | ||
| bar.__loader__ = None | ||
| bar.__spec__ = SimpleNamespace(loader=None) | ||
| self.assertRaises( | ||
| ValueError, | ||
| _bootstrap_external._bless_my_loader, bar.__dict__) | ||
| def test_gh86298_no_spec(self): | ||
| bar = ModuleType('bar') | ||
| bar.__loader__ = object() | ||
| del bar.__spec__ | ||
| with warnings.catch_warnings(): | ||
| self.assertWarns( | ||
| DeprecationWarning, | ||
| _bootstrap_external._bless_my_loader, bar.__dict__) | ||
| def test_gh86298_spec_is_none(self): | ||
| bar = ModuleType('bar') | ||
| bar.__loader__ = object() | ||
| bar.__spec__ = None | ||
| with warnings.catch_warnings(): | ||
| self.assertWarns( | ||
| DeprecationWarning, | ||
| _bootstrap_external._bless_my_loader, bar.__dict__) | ||
| def test_gh86298_no_spec_loader(self): | ||
| bar = ModuleType('bar') | ||
| bar.__loader__ = object() | ||
| bar.__spec__ = SimpleNamespace() | ||
| with warnings.catch_warnings(): | ||
| self.assertWarns( | ||
| DeprecationWarning, | ||
| _bootstrap_external._bless_my_loader, bar.__dict__) | ||
| def test_gh86298_loader_and_spec_loader_disagree(self): | ||
| bar = ModuleType('bar') | ||
| bar.__loader__ = object() | ||
| bar.__spec__ = SimpleNamespace(loader=object()) | ||
| with warnings.catch_warnings(): | ||
| self.assertWarns( | ||
| DeprecationWarning, | ||
| _bootstrap_external._bless_my_loader, bar.__dict__) | ||
| def test_gh86298_no_loader_and_no_spec_loader(self): | ||
| bar = ModuleType('bar') | ||
| del bar.__loader__ | ||
| bar.__spec__ = SimpleNamespace() | ||
| self.assertRaises( | ||
| AttributeError, | ||
| _bootstrap_external._bless_my_loader, bar.__dict__) | ||
| def test_gh86298_no_loader_with_spec_loader_okay(self): | ||
| bar = ModuleType('bar') | ||
| del bar.__loader__ | ||
| loader = object() | ||
| bar.__spec__ = SimpleNamespace(loader=loader) | ||
| self.assertEqual( | ||
| _bootstrap_external._bless_my_loader(bar.__dict__), | ||
| loader) | ||
| if __name__ == "__main__": | ||
| unittest.main() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| In cases where ``warnings.warn_explicit()`` consults the module's loader, an | ||
| ``DeprecationWarning`` is issued when ``m.__loader__`` differs from | ||
| ``m.__spec__.loader``. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -977,19 +977,26 @@ warnings_warn_impl(PyObject *module, PyObject *message, PyObject *category, | ||
| static PyObject * | ||
| get_source_line(PyInterpreterState *interp, PyObject *module_globals, int lineno) | ||
| { | ||
| PyObject *external; | ||
| PyObject *loader; | ||
| PyObject *module_name; | ||
| PyObject *get_source; | ||
| PyObject *source; | ||
| PyObject *source_list; | ||
| PyObject *source_line; | ||
| /* Check/get the requisite pieces needed for the loader. */ | ||
| loader = _PyDict_GetItemWithError(module_globals, &_Py_ID(__loader__)); | ||
| /* stolen from import.c */ | ||
warsaw marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| external = PyObject_GetAttrString(interp->importlib, "_bootstrap_external"); | ||
| if (external == NULL) { | ||
| return NULL; | ||
| } | ||
| loader = PyObject_CallMethod(external, "_bless_my_loader", "O", module_globals, NULL); | ||
| Py_DECREF(external); | ||
| if (loader == NULL) { | ||
| return NULL; | ||
| } | ||
| Py_INCREF(loader); | ||
| module_name = _PyDict_GetItemWithError(module_globals, &_Py_ID(__name__)); | ||
| if (!module_name) { | ||
| Py_DECREF(loader); | ||
Uh oh!
There was an error while loading. Please reload this page.