From e92f16f35a953ce97b0dbb5cad31da7563b4faab Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Fri, 22 Feb 2019 04:15:07 -0800 Subject: [PATCH] Include file path when Version: missing Related to pip's github issue pypa/pip#6194. This has come up in pip's issue tracker (github) multiple times: - pypa/pip#6177 - pypa/pip#6283 - pypa/pip#6194 --- changelog.d/1664.change.rst | 2 + pkg_resources/__init__.py | 37 +++++++++- pkg_resources/tests/test_pkg_resources.py | 84 +++++++++++++++++++++++ 3 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 changelog.d/1664.change.rst diff --git a/changelog.d/1664.change.rst b/changelog.d/1664.change.rst new file mode 100644 index 00000000000..85e40a39db8 --- /dev/null +++ b/changelog.d/1664.change.rst @@ -0,0 +1,2 @@ +Added the path to the ``PKG-INFO`` or ``METADATA`` file in the exception +text when the ``Version:`` header can't be found. diff --git a/pkg_resources/__init__.py b/pkg_resources/__init__.py index e8921f95bbb..97e08d6827e 100644 --- a/pkg_resources/__init__.py +++ b/pkg_resources/__init__.py @@ -1403,8 +1403,15 @@ def get_resource_string(self, manager, resource_name): def has_resource(self, resource_name): return self._has(self._fn(self.module_path, resource_name)) + def _get_metadata_path(self, name): + return self._fn(self.egg_info, name) + def has_metadata(self, name): - return self.egg_info and self._has(self._fn(self.egg_info, name)) + if not self.egg_info: + return self.egg_info + + path = self._get_metadata_path(name) + return self._has(path) def get_metadata(self, name): if not self.egg_info: @@ -1868,6 +1875,9 @@ class FileMetadata(EmptyProvider): def __init__(self, path): self.path = path + def _get_metadata_path(self, name): + return self.path + def has_metadata(self, name): return name == 'PKG-INFO' and os.path.isfile(self.path) @@ -2663,8 +2673,12 @@ def version(self): except AttributeError: version = self._get_version() if version is None: - tmpl = "Missing 'Version:' header and/or %s file" - raise ValueError(tmpl % self.PKG_INFO, self) + path = self._get_metadata_path_for_display(self.PKG_INFO) + msg = ( + "Missing 'Version:' header and/or {} file at path: {}" + ).format(self.PKG_INFO, path) + raise ValueError(msg, self) + return version @property @@ -2722,6 +2736,23 @@ def requires(self, extras=()): ) return deps + def _get_metadata_path_for_display(self, name): + """ + Return the path to the given metadata file, if available. + """ + try: + # We need to access _get_metadata_path() on the provider object + # directly rather than through this class's __getattr__() + # since _get_metadata_path() is marked private. + path = self._provider._get_metadata_path(name) + + # Handle exceptions e.g. in case the distribution's metadata + # provider doesn't support _get_metadata_path(). + except Exception: + return '[could not detect]' + + return path + def _get_metadata(self, name): if self.has_metadata(name): for line in self.get_metadata_lines(name): diff --git a/pkg_resources/tests/test_pkg_resources.py b/pkg_resources/tests/test_pkg_resources.py index 2c2c9c7f970..fb77c685870 100644 --- a/pkg_resources/tests/test_pkg_resources.py +++ b/pkg_resources/tests/test_pkg_resources.py @@ -17,6 +17,7 @@ except ImportError: import mock +from pkg_resources import DistInfoDistribution, Distribution, EggInfoDistribution from pkg_resources.extern.six.moves import map from pkg_resources.extern.six import text_type, string_types @@ -190,6 +191,89 @@ def test_setuptools_not_imported(self): subprocess.check_call(cmd) +# TODO: remove this in favor of Path.touch() when Python 2 is dropped. +def touch_file(path): + """ + Create an empty file. + """ + with open(path, 'w'): + pass + + +def make_distribution_no_version(tmpdir, basename): + """ + Create a distribution directory with no file containing the version. + """ + # Convert the LocalPath object to a string before joining. + dist_dir = os.path.join(str(tmpdir), basename) + os.mkdir(dist_dir) + # Make the directory non-empty so distributions_from_metadata() + # will detect it and yield it. + touch_file(os.path.join(dist_dir, 'temp.txt')) + + dists = list(pkg_resources.distributions_from_metadata(dist_dir)) + assert len(dists) == 1 + dist, = dists + + return dist, dist_dir + + +@pytest.mark.parametrize( + 'suffix, expected_filename, expected_dist_type', + [ + ('egg-info', 'PKG-INFO', EggInfoDistribution), + ('dist-info', 'METADATA', DistInfoDistribution), + ], +) +def test_distribution_version_missing(tmpdir, suffix, expected_filename, + expected_dist_type): + """ + Test Distribution.version when the "Version" header is missing. + """ + basename = 'foo.{}'.format(suffix) + dist, dist_dir = make_distribution_no_version(tmpdir, basename) + + expected_text = ( + "Missing 'Version:' header and/or {} file at path: " + ).format(expected_filename) + metadata_path = os.path.join(dist_dir, expected_filename) + + # Now check the exception raised when the "version" attribute is accessed. + with pytest.raises(ValueError) as excinfo: + dist.version + + err = str(excinfo) + # Include a string expression after the assert so the full strings + # will be visible for inspection on failure. + assert expected_text in err, str((expected_text, err)) + + # Also check the args passed to the ValueError. + msg, dist = excinfo.value.args + assert expected_text in msg + # Check that the message portion contains the path. + assert metadata_path in msg, str((metadata_path, msg)) + assert type(dist) == expected_dist_type + + +def test_distribution_version_missing_undetected_path(): + """ + Test Distribution.version when the "Version" header is missing and + the path can't be detected. + """ + # Create a Distribution object with no metadata argument, which results + # in an empty metadata provider. + dist = Distribution('/foo') + with pytest.raises(ValueError) as excinfo: + dist.version + + msg, dist = excinfo.value.args + expected = ( + "Missing 'Version:' header and/or PKG-INFO file at path: " + '[could not detect]' + ) + assert msg == expected + + class TestDeepVersionLookupDistutils: @pytest.fixture def env(self, tmpdir):