Skip to content

bpo-45324: Capture data in FrozenImporter.find_spec() to use in exec_module(). #28633

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
97bdaa2
Make use of find_frozen() more consistent.
ericsnowcurrently Sep 29, 2021
e2e676d
Drop some one-off helper functions.
ericsnowcurrently Sep 29, 2021
6333212
Factor out unmarshal_frozen_code().
ericsnowcurrently Sep 29, 2021
8cd239b
Add a "data" arg to _imp.get_frozen_object().
ericsnowcurrently Sep 29, 2021
7724019
Add _imp.find_frozen().
ericsnowcurrently Sep 29, 2021
0dd3870
Use _imp.find_frozen() in the frozen importer.
ericsnowcurrently Sep 29, 2021
1a0ad16
Clear loader_state to save memory.
ericsnowcurrently Sep 29, 2021
fdf7689
Only call is_frozen() in the fallback case.
ericsnowcurrently Sep 29, 2021
385d412
Add a NEWS entry.
ericsnowcurrently Sep 29, 2021
e0fe501
Add a missing "static" declaration.
ericsnowcurrently Sep 29, 2021
7221e0e
Treat FROZEN_BAD_NAME as FROZEN_NOT_FOUND in some cases.
ericsnowcurrently Sep 29, 2021
5f68ee8
Make frozen_info.size Py_ssize_t.
ericsnowcurrently Sep 29, 2021
7b85a63
Py_ssize_t is never smaller than int.
ericsnowcurrently Sep 29, 2021
cdbc611
name -> nameobj.
ericsnowcurrently Sep 29, 2021
c6e70cc
Add tests.
ericsnowcurrently Sep 30, 2021
f86dcd5
Tweak the tests.
ericsnowcurrently Oct 1, 2021
d1b1868
Default to using _imp.get_frozen_object().
ericsnowcurrently Oct 1, 2021
4e105a9
Do not use <> to mark source package.
ericsnowcurrently Oct 1, 2021
5d6af1e
Drop unused code.
ericsnowcurrently Oct 1, 2021
017c932
Store data in loader_state directly.
ericsnowcurrently Oct 2, 2021
1a833ed
Set "name" on import errors.
ericsnowcurrently Oct 2, 2021
5363708
Use a narrower exception.
ericsnowcurrently Oct 2, 2021
07c947f
Fix a comment.
ericsnowcurrently Oct 2, 2021
cddf089
Clarify a comment.
ericsnowcurrently Oct 2, 2021
ca0500f
Add a note about why we silence an error.
ericsnowcurrently Oct 2, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions Lib/importlib/_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -826,10 +826,15 @@ def module_repr(m):

@classmethod
def find_spec(cls, fullname, path=None, target=None):
if _imp.is_frozen(fullname):
return spec_from_loader(fullname, cls, origin=cls._ORIGIN)
else:
info = _call_with_frames_removed(_imp.find_frozen, fullname)
if info is None:
return None
data, ispkg = info
spec = spec_from_loader(fullname, cls,
origin=cls._ORIGIN,
is_package=ispkg)
spec.loader_state = data
return spec

@classmethod
def find_module(cls, fullname, path=None):
Expand All @@ -849,11 +854,22 @@ def create_module(spec):

@staticmethod
def exec_module(module):
name = module.__spec__.name
if not _imp.is_frozen(name):
raise ImportError('{!r} is not a frozen module'.format(name),
name=name)
code = _call_with_frames_removed(_imp.get_frozen_object, name)
spec = module.__spec__
name = spec.name
try:
data = spec.loader_state
except AttributeError:
if not _imp.is_frozen(name):
raise ImportError('{!r} is not a frozen module'.format(name),
name=name)
data = None
else:
# We clear the extra data we got from the finder, to save memory.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the loader is one-time use? What happens if you call exec_module() again (e.g. importlib.reload())? Won't this break that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should work fine. If the data is set then the _imp.get_frozen_object() call in exec_module() will unmarshal that. If the data isn't set (e.g. with a reload) then get_frozen_object() will look up the frozen data, in addition to unmarshaling it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added to that comment to clarify.

# Note that if this method is called again (e.g. by
# importlib.reload()) then _imp.get_frozen_object() will notice
# no data was provided and will look it up.
spec.loader_state = None
code = _call_with_frames_removed(_imp.get_frozen_object, name, data)
exec(code, module.__dict__)

@classmethod
Expand Down
76 changes: 53 additions & 23 deletions Lib/test/test_importlib/frozen/test_finder.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
from .. import abc
import os.path
from .. import util

machinery = util.import_importlib('importlib.machinery')

import _imp
import marshal
import os.path
import unittest
import warnings

from test.support import import_helper
from test.support import import_helper, REPO_ROOT


class FindSpecTests(abc.FinderTests):
Expand All @@ -19,39 +21,67 @@ def find(self, name, **kwargs):
with import_helper.frozen_modules():
return finder.find_spec(name, **kwargs)

def check(self, spec, name):
def check_basic(self, spec, name, ispkg=False):
self.assertEqual(spec.name, name)
self.assertIs(spec.loader, self.machinery.FrozenImporter)
self.assertEqual(spec.origin, 'frozen')
self.assertFalse(spec.has_location)
if ispkg:
self.assertIsNotNone(spec.submodule_search_locations)
else:
self.assertIsNone(spec.submodule_search_locations)
self.assertIsNotNone(spec.loader_state)

def check_search_location(self, spec, source=None):
# Frozen packages do not have any path entries.
# (See https://bugs.python.org/issue21736.)
expected = []
self.assertListEqual(spec.submodule_search_locations, expected)

def check_data(self, spec, source=None, ispkg=None):
with import_helper.frozen_modules():
expected = _imp.get_frozen_object(spec.name)
data = spec.loader_state
# We can't compare the marshaled data directly because
# marshal.dumps() would mark "expected" as a ref, which slightly
# changes the output. (See https://bugs.python.org/issue34093.)
code = marshal.loads(data)
self.assertEqual(code, expected)

def test_module(self):
names = [
'__hello__',
'__hello_alias__',
'__hello_only__',
'__phello__.__init__',
'__phello__.spam',
'__phello__.ham.__init__',
'__phello__.ham.eggs',
]
for name in names:
modules = {
'__hello__': None,
'__phello__.__init__': None,
'__phello__.spam': None,
'__phello__.ham.__init__': None,
'__phello__.ham.eggs': None,
'__hello_alias__': '__hello__',
}
for name, source in modules.items():
with self.subTest(name):
spec = self.find(name)
self.check(spec, name)
self.assertEqual(spec.submodule_search_locations, None)
self.check_basic(spec, name)
self.check_data(spec, source)

def test_package(self):
names = [
'__phello__',
'__phello__.ham',
'__phello_alias__',
]
for name in names:
modules = {
'__phello__': None,
'__phello__.ham': None,
'__phello_alias__': '__hello__',
}
for name, source in modules.items():
with self.subTest(name):
spec = self.find(name)
self.check(spec, name)
self.assertEqual(spec.submodule_search_locations, [])
self.check_basic(spec, name, ispkg=True)
self.check_search_location(spec, source)
self.check_data(spec, source, ispkg=True)

def test_frozen_only(self):
name = '__hello_only__'
source = os.path.join(REPO_ROOT, 'Tools', 'freeze', 'flag.py')
spec = self.find(name)
self.check_basic(spec, name)
self.check_data(spec, source)

# These are covered by test_module() and test_package().
test_module_in_package = None
Expand Down
7 changes: 7 additions & 0 deletions Lib/test/test_importlib/frozen/test_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
machinery = util.import_importlib('importlib.machinery')

from test.support import captured_stdout, import_helper
import _imp
import contextlib
import marshal
import types
import unittest
import warnings
Expand Down Expand Up @@ -33,11 +35,14 @@ class ExecModuleTests(abc.LoaderTests):
def exec_module(self, name):
with import_helper.frozen_modules():
is_package = self.machinery.FrozenImporter.is_package(name)
code = _imp.get_frozen_object(name)
data = marshal.dumps(code)
spec = self.machinery.ModuleSpec(
name,
self.machinery.FrozenImporter,
origin='frozen',
is_package=is_package,
loader_state=data,
)
module = types.ModuleType(name)
module.__spec__ = spec
Expand All @@ -61,6 +66,7 @@ def test_module(self):
self.assertEqual(getattr(module, attr), value)
self.assertEqual(output, 'Hello world!\n')
self.assertTrue(hasattr(module, '__spec__'))
self.assertIsNone(module.__spec__.loader_state)

def test_package(self):
name = '__phello__'
Expand All @@ -73,6 +79,7 @@ def test_package(self):
name=name, attr=attr, given=attr_value,
expected=value))
self.assertEqual(output, 'Hello world!\n')
self.assertIsNone(module.__spec__.loader_state)

def test_lacking_parent(self):
name = '__phello__.spam'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
In FrozenImporter.find_spec(), we now preserve the information needed in
exec_module() to load the module. This change mostly impacts internal
details, rather than changing the importer's behavior.
67 changes: 57 additions & 10 deletions Python/clinic/import.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading