`ModuleType.repr` can fail on py312+ #112414

AlexWaygood · 2023-11-25T22:56:11Z

Bug report

Bug description:

Calling repr() on a ModuleType instance can fail with AttributeError on py312+, due to changes made in #98870. This is because the implementation of importlib._bootstrap._module_repr_from_spec assumes that all loaders will have a _path attribute. But if the module had a custom loader, that won't necessarily hold true: _path is an undocumented, internal, private attribute that CPython's import loaders have, but custom third-party loaders won't necessarily have.

An easy way to see this is the following (in a py312 venv with setuptools installed):

Python 3.12.0 (tags/v3.12.0:0fb18b0, Oct  2 2023, 13:03:39) [MSC v.1935 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from setuptools._vendor import packaging
>>> repr(packaging)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 545, in _module_repr
  File "<frozen importlib._bootstrap>", line 830, in _module_repr_from_spec
AttributeError: 'VendorImporter' object has no attribute '_path'

setuptools implements the custom VendorImporter loader here: https://github.com/pypa/setuptools/blob/b1bf87be7869df40872e947e27296ef87e3125ae/setuptools/extern/__init__.py#L5-L70. Instances of VendorImporter do not have a _path attribute.

This kind of thing trips up external tools such as mypy's stubtest.py script, which calls repr() on arbitrary instances of types.ModuleType on certain code paths and (reasonably) expects that to always succeed: https://github.com/python/mypy/blob/1200d1d956e589a0a33c86ef8a7cb3f5a9b64f1f/mypy/stubtest.py#L224

Cc. @FFY00 and @jaraco, as participants in #98139 (the issue #98870 was linked to)

CPython versions tested on:

3.12

Operating systems tested on:

Windows

Linked PRs

The text was updated successfully, but these errors were encountered:

AlexWaygood · 2023-11-25T22:59:38Z

Maybe something like this would be a reasonable fix?

--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -826,8 +826,10 @@ def _module_repr_from_spec(spec):
     if spec.origin is None:
         if spec.loader is None:
             return f'<module {name!r}>'
-        else:
+        elif hasattr(spec.loader, "_path"):
             return f'<module {name!r} (namespace) from {list(spec.loader._path)}>'
+        else:
+            return f'<module {name!r} ({spec.loader!r})>'

TeamSpen210 · 2023-11-25T23:19:08Z

Maybe it'd be better to do an isinstance() check. Then someone defining _path in their own loader wouldn't have the repr mysteriously change. Seems like an entirely reasonable attribute to have, and if it was a string you'd get rather silly results.

AlexWaygood · 2023-11-26T13:19:47Z

Maybe it'd be better to do an isinstance() check. Then someone defining _path in their own loader wouldn't have the repr mysteriously change. Seems like an entirely reasonable attribute to have, and if it was a string you'd get rather silly results.

Makes sense. That implies a diff like this:

--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -824,10 +824,16 @@ def _module_repr_from_spec(spec):
     """Return the repr to use for the module."""
     name = '?' if spec.name is None else spec.name
     if spec.origin is None:
-        if spec.loader is None:
+        loader = spec.loader
+        if loader is None:
             return f'<module {name!r}>'
+        elif (
+            _bootstrap_external is not None
+            and isinstance(loader, _bootstrap_external.NamespaceLoader)
+        ):
+            return f'<module {name!r} (namespace) from {list(loader._path)}>'
         else:
-            return f'<module {name!r} (namespace) from {list(spec.loader._path)}>'
+            # the loader could be a custom third-party object without a `_path` attribute
+            return f'<module {name!r} ({loader!r})>'

…le imported with a custom loader

AlexWaygood · 2023-11-26T13:57:03Z

Another easy way to observe this bug: apply this diff to main:

diff --git a/Lib/test/test_importlib/import_/test___loader__.py b/Lib/test/test_importlib/import_/test___loader__.py
index 858b37effc..4ae60574ed 100644
--- a/Lib/test/test_importlib/import_/test___loader__.py
+++ b/Lib/test/test_importlib/import_/test___loader__.py
@@ -23,6 +23,7 @@ def test___loader__(self):
         with util.uncache('blah'), util.import_state(meta_path=[loader]):
             module = self.__import__('blah')
         self.assertEqual(loader, module.__loader__)
+        repr(module)

And then run python -m test test_importlib.import_.test___loader__ -v:

Test output

Using random seed: 2645428552
0:00:00 Run 1 test sequentially
0:00:00 [1/1] test_importlib.import_.test___loader__
test___loader__ (test.test_importlib.import_.test___loader__.Frozen_SpecLoaderAttributeTests.test___loader__) ... ERROR
test___loader__ (test.test_importlib.import_.test___loader__.Source_SpecLoaderAttributeTests.test___loader__) ... ERROR

======================================================================
ERROR: test___loader__ (test.test_importlib.import_.test___loader__.Frozen_SpecLoaderAttributeTests.test___loader__)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\alexw\coding\cpython\Lib\test\test_importlib\import_\test___loader__.py", line 26, in test___loader__
    repr(module)
  File "<frozen importlib._bootstrap>", line 545, in _module_repr
  File "<frozen importlib._bootstrap>", line 830, in _module_repr_from_spec
AttributeError: 'SpecLoaderMock' object has no attribute '_path'

======================================================================
ERROR: test___loader__ (test.test_importlib.import_.test___loader__.Source_SpecLoaderAttributeTests.test___loader__)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\alexw\coding\cpython\Lib\test\test_importlib\import_\test___loader__.py", line 26, in test___loader__
    repr(module)
  File "<frozen importlib._bootstrap>", line 545, in _module_repr
  File "<frozen importlib._bootstrap>", line 830, in _module_repr_from_spec
AttributeError: 'SpecLoaderMock' object has no attribute '_path'

----------------------------------------------------------------------
Ran 2 tests in 0.022s

FAILED (errors=2)
test test_importlib.import_.test___loader__ failed
test_importlib.import_.test___loader__ failed (2 errors)

== Tests result: FAILURE ==

1 test failed:
    test_importlib.import_.test___loader__

Total duration: 254 ms
Total tests: run=2
Total test files: run=1/1 failed=1
Result: FAILURE

I've filed #112425 to fix it.

…package imported with a custom loader (#112425)

…space package imported with a custom loader (pythonGH-112425) (cherry picked from commit 0622839) Co-authored-by: Alex Waygood <[email protected]>

…espace package imported with a custom loader (GH-112425) (#112440) gh-112414: Fix `AttributeError` when calling `repr()` on a namespace package imported with a custom loader (GH-112425) (cherry picked from commit 0622839) Co-authored-by: Alex Waygood <[email protected]>

…namespace package

…ace package (#112475) Co-authored-by: Eric Snow <[email protected]>

…namespace package (pythonGH-112475) (cherry picked from commit cf20540) Co-authored-by: Alex Waygood <[email protected]> Co-authored-by: Eric Snow <[email protected]>

… namespace package (GH-112475) (#112480) gh-112414: Add additional unit tests for calling `repr()` on a namespace package (GH-112475) (cherry picked from commit cf20540) Co-authored-by: Alex Waygood <[email protected]> Co-authored-by: Eric Snow <[email protected]>

…space package imported with a custom loader (python#112425)

…namespace package (python#112475) Co-authored-by: Eric Snow <[email protected]>

…space package imported with a custom loader (python#112425)

…namespace package (python#112475) Co-authored-by: Eric Snow <[email protected]>

AlexWaygood added type-bug An unexpected behavior, bug, or error topic-importlib 3.12 bugs and security fixes 3.13 bugs and security fixes labels Nov 25, 2023

AlexWaygood added a commit to AlexWaygood/cpython that referenced this issue Nov 26, 2023

pythongh-112414: Fix AttributeError when calling repr() on a modu…

54f03c8

…le imported with a custom loader

bedevere-app bot mentioned this issue Nov 26, 2023

gh-112414: Fix AttributeError when calling repr() on a namespace package imported with a custom loader #112425

Merged

hauntsaninja added a commit to hauntsaninja/cpython that referenced this issue Nov 27, 2023

pythongh-112414: Fix repr for custom loader

e21726d

bedevere-app bot mentioned this issue Nov 27, 2023

gh-112414: Fix repr for custom loader #112436

Closed

AlexWaygood added a commit that referenced this issue Nov 27, 2023

gh-112414: Fix AttributeError when calling repr() on a namespace …

0622839

…package imported with a custom loader (#112425)

bedevere-app bot mentioned this issue Nov 27, 2023

[3.12] gh-112414: Fix AttributeError when calling repr() on a namespace package imported with a custom loader (GH-112425) #112440

Merged

AlexWaygood closed this as completed Nov 27, 2023

AlexWaygood added a commit to AlexWaygood/cpython that referenced this issue Nov 27, 2023

pythongh-112414: Add additional unit tests for calling repr() on a …

e8ae7f0

…namespace package

bedevere-app bot mentioned this issue Nov 27, 2023

gh-112414: Add additional unit tests for calling repr() on a namespace package #112475

Merged

AlexWaygood added a commit that referenced this issue Nov 28, 2023

gh-112414: Add additional unit tests for calling repr() on a namesp…

cf20540

…ace package (#112475) Co-authored-by: Eric Snow <[email protected]>

bedevere-app bot mentioned this issue Nov 28, 2023

[3.12] gh-112414: Add additional unit tests for calling repr() on a namespace package (GH-112475) #112480

Merged

aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024

pythongh-112414: Fix AttributeError when calling repr() on a name…

e030110

…space package imported with a custom loader (python#112425)

aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024

pythongh-112414: Add additional unit tests for calling repr() on a …

7de710c

…namespace package (python#112475) Co-authored-by: Eric Snow <[email protected]>

hmstepanek mentioned this issue Aug 1, 2024

repr(six.moves) throws an AttributeError for cpython 3.12 benjaminp/six#376

Closed

Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024

pythongh-112414: Fix AttributeError when calling repr() on a name…

f5d2397

…space package imported with a custom loader (python#112425)

Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024

pythongh-112414: Add additional unit tests for calling repr() on a …

ba9e5f2

…namespace package (python#112475) Co-authored-by: Eric Snow <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`ModuleType.repr` can fail on py312+ #112414

`ModuleType.repr` can fail on py312+ #112414

AlexWaygood commented Nov 25, 2023 •

edited by bedevere-app bot

Loading

AlexWaygood commented Nov 25, 2023

TeamSpen210 commented Nov 25, 2023

AlexWaygood commented Nov 26, 2023 •

edited

Loading

AlexWaygood commented Nov 26, 2023 •

edited

Loading

ModuleType.__repr__ can fail on py312+ #112414

ModuleType.__repr__ can fail on py312+ #112414

Comments

AlexWaygood commented Nov 25, 2023 • edited by bedevere-app bot Loading

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

AlexWaygood commented Nov 25, 2023

TeamSpen210 commented Nov 25, 2023

AlexWaygood commented Nov 26, 2023 • edited Loading

AlexWaygood commented Nov 26, 2023 • edited Loading

`ModuleType.repr` can fail on py312+ #112414

`ModuleType.repr` can fail on py312+ #112414

AlexWaygood commented Nov 25, 2023 •

edited by bedevere-app bot

Loading

AlexWaygood commented Nov 26, 2023 •

edited

Loading

AlexWaygood commented Nov 26, 2023 •

edited

Loading