Re: What's cooking extra

From: Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com>
Date: May 22, 2010, 19:42 UTC
Message-ID: <50199F1F-3513-43A6-8990-957F3D0AF58C@gmail.com>
In-Reply-To: <20100522130916.GA28452@localhost>

On 22. mai 2010, at 15.09, Clemens Buchacher wrote:

Show 21 quoted lines

> On Wed, May 19, 2010 at 07:06:56PM +0200, Finn Arne Gangstad wrote:
>> On Wed, May 19, 2010 at 07:33:32AM -0700, Junio C Hamano wrote:
>> 
>>> * (Eyvind Bernhardsen and Linus) Fixing the behaviour of crlf attribute;
>>>   ignoring them when core.autocrlf is not in effect was a wrong design
>>>   decision.
>>> 
>>>   I agree with what Linus said in the thread; I haven't yet looked at the
>>>   discussion in the past few days.  Also I don't know where '[PATCH v2]
>>>   Add "core.eol" config variable' fits in the picture.
>> 
>> I think this one is pretty much discussed by now, with the latest
>> changes this should do pretty much what Linus wanted.
> 
> That is not the impression I got. Linus was objecting to the idea of new
> attribute and configuration variables, which essentially do the same thing
> but with slightly different semantics.
> 
> As soon as the existing crlf attribute is given priority over core.autocrlf,
> all the problems discussed originally go away. So what exactly are the new
> attributes supposed to do?

There is one new attribute, "eol", that is used for files which need a specific line ending.  Being able to "force" LF or CRLF line endings has been requested several times on the list, and is already sort of provided for by "crlf=input".

Linus had lots of strong objections, but I also made lots of changes during the course of the discussion.

> Also, could you post a truth table for all the parameters involved (eol,
> crlf, core.autocrlf, core.eol). The documentation in the patches is too
> confusing for me to understand even that.

I'll do my best.  Basically there are two things to keep track of: "should this file be normalized as text?" and "which line endings should this file have in the working directory?".  There is an attribute for each, and two configuration variables that set the default values of the attributes.

Unfortunately my mail client mangles ascii art, so I can't do a table.  This will have to suffice:

Any file with the "text" attribute set will have its line endings normalized to LF in the repository.  If "text" is set to the special value "auto", git will only convert the file if it looks like a text file.

The "eol" attribute is used for files that need a specific line ending.  Setting it also sets "text".

core.eol controls which line endings to use for normalized files that don't have the "eol" attribute set, and defaults to the platform native line ending.

When core.autocrlf is set, the default value of the "text" attribute is set to "auto" but with an extra safety feature: if a file contains CRs in the index, it won't be normalized.  The extra feature comes from Finn Arne's "safe autocrlf" patch.

There is a backwards compatibility wrinkle in that core.autocrlf will override core.eol if the latter isn't explicitly set, so that "core.autocrlf=true" still results in CRLFs in the working directory on Linux.

> And, renaming the crlf attribute to text? Where did Linus suggest that? If
> we do that, we don't even have to talk about backwards compatibility any
> more.

In <alpine.LFD.2.00.1005121824260.3711@i5.linux-foundation.org>:

> So if you rename these things, keep them separate.  Make the "am I a
> text-file" boolean be a boolean (plus "auto"), and just call it "text". 
> And make the "what end of line to use" be just "eol" then.

So he didn't suggest renaming it, but he did suggest a better name and UI than the one I came up with.  I expected objections to renaming the attribute, which is why that is a separate commit, but people seemed supportive overall.

The "crlf" attribute will be used if it is present so backwards compatibility is preserved to a degree.  Scripts that test for the "crlf" attribute explicitly (such as git-cvsserver, which I fixed) will break.  I don't know how big a problem that is going to be in practice, but nobody raised it as an issue during the discussion.

Compatibility with older clients is a valid concern, but older clients will ignore "crlf" as well as "text" unless core.autocrlf is set, so they will cause problems no matter what.

-- 
Eyvind