diff --git a/news/5082.feature b/news/5082.feature new file mode 100644 index 000000000..17c678764 --- /dev/null +++ b/news/5082.feature @@ -0,0 +1,2 @@ +Improve the error message when ``METADATA`` or ``PKG-INFO`` is None when +accessing metadata. diff --git a/src/pip/_internal/exceptions.py b/src/pip/_internal/exceptions.py index 7b291a1e4..096adcd6c 100644 --- a/src/pip/_internal/exceptions.py +++ b/src/pip/_internal/exceptions.py @@ -9,6 +9,7 @@ from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: from typing import Optional + from pip._vendor.pkg_resources import Distribution from pip._internal.req.req_install import InstallRequirement @@ -28,6 +29,36 @@ class UninstallationError(PipError): """General exception during uninstallation""" +class NoneMetadataError(PipError): + """ + Raised when accessing "METADATA" or "PKG-INFO" metadata for a + pip._vendor.pkg_resources.Distribution object and + `dist.has_metadata('METADATA')` returns True but + `dist.get_metadata('METADATA')` returns None (and similarly for + "PKG-INFO"). + """ + + def __init__(self, dist, metadata_name): + # type: (Distribution, str) -> None + """ + :param dist: A Distribution object. + :param metadata_name: The name of the metadata being accessed + (can be "METADATA" or "PKG-INFO"). + """ + self.dist = dist + self.metadata_name = metadata_name + + def __str__(self): + # type: () -> str + # Use `dist` in the error message because its stringification + # includes more information, like the version and location. + return ( + 'None {} metadata found for distribution: {}'.format( + self.metadata_name, self.dist, + ) + ) + + class DistributionNotFound(InstallationError): """Raised when a distribution cannot be found to satisfy a requirement""" diff --git a/src/pip/_internal/utils/packaging.py b/src/pip/_internal/utils/packaging.py index ac27e5600..2d80920df 100644 --- a/src/pip/_internal/utils/packaging.py +++ b/src/pip/_internal/utils/packaging.py @@ -6,6 +6,7 @@ from email.parser import FeedParser from pip._vendor import pkg_resources from pip._vendor.packaging import specifiers, version +from pip._internal.exceptions import NoneMetadataError from pip._internal.utils.misc import display_path from pip._internal.utils.typing import MYPY_CHECK_RUNNING @@ -43,16 +44,27 @@ def check_requires_python(requires_python, version_info): def get_metadata(dist): # type: (Distribution) -> Message + """ + :raises NoneMetadataError: if the distribution reports `has_metadata()` + True but `get_metadata()` returns None. + """ + metadata_name = 'METADATA' if (isinstance(dist, pkg_resources.DistInfoDistribution) and - dist.has_metadata('METADATA')): - metadata = dist.get_metadata('METADATA') + dist.has_metadata(metadata_name)): + metadata = dist.get_metadata(metadata_name) elif dist.has_metadata('PKG-INFO'): - metadata = dist.get_metadata('PKG-INFO') + metadata_name = 'PKG-INFO' + metadata = dist.get_metadata(metadata_name) else: logger.warning("No metadata found in %s", display_path(dist.location)) metadata = '' + if metadata is None: + raise NoneMetadataError(dist, metadata_name) + feed_parser = FeedParser() + # The following line errors out if with a "NoneType" TypeError if + # passed metadata=None. feed_parser.feed(metadata) return feed_parser.close() diff --git a/tests/unit/test_legacy_resolve.py b/tests/unit/test_legacy_resolve.py index e384e2349..4d7f27b20 100644 --- a/tests/unit/test_legacy_resolve.py +++ b/tests/unit/test_legacy_resolve.py @@ -1,33 +1,65 @@ import logging import pytest -from mock import patch +from pip._vendor import pkg_resources -from pip._internal.exceptions import UnsupportedPythonVersion +from pip._internal.exceptions import ( + NoneMetadataError, UnsupportedPythonVersion, +) from pip._internal.legacy_resolve import _check_dist_requires_python +from pip._internal.utils.packaging import get_requires_python -class FakeDist(object): +# We need to inherit from DistInfoDistribution for the `isinstance()` +# check inside `packaging.get_metadata()` to work. +class FakeDist(pkg_resources.DistInfoDistribution): - def __init__(self, project_name): - self.project_name = project_name + def __init__(self, metadata, metadata_name=None): + """ + :param metadata: The value that dist.get_metadata() should return + for the `metadata_name` metadata. + :param metadata_name: The name of the metadata to store + (can be "METADATA" or "PKG-INFO"). Defaults to "METADATA". + """ + if metadata_name is None: + metadata_name = 'METADATA' + + self.project_name = 'my-project' + self.metadata_name = metadata_name + self.metadata = metadata + + def __str__(self): + return ''.format(self.project_name) + + def has_metadata(self, name): + return (name == self.metadata_name) + + def get_metadata(self, name): + assert name == self.metadata_name + return self.metadata -@pytest.fixture -def dist(): - return FakeDist('my-project') +def make_fake_dist(requires_python=None, metadata_name=None): + metadata = 'Name: test\n' + if requires_python is not None: + metadata += 'Requires-Python:{}'.format(requires_python) + + return FakeDist(metadata, metadata_name=metadata_name) -@patch('pip._internal.legacy_resolve.get_requires_python') class TestCheckDistRequiresPython(object): """ Test _check_dist_requires_python(). """ - def test_compatible(self, mock_get_requires, caplog, dist): + def test_compatible(self, caplog): + """ + Test a Python version compatible with the dist's Requires-Python. + """ caplog.set_level(logging.DEBUG) - mock_get_requires.return_value = '== 3.6.5' + dist = make_fake_dist('== 3.6.5') + _check_dist_requires_python( dist, version_info=(3, 6, 5), @@ -35,9 +67,66 @@ class TestCheckDistRequiresPython(object): ) assert not len(caplog.records) - def test_invalid_specifier(self, mock_get_requires, caplog, dist): + def test_incompatible(self): + """ + Test a Python version incompatible with the dist's Requires-Python. + """ + dist = make_fake_dist('== 3.6.4') + with pytest.raises(UnsupportedPythonVersion) as exc: + _check_dist_requires_python( + dist, + version_info=(3, 6, 5), + ignore_requires_python=False, + ) + assert str(exc.value) == ( + "Package 'my-project' requires a different Python: " + "3.6.5 not in '== 3.6.4'" + ) + + def test_incompatible_with_ignore_requires(self, caplog): + """ + Test a Python version incompatible with the dist's Requires-Python + while passing ignore_requires_python=True. + """ caplog.set_level(logging.DEBUG) - mock_get_requires.return_value = 'invalid' + dist = make_fake_dist('== 3.6.4') + _check_dist_requires_python( + dist, + version_info=(3, 6, 5), + ignore_requires_python=True, + ) + assert len(caplog.records) == 1 + record = caplog.records[0] + assert record.levelname == 'DEBUG' + assert record.message == ( + "Ignoring failed Requires-Python check for package 'my-project': " + "3.6.5 not in '== 3.6.4'" + ) + + def test_none_requires_python(self, caplog): + """ + Test a dist with Requires-Python None. + """ + caplog.set_level(logging.DEBUG) + dist = make_fake_dist() + # Make sure our test setup is correct. + assert get_requires_python(dist) is None + assert len(caplog.records) == 0 + + # Then there is no exception and no log message. + _check_dist_requires_python( + dist, + version_info=(3, 6, 5), + ignore_requires_python=False, + ) + assert len(caplog.records) == 0 + + def test_invalid_requires_python(self, caplog): + """ + Test a dist with an invalid Requires-Python. + """ + caplog.set_level(logging.DEBUG) + dist = make_fake_dist('invalid') _check_dist_requires_python( dist, version_info=(3, 6, 5), @@ -51,33 +140,29 @@ class TestCheckDistRequiresPython(object): "Invalid specifier: 'invalid'" ) - def test_incompatible(self, mock_get_requires, dist): - mock_get_requires.return_value = '== 3.6.4' - with pytest.raises(UnsupportedPythonVersion) as exc: + @pytest.mark.parametrize('metadata_name', [ + 'METADATA', + 'PKG-INFO', + ]) + def test_empty_metadata_error(self, caplog, metadata_name): + """ + Test dist.has_metadata() returning True and dist.get_metadata() + returning None. + """ + dist = make_fake_dist(metadata_name=metadata_name) + dist.metadata = None + + # Make sure our test setup is correct. + assert dist.has_metadata(metadata_name) + assert dist.get_metadata(metadata_name) is None + + with pytest.raises(NoneMetadataError) as exc: _check_dist_requires_python( dist, version_info=(3, 6, 5), ignore_requires_python=False, ) assert str(exc.value) == ( - "Package 'my-project' requires a different Python: " - "3.6.5 not in '== 3.6.4'" - ) - - def test_incompatible_with_ignore_requires( - self, mock_get_requires, caplog, dist, - ): - caplog.set_level(logging.DEBUG) - mock_get_requires.return_value = '== 3.6.4' - _check_dist_requires_python( - dist, - version_info=(3, 6, 5), - ignore_requires_python=True, - ) - assert len(caplog.records) == 1 - record = caplog.records[0] - assert record.levelname == 'DEBUG' - assert record.message == ( - "Ignoring failed Requires-Python check for package 'my-project': " - "3.6.5 not in '== 3.6.4'" + "None {} metadata found for distribution: " + "".format(metadata_name) )