diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index 0d910dbc494..f561a0a1ccb 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -13,9 +13,7 @@ import logging import os -import re -from pip._vendor.packaging.markers import Marker from pip._vendor.packaging.requirements import InvalidRequirement, Requirement from pip._vendor.packaging.specifiers import Specifier from pip._vendor.pkg_resources import RequirementParseError, parse_requirements @@ -24,6 +22,11 @@ from pip._internal.models.index import PyPI, TestPyPI from pip._internal.models.link import Link from pip._internal.pyproject import make_pyproject_path +from pip._internal.req.parsing import ( + RequirementParsingError, + _strip_extras, + parse_requirement_text, +) from pip._internal.req.req_install import InstallRequirement from pip._internal.utils.misc import ( ARCHIVE_EXTENSIONS, @@ -33,7 +36,7 @@ ) from pip._internal.utils.typing import MYPY_CHECK_RUNNING from pip._internal.utils.urls import url_to_path -from pip._internal.vcs import is_url, vcs +from pip._internal.vcs import vcs from pip._internal.wheel import Wheel if MYPY_CHECK_RUNNING: @@ -61,19 +64,6 @@ def is_archive_file(name): return False -def _strip_extras(path): - # type: (str) -> Tuple[str, Optional[str]] - m = re.match(r'^(.+)(\[[^\]]+\])$', path) - extras = None - if m: - path_no_extras = m.group(1) - extras = m.group(2) - else: - path_no_extras = path - - return path_no_extras, extras - - def parse_editable(editable_req): # type: (str) -> Tuple[Optional[str], str, Optional[Set[str]]] """Parses an editable requirement into: @@ -156,23 +146,19 @@ def deduce_helpful_msg(req): :params req: Requirements file path """ msg = "" - if os.path.exists(req): - msg = " It does exist." - # Try to parse and check if it is a requirements file. - try: - with open(req, 'r') as fp: - # parse first line only - next(parse_requirements(fp.read())) - msg += " The argument you provided " + \ - "(%s) appears to be a" % (req) + \ - " requirements file. If that is the" + \ - " case, use the '-r' flag to install" + \ - " the packages specified within it." - except RequirementParseError: - logger.debug("Cannot parse '%s' as requirements \ - file" % (req), exc_info=True) - else: - msg += " File '%s' does not exist." % (req) + # Try to parse and check if it is a requirements file. + try: + with open(req, 'r') as fp: + # parse first line only + next(parse_requirements(fp.read())) + msg += " The argument you provided " + \ + "(%s) appears to be a" % (req) + \ + " requirements file. If that is the" + \ + " case, use the '-r' flag to install" + \ + " the packages specified within it." + except RequirementParseError: + logger.debug("Cannot parse '%s' as requirements \ + file" % (req), exc_info=True) return msg @@ -232,105 +218,66 @@ def install_req_from_line( :param line_source: An optional string describing where the line is from, for logging purposes in case of an error. """ - if is_url(name): - marker_sep = '; ' - else: - marker_sep = ';' - if marker_sep in name: - name, markers_as_string = name.split(marker_sep, 1) - markers_as_string = markers_as_string.strip() - if not markers_as_string: - markers = None + def with_source(text): + if not line_source: + return text + return '{} (from {})'.format(text, line_source) + + try: + req = parse_requirement_text(name) + except RequirementParsingError as e: + if e.type_tried not in ['path', 'url'] and ( + '=' in name and not any(op in name for op in operators) + ): + add_msg = "= is not a valid operator. Did you mean == ?" else: - markers = Marker(markers_as_string) - else: - markers = None - name = name.strip() - req_as_string = None - path = os.path.normpath(os.path.abspath(name)) - link = None - extras_as_string = None - - if is_url(name): - link = Link(name) - else: - p, extras_as_string = _strip_extras(path) - looks_like_dir = os.path.isdir(p) and ( - os.path.sep in name or - (os.path.altsep is not None and os.path.altsep in name) or - name.startswith('.') - ) - if looks_like_dir: + add_msg = "(tried parsing as {})".format(e.type_tried) + + msg = with_source('Invalid requirement: {!r}'.format(name)) + msg += '\nHint: {}'.format(add_msg) + raise InstallationError(msg) + + link = req.link + + def get_install_error(text): + return InstallationError(with_source(text)) + + if link and link.scheme == 'file': + p = link.path + if not os.path.exists(p): + raise get_install_error( + "Requirement '{}' looks like a path, but the " + 'file/directory does not exist'.format(name) + ) + + if os.path.isdir(p): if not is_installable_dir(p): - raise InstallationError( + raise get_install_error( "Directory %r is not installable. Neither 'setup.py' " - "nor 'pyproject.toml' found." % name + "nor 'pyproject.toml' found.".format(name) ) - link = Link(path_to_url(p)) - elif is_archive_file(p): - if not os.path.isfile(p): - logger.warning( - 'Requirement %r looks like a filename, but the ' - 'file does not exist', - name - ) - link = Link(path_to_url(p)) - - # it's a local file, dir, or url - if link: - # Handle relative file URLs - if link.scheme == 'file' and re.search(r'\.\./', link.url): - link = Link( - path_to_url(os.path.normpath(os.path.abspath(link.path)))) - # wheel file - if link.is_wheel: - wheel = Wheel(link.filename) # can raise InvalidWheelFilename - req_as_string = "%s==%s" % (wheel.name, wheel.version) - else: - # set the req to the egg fragment. when it's not there, this - # will become an 'unnamed' requirement - req_as_string = link.egg_fragment - - # a requirement specifier - else: - req_as_string = name + elif not is_archive_file(p) and not link.is_wheel: + raise get_install_error( + "Invalid requirement: {!r}, files must be wheels or " + 'archives'.format(name) + deduce_helpful_msg(p) + ) - if extras_as_string: - extras = Requirement("placeholder" + extras_as_string.lower()).extras - else: - extras = () - if req_as_string is not None: + # wheel file + if link and link.is_wheel: + wheel = Wheel(link.filename) # can raise InvalidWheelFilename + wheel_req = "%s==%s" % (wheel.name, wheel.version) try: - req = Requirement(req_as_string) + req.requirement = Requirement(wheel_req) except InvalidRequirement: - if os.path.sep in req_as_string: - add_msg = "It looks like a path." - add_msg += deduce_helpful_msg(req_as_string) - elif ('=' in req_as_string and - not any(op in req_as_string for op in operators)): - add_msg = "= is not a valid operator. Did you mean == ?" - else: - add_msg = '' - if line_source is None: - source = '' - else: - source = ' (from {})'.format(line_source) - msg = ( - 'Invalid requirement: {!r}{}'.format(req_as_string, source) - ) - if add_msg: - msg += '\nHint: {}'.format(add_msg) - raise InstallationError(msg) - else: - req = None + pass return InstallRequirement( - req, comes_from, link=link, markers=markers, + req.requirement, comes_from, link=link, markers=req.markers, use_pep517=use_pep517, isolated=isolated, options=options if options else {}, wheel_cache=wheel_cache, constraint=constraint, - extras=extras, + extras=req.extras, ) diff --git a/src/pip/_internal/req/parsing.py b/src/pip/_internal/req/parsing.py new file mode 100644 index 00000000000..822e554d8d7 --- /dev/null +++ b/src/pip/_internal/req/parsing.py @@ -0,0 +1,198 @@ +import os.path +import re +from contextlib import contextmanager + +from pip._vendor.packaging.markers import Marker +from pip._vendor.packaging.requirements import Requirement + +from pip._internal.models.link import Link +from pip._internal.utils.misc import path_to_url +from pip._internal.utils.typing import MYPY_CHECK_RUNNING + +if MYPY_CHECK_RUNNING: + from typing import Optional, Set, Tuple + + +__all__ = [ + 'RequirementInfo', + 'RequirementParsingError', + 'parse_requirement_text', +] + + +PATH_MARKER_SEP = ';' +URL_MARKER_SEP = '; ' + + +def convert_extras(extras): + # type: (Optional[str]) -> Set[str] + if extras: + return Requirement("placeholder" + extras.lower()).extras + else: + return set() + + +def _strip_extras(path): + # type: (str) -> Tuple[str, Optional[str]] + m = re.match(r'^(.+)(\[[^\]]+\])$', path) + extras = None + if m: + path_no_extras = m.group(1) + extras = m.group(2) + else: + path_no_extras = path + + return path_no_extras, extras + + +def strip_and_convert_extras(text): + # type: (str) -> Tuple[str, Set[str]] + result, extras = _strip_extras(text) + return result, convert_extras(extras) + + +class RequirementParsingError(Exception): + def __init__(self, type_tried, cause): + # type: (str, Exception) -> None + self.type_tried = type_tried + self.cause = cause + + +class RequirementInfo(object): + def __init__( + self, + requirement, # type: Optional[Requirement] + link, # type: Optional[Link] + markers, # type: Optional[Marker] + extras, # type: Set[str] + ): + self.requirement = requirement + self.link = link + self.markers = markers + self.extras = extras + + @property + def is_unnamed(self): + return self.requirement is None + + @property + def is_name_based(self): + return self.link is None + + def __repr__(self): + return ''.format( + self.requirement, self.link, self.markers, self.extras, + ) + + +def requirement_info_from_requirement(text): + # type: (str) -> RequirementInfo + req = Requirement(text) + return RequirementInfo( + req, + Link(req.url) if req.url else None, + req.marker, + req.extras, + ) + + +def requirement_info_from_url(url): + # type: (str) -> RequirementInfo + try: + url, marker_text = url.split(URL_MARKER_SEP) + except ValueError: + marker = None + else: + marker = Marker(marker_text) + + # Prevent stripping extras out of URL fragment + if '#egg=' not in url: + url, extras = strip_and_convert_extras(url) + else: + extras = set() + + link = Link(url) + + egg_fragment = link.egg_fragment + + req = None # type: Optional[Requirement] + if egg_fragment: + req = Requirement(egg_fragment) + # We prefer fragment extras if present. + if req.extras: + extras = req.extras + + return RequirementInfo(req, link, marker, extras) + + +def requirement_info_from_path(path): + # type: (str) -> RequirementInfo + try: + path, markers = path.split(PATH_MARKER_SEP) + except ValueError: + markers = '' + else: + markers = URL_MARKER_SEP + markers + + path, extras = _strip_extras(path) + if extras is None: + extras = '' + + url = path_to_url(path) + + return requirement_info_from_url( + '{}{}{}'.format(url, extras, markers), + ) + + +def looks_like_direct_reference(text): + try: + assert text.index('@') < text.index('://') + except (AssertionError, ValueError): + return False + else: + return True + + +def looks_like_url(text): + # type: (str) -> bool + return '://' in text + + +def looks_like_path(text): + # type: (str) -> bool + return ( + os.path.sep in text or + os.path.altsep is not None and os.path.altsep in text or + text.startswith('.') + ) + + +@contextmanager +def try_parse_as(message): + try: + yield + except Exception as e: + raise RequirementParsingError(message, e) + + +def parse_requirement_text(text): + # type: (str) -> RequirementInfo + # Only search before any ';', since marker strings can + # contain most kinds of text. + search_text = text.split(';', 1)[0] + + if looks_like_direct_reference(search_text): + with try_parse_as('direct reference'): + return requirement_info_from_requirement(text) + + if looks_like_url(search_text): + with try_parse_as('url'): + return requirement_info_from_url(text) + + if looks_like_path(search_text): + with try_parse_as('path'): + return requirement_info_from_path(text) + + with try_parse_as('name-based reference'): + return requirement_info_from_requirement(text) diff --git a/tests/functional/test_freeze.py b/tests/functional/test_freeze.py index 77f83796abc..a2d4d60aead 100644 --- a/tests/functional/test_freeze.py +++ b/tests/functional/test_freeze.py @@ -473,13 +473,15 @@ def test_freeze_bazaar_clone(script, tmpdir): def test_freeze_with_requirement_option_file_url_egg_not_installed( - script, deprecated_python): + script, data, deprecated_python): """ Test "freeze -r requirements.txt" with a local file URL whose egg name is not installed. """ - url = path_to_url('my-package.tar.gz') + '#egg=Does.Not-Exist' + url = path_to_url( + str(data.packages.joinpath('test_tar.tgz')) + ) + '#egg=Does.Not-Exist' requirements_path = script.scratch_path.joinpath('requirements.txt') requirements_path.write_text(url + '\n') diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 7a0e4a9cbee..a3c4305dc5d 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -406,9 +406,7 @@ def test_basic_install_relative_directory(script, data): # Compute relative install path to FSPkg from scratch path. full_rel_path = data.packages.joinpath('FSPkg') - script.scratch_path - full_rel_url = ( - 'file:' + full_rel_path.replace(os.path.sep, '/') + '#egg=FSPkg' - ) + full_rel_url = full_rel_path.replace(os.path.sep, '/') embedded_rel_path = script.scratch_path.joinpath(full_rel_path) # For each relative path, install as either editable or not using either diff --git a/tests/functional/test_install_reqs.py b/tests/functional/test_install_reqs.py index b906c37b642..d401fb4cc8b 100644 --- a/tests/functional/test_install_reqs.py +++ b/tests/functional/test_install_reqs.py @@ -57,9 +57,7 @@ def test_schema_check_in_requirements_file(script): def test_relative_requirements_file(script, data): """ - Test installing from a requirements file with a relative path. For path - URLs, use an egg= definition. - + Test installing from a requirements file with a relative path. """ egg_info_file = ( script.site_packages / 'FSPkg-0.1.dev0-py%s.egg-info' % pyversion @@ -71,7 +69,7 @@ def test_relative_requirements_file(script, data): # Compute relative install path to FSPkg from scratch path. full_rel_path = data.packages.joinpath('FSPkg') - script.scratch_path - full_rel_url = 'file:' + full_rel_path + '#egg=FSPkg' + full_rel_url = full_rel_path embedded_rel_path = script.scratch_path.joinpath(full_rel_path) # For each relative path, install as either editable or not using either diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index f048c548169..49b82cbdbde 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -381,14 +381,18 @@ def test_repr(self): '' ) - def test_invalid_wheel_requirement_raises(self): + def test_invalid_wheel_requirement_raises(self, data): + invalid_wheel_path = data.packages.joinpath('invalid.whl') with pytest.raises(InvalidWheelFilename): - install_req_from_line('invalid.whl') + install_req_from_line(invalid_wheel_path) - def test_wheel_requirement_sets_req_attribute(self): - req = install_req_from_line('simple-0.1-py2.py3-none-any.whl') + def test_wheel_requirement_sets_req_attribute(self, data): + wheel_path = data.packages.joinpath( + 'simplewheel-1.0-py2.py3-none-any.whl' + ) + req = install_req_from_line(wheel_path) assert isinstance(req.req, Requirement) - assert str(req.req) == 'simple==0.1' + assert str(req.req) == 'simplewheel==1.0' def test_url_preserved_line_req(self): """Confirm the url is preserved in a non-editable requirement""" @@ -530,8 +534,8 @@ def test_unexisting_path(self): install_req_from_line( os.path.join('this', 'path', 'does', 'not', 'exist')) err_msg = e.value.args[0] - assert "Invalid requirement" in err_msg - assert "It looks like a path." in err_msg + assert "looks like a path" in err_msg + assert "does not exist" in err_msg def test_single_equal_sign(self): with pytest.raises(InstallationError) as e: @@ -545,7 +549,11 @@ def test_unidentifiable_name(self): with pytest.raises(InstallationError) as e: install_req_from_line(test_name) err_msg = e.value.args[0] - assert "Invalid requirement: '{}'".format(test_name) == err_msg + expected = ( + "Invalid requirement: '{}'\n" + 'Hint: (tried parsing as name-based reference)' + ).format(test_name) + assert expected == err_msg def test_requirement_file(self): req_file_path = os.path.join(self.tempdir, 'test.txt') @@ -555,7 +563,6 @@ def test_requirement_file(self): install_req_from_line(req_file_path) err_msg = e.value.args[0] assert "Invalid requirement" in err_msg - assert "It looks like a path. It does exist." in err_msg assert "appears to be a requirements file." in err_msg assert "If that is the case, use the '-r' flag to install" in err_msg diff --git a/tests/unit/test_req_parsing.py b/tests/unit/test_req_parsing.py new file mode 100644 index 00000000000..87a112d0798 --- /dev/null +++ b/tests/unit/test_req_parsing.py @@ -0,0 +1,154 @@ +import pytest +from pip._vendor.packaging.markers import Marker +from pip._vendor.packaging.requirements import Requirement + +from pip._internal.models.link import Link +from pip._internal.req.parsing import ( + RequirementInfo, + RequirementParsingError, + convert_extras, + looks_like_direct_reference, + looks_like_path, + looks_like_url, + parse_requirement_text, +) +from pip._internal.utils.misc import path_to_url +from pip._internal.utils.typing import MYPY_CHECK_RUNNING + +if MYPY_CHECK_RUNNING: + from typing import Optional, Set, Union + + +@pytest.mark.parametrize('text,expected', [ + ('[extra]', {'extra'}), + ('[extra1,extra2]', {'extra1', 'extra2'}), + ('', set()), + (None, set()), +]) +def test_convert_extras(text, expected): + assert expected == convert_extras(text) + + +@pytest.mark.parametrize('text,expected', [ + ('example @ file://.', True), + ('example@ https://.', True), + ('https://.', False), + ('https://git.example.com/repo@commit', False), + ('https://user:pass@git.example.com/repo', False), +]) +def test_looks_like_direct_reference(text, expected): + assert expected == looks_like_direct_reference(text) + + +@pytest.mark.parametrize('text,expected', [ + ('/example/hello', True), + ('hello', False), + ('/example[extra]', True), + ('.', True), + ('.[extra]', True), +]) +def test_looks_like_path(text, expected): + assert expected == looks_like_path(text) + + +@pytest.mark.skipif('sys.platform != "win32"') +@pytest.mark.parametrize('text,expected', [ + ('C:\\Example', True), + ('.\\', True), +]) +def test_looks_like_path_windows(text, expected): + assert expected == looks_like_path(expected) + + +@pytest.mark.parametrize('text,expected', [ + ('http://example.com/', True), + ('./example', False), +]) +def test_looks_like_url(text, expected): + assert expected == looks_like_url(text) + + +def _assert_requirement(expected, test): + # type: (Optional[Requirement], Optional[Requirement]) -> None + if expected is None or test is None: + assert expected == test + return + + assert expected.name == test.name + assert expected.specifier == test.specifier + assert expected.url == test.url + assert expected.extras == test.extras + assert expected.marker == test.marker + + +def _assert_requirement_info( + expected, test, +): + # type: (RequirementInfo, RequirementInfo) -> None + _assert_requirement(expected.requirement, test.requirement) + assert (expected.link is None) == (test.link is None) + if expected.link is not None: + assert expected.link.url == test.link.url + assert str(expected.markers) == str(test.markers) + assert expected.extras == test.extras + + +INPUT = object() + + +def req_info( + req=None, # type: Optional[Union[str, object]] + link=None, # type: Optional[Union[str, object]] + markers=None, # type: Optional[str] + extras=None, # type: Optional[Set[str]] +): + def get_req_info(text): + _req = req + if _req is INPUT: + _req = text + _link = link + if _link is INPUT: + _link = text + + return RequirementInfo( + Requirement(_req) if _req else None, + Link(_link) if _link else None, + Marker(markers) if markers else None, + set() if extras is None else extras, + ) + + return get_req_info + + +@pytest.mark.parametrize('text,make_expected', [ + ('.', req_info(link=path_to_url('.'))), + ('.[extra1]', req_info(link=path_to_url('.'), extras={'extra1'})), + ('path/to/project', req_info(link=path_to_url('path/to/project'))), + + ('pkg[extra1]', req_info(req=INPUT, extras={'extra1'})), + + ('https://github.com/a/b/c/asdf-1.5.2-cp27-none-xyz.whl' + '; sys_platform == "xyz"', + req_info( + link='https://github.com/a/b/c/asdf-1.5.2-cp27-none-xyz.whl', + markers='sys_platform == "xyz"', + )), + ('git+http://git.example.com/pkgrepo#egg=pkg', + req_info(req='pkg', link=INPUT)), + ('git+http://git.example.com/pkgrepo#egg=pkg[extra1]', + req_info(req='pkg[extra1]', link=INPUT, extras={'extra1'})), +]) +def test_parse_requirement_text(text, make_expected): + _assert_requirement_info( + parse_requirement_text(text), make_expected(text), + ) + + +@pytest.mark.parametrize('text,expected_message', [ + ('file:.', 'name-based'), + ('@ http://example', 'direct reference'), +]) +def test_parse_requirement_text_fail(text, expected_message): + with pytest.raises(RequirementParsingError) as e: + parse_requirement_text(text) + assert expected_message in e.value.type_tried