diff --git a/coverage.ini b/coverage.ini index adcb579d..da82b3df 100644 --- a/coverage.ini +++ b/coverage.ini @@ -2,7 +2,7 @@ branch = true parallel = true omit = - setup* + setup* .tox/*/lib/python*/site-packages/* */tests/*.py */testing/*.py @@ -10,6 +10,7 @@ omit = importlib_resources/__init__.py importlib_resources/_compat.py importlib_resources/abc.py + *.zip* plugins = coverplug diff --git a/docs/changelog.rst b/docs/changelog.rst index bff615dd..113d7f2d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -2,6 +2,14 @@ importlib_resources NEWS ========================== +v3.1.0 +====== + +* #110 and bpo-41490: ``path`` method is more aggressive about + releasing handles to zipfile objects early, enabling use-cases + like ``certifi`` to leave the context open but delete the underlying + zip file. + v3.0.0 ====== diff --git a/importlib_resources/_common.py b/importlib_resources/_common.py index f54c78d7..a7c2bf81 100644 --- a/importlib_resources/_common.py +++ b/importlib_resources/_common.py @@ -93,6 +93,7 @@ def _tempfile(reader, suffix=''): try: os.write(fd, reader()) os.close(fd) + del reader yield Path(raw_path) finally: try: @@ -102,14 +103,12 @@ def _tempfile(reader, suffix=''): @singledispatch -@contextlib.contextmanager def as_file(path): """ Given a Traversable object, return that object as a path on the local file system in a context manager. """ - with _tempfile(path.read_bytes, suffix=path.name) as local: - yield local + return _tempfile(path.read_bytes, suffix=path.name) @as_file.register(Path) diff --git a/importlib_resources/_py3.py b/importlib_resources/_py3.py index 5998f215..7b535d0b 100644 --- a/importlib_resources/_py3.py +++ b/importlib_resources/_py3.py @@ -1,8 +1,9 @@ import os import sys +import io from . import _common -from contextlib import contextmanager, suppress +from contextlib import suppress from importlib.abc import ResourceLoader from io import BytesIO, TextIOWrapper from pathlib import Path @@ -94,22 +95,26 @@ def path( """ reader = _common.get_resource_reader(_common.get_package(package)) return ( - _path_from_reader(reader, resource) + _path_from_reader(reader, _common.normalize_path(resource)) if reader else _common.as_file( _common.files(package).joinpath(_common.normalize_path(resource))) ) -@contextmanager def _path_from_reader(reader, resource): - norm_resource = _common.normalize_path(resource) + return _path_from_resource_path(reader, resource) or \ + _path_from_open_resource(reader, resource) + + +def _path_from_resource_path(reader, resource): with suppress(FileNotFoundError): - yield Path(reader.resource_path(norm_resource)) - return - opener_reader = reader.open_resource(norm_resource) - with _common._tempfile(opener_reader.read, suffix=norm_resource) as res: - yield res + return Path(reader.resource_path(resource)) + + +def _path_from_open_resource(reader, resource): + saved = io.BytesIO(reader.open_resource(resource).read()) + return _common._tempfile(saved.read, suffix=resource) def is_resource(package: Package, name: str) -> bool: diff --git a/importlib_resources/readers.py b/importlib_resources/readers.py index 0e0b17ab..3b07c76c 100644 --- a/importlib_resources/readers.py +++ b/importlib_resources/readers.py @@ -22,8 +22,8 @@ def files(self): class ZipReader(abc.TraversableResources): def __init__(self, loader, module): _, _, name = module.rpartition('.') - prefix = loader.prefix.replace('\\', '/') + name + '/' - self.path = ZipPath(loader.archive, prefix) + self.prefix = loader.prefix.replace('\\', '/') + name + '/' + self.archive = loader.archive def open_resource(self, resource): try: @@ -38,4 +38,4 @@ def is_resource(self, path): return target.is_file() and target.exists() def files(self): - return self.path + return ZipPath(self.archive, self.prefix) diff --git a/importlib_resources/tests/_compat.py b/importlib_resources/tests/_compat.py new file mode 100644 index 00000000..edadf450 --- /dev/null +++ b/importlib_resources/tests/_compat.py @@ -0,0 +1,30 @@ +try: + from test.support import import_helper # type: ignore +except ImportError: + try: + # Python 3.9 and earlier + class import_helper: # type: ignore + from test.support import modules_setup, modules_cleanup + except ImportError: + from . import py27compat + + class import_helper: # type: ignore + modules_setup = staticmethod(py27compat.modules_setup) + modules_cleanup = staticmethod(py27compat.modules_cleanup) + + +try: + from os import fspath # type: ignore +except ImportError: + # Python 3.5 + fspath = str # type: ignore + + +try: + # Python 3.10 + from test.support.os_helper import unlink +except ImportError: + from test.support import unlink as _unlink + + def unlink(target): + return _unlink(fspath(target)) diff --git a/importlib_resources/tests/py27compat.py b/importlib_resources/tests/py27compat.py new file mode 100644 index 00000000..90c959e5 --- /dev/null +++ b/importlib_resources/tests/py27compat.py @@ -0,0 +1,23 @@ +import sys + + +def modules_setup(): + return sys.modules.copy(), + + +def modules_cleanup(oldmodules): + # Encoders/decoders are registered permanently within the internal + # codec cache. If we destroy the corresponding modules their + # globals will be set to None which will trip up the cached functions. + encodings = [(k, v) for k, v in sys.modules.items() + if k.startswith('encodings.')] + sys.modules.clear() + sys.modules.update(encodings) + # XXX: This kind of problem can affect more than just encodings. + # In particular extension modules (such as _ssl) don't cope + # with reloading properly. Really, test modules should be cleaning + # out the test specific modules they know they added (ala test_runpy) + # rather than relying on this function (as test_importhooks and test_pkg + # do currently). Implicitly imported *real* modules should be left alone + # (see issue 10556). + sys.modules.update(oldmodules) diff --git a/importlib_resources/tests/test_resource.py b/importlib_resources/tests/test_resource.py index 8c5a72cb..6920ac16 100644 --- a/importlib_resources/tests/test_resource.py +++ b/importlib_resources/tests/test_resource.py @@ -1,11 +1,15 @@ import sys import unittest import importlib_resources as resources +import uuid + +from importlib_resources._compat import Path from . import data01 from . import zipdata01, zipdata02 from . import util from importlib import import_module +from ._compat import import_helper, unlink class ResourceTests: @@ -166,5 +170,80 @@ def test_namespaces_cannot_have_resources(self): 'importlib_resources.tests.data03.namespace', 'resource1.txt') +class DeletingZipsTest(unittest.TestCase): + """Having accessed resources in a zip file should not keep an open + reference to the zip. + """ + ZIP_MODULE = zipdata01 + + def setUp(self): + modules = import_helper.modules_setup() + self.addCleanup(import_helper.modules_cleanup, *modules) + + data_path = Path(self.ZIP_MODULE.__file__) + data_dir = data_path.parent + self.source_zip_path = data_dir / 'ziptestdata.zip' + self.zip_path = Path('{}.zip'.format(uuid.uuid4())).absolute() + self.zip_path.write_bytes(self.source_zip_path.read_bytes()) + sys.path.append(str(self.zip_path)) + self.data = import_module('ziptestdata') + + def tearDown(self): + try: + sys.path.remove(str(self.zip_path)) + except ValueError: + pass + + try: + del sys.path_importer_cache[str(self.zip_path)] + del sys.modules[self.data.__name__] + except KeyError: + pass + + try: + unlink(self.zip_path) + except OSError: + # If the test fails, this will probably fail too + pass + + def test_contents_does_not_keep_open(self): + c = resources.contents('ziptestdata') + self.zip_path.unlink() + del c + + def test_is_resource_does_not_keep_open(self): + c = resources.is_resource('ziptestdata', 'binary.file') + self.zip_path.unlink() + del c + + def test_is_resource_failure_does_not_keep_open(self): + c = resources.is_resource('ziptestdata', 'not-present') + self.zip_path.unlink() + del c + + @unittest.skip("Desired but not supported.") + def test_path_does_not_keep_open(self): + c = resources.path('ziptestdata', 'binary.file') + self.zip_path.unlink() + del c + + def test_entered_path_does_not_keep_open(self): + # This is what certifi does on import to make its bundle + # available for the process duration. + c = resources.path('ziptestdata', 'binary.file').__enter__() + self.zip_path.unlink() + del c + + def test_read_binary_does_not_keep_open(self): + c = resources.read_binary('ziptestdata', 'binary.file') + self.zip_path.unlink() + del c + + def test_read_text_does_not_keep_open(self): + c = resources.read_text('ziptestdata', 'utf-8.file', encoding='utf-8') + self.zip_path.unlink() + del c + + if __name__ == '__main__': unittest.main() diff --git a/importlib_resources/tests/util.py b/importlib_resources/tests/util.py index 8c26496d..1d6fa729 100644 --- a/importlib_resources/tests/util.py +++ b/importlib_resources/tests/util.py @@ -9,30 +9,7 @@ from . import zipdata01 from .._compat import ABC, Path, PurePath, FileNotFoundError from ..abc import ResourceReader - -try: - from test.support import modules_setup, modules_cleanup -except ImportError: - # Python 2.7. - def modules_setup(): - return sys.modules.copy(), - - def modules_cleanup(oldmodules): - # Encoders/decoders are registered permanently within the internal - # codec cache. If we destroy the corresponding modules their - # globals will be set to None which will trip up the cached functions. - encodings = [(k, v) for k, v in sys.modules.items() - if k.startswith('encodings.')] - sys.modules.clear() - sys.modules.update(encodings) - # XXX: This kind of problem can affect more than just encodings. In - # particular extension modules (such as _ssl) don't cope with reloading - # properly. Really, test modules should be cleaning out the test - # specific modules they know they added (ala test_runpy) rather than - # relying on this function (as test_importhooks and test_pkg do - # currently). Implicitly imported *real* modules should be left alone - # (see issue 10556). - sys.modules.update(oldmodules) +from ._compat import import_helper try: @@ -205,8 +182,8 @@ def tearDownClass(cls): pass def setUp(self): - modules = modules_setup() - self.addCleanup(modules_cleanup, *modules) + modules = import_helper.modules_setup() + self.addCleanup(import_helper.modules_cleanup, *modules) class ZipSetup(ZipSetupBase): diff --git a/tox.ini b/tox.ini index 2aeb8476..d466a725 100644 --- a/tox.ini +++ b/tox.ini @@ -7,8 +7,8 @@ toxworkdir={env:TOX_WORK_DIR:.tox} [testenv] commands = - !cov,!diffcov: python -m unittest discover - cov,diffcov: python -m coverage run {[coverage]rc} -m unittest discover {posargs} + !cov,!diffcov: python -m unittest {posargs:discover} + cov,diffcov: python -m coverage run {[coverage]rc} -m unittest {posargs:discover} cov,diffcov: python -m coverage combine {[coverage]rc} cov: python -m coverage html {[coverage]rc} cov: python -m coverage xml {[coverage]rc}