From 5ea542a799112fe2058848d1e71393312f4d605a Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 3 Dec 2020 12:39:27 +0100 Subject: [PATCH 1/5] Allow getting version despite deps missing --- flit_core/flit_core/common.py | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/flit_core/flit_core/common.py b/flit_core/flit_core/common.py index b0505911..3d9ac797 100644 --- a/flit_core/flit_core/common.py +++ b/flit_core/flit_core/common.py @@ -150,13 +150,19 @@ def get_docstring_and_version_via_import(target): from it. """ log.debug("Loading module %s", target.file) - from importlib.machinery import SourceFileLoader - sl = SourceFileLoader(target.name, str(target.file)) + from importlib.machinery import PathFinder + from importlib.util import spec_from_file_location, module_from_spec + spec = spec_from_file_location(target.name, str(target.file)) + module = module_from_spec(spec) + error = None with _module_load_ctx(): - m = sl.load_module() + try: + spec.loader.exec_module(module) + except ImportError as e: + error = e docstring = m.__dict__.get('__doc__', None) version = m.__dict__.get('__version__', None) - return docstring, version + return docstring, version, error def get_info_from_module(target): @@ -169,21 +175,22 @@ def get_info_from_module(target): # build without necessarily requiring that our built package's # requirements are installed. docstring, version = get_docstring_and_version_via_ast(target) + error = None if not (docstring and version): - docstring, version = get_docstring_and_version_via_import(target) + docstring, version, error = get_docstring_and_version_via_import(target) if (not docstring) or not docstring.strip(): raise NoDocstringError('Flit cannot package module without docstring, ' 'or empty docstring. Please add a docstring to your module ' '({}).'.format(target.file)) - version = check_version(version) + version = check_version(version, error) docstring_lines = docstring.lstrip().splitlines() return {'summary': docstring_lines[0], 'version': version} -def check_version(version): +def check_version(version, error): """ Check whether a given version string match PEP 440, and do normalisation. @@ -195,8 +202,12 @@ def check_version(version): Returns the version in canonical PEP 440 format. """ if not version: - raise NoVersionError('Cannot package module without a version string. ' - 'Please define a `__version__ = "x.y.z"` in your module.') + msg = ('Cannot package module without a version string. ' + 'Please define a `__version__ = "x.y.z"` in your module.') + if error: + msg += (' Encountered error while retrieving version: {} ' + .format(str(error)) + raise NoVersionError(msg) if not isinstance(version, str): raise InvalidVersion('__version__ must be a string, not {}.' .format(type(version))) From 109024bfb3df4e611d4eddf3af512167d472c1e0 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 3 Dec 2020 12:41:07 +0100 Subject: [PATCH 2/5] Whoops --- flit_core/flit_core/common.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/flit_core/flit_core/common.py b/flit_core/flit_core/common.py index 3d9ac797..ede087c1 100644 --- a/flit_core/flit_core/common.py +++ b/flit_core/flit_core/common.py @@ -153,13 +153,13 @@ def get_docstring_and_version_via_import(target): from importlib.machinery import PathFinder from importlib.util import spec_from_file_location, module_from_spec spec = spec_from_file_location(target.name, str(target.file)) - module = module_from_spec(spec) - error = None - with _module_load_ctx(): - try: - spec.loader.exec_module(module) - except ImportError as e: - error = e + m = module_from_spec(spec) + try: + error = None + with _module_load_ctx(): + spec.loader.exec_module(m) + except ImportError as e: + error = e docstring = m.__dict__.get('__doc__', None) version = m.__dict__.get('__version__', None) return docstring, version, error From 5f174877525f21aed65ca9934f6fd634a0eb16eb Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 3 Dec 2020 12:41:38 +0100 Subject: [PATCH 3/5] Unused imports --- flit_core/flit_core/common.py | 1 - 1 file changed, 1 deletion(-) diff --git a/flit_core/flit_core/common.py b/flit_core/flit_core/common.py index ede087c1..d9d7a924 100644 --- a/flit_core/flit_core/common.py +++ b/flit_core/flit_core/common.py @@ -150,7 +150,6 @@ def get_docstring_and_version_via_import(target): from it. """ log.debug("Loading module %s", target.file) - from importlib.machinery import PathFinder from importlib.util import spec_from_file_location, module_from_spec spec = spec_from_file_location(target.name, str(target.file)) m = module_from_spec(spec) From 65b439e70490daa148d46c4138bf8365c96a7418 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Fri, 4 Dec 2020 11:41:40 +0100 Subject: [PATCH 4/5] Fixes --- flit_core/flit_core/common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flit_core/flit_core/common.py b/flit_core/flit_core/common.py index d9d7a924..2bc275c5 100644 --- a/flit_core/flit_core/common.py +++ b/flit_core/flit_core/common.py @@ -189,7 +189,7 @@ def get_info_from_module(target): return {'summary': docstring_lines[0], 'version': version} -def check_version(version, error): +def check_version(version, error=None): """ Check whether a given version string match PEP 440, and do normalisation. @@ -205,7 +205,7 @@ def check_version(version, error): 'Please define a `__version__ = "x.y.z"` in your module.') if error: msg += (' Encountered error while retrieving version: {} ' - .format(str(error)) + .format(str(error))) raise NoVersionError(msg) if not isinstance(version, str): raise InvalidVersion('__version__ must be a string, not {}.' From fad19475190a3882eca7cf31937feb1615ffc41f Mon Sep 17 00:00:00 2001 From: Philipp A Date: Fri, 4 Dec 2020 11:55:26 +0100 Subject: [PATCH 5/5] Tests --- .../samples/constructed_version/cannot_set_version.py | 5 +++++ .../tests/samples/constructed_version/module2.py | 5 +++++ flit_core/flit_core/tests/test_common.py | 9 +++++++++ 3 files changed, 19 insertions(+) create mode 100644 flit_core/flit_core/tests/samples/constructed_version/cannot_set_version.py create mode 100644 flit_core/flit_core/tests/samples/constructed_version/module2.py diff --git a/flit_core/flit_core/tests/samples/constructed_version/cannot_set_version.py b/flit_core/flit_core/tests/samples/constructed_version/cannot_set_version.py new file mode 100644 index 00000000..f9c0644d --- /dev/null +++ b/flit_core/flit_core/tests/samples/constructed_version/cannot_set_version.py @@ -0,0 +1,5 @@ +"""This module imports things that aren’t importable too early""" + +from _a_package_that_definitely_does_not_exist import a_symbol_that_neither_exists + +__version__ = '{v}.{v}'.format(v=1) diff --git a/flit_core/flit_core/tests/samples/constructed_version/module2.py b/flit_core/flit_core/tests/samples/constructed_version/module2.py new file mode 100644 index 00000000..1393230e --- /dev/null +++ b/flit_core/flit_core/tests/samples/constructed_version/module2.py @@ -0,0 +1,5 @@ +"""This module imports things that aren’t importable""" + +__version__ = '{v}.{v}'.format(v=2) + +from _a_package_that_definitely_does_not_exist import a_symbol_that_neither_exists diff --git a/flit_core/flit_core/tests/test_common.py b/flit_core/flit_core/tests/test_common.py index dcc4deb6..73110c80 100644 --- a/flit_core/flit_core/tests/test_common.py +++ b/flit_core/flit_core/tests/test_common.py @@ -55,9 +55,18 @@ def test_get_info_from_module(self): 'version': '1.2.3'} ) + info = get_info_from_module(Module('module2', samples_dir / 'constructed_version')) + self.assertEqual(info, {'summary': 'This module imports things that aren’t importable', + 'version': '2.2'} + ) + with self.assertRaises(InvalidVersion): get_info_from_module(Module('invalid_version1', samples_dir)) + with self.assertRaises(NoVersionError) as cm: + get_info_from_module(Module('cannot_set_version', samples_dir / 'constructed_version')) + assert "error while retrieving version: No module named '_a_package_" in str(cm.exception) + def test_version_raise(self): with pytest.raises(InvalidVersion): check_version('a.1.0.beta0')