From 800d4815682020659e7d99d7aff744b7448d1ae4 Mon Sep 17 00:00:00 2001 From: Andy Kluger Date: Mon, 27 Sep 2021 15:53:24 -0400 Subject: [PATCH 1/4] Add --newline [LF|CRLF|native|preserve] option to compile - Use it in writer to override the line sep used in output, using io.TextIOWrapper - Set default to preserve, which checks the output file, then the input file(s) - Falls back to LF if that's not possible --- piptools/scripts/compile.py | 27 +++++++++++++++++++++++++++ piptools/writer.py | 30 ++++++++++++++++++++++-------- tests/test_writer.py | 1 + 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index 18a3b312f..2642ed49d 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -165,6 +165,12 @@ def _get_default_option(option_name: str) -> Any: "Will be derived from input file otherwise." ), ) +@click.option( + "--newline", + type=click.Choice(("LF", "CRLF", "native", "preserve"), case_sensitive=False), + default="preserve", + help="Override the newline control characters used", +) @click.option( "--allow-unsafe/--no-allow-unsafe", is_flag=True, @@ -279,6 +285,7 @@ def cli( upgrade: bool, upgrade_packages: Tuple[str, ...], output_file: Union[LazyFile, IO[Any], None], + newline: str, allow_unsafe: bool, strip_extras: bool, generate_hashes: bool, @@ -515,6 +522,25 @@ def cli( log.debug("") + # Determine linesep for OutputWriter to use + if newline == "preserve": + for fname in (output_file.name, *src_files): + if os.path.exists(fname): + with open(fname, "rb") as existing_file: + existing_text = existing_file.read().decode() + if "\r\n" in existing_text: + newline = "CRLF" + break + elif "\n" in existing_text: + newline = "LF" + break + linesep = { + "native": os.linesep, + "LF": "\n", + "CRLF": "\r\n", + "preserve": "\n", + }[newline] + ## # Output ## @@ -534,6 +560,7 @@ def cli( index_urls=repository.finder.index_urls, trusted_hosts=repository.finder.trusted_hosts, format_control=repository.finder.format_control, + linesep=linesep, allow_unsafe=allow_unsafe, find_links=repository.finder.find_links, emit_find_links=emit_find_links, diff --git a/piptools/writer.py b/piptools/writer.py index 6ab95f8ce..f2ebece06 100644 --- a/piptools/writer.py +++ b/piptools/writer.py @@ -1,3 +1,4 @@ +import io import os import re import sys @@ -98,6 +99,7 @@ def __init__( index_urls: Iterable[str], trusted_hosts: Iterable[str], format_control: FormatControl, + linesep: str, allow_unsafe: bool, find_links: List[str], emit_find_links: bool, @@ -117,6 +119,7 @@ def __init__( self.index_urls = index_urls self.trusted_hosts = trusted_hosts self.format_control = format_control + self.linesep = linesep self.allow_unsafe = allow_unsafe self.find_links = find_links self.emit_find_links = emit_find_links @@ -257,14 +260,25 @@ def write( hashes: Optional[Dict[InstallRequirement, Set[str]]], ) -> None: - for line in self._iter_lines(results, unsafe_requirements, markers, hashes): - if self.dry_run: - # Bypass the log level to always print this during a dry run - log.log(line) - else: - log.info(line) - self.dst_file.write(unstyle(line).encode()) - self.dst_file.write(os.linesep.encode()) + if not self.dry_run: + dst_file = io.TextIOWrapper( + self.dst_file, + encoding="utf8", + newline=self.linesep, + line_buffering=True, + ) + try: + for line in self._iter_lines(results, unsafe_requirements, markers, hashes): + if self.dry_run: + # Bypass the log level to always print this during a dry run + log.log(line) + else: + log.info(line) + dst_file.write(unstyle(line)) + dst_file.write("\n") + finally: + if not self.dry_run: + dst_file.detach() def _format_requirement( self, diff --git a/tests/test_writer.py b/tests/test_writer.py index 7884a4532..91befd242 100644 --- a/tests/test_writer.py +++ b/tests/test_writer.py @@ -42,6 +42,7 @@ def writer(tmpdir_cwd): index_urls=[], trusted_hosts=[], format_control=FormatControl(set(), set()), + linesep="\n", allow_unsafe=False, find_links=[], emit_find_links=True, From 71d5248d96c2c1f6989bc526d17c94f2aef8caf3 Mon Sep 17 00:00:00 2001 From: Andy Kluger Date: Mon, 27 Sep 2021 17:06:18 -0400 Subject: [PATCH 2/4] Add tests for --newline option --- tests/test_cli_compile.py | 82 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 5b9d57f03..4733465f9 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -936,6 +936,88 @@ def test_generate_hashes_with_annotations(runner): ) +@pytest.mark.network +@pytest.mark.parametrize("gen_hashes", (True, False)) +@pytest.mark.parametrize( + "annotate_options", + ( + ("--no-annotate",), + ("--annotation-style", "line"), + ("--annotation-style", "split"), + ), +) +@pytest.mark.parametrize( + ("nl_options", "must_include", "must_exclude"), + ( + pytest.param(("--newline", "lf"), "\n", "\r\n", id="LF"), + pytest.param(("--newline", "crlf"), "\r\n", "\n", id="CRLF"), + pytest.param( + ("--newline", "native"), + os.linesep, + {"\n": "\r\n", "\r\n": "\n"}[os.linesep], + id="native", + ), + ), +) +def test_override_newline( + runner, gen_hashes, annotate_options, nl_options, must_include, must_exclude +): + opts = annotate_options + nl_options + if gen_hashes: + opts += ("--generate-hashes",) + + with open("requirements.in", "w") as req_in: + req_in.write("six==1.15.0\n") + req_in.write("setuptools\n") + req_in.write("pip-tools @ git+https://github.com/jazzband/pip-tools\n") + + runner.invoke(cli, [*opts, "requirements.in"]) + with open("requirements.txt", "rb") as req_txt: + txt = req_txt.read().decode() + + assert must_include in txt + + if must_exclude in must_include: + txt = txt.replace(must_include, "") + assert must_exclude not in txt + + # Do it again, with --newline=preserve: + + opts = annotate_options + ("--newline", "preserve") + if gen_hashes: + opts += ("--generate-hashes",) + + runner.invoke(cli, [*opts, "requirements.in"]) + with open("requirements.txt", "rb") as req_txt: + txt = req_txt.read().decode() + + assert must_include in txt + + if must_exclude in must_include: + txt = txt.replace(must_include, "") + assert must_exclude not in txt + + +@pytest.mark.network +@pytest.mark.parametrize( + ("linesep", "must_exclude"), + (pytest.param("\n", "\r\n", id="LF"), pytest.param("\r\n", "\n", id="CRLF")), +) +def test_preserve_newline_from_input(runner, linesep, must_exclude): + with open("requirements.in", "wb") as req_in: + req_in.write(f"six{linesep}".encode()) + + runner.invoke(cli, ["--newline=preserve", "requirements.in"]) + with open("requirements.txt", "rb") as req_txt: + txt = req_txt.read().decode() + + assert linesep in txt + + if must_exclude in linesep: + txt = txt.replace(linesep, "") + assert must_exclude not in txt + + @pytest.mark.network def test_generate_hashes_with_split_style_annotations(runner): with open("requirements.in", "w") as fp: From 6dad09dfbd7e5a97d08e6f94e71c9b2358adcc8b Mon Sep 17 00:00:00 2001 From: Andy Kluger Date: Mon, 18 Jul 2022 16:19:15 -0400 Subject: [PATCH 3/4] Move linesep determination to a dedicated function, with improvements - skip decode to save time - catch FileNotFoundError to avoid potential race condition Co-authored-by: Thomas Grainger --- piptools/scripts/compile.py | 50 ++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index 2642ed49d..4d0fa4c56 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -49,6 +49,35 @@ def _get_default_option(option_name: str) -> Any: return getattr(default_values, option_name) +def _determine_linesep( + strategy: str = "preserve", filenames: Tuple[str, ...] = () +) -> str: + """ + Determine and return linesep string for OutputWriter to use. + Valid strategies: "LF", "CRLF", "native", "preserve" + When preserving, files are checked in order for existing newlines. + """ + if strategy == "preserve": + for fname in filenames: + try: + with open(fname, "rb") as existing_file: + existing_text = existing_file.read() + except FileNotFoundError: + continue + if b"\r\n" in existing_text: + strategy = "CRLF" + break + elif b"\n" in existing_text: + strategy = "LF" + break + return { + "native": os.linesep, + "LF": "\n", + "CRLF": "\r\n", + "preserve": "\n", + }[strategy] + + @click.command(context_settings={"help_option_names": ("-h", "--help")}) @click.version_option(**version_option_kwargs) @click.pass_context @@ -522,24 +551,9 @@ def cli( log.debug("") - # Determine linesep for OutputWriter to use - if newline == "preserve": - for fname in (output_file.name, *src_files): - if os.path.exists(fname): - with open(fname, "rb") as existing_file: - existing_text = existing_file.read().decode() - if "\r\n" in existing_text: - newline = "CRLF" - break - elif "\n" in existing_text: - newline = "LF" - break - linesep = { - "native": os.linesep, - "LF": "\n", - "CRLF": "\r\n", - "preserve": "\n", - }[newline] + linesep = _determine_linesep( + strategy=newline, filenames=(output_file.name, *src_files) + ) ## # Output From d247f43394151fcce488fbaa2c0862f2c170f826 Mon Sep 17 00:00:00 2001 From: Andy Kluger Date: Mon, 18 Jul 2022 17:50:34 -0400 Subject: [PATCH 4/4] Replace newline choices with --force-lf-newlines flag The default behavior remains identical to what was --newline=preserve. --- piptools/scripts/compile.py | 55 +++++++++++++++-------------------- tests/test_cli_compile.py | 58 +++++++++++++------------------------ 2 files changed, 43 insertions(+), 70 deletions(-) diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index 4d0fa4c56..f35b5f95d 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -49,33 +49,24 @@ def _get_default_option(option_name: str) -> Any: return getattr(default_values, option_name) -def _determine_linesep( - strategy: str = "preserve", filenames: Tuple[str, ...] = () -) -> str: +def _existing_linesep(*filenames: str) -> str: """ - Determine and return linesep string for OutputWriter to use. - Valid strategies: "LF", "CRLF", "native", "preserve" - When preserving, files are checked in order for existing newlines. + Check files in order for an existing linesep and return it, if possible. + Otherwise, return LF ("\n"). """ - if strategy == "preserve": - for fname in filenames: - try: - with open(fname, "rb") as existing_file: - existing_text = existing_file.read() - except FileNotFoundError: - continue - if b"\r\n" in existing_text: - strategy = "CRLF" - break - elif b"\n" in existing_text: - strategy = "LF" - break - return { - "native": os.linesep, - "LF": "\n", - "CRLF": "\r\n", - "preserve": "\n", - }[strategy] + linesep = "\n" + for fname in filenames: + try: + with open(fname, "rb") as existing_file: + existing_text = existing_file.read() + except FileNotFoundError: + continue + if b"\r\n" in existing_text: + linesep = "\r\n" + break + elif b"\n" in existing_text: + break + return linesep @click.command(context_settings={"help_option_names": ("-h", "--help")}) @@ -195,10 +186,10 @@ def _determine_linesep( ), ) @click.option( - "--newline", - type=click.Choice(("LF", "CRLF", "native", "preserve"), case_sensitive=False), - default="preserve", - help="Override the newline control characters used", + "--force-lf-newlines", + is_flag=True, + default=False, + help="Always use LF newlines, rather than auto-detecting from existing files.", ) @click.option( "--allow-unsafe/--no-allow-unsafe", @@ -314,7 +305,7 @@ def cli( upgrade: bool, upgrade_packages: Tuple[str, ...], output_file: Union[LazyFile, IO[Any], None], - newline: str, + force_lf_newlines: bool, allow_unsafe: bool, strip_extras: bool, generate_hashes: bool, @@ -551,8 +542,8 @@ def cli( log.debug("") - linesep = _determine_linesep( - strategy=newline, filenames=(output_file.name, *src_files) + linesep = ( + "\n" if force_lf_newlines else _existing_linesep(output_file.name, *src_files) ) ## diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 4733465f9..c0a055052 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -946,23 +946,8 @@ def test_generate_hashes_with_annotations(runner): ("--annotation-style", "split"), ), ) -@pytest.mark.parametrize( - ("nl_options", "must_include", "must_exclude"), - ( - pytest.param(("--newline", "lf"), "\n", "\r\n", id="LF"), - pytest.param(("--newline", "crlf"), "\r\n", "\n", id="CRLF"), - pytest.param( - ("--newline", "native"), - os.linesep, - {"\n": "\r\n", "\r\n": "\n"}[os.linesep], - id="native", - ), - ), -) -def test_override_newline( - runner, gen_hashes, annotate_options, nl_options, must_include, must_exclude -): - opts = annotate_options + nl_options +def test_force_lf_newlines(runner, gen_hashes, annotate_options): + opts = (*annotate_options, "--force-lf-newlines") if gen_hashes: opts += ("--generate-hashes",) @@ -975,39 +960,36 @@ def test_override_newline( with open("requirements.txt", "rb") as req_txt: txt = req_txt.read().decode() - assert must_include in txt + assert "\n" in txt + assert "\r\n" not in txt - if must_exclude in must_include: - txt = txt.replace(must_include, "") - assert must_exclude not in txt - - # Do it again, with --newline=preserve: - opts = annotate_options + ("--newline", "preserve") - if gen_hashes: - opts += ("--generate-hashes",) +@pytest.mark.network +@pytest.mark.parametrize( + ("linesep", "must_exclude"), + (pytest.param("\n", "\r\n", id="LF"), pytest.param("\r\n", "\n", id="CRLF")), +) +def test_preserve_newlines_from_output_or_input(runner, linesep, must_exclude): + with open("requirements.in", "wb") as req_in: + req_in.write(f"six{linesep}".encode()) - runner.invoke(cli, [*opts, "requirements.in"]) + runner.invoke(cli, ["requirements.in"]) with open("requirements.txt", "rb") as req_txt: txt = req_txt.read().decode() - assert must_include in txt + assert linesep in txt - if must_exclude in must_include: - txt = txt.replace(must_include, "") + if must_exclude in linesep: + txt = txt.replace(linesep, "") assert must_exclude not in txt + # Now that we have good output, + # see that it's preserved when we have bad input: -@pytest.mark.network -@pytest.mark.parametrize( - ("linesep", "must_exclude"), - (pytest.param("\n", "\r\n", id="LF"), pytest.param("\r\n", "\n", id="CRLF")), -) -def test_preserve_newline_from_input(runner, linesep, must_exclude): with open("requirements.in", "wb") as req_in: - req_in.write(f"six{linesep}".encode()) + req_in.write(f"six{must_exclude}".encode()) - runner.invoke(cli, ["--newline=preserve", "requirements.in"]) + runner.invoke(cli, ["requirements.in"]) with open("requirements.txt", "rb") as req_txt: txt = req_txt.read().decode()