Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --newline=[LF|CRLF|native|preserve] option to pip-compile #1652

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 41 additions & 0 deletions piptools/scripts/compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -159,6 +188,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,
Expand Down Expand Up @@ -272,6 +307,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,
Expand Down Expand Up @@ -503,6 +539,10 @@ def cli(

log.debug("")

linesep = _determine_linesep(
strategy=newline, filenames=(output_file.name, *src_files)
)

##
# Output
##
Expand All @@ -522,6 +562,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,
Expand Down
30 changes: 22 additions & 8 deletions piptools/writer.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import io
import os
import re
import sys
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW this could be cleaner if implemented with a context manager conditionally returning a dummy I/O object.

Copy link
Member

@graingert graingert Oct 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done here #1698

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,
Expand Down
82 changes: 82 additions & 0 deletions tests/test_cli_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like with modern Pythons it'd be nicer if Pathlib.write_text() was used. But more importantly, why don't you use the built-in tmp_path fixture to write temporary files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to make any changes you like, and thanks! This change isn't for me personally, and I have COVID right now and can't bring my full focus to the task.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try and make a PR with this fix in!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done here: #1694

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:
Expand Down
1 change: 1 addition & 0 deletions tests/test_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down