diff --git a/CHANGES.txt b/CHANGES.txt index 7c6d85024d3..925c3891c65 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -37,6 +37,9 @@ * Fix user directory expansion when ``HOME=/``. Workaround for Python bug http://bugs.python.org/issue14768, reported in :issue:`2996`. +* Fixed :issue:`3009`, correct reporting of requirements file line numbers + (:pull:`3125`) + **7.1.2 (2015-08-22)** diff --git a/docs/reference/pip_install.rst b/docs/reference/pip_install.rst index ff7d5659221..489e0b09ff1 100644 --- a/docs/reference/pip_install.rst +++ b/docs/reference/pip_install.rst @@ -99,6 +99,8 @@ treated as a comment. A line ending in an unescaped ``\`` is treated as a line continuation and the newline following it is effectively ignored. +Comments are stripped *before* line continuations are processed. + Additionally, the following Package Index Options are supported: * :ref:`-i, --index-url <--index-url>` diff --git a/pip/req/req_file.py b/pip/req/req_file.py index 4b3f683c6db..7a1aa53e7bd 100644 --- a/pip/req/req_file.py +++ b/pip/req/req_file.py @@ -65,7 +65,7 @@ def parse_requirements(filename, finder=None, comes_from=None, options=None, :param filename: Path or url of requirements file. :param finder: Instance of pip.index.PackageFinder. :param comes_from: Origin description of requirements. - :param options: Global options. + :param options: cli options. :param session: Instance of pip.download.PipSession. :param constraint: If true, parsing a constraint file rather than requirements file. @@ -81,12 +81,9 @@ def parse_requirements(filename, finder=None, comes_from=None, options=None, filename, comes_from=comes_from, session=session ) - lines = content.splitlines() - lines = ignore_comments(lines) - lines = join_lines(lines) - lines = skip_regex(lines, options) + lines_enum = preprocess(content, options) - for line_number, line in enumerate(lines, 1): + for line_number, line in lines_enum: req_iter = process_line(line, filename, line_number, finder, comes_from, options, session, wheel_cache, constraint=constraint) @@ -94,6 +91,19 @@ def parse_requirements(filename, finder=None, comes_from=None, options=None, yield req +def preprocess(content, options): + """Split, filter, and join lines, and return a line iterator + + :param content: the content of the requirements file + :param options: cli options + """ + lines_enum = enumerate(content.splitlines(), start=1) + lines_enum = ignore_comments(lines_enum) + lines_enum = join_lines(lines_enum) + lines_enum = skip_regex(lines_enum, options) + return lines_enum + + def process_line(line, filename, line_number, finder=None, comes_from=None, options=None, session=None, wheel_cache=None, constraint=False): @@ -267,42 +277,53 @@ def parser_exit(self, msg): return parser -def join_lines(iterator): +def join_lines(lines_enum): + """Joins a line ending in '\' with the previous line. The joined line takes on + the index of the first line. """ - Joins a line ending in '\' with the previous line. - """ - lines = [] - for line in iterator: + primary_line_number = None + new_line = [] + for line_number, line in lines_enum: if not line.endswith('\\'): - if lines: - lines.append(line) - yield ''.join(lines) - lines = [] + if new_line: + new_line.append(line) + yield primary_line_number, ''.join(new_line) + new_line = [] else: - yield line + yield line_number, line else: - lines.append(line.strip('\\')) + if not new_line: + primary_line_number = line_number + new_line.append(line.strip('\\')) + + # last line contains \ + if new_line: + yield primary_line_number, ''.join(new_line) # TODO: handle space after '\'. - # TODO: handle '\' on last line. -def ignore_comments(iterator): +def ignore_comments(lines_enum): """ - Strips and filters empty or commented lines. + Strips comments and filter empty lines. """ - for line in iterator: + for line_number, line in lines_enum: line = COMMENT_RE.sub('', line) line = line.strip() if line: - yield line + yield line_number, line -def skip_regex(lines, options): +def skip_regex(lines_enum, options): """ - Optionally exclude lines that match '--skip-requirements-regex' + Skip lines that match '--skip-requirements-regex' pattern + + Note: the regex pattern is only built once """ skip_regex = options.skip_requirements_regex if options else None if skip_regex: - lines = filterfalse(re.compile(skip_regex).search, lines) - return lines + pattern = re.compile(skip_regex) + lines_enum = filterfalse( + lambda e: pattern.search(e[1]), + lines_enum) + return lines_enum diff --git a/tests/unit/test_req_file.py b/tests/unit/test_req_file.py index 1e3dbbbe5ce..d714baacd16 100644 --- a/tests/unit/test_req_file.py +++ b/tests/unit/test_req_file.py @@ -1,6 +1,6 @@ import os import subprocess -from textwrap import dedent +import textwrap from mock import patch, Mock import pytest @@ -12,7 +12,8 @@ from pip.index import PackageFinder from pip.req.req_install import InstallRequirement from pip.req.req_file import (parse_requirements, process_line, join_lines, - ignore_comments, break_args_options) + ignore_comments, break_args_options, skip_regex, + preprocess) @pytest.fixture @@ -33,43 +34,132 @@ def options(session): format_control=pip.index.FormatControl(set(), set())) +class TestPreprocess(object): + """tests for `preprocess`""" + + def test_comments_processed_before_joining_case1(self): + content = textwrap.dedent("""\ + req1 \\ + # comment \\ + req2 + """) + result = preprocess(content, None) + assert list(result) == [(1, 'req1 req2')] + + def test_comments_processed_before_joining_case2(self): + content = textwrap.dedent("""\ + req1\\ + # comment + """) + result = preprocess(content, None) + assert list(result) == [(1, 'req1')] + + def test_comments_processed_before_joining_case3(self): + content = textwrap.dedent("""\ + req1 \\ + # comment + req2 + """) + result = preprocess(content, None) + assert list(result) == [(1, 'req1 req2')] + + def test_skip_regex_after_joining_case1(self, options): + content = textwrap.dedent("""\ + patt\\ + ern + line2 + """) + options.skip_requirements_regex = 'pattern' + result = preprocess(content, options) + assert list(result) == [(3, 'line2')] + + def test_skip_regex_after_joining_case2(self, options): + content = textwrap.dedent("""\ + pattern \\ + line2 + line3 + """) + options.skip_requirements_regex = 'pattern' + result = preprocess(content, options) + assert list(result) == [(3, 'line3')] + + class TestIgnoreComments(object): """tests for `ignore_comment`""" - def test_strip_empty_line(self): - lines = ['req1', '', 'req2'] + def test_ignore_line(self): + lines = [(1, ''), (2, 'req1'), (3, 'req2')] + result = ignore_comments(lines) + assert list(result) == [(2, 'req1'), (3, 'req2')] + + def test_ignore_comment(self): + lines = [(1, 'req1'), (2, '# comment'), (3, 'req2')] result = ignore_comments(lines) - assert list(result) == ['req1', 'req2'] + assert list(result) == [(1, 'req1'), (3, 'req2')] def test_strip_comment(self): - lines = ['req1', '# comment', 'req2'] + lines = [(1, 'req1'), (2, 'req # comment'), (3, 'req2')] result = ignore_comments(lines) - assert list(result) == ['req1', 'req2'] + assert list(result) == [(1, 'req1'), (2, 'req'), (3, 'req2')] class TestJoinLines(object): """tests for `join_lines`""" def test_join_lines(self): - lines = dedent('''\ - line 1 - line 2:1 \\ - line 2:2 - line 3:1 \\ - line 3:2 \\ - line 3:3 - line 4 - ''').splitlines() - + lines = enumerate([ + 'line 1', + 'line 2:1 \\', + 'line 2:2', + 'line 3:1 \\', + 'line 3:2 \\', + 'line 3:3', + 'line 4' + ], start=1) expect = [ + (1, 'line 1'), + (2, 'line 2:1 line 2:2'), + (4, 'line 3:1 line 3:2 line 3:3'), + (7, 'line 4'), + ] + assert expect == list(join_lines(lines)) + + def test_last_line_with_escape(self): + lines = enumerate([ 'line 1', - 'line 2:1 line 2:2', - 'line 3:1 line 3:2 line 3:3', - 'line 4', + 'line 2 \\', + ], start=1) + expect = [ + (1, 'line 1'), + (2, 'line 2 '), ] assert expect == list(join_lines(lines)) +class TestSkipRegex(object): + """tests for `skip_reqex``""" + + def test_skip_regex_pattern_match(self): + options = stub(skip_requirements_regex='.*Bad.*') + line = '--extra-index-url Bad' + assert [] == list(skip_regex(enumerate([line]), options)) + + def test_skip_regex_pattern_not_match(self): + options = stub(skip_requirements_regex='.*Bad.*') + line = '--extra-index-url Good' + assert [(0, line)] == list(skip_regex(enumerate([line]), options)) + + def test_skip_regex_no_options(self): + options = None + line = '--extra-index-url Good' + assert [(0, line)] == list(skip_regex(enumerate([line]), options)) + + def test_skip_regex_no_skip_option(self): + options = stub(skip_requirements_regex=None) + line = '--extra-index-url Good' + assert [(0, line)] == list(skip_regex(enumerate([line]), options)) + + class TestProcessLine(object): """tests for `process_line`"""