770286 - Add .gitattributes file

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

We should probably have a .gitattributes file checked in to the root directory of mozilla-central to enforce conventions.

https://www.kernel.org/pub/software/scm/git/docs/gitattributes.html

At the very least, we should probably define EOL conventions so LF is preserved everywhere (I'm pretty sure checkouts on Windows using Mercurial use LF, so we should force LF everywhere).

We could also add in default diff arguments so patches are produced with Mozilla's preferred 8 lines of context.

And, if we want to be slightly sneaky, we could probably set some whitespace config parameters so patches with trailing whitespace are treated as errors. (This is the default git behavior, but some people may have it disabled.)

Comment 1

[Justin Wood (:Callek)]

CC Hal, as he is working on m-c<->github mirroring

Comment 2

[Hal Wine [:hwine] use NI!]

(In reply to Justin Wood (:Callek) from comment #1)
> CC Hal, as he is working on m-c<->github mirroring

Correction: we're supporting m-c -> git read only mirroring. All changes to m-c must be landed on hg.m.o

I believe this is simply a coding style issue for the dev community to reach consensus on and implement. It should have zero impact on either the CI system or hg -> git work.

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

As of now this is not just a coding style issue. Building SpiderMonkey on Windows Subsystem for Linux fails when the local repository is cloned on Windows as CRLF autoconversion causes shell syntax error and Rust file checksum errors.

Comment 5

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: 0001-Bug-770286-Force-LF-normalization-for-git.patch

Comment 6

[Kagami Rosylight [:saschanaz] (they/them)]

Adding needinfo to the relevant person...

Comment 7

[Hal Wine [:hwine] use NI!]

I'm no longer relevant here -- Connor?

Comment 8

[Connor Sheehan [:sheehan]]

I'm not sure if the patch from comment 5 would fix the issue reported in comment 4, or if there's a better way to resolve that issue.

glandium, does this look okay to you?

As an aside, we're no longer accepting patches through Bugzilla. Please submit this through Phabricator.

Comment 9

[Mike Hommey [:glandium]]

The premise in comment 0 doesn't seem to be true. AFAICT, committing a file with CRLF endings on Windows with mercurial keeps the file with CRLF endings in the repo. That's probably why we have some files that this in the tree.
The .gitattributes attached in comment 5 would make the git behavior nice, sure, but it would also be different from mercurial.

Comment 10

[Kagami Rosylight [:saschanaz] (they/them)]

(In reply to Mike Hommey [:glandium] from comment #9)

committing a file with CRLF endings on Windows with mercurial keeps the file with CRLF endings in the repo.

In that case I suggest two things:

1. Turn off git autocrlf completely (* binary) so that it matches the mercurial behavior.
2. Or, add a mercurial config to use LF on Windows.

Comment 11

[Kagami Rosylight [:saschanaz] (they/them)]

I tried doing git checkout with * text=auto eol=lf and found the files that already added with CRLF in the repo won't lose their CR even with the settings. So I think my previous suggestion already matches mercurial checkout behavior. What do you think?

Comment 12

[Mike Hommey [:glandium]]

I don't personally have a strong opinion on which particuliar behavior should be hardcoded, but whatever it is, we should make sure both mercurial and git do the same thing.

Comment 13

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 770286: Prevent git LF-to-CRLF autoconversion

Currently git for Windows converts LF to CRLF by default, so this change prevents that to match the mercurial behavior.

Comment 14

[Kagami Rosylight [:saschanaz] (they/them)]

We could do * -text but it will break current git clones by detecting all already normalized files as uncommitted new changes. I found it too scary, I prefer we do * text=auto eol=lf to turn off the current unexpected LF-to-CRLF conversions first and then discuss about -text. Thoughts?

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4762f5a28a72
Prevent git LF-to-CRLF autoconversion r=glandium

Comment 16

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/4762f5a28a72