From f82ea77217460ab485676fc743119ad4adcb4037 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Thu, 23 May 2019 22:59:30 -0700 Subject: [PATCH] Simplify the _check_dist_requires_python() call site. --- src/pip/_internal/index.py | 4 +- src/pip/_internal/resolve.py | 81 +++++++++++++-------- src/pip/_internal/utils/packaging.py | 17 +++++ tests/functional/test_install.py | 15 ++-- tests/unit/test_resolve.py | 101 ++++++++++++++++++++------- 5 files changed, 157 insertions(+), 61 deletions(-) diff --git a/src/pip/_internal/index.py b/src/pip/_internal/index.py index fa8227802..62845b657 100644 --- a/src/pip/_internal/index.py +++ b/src/pip/_internal/index.py @@ -272,7 +272,7 @@ def _check_link_requires_python( value if the given Python version isn't compatible. """ try: - support_this_python = check_requires_python( + is_compatible = check_requires_python( link.requires_python, version_info=version_info, ) except specifiers.InvalidSpecifier: @@ -281,7 +281,7 @@ def _check_link_requires_python( link.requires_python, link, ) else: - if not support_this_python: + if not is_compatible: version = '.'.join(map(str, version_info)) if not ignore_requires_python: logger.debug( diff --git a/src/pip/_internal/resolve.py b/src/pip/_internal/resolve.py index e0a0977e6..a6714cb1f 100644 --- a/src/pip/_internal/resolve.py +++ b/src/pip/_internal/resolve.py @@ -17,7 +17,6 @@ from itertools import chain from pip._vendor.packaging import specifiers -from pip._internal import exceptions from pip._internal.exceptions import ( BestVersionAlreadyInstalled, DistributionNotFound, HashError, HashErrors, UnsupportedPythonVersion, @@ -25,7 +24,9 @@ from pip._internal.exceptions import ( from pip._internal.req.constructors import install_req_from_req_string from pip._internal.utils.logging import indent_log from pip._internal.utils.misc import dist_in_usersite, ensure_dir -from pip._internal.utils.packaging import check_requires_python, get_metadata +from pip._internal.utils.packaging import ( + check_requires_python, get_requires_python, +) from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: @@ -43,34 +44,53 @@ if MYPY_CHECK_RUNNING: logger = logging.getLogger(__name__) -def check_dist_requires_python( - dist, # type: pkg_resources.Distribution - version_info, # type: Optional[Tuple[int, ...]] +def _check_dist_requires_python( + dist, # type: pkg_resources.Distribution + version_info, # type: Tuple[int, ...] + ignore_requires_python=False, # type: bool ): # type: (...) -> None """ - :param version_info: A 3-tuple of ints representing the Python - major-minor-micro version to check (e.g. `sys.version_info[:3]`). + Check whether the given Python version is compatible with a distribution's + "Requires-Python" value. + + :param version_info: The Python version to use to check, as a 3-tuple + of ints (major-minor-micro). + :param ignore_requires_python: Whether to ignore the "Requires-Python" + value if the given Python version isn't compatible. + + :raises UnsupportedPythonVersion: When the given Python version isn't + compatible. """ - pkg_info_dict = get_metadata(dist) - requires_python = pkg_info_dict.get('Requires-Python') + requires_python = get_requires_python(dist) try: - if not check_requires_python( + is_compatible = check_requires_python( requires_python, version_info=version_info, - ): - raise exceptions.UnsupportedPythonVersion( - "%s requires Python '%s' but the running Python is %s" % ( - dist.project_name, - requires_python, - '.'.join(map(str, version_info)),) - ) - except specifiers.InvalidSpecifier as e: + ) + except specifiers.InvalidSpecifier as exc: logger.warning( - "Package %s has an invalid Requires-Python entry %s - %s", - dist.project_name, requires_python, e, + "Package %r has an invalid Requires-Python: %s", + dist.project_name, exc, ) return + if is_compatible: + return + + version = '.'.join(map(str, version_info)) + if ignore_requires_python: + logger.debug( + 'Ignoring failed Requires-Python check for package %r: ' + '%s not in %r', + dist.project_name, version, requires_python, + ) + return + + raise UnsupportedPythonVersion( + 'Package {!r} requires a different Python: {} not in {!r}'.format( + dist.project_name, version, requires_python, + )) + class Resolver(object): """Resolves which packages need to be installed/uninstalled to perform \ @@ -92,12 +112,18 @@ class Resolver(object): force_reinstall, # type: bool isolated, # type: bool upgrade_strategy, # type: str - use_pep517=None # type: Optional[bool] + use_pep517=None, # type: Optional[bool] + py_version_info=None, # type: Optional[Tuple[int, ...]] ): # type: (...) -> None super(Resolver, self).__init__() assert upgrade_strategy in self._allowed_strategies + if py_version_info is None: + py_version_info = sys.version_info[:3] + + self._py_version_info = py_version_info + self.preparer = preparer self.finder = finder self.session = session @@ -329,13 +355,12 @@ class Resolver(object): # Parse and return dependencies dist = abstract_dist.dist() - try: - check_dist_requires_python(dist, version_info=sys.version_info[:3]) - except UnsupportedPythonVersion as err: - if self.ignore_requires_python: - logger.warning(err.args[0]) - else: - raise + # This will raise UnsupportedPythonVersion if the given Python + # version isn't compatible with the distribution's Requires-Python. + _check_dist_requires_python( + dist, version_info=self._py_version_info, + ignore_requires_python=self.ignore_requires_python, + ) more_reqs = [] # type: List[InstallRequirement] diff --git a/src/pip/_internal/utils/packaging.py b/src/pip/_internal/utils/packaging.py index 0c2a37992..ac27e5600 100644 --- a/src/pip/_internal/utils/packaging.py +++ b/src/pip/_internal/utils/packaging.py @@ -57,6 +57,23 @@ def get_metadata(dist): return feed_parser.close() +def get_requires_python(dist): + # type: (pkg_resources.Distribution) -> Optional[str] + """ + Return the "Requires-Python" metadata for a distribution, or None + if not present. + """ + pkg_info_dict = get_metadata(dist) + requires_python = pkg_info_dict.get('Requires-Python') + + if requires_python is not None: + # Convert to a str to satisfy the type checker, since requires_python + # can be a Header object. + requires_python = str(requires_python) + + return requires_python + + def get_installer(dist): # type: (Distribution) -> str if dist.has_metadata('INSTALLER'): diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index e8c5caed0..400e4d9b4 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -1305,6 +1305,12 @@ def test_double_install_fail(script): assert msg in result.stderr +def _get_expected_error_text(): + return ( + "Package 'pkga' requires a different Python: {} not in '<1.0'" + ).format(sys.version.split()[0]) + + def test_install_incompatible_python_requires(script): script.scratch_path.join("pkga").mkdir() pkga_path = script.scratch_path / 'pkga' @@ -1315,8 +1321,7 @@ def test_install_incompatible_python_requires(script): version='0.1') """)) result = script.pip('install', pkga_path, expect_error=True) - assert ("pkga requires Python '<1.0' " - "but the running Python is ") in result.stderr, str(result) + assert _get_expected_error_text() in result.stderr, str(result) def test_install_incompatible_python_requires_editable(script): @@ -1330,8 +1335,7 @@ def test_install_incompatible_python_requires_editable(script): """)) result = script.pip( 'install', '--editable=%s' % pkga_path, expect_error=True) - assert ("pkga requires Python '<1.0' " - "but the running Python is ") in result.stderr, str(result) + assert _get_expected_error_text() in result.stderr, str(result) def test_install_incompatible_python_requires_wheel(script, with_wheel): @@ -1347,8 +1351,7 @@ def test_install_incompatible_python_requires_wheel(script, with_wheel): 'python', 'setup.py', 'bdist_wheel', '--universal', cwd=pkga_path) result = script.pip('install', './pkga/dist/pkga-0.1-py2.py3-none-any.whl', expect_error=True) - assert ("pkga requires Python '<1.0' " - "but the running Python is ") in result.stderr + assert _get_expected_error_text() in result.stderr, str(result) def test_install_compatible_python_requires(script): diff --git a/tests/unit/test_resolve.py b/tests/unit/test_resolve.py index 674725eda..91d39c65e 100644 --- a/tests/unit/test_resolve.py +++ b/tests/unit/test_resolve.py @@ -1,32 +1,83 @@ -import sys +import logging import pytest -from mock import Mock +from mock import patch from pip._internal.exceptions import UnsupportedPythonVersion -from pip._internal.resolve import check_dist_requires_python +from pip._internal.resolve import _check_dist_requires_python -class TestCheckRequiresPython(object): +class FakeDist(object): - @pytest.mark.parametrize( - ("metadata", "should_raise"), - [ - ("Name: test\n", False), - ("Name: test\nRequires-Python:", False), - ("Name: test\nRequires-Python: invalid_spec", False), - ("Name: test\nRequires-Python: <=1", True), - ], - ) - def test_check_requires(self, metadata, should_raise): - fake_dist = Mock( - has_metadata=lambda _: True, - get_metadata=lambda _: metadata) - version_info = sys.version_info[:3] - if should_raise: - with pytest.raises(UnsupportedPythonVersion): - check_dist_requires_python( - fake_dist, version_info=version_info, - ) - else: - check_dist_requires_python(fake_dist, version_info=version_info) + def __init__(self, project_name): + self.project_name = project_name + + +@pytest.fixture +def dist(): + return FakeDist('my-project') + + +@patch('pip._internal.resolve.get_requires_python') +class TestCheckDistRequiresPython(object): + + """ + Test _check_dist_requires_python(). + """ + + def test_compatible(self, mock_get_requires, caplog, dist): + caplog.set_level(logging.DEBUG) + mock_get_requires.return_value = '== 3.6.5' + _check_dist_requires_python( + dist, + version_info=(3, 6, 5), + ignore_requires_python=False, + ) + assert not len(caplog.records) + + def test_invalid_specifier(self, mock_get_requires, caplog, dist): + caplog.set_level(logging.DEBUG) + mock_get_requires.return_value = 'invalid' + _check_dist_requires_python( + dist, + version_info=(3, 6, 5), + ignore_requires_python=False, + ) + assert len(caplog.records) == 1 + record = caplog.records[0] + assert record.levelname == 'WARNING' + assert record.message == ( + "Package 'my-project' has an invalid Requires-Python: " + "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: + _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'" + )