From f2e49b3946423c07a46398f691f721771b80b38a Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 13 Feb 2020 10:18:10 +0000 Subject: [PATCH 1/6] Use a new ParsedRequirement class to communicate between handle_line and parse_requirements --- src/pip/_internal/req/req_file.py | 76 +++++++++++++++++++++++++++---- tests/unit/test_req.py | 5 +- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/src/pip/_internal/req/req_file.py b/src/pip/_internal/req/req_file.py index baf8d03ea2a..0263dc15388 100644 --- a/src/pip/_internal/req/req_file.py +++ b/src/pip/_internal/req/req_file.py @@ -33,7 +33,7 @@ if MYPY_CHECK_RUNNING: from optparse import Values from typing import ( - Any, Callable, Iterator, List, NoReturn, Optional, Text, Tuple, + Any, Callable, Iterator, List, NoReturn, Optional, Text, Tuple, Dict, ) from pip._internal.req import InstallRequirement @@ -84,6 +84,58 @@ SUPPORTED_OPTIONS_REQ_DEST = [str(o().dest) for o in SUPPORTED_OPTIONS_REQ] +class ParsedRequirement(object): + def __init__( + self, + is_editable, # type: bool + comes_from, # type: str + use_pep517, # type: Optional[bool] + isolated, # type: bool + wheel_cache, # type: Optional[WheelCache] + constraint, # type: bool + args=None, # type: Optional[str] + editables=None, # type: Optional[str] + options=None, # type: Optional[Dict[str, Any]] + line_source=None, # type: Optional[str] + ): + # type: (...) -> None + self.args = args + self.editables = editables + self.is_editable = is_editable + self.comes_from = comes_from + self.use_pep517 = use_pep517 + self.isolated = isolated + self.options = options + self.wheel_cache = wheel_cache + self.constraint = constraint + self.line_source = line_source + + def make_requirement(self): + # type: (...) -> InstallRequirement + if self.is_editable: + req = install_req_from_editable( + self.editables, + comes_from=self.comes_from, + use_pep517=self.use_pep517, + constraint=self.constraint, + isolated=self.isolated, + wheel_cache=self.wheel_cache + ) + + else: + req = install_req_from_line( + self.args, + comes_from=self.comes_from, + use_pep517=self.use_pep517, + isolated=self.isolated, + options=self.options, + wheel_cache=self.wheel_cache, + constraint=self.constraint, + line_source=self.line_source, + ) + return req + + class ParsedLine(object): def __init__( self, @@ -135,11 +187,11 @@ def parse_requirements( ) for parsed_line in parser.parse(filename, constraint): - req = handle_line( + parsed_req = handle_line( parsed_line, finder, options, session, wheel_cache, use_pep517 ) - if req is not None: - yield req + if parsed_req is not None: + yield parsed_req.make_requirement() def preprocess(content, skip_requirements_regex): @@ -166,7 +218,7 @@ def handle_line( wheel_cache=None, # type: Optional[WheelCache] use_pep517=None, # type: Optional[bool] ): - # type: (...) -> Optional[InstallRequirement] + # type: (...) -> Optional[ParsedRequirement] """Handle a single parsed requirements line; This can result in creating/yielding requirements, or updating the finder. @@ -198,8 +250,9 @@ def handle_line( if dest in line.opts.__dict__ and line.opts.__dict__[dest]: req_options[dest] = line.opts.__dict__[dest] line_source = 'line {} of {}'.format(line.lineno, line.filename) - return install_req_from_line( - line.args, + return ParsedRequirement( + args=line.args, + is_editable=False, comes_from=line_comes_from, use_pep517=use_pep517, isolated=isolated, @@ -212,10 +265,13 @@ def handle_line( # return an editable requirement elif line.opts.editables: isolated = options.isolated_mode if options else False - return install_req_from_editable( - line.opts.editables[0], comes_from=line_comes_from, + return ParsedRequirement( + editables=line.opts.editables[0], + is_editable=True, + comes_from=line_comes_from, use_pep517=use_pep517, - constraint=line.constraint, isolated=isolated, + constraint=line.constraint, + isolated=isolated, wheel_cache=wheel_cache ) diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index 29a23926257..3ce7cedc934 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -48,8 +48,9 @@ def get_processed_req_from_line(line, fname='file', lineno=1): opts, False, ) - req = handle_line(parsed_line) - assert req is not None + parsed_req = handle_line(parsed_line) + assert parsed_req is not None + req = parsed_req.make_requirement() req.is_direct = True return req From e2a57fd1e6a74821edb1cf8cf37ec63f59c04fb5 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 13 Feb 2020 16:16:32 +0000 Subject: [PATCH 2/6] Refactor to reduce coupling. * Make ParsedLine record the type of line * Split handle_line to allow passing arguments only where needed * Remove unneeded attributes from ParsedRequirement --- src/pip/_internal/req/req_file.py | 205 ++++++++++++++++++------------ tests/unit/test_req.py | 8 +- 2 files changed, 132 insertions(+), 81 deletions(-) diff --git a/src/pip/_internal/req/req_file.py b/src/pip/_internal/req/req_file.py index 0263dc15388..4d1a02d8f61 100644 --- a/src/pip/_internal/req/req_file.py +++ b/src/pip/_internal/req/req_file.py @@ -87,49 +87,46 @@ class ParsedRequirement(object): def __init__( self, + requirement, # type:str is_editable, # type: bool comes_from, # type: str - use_pep517, # type: Optional[bool] - isolated, # type: bool - wheel_cache, # type: Optional[WheelCache] constraint, # type: bool - args=None, # type: Optional[str] - editables=None, # type: Optional[str] options=None, # type: Optional[Dict[str, Any]] line_source=None, # type: Optional[str] ): # type: (...) -> None - self.args = args - self.editables = editables + self.requirement = requirement self.is_editable = is_editable self.comes_from = comes_from - self.use_pep517 = use_pep517 - self.isolated = isolated self.options = options - self.wheel_cache = wheel_cache self.constraint = constraint self.line_source = line_source - def make_requirement(self): + def make_requirement( + self, + isolated=False, # type: bool + wheel_cache=None, # type: Optional[WheelCache] + use_pep517=None # type: Optional[bool] + ): # type: (...) -> InstallRequirement if self.is_editable: req = install_req_from_editable( - self.editables, + self.requirement, comes_from=self.comes_from, - use_pep517=self.use_pep517, + use_pep517=use_pep517, constraint=self.constraint, - isolated=self.isolated, - wheel_cache=self.wheel_cache + isolated=isolated, + wheel_cache=wheel_cache ) else: req = install_req_from_line( - self.args, + self.requirement, comes_from=self.comes_from, - use_pep517=self.use_pep517, - isolated=self.isolated, + use_pep517=use_pep517, + isolated=isolated, options=self.options, - wheel_cache=self.wheel_cache, + wheel_cache=wheel_cache, constraint=self.constraint, line_source=self.line_source, ) @@ -150,10 +147,21 @@ def __init__( self.filename = filename self.lineno = lineno self.comes_from = comes_from - self.args = args self.opts = opts self.constraint = constraint + if args: + self.is_requirement = True + self.is_editable = False + self.requirement = args + elif opts.editables: + self.is_requirement = True + self.is_editable = True + # We don't support multiple -e on one line + self.requirement = opts.editables[0] + else: + self.is_requirement = False + def parse_requirements( filename, # type: str @@ -188,10 +196,18 @@ def parse_requirements( for parsed_line in parser.parse(filename, constraint): parsed_req = handle_line( - parsed_line, finder, options, session, wheel_cache, use_pep517 + parsed_line, + options=options, + finder=finder, + session=session ) if parsed_req is not None: - yield parsed_req.make_requirement() + isolated = options.isolated_mode if options else False + yield parsed_req.make_requirement( + isolated, + wheel_cache, + use_pep517 + ) def preprocess(content, skip_requirements_regex): @@ -210,91 +226,80 @@ def preprocess(content, skip_requirements_regex): return lines_enum -def handle_line( +def handle_requirement_line( line, # type: ParsedLine - finder=None, # type: Optional[PackageFinder] options=None, # type: Optional[optparse.Values] - session=None, # type: Optional[PipSession] - wheel_cache=None, # type: Optional[WheelCache] - use_pep517=None, # type: Optional[bool] ): - # type: (...) -> Optional[ParsedRequirement] - """Handle a single parsed requirements line; This can result in - creating/yielding requirements, or updating the finder. - - For lines that contain requirements, the only options that have an effect - are from SUPPORTED_OPTIONS_REQ, and they are scoped to the - requirement. Other options from SUPPORTED_OPTIONS may be present, but are - ignored. - - For lines that do not contain requirements, the only options that have an - effect are from SUPPORTED_OPTIONS. Options from SUPPORTED_OPTIONS_REQ may - be present, but are ignored. These lines may contain multiple options - (although our docs imply only one is supported), and all our parsed and - affect the finder. - """ + # type: (...) -> ParsedRequirement # preserve for the nested code path line_comes_from = '{} {} (line {})'.format( '-c' if line.constraint else '-r', line.filename, line.lineno, ) - # return a line requirement - if line.args: - isolated = options.isolated_mode if options else False + assert line.is_requirement + + if line.is_editable: + # For editable requirements, we don't support per-requirement + # options, so just return the parsed requirement. + return ParsedRequirement( + requirement=line.requirement, + is_editable=line.is_editable, + comes_from=line_comes_from, + constraint=line.constraint, + ) + else: if options: + # Disable wheels if the user has specified build options cmdoptions.check_install_build_global(options, line.opts) + # get the options that apply to requirements req_options = {} for dest in SUPPORTED_OPTIONS_REQ_DEST: if dest in line.opts.__dict__ and line.opts.__dict__[dest]: req_options[dest] = line.opts.__dict__[dest] + line_source = 'line {} of {}'.format(line.lineno, line.filename) return ParsedRequirement( - args=line.args, - is_editable=False, + requirement=line.requirement, + is_editable=line.is_editable, comes_from=line_comes_from, - use_pep517=use_pep517, - isolated=isolated, - options=req_options, - wheel_cache=wheel_cache, constraint=line.constraint, + options=req_options, line_source=line_source, ) - # return an editable requirement - elif line.opts.editables: - isolated = options.isolated_mode if options else False - return ParsedRequirement( - editables=line.opts.editables[0], - is_editable=True, - comes_from=line_comes_from, - use_pep517=use_pep517, - constraint=line.constraint, - isolated=isolated, - wheel_cache=wheel_cache - ) + +def handle_option_line( + opts, # type: Values + filename, # type: str + lineno, # type: int + finder=None, # type: Optional[PackageFinder] + options=None, # type: Optional[optparse.Values] + session=None, # type: Optional[PipSession] +): + # type: (...) -> None # percolate hash-checking option upward - elif line.opts.require_hashes: - options.require_hashes = line.opts.require_hashes + if opts.require_hashes: + options.require_hashes = opts.require_hashes # set finder options elif finder: find_links = finder.find_links index_urls = finder.index_urls - if line.opts.index_url: - index_urls = [line.opts.index_url] - if line.opts.no_index is True: + if opts.index_url: + index_urls = [opts.index_url] + if opts.no_index is True: index_urls = [] - if line.opts.extra_index_urls: - index_urls.extend(line.opts.extra_index_urls) - if line.opts.find_links: + if opts.extra_index_urls: + index_urls.extend(opts.extra_index_urls) + if opts.find_links: # FIXME: it would be nice to keep track of the source # of the find_links: support a find-links local path # relative to a requirements file. - value = line.opts.find_links[0] - req_dir = os.path.dirname(os.path.abspath(line.filename)) + value = opts.find_links[0] + req_dir = os.path.dirname(os.path.abspath(filename)) relative_to_reqs_file = os.path.join(req_dir, value) if os.path.exists(relative_to_reqs_file): value = relative_to_reqs_file @@ -306,15 +311,58 @@ def handle_line( ) finder.search_scope = search_scope - if line.opts.pre: + if opts.pre: finder.set_allow_all_prereleases() if session: - for host in line.opts.trusted_hosts or []: - source = 'line {} of {}'.format(line.lineno, line.filename) + for host in opts.trusted_hosts or []: + source = 'line {} of {}'.format(lineno, filename) session.add_trusted_host(host, source=source) - return None + +def handle_line( + line, # type: ParsedLine + options=None, # type: Optional[optparse.Values] + finder=None, # type: Optional[PackageFinder] + session=None, # type: Optional[PipSession] +): + # type: (...) -> Optional[ParsedRequirement] + """Handle a single parsed requirements line; This can result in + creating/yielding requirements, or updating the finder. + + :param line: The parsed line to be processed. + :param options: CLI options. + :param finder: The finder - updated by non-requirement lines. + :param session: The session - updated by non-requirement lines. + + Returns a ParsedRequirement object if the line is a requirement line, + otherwise returns None. + + For lines that contain requirements, the only options that have an effect + are from SUPPORTED_OPTIONS_REQ, and they are scoped to the + requirement. Other options from SUPPORTED_OPTIONS may be present, but are + ignored. + + For lines that do not contain requirements, the only options that have an + effect are from SUPPORTED_OPTIONS. Options from SUPPORTED_OPTIONS_REQ may + be present, but are ignored. These lines may contain multiple options + (although our docs imply only one is supported), and all our parsed and + affect the finder. + """ + + if parsed_line.is_requirement: + parsed_req = handle_requirement_line(parsed_line, options) + return parsed_req + else: + handle_option_line( + parsed_line.opts, + parsed_line.filename, + parsed_line.lineno, + finder, + options, + session, + ) + return None class RequirementsFileParser(object): @@ -342,8 +390,7 @@ def _parse_and_recurse(self, filename, constraint): # type: (str, bool) -> Iterator[ParsedLine] for line in self._parse_file(filename, constraint): if ( - not line.args and - not line.opts.editables and + not line.is_requirement and (line.opts.requirements or line.opts.constraints) ): # parse a nested requirements file diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index 3ce7cedc934..d633c2ce13b 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -31,7 +31,11 @@ install_req_from_req_string, parse_editable, ) -from pip._internal.req.req_file import ParsedLine, get_line_parser, handle_line +from pip._internal.req.req_file import ( + ParsedLine, + get_line_parser, + handle_requirement_line, +) from pip._internal.req.req_tracker import get_requirement_tracker from pip._internal.utils.urls import path_to_url from tests.lib import assert_raises_regexp, make_test_finder, requirements_file @@ -48,7 +52,7 @@ def get_processed_req_from_line(line, fname='file', lineno=1): opts, False, ) - parsed_req = handle_line(parsed_line) + parsed_req = handle_requirement_line(parsed_line) assert parsed_req is not None req = parsed_req.make_requirement() req.is_direct = True From 90e4eb3eedc7c550c2dd036b7a535cdec50a93d5 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 14 Feb 2020 11:52:53 +0000 Subject: [PATCH 3/6] Make parse_requirements return a ParsedRequirement --- src/pip/_internal/cli/req_command.py | 17 ++++-- src/pip/_internal/commands/uninstall.py | 5 +- src/pip/_internal/req/req_file.py | 23 +++------ tests/unit/test_req_file.py | 69 ++++++++++++++++--------- 4 files changed, 67 insertions(+), 47 deletions(-) diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py index 29bbb8fe72d..e67a7ead96d 100644 --- a/src/pip/_internal/cli/req_command.py +++ b/src/pip/_internal/cli/req_command.py @@ -287,10 +287,14 @@ def get_requirements( check_supported_wheels=check_supported_wheels ) for filename in options.constraints: - for req_to_add in parse_requirements( + for parsed_req in parse_requirements( filename, constraint=True, finder=finder, options=options, - session=session, wheel_cache=wheel_cache): + session=session): + req_to_add = parsed_req.make_requirement( + isolated=options.isolated_mode, + wheel_cache=wheel_cache, + ) req_to_add.is_direct = True requirement_set.add_requirement(req_to_add) @@ -315,11 +319,14 @@ def get_requirements( # NOTE: options.require_hashes may be set if --require-hashes is True for filename in options.requirements: - for req_to_add in parse_requirements( + for parsed_req in parse_requirements( filename, - finder=finder, options=options, session=session, + finder=finder, options=options, session=session): + req_to_add = parsed_req.make_requirement( + isolated=options.isolated_mode, wheel_cache=wheel_cache, - use_pep517=options.use_pep517): + use_pep517=options.use_pep517 + ) req_to_add.is_direct = True requirement_set.add_requirement(req_to_add) diff --git a/src/pip/_internal/commands/uninstall.py b/src/pip/_internal/commands/uninstall.py index 1bde414a6c1..71b00483b94 100644 --- a/src/pip/_internal/commands/uninstall.py +++ b/src/pip/_internal/commands/uninstall.py @@ -58,10 +58,13 @@ def run(self, options, args): if req.name: reqs_to_uninstall[canonicalize_name(req.name)] = req for filename in options.requirements: - for req in parse_requirements( + for parsed_req in parse_requirements( filename, options=options, session=session): + req = parsed_req.make_requirement( + isolated=options.isolated_mode + ) if req.name: reqs_to_uninstall[canonicalize_name(req.name)] = req if not reqs_to_uninstall: diff --git a/src/pip/_internal/req/req_file.py b/src/pip/_internal/req/req_file.py index 4d1a02d8f61..26d2885425f 100644 --- a/src/pip/_internal/req/req_file.py +++ b/src/pip/_internal/req/req_file.py @@ -170,10 +170,8 @@ def parse_requirements( comes_from=None, # type: Optional[str] options=None, # type: Optional[optparse.Values] constraint=False, # type: bool - wheel_cache=None, # type: Optional[WheelCache] - use_pep517=None # type: Optional[bool] ): - # type: (...) -> Iterator[InstallRequirement] + # type: (...) -> Iterator[ParsedRequirement] """Parse a requirements file and yield InstallRequirement instances. :param filename: Path or url of requirements file. @@ -183,8 +181,6 @@ def parse_requirements( :param options: cli options. :param constraint: If true, parsing a constraint file rather than requirements file. - :param wheel_cache: Instance of pip.wheel.WheelCache - :param use_pep517: Value of the --use-pep517 option. """ skip_requirements_regex = ( options.skip_requirements_regex if options else None @@ -202,12 +198,7 @@ def parse_requirements( session=session ) if parsed_req is not None: - isolated = options.isolated_mode if options else False - yield parsed_req.make_requirement( - isolated, - wheel_cache, - use_pep517 - ) + yield parsed_req def preprocess(content, skip_requirements_regex): @@ -350,14 +341,14 @@ def handle_line( affect the finder. """ - if parsed_line.is_requirement: - parsed_req = handle_requirement_line(parsed_line, options) + if line.is_requirement: + parsed_req = handle_requirement_line(line, options) return parsed_req else: handle_option_line( - parsed_line.opts, - parsed_line.filename, - parsed_line.lineno, + line.opts, + line.filename, + line.lineno, finder, options, session, diff --git a/tests/unit/test_req_file.py b/tests/unit/test_req_file.py index 597fc2f82f2..1eee1024e41 100644 --- a/tests/unit/test_req_file.py +++ b/tests/unit/test_req_file.py @@ -48,6 +48,24 @@ def options(session): format_control=FormatControl(set(), set())) +def parse_reqfile( + filename, + session, + finder=None, + options=None, + constraint=False, + isolated=False, +): + # Wrap parse_requirements/make_requirement to + # avoid having to write the same chunk of code in + # lots of tests. + for parsed_req in parse_requirements( + filename, session, finder=finder, + options=options, constraint=constraint, + ): + yield parsed_req.make_requirement(isolated=isolated) + + class TestPreprocess(object): """tests for `preprocess`""" @@ -191,12 +209,13 @@ def process_line( path.parent.mkdir(exist_ok=True) path.write_text(prefix + line) monkeypatch.chdir(str(tmpdir)) - return list(parse_requirements( + return list(parse_reqfile( filename, finder=finder, options=options, session=session, constraint=constraint, + isolated=options.isolated_mode if options else False )) return process_line @@ -309,7 +328,7 @@ def test_nested_constraints_file(self, monkeypatch, tmpdir): monkeypatch.chdir(str(tmpdir)) reqs = list( - parse_requirements('./parent/req_file.txt', session=session) + parse_reqfile('./parent/req_file.txt', session=session) ) assert len(reqs) == 1 assert reqs[0].name == req_name @@ -447,7 +466,7 @@ def get_file_content(filename, *args, **kwargs): pip._internal.req.req_file, 'get_file_content', get_file_content ) - result = list(parse_requirements(req_file, session=session)) + result = list(parse_reqfile(req_file, session=session)) assert len(result) == 1 assert result[0].name == req_name assert not result[0].constraint @@ -467,7 +486,7 @@ def test_relative_local_nested_req_files( monkeypatch.chdir(str(tmpdir)) reqs = list( - parse_requirements('./parent/req_file.txt', session=session) + parse_reqfile('./parent/req_file.txt', session=session) ) assert len(reqs) == 1 assert reqs[0].name == req_name @@ -490,7 +509,7 @@ def test_absolute_local_nested_req_files( req_file.write_text('-r {}'.format(other_req_file_str)) other_req_file.write_text(req_name) - reqs = list(parse_requirements(str(req_file), session=session)) + reqs = list(parse_reqfile(str(req_file), session=session)) assert len(reqs) == 1 assert reqs[0].name == req_name assert not reqs[0].constraint @@ -516,7 +535,7 @@ def get_file_content(filename, *args, **kwargs): pip._internal.req.req_file, 'get_file_content', get_file_content ) - result = list(parse_requirements(req_file, session=session)) + result = list(parse_reqfile(req_file, session=session)) assert len(result) == 1 assert result[0].name == req_name assert not result[0].constraint @@ -565,7 +584,7 @@ def test_variant5(self, line_processor, finder): class TestParseRequirements(object): - """tests for `parse_requirements`""" + """tests for `parse_reqfile`""" @pytest.mark.network def test_remote_reqs_parse(self): @@ -574,7 +593,7 @@ def test_remote_reqs_parse(self): """ # this requirements file just contains a comment previously this has # failed in py3: https://github.com/pypa/pip/issues/760 - for req in parse_requirements( + for req in parse_reqfile( 'https://raw.githubusercontent.com/pypa/' 'pip-test-package/master/' 'tests/req_just_comment.txt', session=PipSession()): @@ -585,8 +604,8 @@ def test_multiple_appending_options(self, tmpdir, finder, options): fp.write("--extra-index-url url1 \n") fp.write("--extra-index-url url2 ") - list(parse_requirements(tmpdir.joinpath("req1.txt"), finder=finder, - session=PipSession(), options=options)) + list(parse_reqfile(tmpdir.joinpath("req1.txt"), finder=finder, + session=PipSession(), options=options)) assert finder.index_urls == ['url1', 'url2'] @@ -596,8 +615,8 @@ def test_skip_regex(self, tmpdir, finder, options): fp.write("--extra-index-url Bad \n") fp.write("--extra-index-url Good ") - list(parse_requirements(tmpdir.joinpath("req1.txt"), finder=finder, - options=options, session=PipSession())) + list(parse_reqfile(tmpdir.joinpath("req1.txt"), finder=finder, + options=options, session=PipSession())) assert finder.index_urls == ['Good'] @@ -617,7 +636,7 @@ def test_expand_existing_env_variables(self, tmpdir, finder): with patch('pip._internal.req.req_file.os.getenv') as getenv: getenv.side_effect = lambda n: dict(env_vars)[n] - reqs = list(parse_requirements( + reqs = list(parse_reqfile( tmpdir.joinpath('req1.txt'), finder=finder, session=PipSession() @@ -642,7 +661,7 @@ def test_expand_missing_env_variables(self, tmpdir, finder): with patch('pip._internal.req.req_file.os.getenv') as getenv: getenv.return_value = '' - reqs = list(parse_requirements( + reqs = list(parse_reqfile( tmpdir.joinpath('req1.txt'), finder=finder, session=PipSession() @@ -657,13 +676,13 @@ def test_join_lines(self, tmpdir, finder): with open(tmpdir.joinpath("req1.txt"), "w") as fp: fp.write("--extra-index-url url1 \\\n--extra-index-url url2") - list(parse_requirements(tmpdir.joinpath("req1.txt"), finder=finder, - session=PipSession())) + list(parse_reqfile(tmpdir.joinpath("req1.txt"), finder=finder, + session=PipSession())) assert finder.index_urls == ['url1', 'url2'] def test_req_file_parse_no_only_binary(self, data, finder): - list(parse_requirements( + list(parse_reqfile( data.reqfiles.joinpath("supported_options2.txt"), finder=finder, session=PipSession())) @@ -677,7 +696,7 @@ def test_req_file_parse_comment_start_of_line(self, tmpdir, finder): with open(tmpdir.joinpath("req1.txt"), "w") as fp: fp.write("# Comment ") - reqs = list(parse_requirements(tmpdir.joinpath("req1.txt"), + reqs = list(parse_reqfile(tmpdir.joinpath("req1.txt"), finder=finder, session=PipSession())) @@ -690,7 +709,7 @@ def test_req_file_parse_comment_end_of_line_with_url(self, tmpdir, finder): with open(tmpdir.joinpath("req1.txt"), "w") as fp: fp.write("https://example.com/foo.tar.gz # Comment ") - reqs = list(parse_requirements(tmpdir.joinpath("req1.txt"), + reqs = list(parse_reqfile(tmpdir.joinpath("req1.txt"), finder=finder, session=PipSession())) @@ -704,7 +723,7 @@ def test_req_file_parse_egginfo_end_of_line_with_url(self, tmpdir, finder): with open(tmpdir.joinpath("req1.txt"), "w") as fp: fp.write("https://example.com/foo.tar.gz#egg=wat") - reqs = list(parse_requirements(tmpdir.joinpath("req1.txt"), + reqs = list(parse_reqfile(tmpdir.joinpath("req1.txt"), finder=finder, session=PipSession())) @@ -724,7 +743,7 @@ def test_req_file_no_finder(self, tmpdir): --no-index """) - parse_requirements(tmpdir.joinpath("req.txt"), session=PipSession()) + parse_reqfile(tmpdir.joinpath("req.txt"), session=PipSession()) def test_install_requirements_with_options(self, tmpdir, finder, session, options): @@ -738,10 +757,10 @@ def test_install_requirements_with_options(self, tmpdir, finder, session, '''.format(global_option=global_option, install_option=install_option) with requirements_file(content, tmpdir) as reqs_file: - req = next(parse_requirements(reqs_file.resolve(), - finder=finder, - options=options, - session=session)) + req = next(parse_reqfile(reqs_file.resolve(), + finder=finder, + options=options, + session=session)) req.source_dir = os.curdir with patch.object(subprocess, 'Popen') as popen: From aac5d821f97a088d044003b5ee37e7c73cc1b5bc Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 14 Feb 2020 12:22:50 +0000 Subject: [PATCH 4/6] Move make_requirement to pip._internal.req.constructors (and rename it) --- src/pip/_internal/cli/req_command.py | 7 +++-- src/pip/_internal/commands/uninstall.py | 8 ++++-- src/pip/_internal/req/constructors.py | 32 ++++++++++++++++++++++ src/pip/_internal/req/req_file.py | 36 ------------------------- tests/unit/test_req.py | 3 ++- tests/unit/test_req_file.py | 11 +++++--- 6 files changed, 52 insertions(+), 45 deletions(-) diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py index e67a7ead96d..0aa02ab1f9a 100644 --- a/src/pip/_internal/cli/req_command.py +++ b/src/pip/_internal/cli/req_command.py @@ -22,6 +22,7 @@ from pip._internal.req.constructors import ( install_req_from_editable, install_req_from_line, + install_req_from_parsed_requirement, install_req_from_req_string, ) from pip._internal.req.req_file import parse_requirements @@ -291,7 +292,8 @@ def get_requirements( filename, constraint=True, finder=finder, options=options, session=session): - req_to_add = parsed_req.make_requirement( + req_to_add = install_req_from_parsed_requirement( + parsed_req, isolated=options.isolated_mode, wheel_cache=wheel_cache, ) @@ -322,7 +324,8 @@ def get_requirements( for parsed_req in parse_requirements( filename, finder=finder, options=options, session=session): - req_to_add = parsed_req.make_requirement( + req_to_add = install_req_from_parsed_requirement( + parsed_req, isolated=options.isolated_mode, wheel_cache=wheel_cache, use_pep517=options.use_pep517 diff --git a/src/pip/_internal/commands/uninstall.py b/src/pip/_internal/commands/uninstall.py index 71b00483b94..e93ad74e29e 100644 --- a/src/pip/_internal/commands/uninstall.py +++ b/src/pip/_internal/commands/uninstall.py @@ -9,7 +9,10 @@ from pip._internal.cli.req_command import SessionCommandMixin from pip._internal.exceptions import InstallationError from pip._internal.req import parse_requirements -from pip._internal.req.constructors import install_req_from_line +from pip._internal.req.constructors import ( + install_req_from_line, + install_req_from_parsed_requirement, +) from pip._internal.utils.misc import protect_pip_from_modification_on_windows @@ -62,7 +65,8 @@ def run(self, options, args): filename, options=options, session=session): - req = parsed_req.make_requirement( + req = install_req_from_parsed_requirement( + parsed_req, isolated=options.isolated_mode ) if req.name: diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index c350aaa8da7..e4294b52aa3 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -37,6 +37,7 @@ Any, Dict, Optional, Set, Tuple, Union, ) from pip._internal.cache import WheelCache + from pip._internal.req.req_file import ParsedRequirement __all__ = [ @@ -441,3 +442,34 @@ def install_req_from_req_string( req, comes_from, isolated=isolated, wheel_cache=wheel_cache, use_pep517=use_pep517 ) + + +def install_req_from_parsed_requirement( + parsed_req, # type: ParsedRequirement + isolated=False, # type: bool + wheel_cache=None, # type: Optional[WheelCache] + use_pep517=None # type: Optional[bool] +): + # type: (...) -> InstallRequirement + if parsed_req.is_editable: + req = install_req_from_editable( + parsed_req.requirement, + comes_from=parsed_req.comes_from, + use_pep517=use_pep517, + constraint=parsed_req.constraint, + isolated=isolated, + wheel_cache=wheel_cache + ) + + else: + req = install_req_from_line( + parsed_req.requirement, + comes_from=parsed_req.comes_from, + use_pep517=use_pep517, + isolated=isolated, + options=parsed_req.options, + wheel_cache=wheel_cache, + constraint=parsed_req.constraint, + line_source=parsed_req.line_source, + ) + return req diff --git a/src/pip/_internal/req/req_file.py b/src/pip/_internal/req/req_file.py index 26d2885425f..ec47abab9f7 100644 --- a/src/pip/_internal/req/req_file.py +++ b/src/pip/_internal/req/req_file.py @@ -22,10 +22,6 @@ RequirementsFileParseError, ) from pip._internal.models.search_scope import SearchScope -from pip._internal.req.constructors import ( - install_req_from_editable, - install_req_from_line, -) from pip._internal.utils.encoding import auto_decode from pip._internal.utils.typing import MYPY_CHECK_RUNNING from pip._internal.utils.urls import get_url_scheme @@ -36,8 +32,6 @@ Any, Callable, Iterator, List, NoReturn, Optional, Text, Tuple, Dict, ) - from pip._internal.req import InstallRequirement - from pip._internal.cache import WheelCache from pip._internal.index.package_finder import PackageFinder from pip._internal.network.session import PipSession @@ -102,36 +96,6 @@ def __init__( self.constraint = constraint self.line_source = line_source - def make_requirement( - self, - isolated=False, # type: bool - wheel_cache=None, # type: Optional[WheelCache] - use_pep517=None # type: Optional[bool] - ): - # type: (...) -> InstallRequirement - if self.is_editable: - req = install_req_from_editable( - self.requirement, - comes_from=self.comes_from, - use_pep517=use_pep517, - constraint=self.constraint, - isolated=isolated, - wheel_cache=wheel_cache - ) - - else: - req = install_req_from_line( - self.requirement, - comes_from=self.comes_from, - use_pep517=use_pep517, - isolated=isolated, - options=self.options, - wheel_cache=wheel_cache, - constraint=self.constraint, - line_source=self.line_source, - ) - return req - class ParsedLine(object): def __init__( diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index d633c2ce13b..f301e11054c 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -28,6 +28,7 @@ _looks_like_path, install_req_from_editable, install_req_from_line, + install_req_from_parsed_requirement, install_req_from_req_string, parse_editable, ) @@ -54,7 +55,7 @@ def get_processed_req_from_line(line, fname='file', lineno=1): ) parsed_req = handle_requirement_line(parsed_line) assert parsed_req is not None - req = parsed_req.make_requirement() + req = install_req_from_parsed_requirement(parsed_req) req.is_direct = True return req diff --git a/tests/unit/test_req_file.py b/tests/unit/test_req_file.py index 1eee1024e41..5b8d5e7c376 100644 --- a/tests/unit/test_req_file.py +++ b/tests/unit/test_req_file.py @@ -18,6 +18,7 @@ from pip._internal.req.constructors import ( install_req_from_editable, install_req_from_line, + install_req_from_parsed_requirement, ) from pip._internal.req.req_file import ( break_args_options, @@ -56,14 +57,16 @@ def parse_reqfile( constraint=False, isolated=False, ): - # Wrap parse_requirements/make_requirement to - # avoid having to write the same chunk of code in - # lots of tests. + # Wrap parse_requirements/install_req_from_parsed_requirement to + # avoid having to write the same chunk of code in lots of tests. for parsed_req in parse_requirements( filename, session, finder=finder, options=options, constraint=constraint, ): - yield parsed_req.make_requirement(isolated=isolated) + yield install_req_from_parsed_requirement( + parsed_req, + isolated=isolated + ) class TestPreprocess(object): From 29a06ef947c768ee9c185b627ac0370231f37489 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 14 Feb 2020 12:25:01 +0000 Subject: [PATCH 5/6] Add a trivial news marker --- news/69a4dd1e-c03f-4780-ae6f-892f818fb367.trivial | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 news/69a4dd1e-c03f-4780-ae6f-892f818fb367.trivial diff --git a/news/69a4dd1e-c03f-4780-ae6f-892f818fb367.trivial b/news/69a4dd1e-c03f-4780-ae6f-892f818fb367.trivial new file mode 100644 index 00000000000..e69de29bb2d From 2b6fb95ba46e712071282126e8fc693890bcd588 Mon Sep 17 00:00:00 2001 From: Christopher Hunt Date: Sun, 23 Feb 2020 23:44:13 +0800 Subject: [PATCH 6/6] Reorder imports --- src/pip/_internal/req/req_file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/req/req_file.py b/src/pip/_internal/req/req_file.py index ec47abab9f7..cdb61a4ca28 100644 --- a/src/pip/_internal/req/req_file.py +++ b/src/pip/_internal/req/req_file.py @@ -29,7 +29,7 @@ if MYPY_CHECK_RUNNING: from optparse import Values from typing import ( - Any, Callable, Iterator, List, NoReturn, Optional, Text, Tuple, Dict, + Any, Callable, Dict, Iterator, List, NoReturn, Optional, Text, Tuple, ) from pip._internal.index.package_finder import PackageFinder