Skip to content

Commit

Permalink
gh-65961: Do not rely solely on __cached__ (GH-97990)
Browse files Browse the repository at this point in the history
Make sure `__spec__.cached` (at minimum) can be used.
  • Loading branch information
brettcannon authored Oct 6, 2022
1 parent f8edc6f commit e1c4d56
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 35 deletions.
9 changes: 9 additions & 0 deletions Doc/c-api/import.rst
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,11 @@ Importing Modules
See also :c:func:`PyImport_ExecCodeModuleEx` and
:c:func:`PyImport_ExecCodeModuleWithPathnames`.
.. versionchanged:: 3.12
The setting of :attr:`__cached__` and :attr:`__loader__` is
deprecated. See :class:`~importlib.machinery.ModuleSpec` for
alternatives.
.. c:function:: PyObject* PyImport_ExecCodeModuleEx(const char *name, PyObject *co, const char *pathname)
Expand All @@ -167,6 +172,10 @@ Importing Modules
.. versionadded:: 3.3
.. versionchanged:: 3.12
Setting :attr:`__cached__` is deprecated. See
:class:`~importlib.machinery.ModuleSpec` for alternatives.
.. c:function:: PyObject* PyImport_ExecCodeModuleWithPathnames(const char *name, PyObject *co, const char *pathname, const char *cpathname)
Expand Down
9 changes: 9 additions & 0 deletions Doc/library/runpy.rst
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ The :mod:`runpy` module provides two functions:
run this way, as well as ensuring the real module name is always
accessible as ``__spec__.name``.

.. versionchanged:: 3.12
The setting of ``__cached__``, ``__loader__``, and
``__package__`` are deprecated. See
:class:`~importlib.machinery.ModuleSpec` for alternatives.

.. function:: run_path(path_name, init_globals=None, run_name=None)

.. index::
Expand Down Expand Up @@ -163,6 +168,10 @@ The :mod:`runpy` module provides two functions:
case where ``__main__`` is imported from a valid sys.path entry rather
than being executed directly.

.. versionchanged:: 3.12
The setting of ``__cached__``, ``__loader__``, and
``__package__`` are deprecated.

.. seealso::

:pep:`338` -- Executing modules as scripts
Expand Down
4 changes: 2 additions & 2 deletions Doc/whatsnew/3.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,8 @@ Pending Removal in Python 3.14
* Creating :c:data:`immutable types <Py_TPFLAGS_IMMUTABLETYPE>` with mutable
bases using the C API.

* ``__package__`` will cease to be set or taken into consideration by
the import system (:gh:`97879`).
* ``__package__`` and ``__cached__`` will cease to be set or taken
into consideration by the import system (:gh:`97879`).


Pending Removal in Future Versions
Expand Down
8 changes: 6 additions & 2 deletions Lib/cProfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
__all__ = ["run", "runctx", "Profile"]

import _lsprof
import importlib.machinery
import profile as _pyprofile

# ____________________________________________________________
Expand Down Expand Up @@ -169,9 +170,12 @@ def main():
sys.path.insert(0, os.path.dirname(progname))
with open(progname, 'rb') as fp:
code = compile(fp.read(), progname, 'exec')
spec = importlib.machinery.ModuleSpec(name='__main__', loader=None,
origin=progname)
globs = {
'__file__': progname,
'__name__': '__main__',
'__spec__': spec,
'__file__': spec.origin,
'__name__': spec.name,
'__package__': None,
'__cached__': None,
}
Expand Down
28 changes: 18 additions & 10 deletions Lib/importlib/_bootstrap_external.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,16 @@ def _path_isabs(path):
return path.startswith(path_separators)


def _path_abspath(path):
"""Replacement for os.path.abspath."""
if not _path_isabs(path):
for sep in path_separators:
path = path.removeprefix(f".{sep}")
return _path_join(_os.getcwd(), path)
else:
return path


def _write_atomic(path, data, mode=0o666):
"""Best-effort function to write data to a path atomically.
Be prepared to handle a FileExistsError if concurrent writing of the
Expand Down Expand Up @@ -494,8 +504,7 @@ def cache_from_source(path, debug_override=None, *, optimization=None):
# make it absolute (`C:\Somewhere\Foo\Bar`), then make it root-relative
# (`Somewhere\Foo\Bar`), so we end up placing the bytecode file in an
# unambiguous `C:\Bytecode\Somewhere\Foo\Bar\`.
if not _path_isabs(head):
head = _path_join(_os.getcwd(), head)
head = _path_abspath(head)

# Strip initial drive from a Windows path. We know we have an absolute
# path here, so the second part of the check rules out a POSIX path that
Expand Down Expand Up @@ -808,11 +817,10 @@ def spec_from_file_location(name, location=None, *, loader=None,
pass
else:
location = _os.fspath(location)
if not _path_isabs(location):
try:
location = _path_join(_os.getcwd(), location)
except OSError:
pass
try:
location = _path_abspath(location)
except OSError:
pass

# If the location is on the filesystem, but doesn't actually exist,
# we could return None here, indicating that the location is not
Expand Down Expand Up @@ -1564,10 +1572,8 @@ def __init__(self, path, *loader_details):
# Base (directory) path
if not path or path == '.':
self.path = _os.getcwd()
elif not _path_isabs(path):
self.path = _path_join(_os.getcwd(), path)
else:
self.path = path
self.path = _path_abspath(path)
self._path_mtime = -1
self._path_cache = set()
self._relaxed_path_cache = set()
Expand Down Expand Up @@ -1717,6 +1723,8 @@ def _fix_up_module(ns, name, pathname, cpathname=None):
loader = SourceFileLoader(name, pathname)
if not spec:
spec = spec_from_file_location(name, pathname, loader=loader)
if cpathname:
spec.cached = _path_abspath(cpathname)
try:
ns['__spec__'] = spec
ns['__loader__'] = loader
Expand Down
21 changes: 3 additions & 18 deletions Lib/inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,30 +281,15 @@ def get_annotations(obj, *, globals=None, locals=None, eval_str=False):

# ----------------------------------------------------------- type-checking
def ismodule(object):
"""Return true if the object is a module.
Module objects provide these attributes:
__cached__ pathname to byte compiled file
__doc__ documentation string
__file__ filename (missing for built-in modules)"""
"""Return true if the object is a module."""
return isinstance(object, types.ModuleType)

def isclass(object):
"""Return true if the object is a class.
Class objects provide these attributes:
__doc__ documentation string
__module__ name of module in which this class was defined"""
"""Return true if the object is a class."""
return isinstance(object, type)

def ismethod(object):
"""Return true if the object is an instance method.
Instance method objects provide these attributes:
__doc__ documentation string
__name__ name with which this method was defined
__func__ function object containing implementation of method
__self__ instance to which this method is bound"""
"""Return true if the object is an instance method."""
return isinstance(object, types.MethodType)

def ismethoddescriptor(object):
Expand Down
8 changes: 6 additions & 2 deletions Lib/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
# governing permissions and limitations under the License.


import importlib.machinery
import sys
import time
import marshal
Expand Down Expand Up @@ -589,9 +590,12 @@ def main():
sys.path.insert(0, os.path.dirname(progname))
with open(progname, 'rb') as fp:
code = compile(fp.read(), progname, 'exec')
spec = importlib.machinery.ModuleSpec(name='__main__', loader=None,
origin=progname)
globs = {
'__file__': progname,
'__name__': '__main__',
'__spec__': spec,
'__file__': spec.origin,
'__name__': spec.name,
'__package__': None,
'__cached__': None,
}
Expand Down
71 changes: 71 additions & 0 deletions Lib/test/test_importlib/import_/test_helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
"""Tests for helper functions used by import.c ."""

from importlib import _bootstrap_external, machinery
import os.path
import unittest

from .. import util


class FixUpModuleTests:

def test_no_loader_but_spec(self):
loader = object()
name = "hello"
path = "hello.py"
spec = machinery.ModuleSpec(name, loader)
ns = {"__spec__": spec}
_bootstrap_external._fix_up_module(ns, name, path)

expected = {"__spec__": spec, "__loader__": loader, "__file__": path,
"__cached__": None}
self.assertEqual(ns, expected)

def test_no_loader_no_spec_but_sourceless(self):
name = "hello"
path = "hello.py"
ns = {}
_bootstrap_external._fix_up_module(ns, name, path, path)

expected = {"__file__": path, "__cached__": path}

for key, val in expected.items():
with self.subTest(f"{key}: {val}"):
self.assertEqual(ns[key], val)

spec = ns["__spec__"]
self.assertIsInstance(spec, machinery.ModuleSpec)
self.assertEqual(spec.name, name)
self.assertEqual(spec.origin, os.path.abspath(path))
self.assertEqual(spec.cached, os.path.abspath(path))
self.assertIsInstance(spec.loader, machinery.SourcelessFileLoader)
self.assertEqual(spec.loader.name, name)
self.assertEqual(spec.loader.path, path)
self.assertEqual(spec.loader, ns["__loader__"])

def test_no_loader_no_spec_but_source(self):
name = "hello"
path = "hello.py"
ns = {}
_bootstrap_external._fix_up_module(ns, name, path)

expected = {"__file__": path, "__cached__": None}

for key, val in expected.items():
with self.subTest(f"{key}: {val}"):
self.assertEqual(ns[key], val)

spec = ns["__spec__"]
self.assertIsInstance(spec, machinery.ModuleSpec)
self.assertEqual(spec.name, name)
self.assertEqual(spec.origin, os.path.abspath(path))
self.assertIsInstance(spec.loader, machinery.SourceFileLoader)
self.assertEqual(spec.loader.name, name)
self.assertEqual(spec.loader.path, path)
self.assertEqual(spec.loader, ns["__loader__"])


FrozenFixUpModuleTests, SourceFixUpModuleTests = util.test_both(FixUpModuleTests)

if __name__ == "__main__":
unittest.main()
3 changes: 3 additions & 0 deletions Lib/test/test_inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -4358,8 +4358,11 @@ def test_details(self):
'unittest', '--details')
output = out.decode()
# Just a quick sanity check on the output
self.assertIn(module.__spec__.name, output)
self.assertIn(module.__name__, output)
self.assertIn(module.__spec__.origin, output)
self.assertIn(module.__file__, output)
self.assertIn(module.__spec__.cached, output)
self.assertIn(module.__cached__, output)
self.assertEqual(err, b'')

Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_pydoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ def test_synopsis(self):
def test_synopsis_sourceless(self):
os = import_helper.import_fresh_module('os')
expected = os.__doc__.splitlines()[0]
filename = os.__cached__
filename = os.__spec__.cached
synopsis = pydoc.synopsis(filename)

self.assertEqual(synopsis, expected)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Do not rely solely on ``__cached__`` on modules; code will also support
``__spec__.cached``.

0 comments on commit e1c4d56

Please sign in to comment.