Merge pull request #1706 from cjerdonek/issue-1664-include-metadata-location

Include path in error message when version is missing (Fixes #1664)

changelog.d/1664.change.rst

+Added the path to the ``PKG-INFO`` or ``METADATA`` file in the exception
+text when the ``Version:`` header can't be found.

pkg_resources/__init__.py

@@ -1403,8 +1403,15 @@ class NullProvider:
     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)
 
@@ -2661,10 +2671,14 @@ class Distribution:
         try:
             return self._version
         except AttributeError:
-            version = _version_from_file(self._get_metadata(self.PKG_INFO))
+            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,11 +2736,34 @@ class Distribution:
                 )
         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):
                 yield line
 
+    def _get_version(self):
+        lines = self._get_metadata(self.PKG_INFO)
+        version = _version_from_file(lines)
+
+        return version
+
     def activate(self, path=None, replace=False):
         """Ensure distribution is importable on `path` (default=sys.path)"""
         if path is None:
@@ -2945,7 +2982,7 @@ class EggInfoDistribution(Distribution):
         take an extra step and try to get the version number from
         the metadata file itself instead of the filename.
         """
-        md_version = _version_from_file(self._get_metadata(self.PKG_INFO))
+        md_version = self._get_version()
         if md_version:
             self._version = md_version
         return self

pkg_resources/tests/test_pkg_resources.py

@@ -17,6 +17,7 @@ try:
 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 @@ class TestResourceManager:
         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):