[rush] warning: CRLF will be replaced by LF in $file

[moltar]

Is this a feature or a bug?

• [ ] Feature
• [x] Bug

Please describe the actual behavior.

rush init produces files with Windows line breaks.

If that is not intentional, then it is a bug.

But since this is Microsoft, then maybe that is intentional after all.

mkdir foo
cd foo
git init .
rush init
git add .

warning: CRLF will be replaced by LF in .gitattributes.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in .gitignore.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in .travis.yml.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in common/config/rush/.npmrc.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in common/config/rush/command-line.json.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in common/config/rush/common-versions.json.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in common/config/rush/pnpmfile.js.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in common/config/rush/version-policies.json.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in rush.json.
The file will have its original line endings in your working directory

If the issue is a bug, how can we reproduce it? Please provide detailed steps and include a GitHub branch if applicable. Your issue will get resolved faster if you can make it easy to investigate.

What is the expected behavior?

To have LF by default.

If this is a bug, please provide the tool version, Node.js version, and OS.

• Tool: rush
• Tool Version: 5.11.2
• Node Version: 10.15.3
  • Is this a LTS version? yes
  • Have you tested on a LTS version? yes
• OS: macOS

[iclanton]

@octogonz - We intentionally picked one style. Do you remember what the rationale was for picking the one we picked?

[octogonz]

We originally standardized on CRLF because it was handled correctly by all editors on all OS's, whereas LF was mishandled by some editors (e.g. Notepad.exe and cmd.exe on Windows). The convention doesn't matter too much in our repos, because our .gitattributes file automatically normalizes line endings during commit.

(BTW this normalization is uniform across all OS's. I'm very opposed to the idea of writing LF on Mac and CRLF on Windows, because we've seen cases where this leads to build errors that only repro on a specific OS.)

These normalization lines are missing from the .gitattributes file written by rush init, which I suppose is why that warning is being reported. In other words, the rush init output produces Git warnings. That's definitely a bug.

As of Windows 10, pretty much all editors now support LF correctly. Thus, I have been wanting to change our convention to be LF instead of CRLF. LF is more widely used in the NodeJS community, and many libraries write \n instead of os.EOL, so correctly implementing CRLF often requires an annoying extra postprocessing.

Proposal: Let's update all of our tools to always write LF by default. @iclanton What do you think?

[octogonz]

@iclanton @patmill @lahuey If we make this change, it would impact SPFx as well.

[iclanton]

I support moving everything to LF. I doubt the change itself will be too complicated, but this change will need to be extensively tested.

[octogonz]

Cool, let's do it.

[elliot-nelson]

Arrived here via the zulipchat... I'm a little surprised that we would want the LF or CRLF globally instead of OS-specific.

Especially if we are updating a file that already exists, I'd think the gold standard would be to detect the existing line endings and then use those when we update the file -- this way rush's edits are unobtrusive (and avoid unwanted git diff) whether a Windows user uses LF or CRLF checkouts.

[berenddeboer]

And we still have the dos line endings.

[octogonz]

I'd think the gold standard would be to detect the existing line endings and then use those when we update the file

This would produce nondeterministic results, and the behavior is undefined for a file that has no newlines.

[octogonz]

@iclanton and I discussed this today, and came to these conclusions:

• Which behavior is ideal depends on Git's core.autocrlf setting, but it's impractical for our tools to query that (i.e. read your Git config and/or .gitattributes file).

• Thus, Rush needs to make an opinionated recommendation for core.autocrlf and then update our tools around that policy

• Git's default setting for core.autocrlf is to preserve newlines, which is obviously bad (because it allows a file to be committed with both CR/CRLF). So that can't be our recommendation.

• The Windows installer for Git gives the user three options:

  

• Option 3 is the bad Git default, so basically our choice is between options 1 and 2

• Both choices behave the same on non-Windows operating systems, so Mac/Linux users should not get a vote 😁

The pro/cons break down as follows:

1. CRLF on Windows, LF on Mac/Linux: (core.autocrlf=true) This setting is probably the most widespread default. It is the initially selected radio button in the screenshot above. And for example, if you create a new file on Windows, pretty much any text editor will default to CRLF endings.
2. LF on every OS: (core.autocrlf=input) If we used LF consistently everywhere, it would reduce the risk of OS-specific build breaks.👍 It also would eliminate the need to litter our code base with annoying calls to APIs like os.EOL and Text.convertTo().👍 Now that Windows 10 thoroughly supports LF, there's really no technical downside to using LF.

As a purist, I'm strongly attracted to option 2. But as a member of an open source community where users interact with hundreds of different Git repos, it seems like an uphill battle to reconfigure all the tools that default to CRLF on Windows. If we have to deal with CRLF at all, injecting it into every file has a benefit of forcing us to test CRLF more thoroughly and catch any bugs more quickly.

Thus after some deliberation, we've decided to go with option 1 (core.autocrlf=true).

👉If anyone has feedback or concerns, please reply to this thread.

The rush init templates will get fixed wtih PR #3114, but I'd like to keep issue #1467 open until this policy has been rolled out to every list Rush Stack project.

[octogonz]

Footnote: A possible fourth option would be to set core.safecrlf=false which suppresses Git's CRLF will be replaced by LF warning. That would technically resolve this GitHub issue, but it seems like a sloppy solution. :-)

[eldarshamukhamedov]

@octogonz catching up on the CRLF vs LF debates, so forgive me if I missed something 😅 . I am running Rush 5.64.0 and rush update still consistently generates browser-approved-packages.json files with CRLF, even though I'm running on a Linux environment. Is that expected? I've been using Prettier to force LF in a Git hook, so it's not really an issue, but was curious.