Add a way to choose line endings in compiled files

[Jackenmen]

What's the problem this feature will solve?

I'm trying to use the pip-compile hook with my existing pre-commit configuration but one of the hooks I have is a mixed line ending auto-fixer (which I use to ensure all files in the repository use LF for line endings):

repos:
  - repo: https://github.com/jazzband/pip-tools
    rev: 6.2.0
    hooks:
      - id: pip-compile
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v3.4.0
    hooks:
      - id: mixed-line-ending
        args:
          - "--fix=lf"

This configuration causes issues whenever I try to touch a requirements file.

Describe the solution you'd like

A way to set the line endings used for the generated requirements.txt file. The alternative would be for pip-compile to always use \n but then I'm guessing someone else might complain about it in the future like I'm doing now :)

Alternative Solutions

I can work around this issue by excluding requirement files from the check but this doesn't really solve it as I still end up with requirements files ending with CRLF, it just allows me to continue to use both hooks.

Another solution would be a way for --dry-run to return non-zero exit code when the file would have any dependencies changed (like what #882 and #1215 suggest) but this doesn't fully solve it either as then you lose the benefit of pip-compile auto-fixing the issue during pre-commit run.

Additional context

I'm using on Windows - os.linesep is equal to "\r\n" there

[graingert]

Imho pip-compile should reuse whatever line separator it finds first in requirements.txt, and fall back to the first line separator it finds in requirements.in

[AndydeCleyre]

This configuration causes issues whenever I try to touch a requirements file.

Can you please elaborate on the problem? Do you not have a guaranteed order of hook runs (to ensure the line-ending hook runs last)?

[Jackenmen]

Run this on Windows (due to os.linesep being equal to "\r\n" there):

python -m pip install -U pre-commit
git clone https://github.com/jack1142/pip-tools-issue-1448
cd pip-tools-issue-1448
pre-commit run --all-files

See the output:

pip-compile..............................................................Passed
Mixed line ending........................................................Failed
- hook id: mixed-line-ending
- exit code: 1

requirements.txt: fixed mixed line endings

The problem is that pip-compile reintroduces the issue with line endings whenever it runs therefore causing the mixed-line-ending hook to be triggered whenever requirements.in/txt is modified in the commit or when using pre-commit run --all-files. The guaranteed order of hook runs doesn't matter here, pre-commit will fail the commit if any of the hooks fail and that's what is happening here.

[AndydeCleyre]

I've done a first try at https://github.com/AndydeCleyre/pip-tools/tree/feature/linesep -- no tests yet, no PR yet.

@jack1142 can you test out that branch in your pre-commit setup, and add the arg --newline=lf to the config?

[Jackenmen]

@AndydeCleyre Seems to work as intended.

[AndydeCleyre]

If any Windows users are reading this who haven't already responded to the poll about the best default behavior, please go have a look, thanks!

[AndydeCleyre]

@graingert

Imho pip-compile should reuse whatever line separator it finds first in requirements.txt, and fall back to the first line separator it finds in requirements.in

The PR is now defaulting to preserve behavior, which does intend to reuse what it finds in requirements.txt.

Currently there is no checking of input file(s). I would prefer not to bother with that, as for example we may have multiple input files with differing newlines. Do you think this is good enough, for the preserve behavior?

[AndydeCleyre]

Checking in:

There is a PR in good condition that addresses this in a flexible way, ~#1491~ #1652, but an alternative approach has been proposed to just use LF no matter what.

Either solution Works For Me, but I'm requesting comments from Windows users and those with mixed OS dev teams.

[AndydeCleyre]

@ssbarnea @jack1142

Maybe the move forward is to collpase ~#1491~ #1652 into a single new flag, --preserve-newlines (or similar):

• without it, always use LF
• with it, and existing newlines can be detected, use those
• with it, and no existing newlines are detected, use LF

What do you think?

[graingert]

In the usecase stated in the issue "preserve" is a better choice. The pre-commit hook https://github.com/pre-commit/pre-commit-hooks#mixed-line-ending in combination with "preserve" will handle this

[AndydeCleyre]

So more like an --LF flag is called for?

[Jackenmen]

@ssbarnea @jack1142

Maybe the move forward is to collpase #1491 into a single new flag, --preserve-newlines (or similar):

• without it, always use LF
• with it, and existing newlines can be detected, use those
• with it, and no existing newlines are detected, use LF

What do you think?

Seems fine to me though Thomas commented in the PR that preserve should be default (which I'm not sure I agree with but it doesn't matter to me since it's going to end up as LF in both cases anyway)

[ssbarnea]

I am for LF implicit and having a single opt-in preserve option for those looking for one. That keeps the added complexity out of the execution path for most users.

TBH, I do find any developer that does not configure git to force LF as looking for trouble, especially as most tools work well with LF, even on Windows. Just think about mounting and cross platform issues.

[MarkKoz]

TBH, I do find any developer that does not configure git to force LF as looking for trouble, especially as most tools work well with LF, even on Windows. Just think about mounting and cross platform issues.

In my experience, trying to force LF has resulted in similar issues, where some tool edits a file and naïvely uses CRLF. Git does a good job at converting back to LF on check in, so there shouldn't be a need to force a specific line ending on check out.

[AndydeCleyre]

Poll:

• 👀 preserve by default, and add --force-unix-newlines
• 🎉 preserve by default, and add --newline=[LF|CRLF|native|preserve]
• 🚀 LF by default, and add --preserve-newlines
• 😆 preserve, with no alternatives
• 😕 none of the above

You can vote for one or more if you're feeling flexible.

[MarkKoz]

How about preserve by default, and fall back to the line endings in requirements.in / setup.py / other input file as someone suggested? Don't add any option to convert the line endings. I'm not strongly against the conversion options, but they seem redundant. They'd be extra work to support and there'd be more cognitive burden to understand the extra options.

With just the preserve by default behaviour, there will already be several options to force a specific line ending:

1. Don't use the undesired line ending to begin with in the input. If one wants LF for pip-tools's output, then surely they are already using LF for the input files. And if this is the case, then LF will get preserved by pip-tools.
2. Use a pre-commit hook to convert them
3. Rely on git to convert them

Are these insufficient?

[AndydeCleyre]

How about preserve by default, and fall back to the line endings in requirements.in / setup.py / other input file as someone suggested?

To be clear, the "preserve newlines" behavior as implemented in alternative PRs ~#1491~ #1652 and #1584 does include that fall-back-to-input bit.

If one wants LF for pip-tools's output, then surely they are already using LF for the input files.

I'm less sure, and welcome a description of any real user cases violating this prediction.

Theoretically some users may not be using git...

Anyway I'll add an option to the poll:

• 😆 preserve, with no alternatives

[MarkKoz]

Theoretically some users may not be using git...

Yeah, fair enough. The first point is indeed the strongest one I think.

[graingert]

preserve, with no alternatives

means pip-compile crashes when used with data from stdin?

[AndydeCleyre]

preserve, with no alternatives

means pip-compile crashes when used with data from stdin?

No, it means try to use newline from existing output, and failing that, try to use newline from input file, and failing that, use LF.

[AndydeCleyre]

Are all users affected by this issue equally satisfied by alternatives #1652 and #1584?

[Jackenmen]

1584 could use a PR description as the difference between these PRs is not immediately clear.

Based on the code read, it seems that #1584 has the behavior of --newline=LF (from #1652) when --force-lf-newlines is used and --newline=preserve otherwise (with the default being LF for new files). Since I personally am most interested in forcing LF, this does satisfy my needs. I consider preserve the second best option since if the file is already using LF then it's just going to reuse LF and if the file doesn't already exist then it will use LF as default.

I personally did not see the need for --newline=CRLF or the worst of all - --newline=native which makes the output dependent on the OS you're using.

[MarkKoz]

The most important thing to me is that pip-tools stops producing files with mixed line endings. I don't care what the line ending is as long as its consistent. Both PRs seem to accomplish this, but I lean towards #1584 since it's simpler. In fact, I don't think a way to force line endings is needed (as outlined in https://github.com/jazzband/pip-tools/issues/1448#issuecomment-1057494252, there are alternatives to accomplish this). Not a dealbreaker though; it's an extra argument but at least it doesn't seem to have much impact on the complexity of the implementation.

[graingert]

--newline=native is useful for backwards compatibility --newline=CRLF is useful for some users who are Windows only