[PATCH/RFC v2 0/4] End-of-line normalization, take 2 (now only slightly scary)

[PATCH/RFC v2 0/4] End-of-line normalization, take 2 (now only slightly scary)

[Eyvind Bernhardsen]

Okay, here's a second shot at my end-of-line conversion sanification
series.  The main change is that instead of introducing a new attribute,
I've added a new mode to the "crlf" attribute: "crlf=auto".

The semantics of "crlf" have also been modified slightly in that the
attribute now enables end-of-line normalization whenever it is set,
instead of simply amending what happens when "core.autocrlf" is enabled
as it did before.

I think this version is a step in a direction that everyone can agree
on.  The new config variable seems to be a point of contention, but
while I'm not married to the name "core.eolStyle" by any means, I don't
think "core.crlf" explains what it is, and I don't think "input" and
"true" are good ways of describing what it does.

I agree with Linus that "crlf=input" seems crazy, and I agree with
Dmitry that "crlf" is a bad name for the attribute to begin with.  This
isn't bikeshedding: git has lost a lot of its sharp edges recently, but
this one remains, and is quite an important one for Windows users.

For now, though, I'd like the focus for this patch series to be creating
a mechanism that allows a project to enforce line ending normalization
for all contributors to the repository, while letting the user decide
what line endings he likes to see in his working directory.  If we end
up not shooting anybody in the head, all the better.

Thanks for all your inputs.  I was worried that nobody would even notice
this series :)  I've expanded the Cc list a bit, hope that's okay.

Eyvind Bernhardsen (4):
    Add "core.eolStyle" variable to control end-of-line conversion
    Add tests for per-repository eol normalization
    Pass eol conv mode as an argument instead of using global auto_crlf
    Add per-repository eol normalization

 Documentation/config.txt        |   11 ++-
 Documentation/gitattributes.txt |   71 +++++++++++-----
 Makefile                        |    3 +
 cache.h                         |   19 ++++
 config.c                        |   16 +++-
 convert.c                       |   52 +++++++++---
 environment.c                   |    1 +
 t/t0025-crlf-auto.sh            |  180 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 317 insertions(+), 36 deletions(-)
 create mode 100755 t/t0025-crlf-auto.sh

[PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

Introduce a new configuration variable, "core.eolStyle", that allows the
user to set which line endings to use for end-of-line-normalized files
in the working directory.  It defaults to "native", which means CRLF on
Windows and LF everywhere else.

Signed-off-by: Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com>
---
 Documentation/config.txt |    7 +++++++
 Makefile                 |    3 +++
 cache.h                  |   19 +++++++++++++++++++
 config.c                 |   16 +++++++++++++++-
 environment.c            |    1 +
 5 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 92f851e..3956ff7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -207,6 +207,13 @@ core.autocrlf::
 	the file's `crlf` attribute, or if `crlf` is unspecified,
 	based on the file's contents.  See linkgit:gitattributes[5].
 
+core.eolStyle::
+	Sets the line ending type to use for text files in the working
+	directory when the `auto-eol` property is set.  Alternatives are
+	'lf', 'crlf', 'native' and 'false'.  'native', the default, uses
+	the platform's native line ending.  'false' disables `auto-eol`
+	line ending conversion.  See linkgit:gitattributes[5].
+
 core.safecrlf::
 	If true, makes git check if converting `CRLF` as controlled by
 	`core.autocrlf` is reversible.  Git will verify if a command
diff --git a/Makefile b/Makefile
index 910f471..419532e 100644
--- a/Makefile
+++ b/Makefile
@@ -224,6 +224,8 @@ all::
 #
 # Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded
 # dependency rules.
+#
+# Define NATIVE_CRLF if your platform uses CRLF for line endings.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -989,6 +991,7 @@ ifeq ($(uname_S),Windows)
 	NO_CURL = YesPlease
 	NO_PYTHON = YesPlease
 	BLK_SHA1 = YesPlease
+	NATIVE_CRLF = YesPlease
 
 	CC = compat/vcbuild/scripts/clink.pl
 	AR = compat/vcbuild/scripts/lib.pl
diff --git a/cache.h b/cache.h
index 5eb0573..690511e 100644
--- a/cache.h
+++ b/cache.h
@@ -561,6 +561,25 @@ enum safe_crlf {
 
 extern enum safe_crlf safe_crlf;
 
+enum auto_crlf {
+	AUTO_CRLF_FALSE = 0,
+	AUTO_CRLF_TRUE = 1,
+	AUTO_CRLF_INPUT = -1,
+};
+
+enum eol_style {
+	EOL_STYLE_FALSE = AUTO_CRLF_FALSE,
+	EOL_STYLE_CRLF = AUTO_CRLF_TRUE,
+	EOL_STYLE_LF = AUTO_CRLF_INPUT,
+#ifdef NATIVE_CRLF
+	EOL_STYLE_NATIVE = EOL_STYLE_CRLF,
+#else
+	EOL_STYLE_NATIVE = EOL_STYLE_LF,
+#endif
+};
+
+extern enum eol_style eol_style;
+
 enum branch_track {
 	BRANCH_TRACK_UNSPECIFIED = -1,
 	BRANCH_TRACK_NEVER = 0,
diff --git a/config.c b/config.c
index 6963fbe..8a11052 100644
--- a/config.c
+++ b/config.c
@@ -461,7 +461,7 @@ static int git_default_core_config(const char *var, const char *value)
 
 	if (!strcmp(var, "core.autocrlf")) {
 		if (value && !strcasecmp(value, "input")) {
-			auto_crlf = -1;
+			auto_crlf = AUTO_CRLF_INPUT;
 			return 0;
 		}
 		auto_crlf = git_config_bool(var, value);
@@ -477,6 +477,20 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.eolstyle")) {
+		if (value && !strcasecmp(value, "lf"))
+			eol_style = EOL_STYLE_LF;
+		else if (value && !strcasecmp(value, "crlf"))
+			eol_style = EOL_STYLE_CRLF;
+		else if (value && !strcasecmp(value, "native"))
+			eol_style = EOL_STYLE_NATIVE;
+		else if (! git_config_bool(var, value))
+			eol_style = EOL_STYLE_FALSE;
+		else
+			return error("Malformed value for %s", var);
+		return 0;
+	}
+
 	if (!strcmp(var, "core.notesref")) {
 		notes_ref_name = xstrdup(value);
 		return 0;
diff --git a/environment.c b/environment.c
index 876c5e5..05cd1d5 100644
--- a/environment.c
+++ b/environment.c
@@ -40,6 +40,7 @@ const char *editor_program;
 const char *excludes_file;
 int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
 int read_replace_refs = 1;
+enum eol_style eol_style = EOL_STYLE_NATIVE;
 enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
 enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
-- 
1.7.1.3.gb95c9

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Linus Torvalds]

On Sat, 8 May 2010, Eyvind Bernhardsen wrote:
>
> Introduce a new configuration variable, "core.eolStyle", that allows the
> user to set which line endings to use for end-of-line-normalized files
> in the working directory.  It defaults to "native", which means CRLF on
> Windows and LF everywhere else.

So I at least agree with the semantics now, but I think that we really 
would be better off just calling it "core.crlf". I don't _care_ whether 
people think it's a bad name - much worse than a bad name is to use a name 
that is not consistent. 

Having two config variables named "core.autocrlf" and "core.crlf" at least 
is consistent. Having "autocrlf" and "eolStyle" is just messed up.

		Linus

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

On 8. mai 2010, at 23.57, Linus Torvalds <torvalds@linux- 
foundation.org> wrote:

> Having two config variables named "core.autocrlf" and "core.crlf" at  
> least
> is consistent. Having "autocrlf" and "eolStyle" is just messed up.

I see your point, but "crlf=lf" makes no sense and "crlf=input" is  
hideous. It's bad enough that autocrlf uses "true" and "input"; this  
is a chance to reduce the insanity, not perpetuate it.

I'll try to think of a better name.
-- 
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

On 9. mai 2010, at 00.17, Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com 
 > wrote:

> I'll try to think of a better name.

Heh. How about "localcrlf={true,false,native}"?

It breaks down a bit if we ever decide to support old-school-Mac-style  
CR line endings, but at that point you're approaching the borders of  
madness anyway.
-- 
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Linus Torvalds]

On Sun, 9 May 2010, Eyvind Bernhardsen wrote:
> 
> Heh. How about "localcrlf={true,false,native}"?

I really don't understand that. What would it even mean?

An you _do_ realize that like it or not, we already have "crlf=input" as 
the syntax in our .gitattributes files? So that exact syntax already 
exists in one place.

As mentioned, I really can understand people not liking the name, but we 
already _have_ that name, and that syntax. I think it makes more sense to 
try to have a unified syntax than have two different strings for the same 
thing.

So I think we'd be better off with good documentation with a couple of 
real examples (and easily findable), so that the naming is at least 
something people can look up and see the semantics for. The "eol" vs 
"crlf" thing is just bike shedding, and we already ended up with "crlf". 
In contrast, making docs understandable is more than bikeshedding.

			Linus

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

On 9. mai 2010, at 01.08, Linus Torvalds wrote:

> On Sun, 9 May 2010, Eyvind Bernhardsen wrote:
>> 
>> Heh. How about "localcrlf={true,false,native}"?
> 
> I really don't understand that. What would it even mean?

Sorry, I should have explained it.  I chided you for "crlf=input" being crazy, but then I realized that "crlf=true" really isn't too bad a config variable for saying that you want CRLFs.  Just "crlf" seems a bit obscure still, so I came up with "localcrlf": if it's "true" it means you want CRLFs locally (in your working directory), if it's "false" you want LFs.

It helps to think about the situations where you're likely to change this. If you're on Linux but you want CRLFs in your working directory, set "localcrlf=true".  If you're on Windows but don't want CRLFs, "localcrlf=false".  Thinking about it, "input" could be an alias for "false", which would make it _exactly_ what you suggested except for a slight clarification in the name.

> An you _do_ realize that like it or not, we already have "crlf=input" as 
> the syntax in our .gitattributes files? So that exact syntax already 
> exists in one place.

I'm sorry.  Are you the same Linus Torvalds who wrote this:

> Btw, since we're discussing this, I do think that our current "crlf=input" 
> syntax for .gitattributes is pretty dubious.

?  I agree with that sentiment.  "crlf=input" is silly; if you have a file that always has to be LF no matter what, just set it to "-crlf" and make sure you don't accidentally introduce any CRs.  That is what you have to do for a file that always has to be CRLF, after all.  As always, I'm not suggesting getting rid of "crlf=input"; I would keep it for backwards compatibility, but not emphasize it in the documentation.

> As mentioned, I really can understand people not liking the name, but we 
> already _have_ that name, and that syntax. I think it makes more sense to 
> try to have a unified syntax than have two different strings for the same 
> thing.

I agree with this!  I just don't agree that the unified syntax should be married to the old, poor syntax, and "crlf=input" is a straw man.  It's both ugly and unnecessary.

> So I think we'd be better off with good documentation with a couple of 
> real examples (and easily findable), so that the naming is at least 
> something people can look up and see the semantics for. The "eol" vs 
> "crlf" thing is just bike shedding, and we already ended up with "crlf". 
> In contrast, making docs understandable is more than bikeshedding.

Making the docs understandable is important.  I have changed the docs, and I will keep trying to improve them.  But making the configuration understandable is _also_ important!  It's not just the colour of the bike shed, it's how it _works_.  This discussion is not a waste of time.

But if you insist, here's how it looks to me: I'm the one trying to build a new bike shed from the parts of the grotty one that nobody really likes to use, and you're the one saying I need to paint it in the same 70s brown-and-mustard colour scheme as the old one.

The frustrating thing is that you're also coming up with some interesting colour combinations elsewhere, but you seem resigned to the idea that we're stuck with the ugliness.  I don't think we are.
-- 
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Linus Torvalds]

On Sun, 9 May 2010, Eyvind Bernhardsen wrote:
> 
> I'm sorry.  Are you the same Linus Torvalds who wrote this:
> 
> > Btw, since we're discussing this, I do think that our current "crlf=input" 
> > syntax for .gitattributes is pretty dubious.

Yes, it's dubious. But as with the kernel, we need to support backwards 
compatibility for things that have reasonably been used (and "input" has).

I really brought it up as an example of things that weren't necessarily 
all that well designed.

That said, it looks like people actually do want per-file line-ending 
settings, ie not just a global "I want CRLF vs LF". So it looks like 
crlf=input is actually useful in a .gitattributes files, if only because 
some people seem to want to mix CRLF and just LF in the same repository.

It also sounds like people actually want to have the reverse (ie not just 
"input", but have a mode where LF may be the default, but then some 
particular files must always be CRLF even if most files are normal text).

So I suspect we want to really have support for all four combinations 
_both_ in the .git/config file, _and_ in the .gitattributes file.

The four cases would be "none" ("binary" or "-crlf"), "lf" ("input" or 
"crlf=input"), "system default", and "force crlf".

Honestly, I would personally have preferred to have just a repo-wide "this 
is the line ending". Not some path-specific endings like "crlf=input" in 
the .gitattributes. But people do seem to want it.

			Linus

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

On 9. mai 2010, at 20.11, Linus Torvalds wrote:

> On Sun, 9 May 2010, Eyvind Bernhardsen wrote:
>> 
>> I'm sorry.  Are you the same Linus Torvalds who wrote this:
>> 
>>> Btw, since we're discussing this, I do think that our current "crlf=input" 
>>> syntax for .gitattributes is pretty dubious.
> 
> Yes, it's dubious. But as with the kernel, we need to support backwards 
> compatibility for things that have reasonably been used (and "input" has).

Oh, absolutely.  I'm not suggesting removing support for it, I just don't think it's a good reason to keep the old naming alive.

> I really brought it up as an example of things that weren't necessarily 
> all that well designed.
> 
> That said, it looks like people actually do want per-file line-ending 
> settings, ie not just a global "I want CRLF vs LF". So it looks like 
> crlf=input is actually useful in a .gitattributes files, if only because 
> some people seem to want to mix CRLF and just LF in the same repository.
> 
> It also sounds like people actually want to have the reverse (ie not just 
> "input", but have a mode where LF may be the default, but then some 
> particular files must always be CRLF even if most files are normal text).

My plan was to "support" that by disabling conversion for those files; git wouldn't enforce the line endings, but at least it wouldn't break them.

I guess there's no reason not to have the option to enforce in there if there is a huge demand for that.

> So I suspect we want to really have support for all four combinations 
> _both_ in the .git/config file, _and_ in the .gitattributes file.
> 
> The four cases would be "none" ("binary" or "-crlf"), "lf" ("input" or 
> "crlf=input"), "system default", and "force crlf".

Well, "crlf=crlf" and "crlf=lf" would look silly, but no more than I could live with.  .gitconfig is already covered in my series.
-- 
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Dmitry Potapov]

On Sun, May 09, 2010 at 12:53:17AM +0200, Eyvind Bernhardsen wrote:
> On 9. mai 2010, at 00.17, Eyvind Bernhardsen
> <eyvind.bernhardsen@gmail.com> wrote:
> 
> >I'll try to think of a better name.
> 
> Heh. How about "localcrlf={true,false,native}"?

IMHO, the 'local' prefix certainly does not improve anything. Also,
I would rather call default as "default" instead of "native". So,
why not use "core.crlf={true, false, default}"?

Though crlf is not my preferable name, I think consistency is important,
and we should use the same name here as in git attributes.


Dmitry

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[hasen j]

On 9 May 2010 01:00, Dmitry Potapov <dpotapov@gmail.com> wrote:
> [...] Also,
> I would rather call default as "default" instead of "native". So,
> why not use "core.crlf={true, false, default}"?

default and native have completely different connotations. default
makes me think "one of true or false, which ever happens to be the
default". native is a better fit here.

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Dmitry Potapov]

On Sun, May 09, 2010 at 01:30:10AM -0600, hasen j wrote:
> On 9 May 2010 01:00, Dmitry Potapov <dpotapov@gmail.com> wrote:
> > [...] Also,
> > I would rather call default as "default" instead of "native". So,
> > why not use "core.crlf={true, false, default}"?
> 
> default and native have completely different connotations. default
> makes me think "one of true or false, which ever happens to be the
> default". native is a better fit here.

but indeed core.crlf=default means that it is either true or false
whatever happened the system default, as well as "default" means the
same as if it would be if it is not specified at all.  If it were
eol=native, it would be okay, but crlf=native looks strange to me.


Dmitry

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

On 9. mai 2010, at 09.00, Dmitry Potapov wrote:

> On Sun, May 09, 2010 at 12:53:17AM +0200, Eyvind Bernhardsen wrote:
>> On 9. mai 2010, at 00.17, Eyvind Bernhardsen
>> <eyvind.bernhardsen@gmail.com> wrote:
>> 
>>> I'll try to think of a better name.
>> 
>> Heh. How about "localcrlf={true,false,native}"?
> 
> IMHO, the 'local' prefix certainly does not improve anything. Also,
> I would rather call default as "default" instead of "native". So,
> why not use "core.crlf={true, false, default}"?
> 
> Though crlf is not my preferable name, I think consistency is important,
> and we should use the same name here as in git attributes.

But the attribute does something different!  The attribute turns eol conversion on and off, the configuration variable decides which line endings to use when conversion is on.  They should be related, but making them identical doesn't make any kind of sense.

To be consistent, I would expect a variable called "core.crlf" to do the same thing as the attribute, so "core.crlf=auto" would replace "core.autocrlf={true,input}", allowing you to turn on line ending conversion without having to modify the repository.

If we added this consistently named "core.crlf", we'd still need a separate config variable to decide between LFs and CRLFs in the working directory, so what should that variable be called?

"localcrlf" at least conveys the idea that this is a local setting, so it's not too much of a stretch to guess that it controls the working directory.
-- 
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

I guess I should nail my flag to the mast: Here's what I would have done, with the benefit of plenty of hindsight, had we not had core.autocrlf, and also what I think we should do to approach that ideal.

Please don't get hung up too much on the names, they were chosen to not match anything suggested so far so that I can refer back to them unambiguously.

My user interface would have been:

- an attribute "eolconv" that enables or disables line ending conversion
- a config variable "core.eolconv" that sets "eolconv" for all files where it is unset
- a config variable "core.localeol" that decides whether LF or CRLF is preferred

This provides the means to enable normalization on a per-project ("eolconv") or per-repository ("core.eolconv") basis, and allows the user to override the platform native line ending when normalization is in effect.


Now, how does that compare to Git's current implementation of autocrlf?

- The config variable "core.autocrlf" enables or disables line ending conversion and decides if conversion occurs in "both" directions ("autocrlf=true") or just towards the repository ("autocrlf=input").

- The attribute "crlf" allows the automatic detection of binary files to be overridden, and forcing one-way conversion even if "core.autocrlf" is true ("crlf=input").  It only has an effect when "core.autocrlf" is enabled.

I think I've stated my case against these settings before, but just to be clear, I think the biggest problem is that there's no way to tell if a repository is normalized from the contents of the repository, and it's not safe to enable autocrlf if the repository isn't normalized.

The second biggest problem is that "true" and "input" are bad names for "normalize and put CRLFs in my working directory" and "normalize and put LFs in my working directory", respectively.


So how to progress from here?

As Junio observed, the "crlf" attribute can act just like my hypothetical "eolconv" if you squeeze its semantics a bit and add a new setting.  Disregarding "crlf=input" makes it exactly equivalent to "eolconv".  The name is a bit off, but it's not monstrous.

"core.localeol" has no equivalent in the current implementation.  Using "core.autocrlf" as a stand in would not work, since that setting is expected to actually enable normalization, so if we want a moral equivalent of "eolconv", we need a new setting.  "core.crlf" has been suggested to match the "crlf" attribute, but that causes confusion: why doesn't setting "core.crlf=auto" mean the same thing as setting "crlf=auto" on all files?  I think a new variable is needed.

"core.autocrlf" is problematic because it mixes up two things: you use it to turn on normalization _and_ to decide which line endings you prefer, and to maintain backwards compatibility its semantics can't be changed too much.  A new setting "core.autocrlf=auto" (meaning the same as "eolconv=auto" on all files where "eolconv" isn't explicitly set) is a possibility, if kind of ugly.  Another alternative is to make "core.autocrlf=true" respect "core.localeol", but that would be a v1.8.0 kind of change.


My current thinking on how to change my series now runs along these lines:

- keep the current "crlf=auto" change
- rename "core.eolStyle" to "core.localcrlf"
- add a "core.crlf" that sets the "crlf" attribute on paths where it isn't explicitly configured
- keep "core.autocrlf" for backwards compatibility, but make "core.autocrlf=input" and "core.autocrlf=true" complain if they are in conflict with the other config settings.



An apology: I know I've said a lot of nasty things about autocrlf, but I haven't had to touch a thing about its implementation, which works very well.  My only beef is with its user interface (which probably made sense when it was implemented); the reason I don't think I'm just whining about the colour of the bike shed is that a lot of other people trip over that interface, to the point where the standard recommendation is simply to disable the feature, and I'd like to make it both more functional and more understandable.

In short, I'm burning autocrlf to save it :)
-- 
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Robert Buck]

> My user interface would have been:
>
> - an attribute "eolconv" that enables or disables line ending conversion
> - a config variable "core.eolconv" that sets "eolconv" for all files where it is unset
> - a config variable "core.localeol" that decides whether LF or CRLF is preferred
>

[...]

> My current thinking on how to change my series now runs along these lines:
>
> - keep the current "crlf=auto" change
> - rename "core.eolStyle" to "core.localcrlf"
> - add a "core.crlf" that sets the "crlf" attribute on paths where it isn't explicitly configured
> - keep "core.autocrlf" for backwards compatibility, but make "core.autocrlf=input" and "core.autocrlf=true" complain if they are in conflict with the other config settings.

So, the meanings of these would become...

core.crlf [ auto | input | false ] : 'auto' means to enable
bidirectional normalization, and 'false' would mean do not
normalization, and 'input' would mean normalize on input only,
otherwise output lf. Is this true?
core.localcrlf [ crlf | lf ] : this is obvious, and use-friendly

For the above case have you considered using 'core.crlflocal' instead?
Usability-wise the related properties start with the same name prefix.

>From a usability standpoint, I personally prefer something similar to
what you (see "my user interface would have been") specified, slight
adjustment to the names only:

core.eolconv [ true | false ] - whether or not to turn on conversions
core.eoltype [ lf | crlf ] - by default what to convert to for text files

I like this purely because, from the users standpoint, saying
something like "localcrlf crlf" is strange; meaning the term "crlf" is
on both sides of the assignment. I do prefer "eol... crlf", where eol
refers to the applicability of the property and crlf is only one such
value.

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

On 9. mai 2010, at 13.14, Robert Buck wrote:

> So, the meanings of these would become...
> 
> core.crlf [ auto | input | false ] : 'auto' means to enable
> bidirectional normalization, and 'false' would mean do not
> normalization, and 'input' would mean normalize on input only,
> otherwise output lf. Is this true?

No, "auto" means to enable normalization for files git doesn't identify as text files, "true" means to always normalize, and "false" means never normalize.  I probably wouldn't implement "input" unless there was a lot of demand.  The idea is to make it act exactly like the "crlf" attribute, even though "core.crlf=true/false" would probably be used very rarely...  I'm having second thoughts, actually.

> core.localcrlf [ crlf | lf ] : this is obvious, and use-friendly

Well, yes.  I was thinking "true|false" (ie "I want crlf" or "I don't want crlf"), but I'm having second thoughts about that, too.

> For the above case have you considered using 'core.crlflocal' instead?
> Usability-wise the related properties start with the same name prefix.

I didn't think too much about the name, so a completely different name might be even better.

Because of this and my second thoughts, I'm going to wait a few days before I make any more changes to allow good ideas to appear and give them time to sink in.

> From a usability standpoint, I personally prefer something similar to
> what you (see "my user interface would have been") specified, slight
> adjustment to the names only:
> 
> core.eolconv [ true | false ] - whether or not to turn on conversions
> core.eoltype [ lf | crlf ] - by default what to convert to for text files

Agreed, but I think getting this feature included is more important than getting the user interface exactly right.  A compromise between backwards compatibility and user friendliness is okay.

> I like this purely because, from the users standpoint, saying
> something like "localcrlf crlf" is strange; meaning the term "crlf" is
> on both sides of the assignment. I do prefer "eol... crlf", where eol
> refers to the applicability of the property and crlf is only one such
> value.

Yes, my "localcrlf" would be true/false instead of crlf/lf.  It's definitely a compromise :)
-- 
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Robert Buck]

> On 9. mai 2010, at 13.14, Robert Buck wrote:
>
>> So, the meanings of these would become...
>>
>> core.crlf [ auto | input | false ] : 'auto' means to enable
>> bidirectional normalization, and 'false' would mean do not
>> normalization, and 'input' would mean normalize on input only,
>> otherwise output lf. Is this true?
>
> No, "auto" means to enable normalization for files git doesn't identify as text files, "true" means to always normalize, and "false" means never normalize.

I probably missed something. The part that confuses me in this
statement is that you said "for files git doesn't identify as text
files". The convert.c source is the heart of this, and if a file is
not identified as text it is presumed to be binary. The statement made
seems to imply you'd auto-convert PDF files? I know you did not mean
that, but it could have been read that way.

What specifically happens in the three modes? Would it be precise to
say the following?

    "Files subject to EOL conversion are those that are explicitly
identified through attributes to be text files, or those
algorithmically determined to be text files which happen to not bear
the "text" file attribute. Otherwise the default value, "false",
applies and no EOL conversions occur."

-Bob

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

On 9. mai 2010, at 22.46, Robert Buck <buck.robert.j@gmail.com> wrote:

>> No, "auto" means to enable normalization for files git doesn't  
>> identify as text files, "true" means to always normalize, and  
>> "false" means never normalize.
>
> I probably missed something. The part that confuses me in this
> statement is that you said "for files git doesn't identify as text
> files". The convert.c source is the heart of this, and if a file is
> not identified as text it is presumed to be binary. The statement made
> seems to imply you'd auto-convert PDF files? I know you did not mean
> that, but it could have been read that way.

Doh! I meant to write "files git _does_ identify as text files". Sorry  
for the confusion.

> What specifically happens in the three modes? Would it be precise to
> say the following?
>
>    "Files subject to EOL conversion are those that are explicitly
> identified through attributes to be text files, or those
> algorithmically determined to be text files which happen to not bear
> the "text" file attribute. Otherwise the default value, "false",
> applies and no EOL conversions occur."

Very close, but my thinko threw you off. The "algorithmic  
determination" of text files is only performed when crlf=auto, either  
by the attribute or the config variable being set that way.

The point of the "core.crlf" config variable would be to provide a  
default value for the "crlf" attribute.
-- 
>
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Robert Buck]

On Mon, May 10, 2010 at 12:33 AM, Eyvind Bernhardsen
<eyvind.bernhardsen@gmail.com> wrote:
> On 9. mai 2010, at 22.46, Robert Buck <buck.robert.j@gmail.com> wrote:
>
>>> No, "auto" means to enable normalization for files git doesn't identify
>>> as text files, "true" means to always normalize, and "false" means never
>>> normalize.
>>
>> I probably missed something. The part that confuses me in this
>> statement is that you said "for files git doesn't identify as text
>> files". The convert.c source is the heart of this, and if a file is
>> not identified as text it is presumed to be binary. The statement made
>> seems to imply you'd auto-convert PDF files? I know you did not mean
>> that, but it could have been read that way.
>
> Doh! I meant to write "files git _does_ identify as text files". Sorry for
> the confusion.
>
>> What specifically happens in the three modes? Would it be precise to
>> say the following?
>>
>>   "Files subject to EOL conversion are those that are explicitly
>> identified through attributes to be text files, or those
>> algorithmically determined to be text files which happen to not bear
>> the "text" file attribute. Otherwise the default value, "false",
>> applies and no EOL conversions occur."
>
> Very close, but my thinko threw you off. The "algorithmic determination" of
> text files is only performed when crlf=auto, either by the attribute or the
> config variable being set that way.
>
> The point of the "core.crlf" config variable would be to provide a default
> value for the "crlf" attribute.

Okay, so that makes sense...

   "Files subject to EOL conversion are those that are explicitly
 identified through attributes to be text files, or provided core.crlf is
set to auto, those files algorithmically determined to be text files
which happen to not bear
 the "text" file attribute. Otherwise the default value, "false",
applies and no EOL conversions occur. When conversions occur the EOL
character changes from the internal LF format to the format specified
by core.crlftype."

This would work out well if file type maps were ever introduced. Type
maps would not short circuit the explicit attributes identified by the
first clause, then for those files without attributes you'd check the
file-type maps, then fall back to the algorithmic if auto is enabled.

I like that.

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Robert Buck]

Today I am writing up some documentation related to Git use at the
company, and planning to check in all the data from Git vs Hg testing,
and all on-boarding/first-day documentation related to Git use. So
within the directory containing all these files I init-ed the
repository and got this when I attempted to add multiple files. Mind
you, this is on Linux!

  "warning: LF will be replaced by CRLF"

This _really_ of scares me being that this was on Linux. Is this one
of the problem spots that the topic of this thread attempts to
resolve?

Bob

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Dmitry Potapov]

On Mon, May 10, 2010 at 09:25:11AM -0400, Robert Buck wrote:
> Today I am writing up some documentation related to Git use at the
> company, and planning to check in all the data from Git vs Hg testing,
> and all on-boarding/first-day documentation related to Git use. So
> within the directory containing all these files I init-ed the
> repository and got this when I attempted to add multiple files. Mind
> you, this is on Linux!
> 
>   "warning: LF will be replaced by CRLF"

It looks like you set core.autocrlf to true on Linux.

The problem is core.autocrlf says two things -- whether you want to
enable automatic eol conversion for text files and whether you have CRLF
for text files. Only if the answer is "yes" to both then you should set
it to true. In other words, you should never set it to true on Linux.

You may want use core.autocrlf=input on Linux to avoid text files being
accidentally commit with a wrong encoding, when they were copied from
Windows directly.

> 
> This _really_ of scares me being that this was on Linux. Is this one
> of the problem spots that the topic of this thread attempts to
> resolve?

Yes, we want to have a separate setting, whihc will tell what is EOL on
your system, and it should have sensible default, i.e. LF on Linux and
CRLF on Windows.


Dmitry

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Jay Soffian]

On Sun, May 9, 2010 at 6:42 AM, Eyvind Bernhardsen
<eyvind.bernhardsen@gmail.com> wrote:
> I guess I should nail my flag to the mast: Here's what I would have done, with the benefit of plenty of hindsight, had we not had core.autocrlf, and also what I think we should do to approach that ideal.
>
> Please don't get hung up too much on the names, they were chosen to not match anything suggested so far so that I can refer back to them unambiguously.
>
> My user interface would have been:
>
> - an attribute "eolconv" that enables or disables line ending conversion
> - a config variable "core.eolconv" that sets "eolconv" for all files where it is unset
> - a config variable "core.localeol" that decides whether LF or CRLF is preferred
>
> This provides the means to enable normalization on a per-project ("eolconv") or per-repository ("core.eolconv") basis, and allows the user to override the platform native line ending when normalization is in effect.
>
> [...]
>
> My current thinking on how to change my series now runs along these lines:
>
> - keep the current "crlf=auto" change
> - rename "core.eolStyle" to "core.localcrlf"
> - add a "core.crlf" that sets the "crlf" attribute on paths where it isn't explicitly configured
> - keep "core.autocrlf" for backwards compatibility, but make "core.autocrlf=input" and "core.autocrlf=true" complain if they are in conflict with the other config settings.

Bah. I think relegating the old names to "deprecated, for
compatibility" is absolutely the right thing to do. Is there a use
case where the existing crlf setup is preferable? If not, why not just
mark them as deprecated in the documentation and say "see ..."
pointing to the new functionality and use the new names as you
suggest.

$0.02. :-)

j.

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Jay Soffian]

One more point, which I'm sure will be inflammatory. After all, it has
nothing to do with anything, but let's look at how svn deals with EOL
issues:

1) A property for whether a file is binary or text
2) A property for the EOL style which applies only to text files:

http://svnbook.red-bean.com/en/1.5/svn.advanced.props.file-portability.html#svn.advanced.props.special.eol-style

And Mercurial's plan to deal with it:

http://mercurial.selenic.com/wiki/EOLTranslationPlan

:-)

j.

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

On 9. mai 2010, at 19.02, Jay Soffian wrote:

> Bah. I think relegating the old names to "deprecated, for
> compatibility" is absolutely the right thing to do. Is there a use
> case where the existing crlf setup is preferable? If not, why not just
> mark them as deprecated in the documentation and say "see ..."
> pointing to the new functionality and use the new names as you
> suggest.

I don't know if it would get accepted, but I'll add a commit that does that to the next iteration :)
-- 
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Junio C Hamano]

Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com> writes:

> My user interface would have been:
>
> - an attribute "eolconv" that enables or disables line ending conversion
> - a config variable "core.eolconv" that sets "eolconv" for all files where it is unset
> - a config variable "core.localeol" that decides whether LF or CRLF is preferred

I am puzzled about this second item; what is its type and what is its
purpose?  If it is to allow project-wide default to be specified, then
isn't having "* eolconv=true" in .gitattributes a much better option and
is already supported by the first item?

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Finn Arne Gangstad]

On Sun, May 09, 2010 at 10:45:35AM -0700, Junio C Hamano wrote:
> Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com> writes:
> 
> > My user interface would have been:
> >
> > - an attribute "eolconv" that enables or disables line ending conversion
> > - a config variable "core.eolconv" that sets "eolconv" for all files where it is unset
> > - a config variable "core.localeol" that decides whether LF or CRLF is preferred
> 
> I am puzzled about this second item; what is its type and what is its
> purpose?  If it is to allow project-wide default to be specified, then
> isn't having "* eolconv=true" in .gitattributes a much better option and
> is already supported by the first item?

The way I understood it core.eolconv has exactly the same possible
values as the "eolconv" attribute, and serves as a default value for
"eolconv" if it isn't set. This would (mostly, I guess) be for Windows
users who would like to check out a project that is primarily
developed on Unix and didn't bother to set any eolconv attributes, and
still get CRLF line endings. core.eolconv has some of the drawbacks
of autocrlf, so shouldn't really be used if you can convince projects
to add eolconv attributes instead.

Are you thinking we could live completely without it? Most other
popular vcs-systems have eol-conversion/normalisation on by default,
while git has it disabled by default. The config variable can change
the default behaviour, but is isn't as helpful as it should have been
perhaps.

To do a better job with old un-normalised repos, we could for each
file remember what their eol-style was (CR, LF, CRLF, mixed), and then
even if doing output conversion on checkout, convert back to the same
eol style on commit. This would perhaps break down a bit for renames,
but would be lovely if it worked for merges for example...

- Finn Arne

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Junio C Hamano]

Finn Arne Gangstad <finnag@pvv.org> writes:

> Are you thinking we could live completely without it?

Yes, and your description makes it sound like it doesn't buy us anything
that an entry '* eolconv=true" in .git/info/attributes wouldn't.

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

On 9. mai 2010, at 23.57, Junio C Hamano wrote:

> Finn Arne Gangstad <finnag@pvv.org> writes:
> 
>> Are you thinking we could live completely without it?
> 
> Yes, and your description makes it sound like it doesn't buy us anything
> that an entry '* eolconv=true" in .git/info/attributes wouldn't.

I'm not sure I even knew about .git/info/attributes, but it makes a lot of sense.  The config variable would allow a user to turn on normalization globally, I guess, but that's probably not worth it.
-- 
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Eyvind Bernhardsen]

On 9. mai 2010, at 19.45, Junio C Hamano wrote:

> Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com> writes:
> 
>> My user interface would have been:
>> 
>> - an attribute "eolconv" that enables or disables line ending conversion
>> - a config variable "core.eolconv" that sets "eolconv" for all files where it is unset
>> - a config variable "core.localeol" that decides whether LF or CRLF is preferred
> 
> I am puzzled about this second item; what is its type and what is its
> purpose?  If it is to allow project-wide default to be specified, then
> isn't having "* eolconv=true" in .gitattributes a much better option and
> is already supported by the first item?

Yes, but if the repository maintainers don't want to make that change (for whatever reason), it might be nice for the user to be able to enable normalization locally.  It would be a boolean, and setting it would mean the equivalent of "crlf=auto" on all files where "crlf" is not explicitly set.
-- 
Eyvind

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Finn Arne Gangstad]

On Sun, May 09, 2010 at 12:42:17PM +0200, Eyvind Bernhardsen wrote:

> I guess I should nail my flag to the mast: Here's what I would have
> done, with the benefit of plenty of hindsight, had we not had
> core.autocrlf, and also what I think we should do to approach that
> ideal.
> [...]

After some meditation, I think the following:

The most important thing is how we can fix git on Windows to operate
well by default. This is what we really should try to fix.

The way git on Windows would work best, while at the same time cause
the minimum amount of damage, would IMHO be:

- Convert LF-only text files to CRLF on checkout
- Convert those same files (but only those!) back to LF on commit
- Optionally: Convert new CRLF text files to LF on commit

And all this should happen without setting any configuration, and not
setting any attributes.

So maybe, just maybe, we can make everything sufficiently good by
repairing "core.autocrlf = {input,true}" so that git will not convert
a CRLF already in the repo. This would make autocrlf = true a safe
default value (and probably input too, but you'd have to "do
something" to get a new text file with CRLF into the repo then).

Then we go from a situation that didn't work, to a working situation,
and break nothing that used to work. You no longer have to normalise
your repos to work from Windows.

- Finn Arne

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Dmitry Potapov]

On Sun, May 09, 2010 at 10:09:37PM +0200, Finn Arne Gangstad wrote:
> 
> So maybe, just maybe, we can make everything sufficiently good by
> repairing "core.autocrlf = {input,true}" so that git will not convert
> a CRLF already in the repo. This would make autocrlf = true a safe
> default value (and probably input too, but you'd have to "do
> something" to get a new text file with CRLF into the repo then).

First of autocrlf is safe as it is implemented now. Second, to do
something to get a new with CRLF into the repo is really stupid.
The whole point of autocrlf is being automatic and do not have the
user to worry about CRLF when he adds a new file.

The only real problem I am aware of is that some repository are not
compatible with autocrlf conversion, because they store text files
with different endings and do not have appropriate .gitatributes to
describle what text files should and should not be converted. So, each
user has to disable autocrlf in them manually and then re-checkout all
files using (rm -rf * && git checkout -f), which is confusing for many
users. In fact, you do not have to disable autocrlf, you can add a few
lines to .git/info/attributes to make it autocrlf compatible, but again
many users even not aware about this file, let alone what needs to be
added. Further, the problem amplified by the fact that you have to do
the same procedure every time when you do cloning, and though cloning
is not most common operation, it happens often enough to annoy many
users.

I believe that the right solution is to be able to enable autocrlf but
only for those repositories that are marked as autocrlf compatible by
upstream.


Dmitry

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Finn Arne Gangstad]

On Mon, May 10, 2010 at 12:13:58PM +0400, Dmitry Potapov wrote:
> 
> First of autocrlf is safe as it is implemented now.

No, it isn't. autocrlf as it is implemented now is destructive for any
file that contains CRLF in the repo (it also gives dirty files after
checkout and so on).

> I believe that the right solution is to be able to enable autocrlf but
> only for those repositories that are marked as autocrlf compatible by
> upstream.

Yes absolutely. But how do you tell autocrlf that the repository is
compatible with it?  This is what is causing all the problems.

Now, I propose to change autocrlf in such a way that it will work as
before for all repositories that are "compatible with it", but _also_
so that it works reasonably with those that aren't.

- Finn Arne

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Dmitry Potapov]

On Mon, May 10, 2010 at 01:14:39PM +0200, Finn Arne Gangstad wrote:
> On Mon, May 10, 2010 at 12:13:58PM +0400, Dmitry Potapov wrote:
> > 
> > First of autocrlf is safe as it is implemented now.
> 
> No, it isn't. autocrlf as it is implemented now is destructive for any
> file that contains CRLF in the repo (it also gives dirty files after
> checkout and so on).

It may give dirty files in your working tree, but you do not lose any
information silently (as it happened with CVS), so it is safe in this
respect.

> 
> > I believe that the right solution is to be able to enable autocrlf but
> > only for those repositories that are marked as autocrlf compatible by
> > upstream.
> 
> Yes absolutely. But how do you tell autocrlf that the repository is
> compatible with it?  This is what is causing all the problems.

I suppose the original design assumption of autocrlf was that nearly all
repo should be compatible, because autocrlf is very good on detecting
text files. The problem with that is that many people want to store text
files with different endings, but they cannot be bothered to add a few
lines to .gitattributes. And while it is possible now to disable autocrlf
in any repo just by adding one line to .gitattributes:
* -crlf
but it is impossible to say the opposite.

So, I believe we should add "crlf=auto" as Eyvind proposed, as well as
to do eol conversion for files marked as "crlf" even if autocrlf is not
set as Linus suggested earlier. (Certainly, "eolconv" would be a better
name than "crlf", but maybe it is not the right time to replace it right
now).

> 
> Now, I propose to change autocrlf in such a way that it will work as
> before for all repositories that are "compatible with it", but _also_
> so that it works reasonably with those that aren't.

No, you proposed something different. You said that conversion for new
files would become optional. I don't know what exactly you mean by
optional, but it sounds incompatible with what we have now. In fact,
what I really like about autocrlf is that I do not need to think about
when I add a new file.

Moreover, you said "Convert LF-only text files to CRLF on checkout",
which raises two questions:

1. Where should information about what file was and what was not
converted be stored? (Storying it in the index can make the index
incompatible with old versions of Git).

2. How does it solve the problem with new files? (Just saying that
this conversation will be optional, does not it mean that it will be
right for this particular repo.)



Dmitry

Re: [PATCH/RFC v2 1/4] Add "core.eolStyle" variable to control end-of-line conversion

[Finn Arne Gangstad]

On Sun, May 09, 2010 at 12:53:17AM +0200, Eyvind Bernhardsen wrote:
> On 9. mai 2010, at 00.17, Eyvind Bernhardsen 
> <eyvind.bernhardsen@gmail.com> wrote:
>
>> I'll try to think of a better name.
>
> Heh. How about "localcrlf={true,false,native}"?
>
> It breaks down a bit if we ever decide to support old-school-Mac-style  
> CR line endings, but at that point you're approaching the borders of  
> madness anyway.

It seems that autocrlf is currently so troublesome that most people end up
disabling it, and handling it in other ways (we certainly do).

Instead of keeping the names and trying to graft on something that is
almost backwards compatible, and forever live with names that we agree
are bad, what about:

Deprecate core.autocrlf and the crlf attribute.

Make these instead:

Configs:
core.eol = lf, crlf, native [default=native]
core.eolconversion = true, false, auto [default=unset]

Attribute:
eolconversion = true, false, auto [default=core.eolconversion]

- Finn Arne

[PATCH/RFC v2 2/4] Add tests for per-repository eol normalization

[Eyvind Bernhardsen]

Signed-off-by: Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com>
---
 t/t0025-crlf-auto.sh |  180 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 180 insertions(+), 0 deletions(-)
 create mode 100755 t/t0025-crlf-auto.sh

diff --git a/t/t0025-crlf-auto.sh b/t/t0025-crlf-auto.sh
new file mode 100755
index 0000000..367b459
--- /dev/null
+++ b/t/t0025-crlf-auto.sh
@@ -0,0 +1,180 @@
+#!/bin/sh
+
+test_description='CRLF conversion'
+
+. ./test-lib.sh
+
+has_cr() {
+	tr '\015' Q <"$1" | grep Q >/dev/null
+}
+
+test_expect_success setup '
+
+	git config core.autocrlf false &&
+
+	for w in Hello world how are you; do echo $w; done >one &&
+	for w in I am very very fine thank you; do echo ${w}Q; done | q_to_cr >two &&
+	git add . &&
+
+	git commit -m initial &&
+
+	one=`git rev-parse HEAD:one` &&
+	two=`git rev-parse HEAD:two` &&
+
+	for w in Some extra lines here; do echo $w; done >>one &&
+	git diff >patch.file &&
+	patched=`git hash-object --stdin <one` &&
+	git read-tree --reset -u HEAD &&
+
+	echo happy.
+'
+
+test_expect_success 'default settings cause no changes' '
+
+	rm -f .gitattributes tmp one two &&
+	git read-tree --reset -u HEAD &&
+
+	if has_cr one || ! has_cr two
+	then
+		echo "Eh? $f"
+		false
+	fi &&
+	onediff=`git diff one` &&
+	twodiff=`git diff two` &&
+	test -z "$onediff" -a -z "$twodiff"
+'
+
+test_expect_success 'no crlf=auto, explicit eolstyle=native causes no changes' '
+
+	rm -f .gitattributes tmp one two &&
+	git config core.eolstyle native &&
+	git read-tree --reset -u HEAD &&
+
+	if has_cr one || ! has_cr two
+	then
+		echo "Eh? $f"
+		false
+	fi &&
+	onediff=`git diff one` &&
+	twodiff=`git diff two` &&
+	test -z "$onediff" -a -z "$twodiff"
+'
+
+test_expect_failure 'crlf=auto, eolStyle=crlf <=> autocrlf=true' '
+
+	rm -f .gitattributes tmp one two &&
+	git config core.autocrlf false &&
+	git config core.eolstyle crlf &&
+	echo "* crlf=auto" > .gitattributes &&
+	git read-tree --reset -u HEAD &&
+	unset missing_cr &&
+
+	for f in one two
+	do
+		if ! has_cr "$f"
+		then
+			echo "Eh? $f"
+			missing_cr=1
+			break
+		fi
+	done &&
+	test -z "$missing_cr"
+'
+
+test_expect_failure 'crlf=auto, eolStyle=lf <=> autocrlf=input' '
+
+	rm -f .gitattributes tmp one two &&
+	git config core.autocrlf false &&
+	git config core.eolstyle lf &&
+	echo "* crlf=auto" > .gitattributes &&
+	git read-tree --reset -u HEAD &&
+
+	if has_cr one || ! has_cr two
+	then
+		echo "Eh? $f"
+		false
+	fi &&
+	onediff=`git diff one` &&
+	twodiff=`git diff two` &&
+	test -z "$onediff" -a -n "$twodiff"
+'
+
+test_expect_success 'crlf=auto, eolStyle=false <=> autocrlf=false' '
+
+	rm -f .gitattributes tmp one two &&
+	git config core.autocrlf false &&
+	git config core.eolstyle false &&
+	echo "* crlf=auto" > .gitattributes &&
+	git read-tree --reset -u HEAD &&
+
+	if has_cr one || ! has_cr two
+	then
+		echo "Eh? $f"
+		false
+	fi
+	onediff=`git diff one` &&
+	twodiff=`git diff two` &&
+	test -z "$onediff" -a -z "$twodiff"
+'
+
+test_expect_success 'autocrlf=true overrides crlf=auto, eolStyle=lf' '
+
+	rm -f .gitattributes tmp one two &&
+	git config core.autocrlf true &&
+	git config core.eolstyle lf &&
+	echo "* crlf=auto" > .gitattributes &&
+	git read-tree --reset -u HEAD &&
+	unset missing_cr &&
+
+	for f in one two
+	do
+		if ! has_cr "$f"
+		then
+			echo "Eh? $f"
+			missing_cr=1
+			break
+		fi
+	done &&
+	test -z "$missing_cr"
+'
+
+test_expect_success 'autocrlf=input overrides crlf=auto, eolStyle=crlf' '
+
+	rm -f .gitattributes tmp one two &&
+	git config core.autocrlf input &&
+	git config core.eolstyle crlf &&
+	echo "* crlf=auto" > .gitattributes &&
+	git read-tree --reset -u HEAD &&
+
+	if has_cr one || ! has_cr two
+	then
+		echo "Eh? $f"
+		false
+	fi &&
+	onediff=`git diff one` &&
+	twodiff=`git diff two` &&
+	test -z "$onediff" -a -n "$twodiff"
+'
+
+test_expect_success 'autocrlf=true overrides crlf=auto, eolStyle=false' '
+
+	rm -f .gitattributes tmp one two &&
+	git config core.autocrlf true &&
+	git config core.eolstyle false &&
+	echo "* crlf=auto" > .gitattributes &&
+	git read-tree --reset -u HEAD &&
+	unset missing_cr &&
+
+	for f in one two
+	do
+		if ! has_cr "$f"
+		then
+			echo "Eh? $f"
+			missing_cr=1
+			break
+		fi
+	done &&
+	test -z "$missing_cr"
+'
+
+test_done
-- 
1.7.1.3.gb95c9

[PATCH/RFC v2 3/4] Pass eol conv mode as an argument instead of using global auto_crlf

[Eyvind Bernhardsen]

This patch has no semantic changes, but makes the next commit easier to
review.

Signed-off-by: Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com>
---
 convert.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/convert.c b/convert.c
index 4f8fcb7..2eef2f6 100644
--- a/convert.c
+++ b/convert.c
@@ -90,12 +90,13 @@ static int is_binary(unsigned long size, struct text_stat *stats)
 }
 
 static void check_safe_crlf(const char *path, int action,
-                            struct text_stat *stats, enum safe_crlf checksafe)
+			    struct text_stat *stats, enum safe_crlf checksafe,
+			    int eol_conversion)
 {
 	if (!checksafe)
 		return;
 
-	if (action == CRLF_INPUT || auto_crlf <= 0) {
+	if (action == CRLF_INPUT || eol_conversion <= 0) {
 		/*
 		 * CRLFs would not be restored by checkout:
 		 * check if we'd remove CRLFs
@@ -106,7 +107,7 @@ static void check_safe_crlf(const char *path, int action,
 			else /* i.e. SAFE_CRLF_FAIL */
 				die("CRLF would be replaced by LF in %s.", path);
 		}
-	} else if (auto_crlf > 0) {
+	} else if (eol_conversion > 0) {
 		/*
 		 * CRLFs would be added by checkout:
 		 * check if we have "naked" LFs
@@ -121,12 +122,13 @@ static void check_safe_crlf(const char *path, int action,
 }
 
 static int crlf_to_git(const char *path, const char *src, size_t len,
-                       struct strbuf *buf, int action, enum safe_crlf checksafe)
+		       struct strbuf *buf, int action, enum safe_crlf checksafe,
+		       int eol_conversion)
 {
 	struct text_stat stats;
 	char *dst;
 
-	if ((action == CRLF_BINARY) || !auto_crlf || !len)
+	if ((action == CRLF_BINARY) || !eol_conversion || !len)
 		return 0;
 
 	gather_stats(src, len, &stats);
@@ -147,7 +149,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
 			return 0;
 	}
 
-	check_safe_crlf(path, action, &stats, checksafe);
+	check_safe_crlf(path, action, &stats, checksafe, eol_conversion);
 
 	/* Optimization: No CR? Nothing to convert, regardless. */
 	if (!stats.cr)
@@ -180,13 +182,13 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
 }
 
 static int crlf_to_worktree(const char *path, const char *src, size_t len,
-                            struct strbuf *buf, int action)
+			    struct strbuf *buf, int action, int eol_conversion)
 {
 	char *to_free = NULL;
 	struct text_stat stats;
 
 	if ((action == CRLF_BINARY) || (action == CRLF_INPUT) ||
-	    auto_crlf <= 0)
+	    eol_conversion <= 0)
 		return 0;
 
 	if (!len)
@@ -591,7 +593,7 @@ int convert_to_git(const char *path, const char *src, size_t len,
 		src = dst->buf;
 		len = dst->len;
 	}
-	ret |= crlf_to_git(path, src, len, dst, crlf, checksafe);
+	ret |= crlf_to_git(path, src, len, dst, crlf, checksafe, auto_crlf);
 	if (ret) {
 		src = dst->buf;
 		len = dst->len;
@@ -621,7 +623,7 @@ int convert_to_working_tree(const char *path, const char *src, size_t len, struc
 		src = dst->buf;
 		len = dst->len;
 	}
-	ret |= crlf_to_worktree(path, src, len, dst, crlf);
+	ret |= crlf_to_worktree(path, src, len, dst, crlf, auto_crlf);
 	if (ret) {
 		src = dst->buf;
 		len = dst->len;
-- 
1.7.1.3.gb95c9

[PATCH/RFC v2 4/4] Add per-repository eol normalization

[Eyvind Bernhardsen]

Change the semantics of the "crlf" attribute so that it enables
end-of-line normalization when it is set.  Add a new setting, "auto",
which enables end-of-line conversion but does not override the automatic
binary file detection.

The effect of this change is that a project can enable end-of-line
normalization for all files.  This is similar to the "core.autocrlf"
configuration variable, but since the setting is part of the content, it
is cloned when the project is cloned and can be changed if a previously
un-normalized repository is normalized.

Since the line ending style to be used in the working directory is a
user preference, the configuration variable "core.eolStyle" controls it.

"core.autocrlf" can still be used to enable conversion, and every effort
has been taken to avoid backwards incompatibility.  There is one small
semantic change: if "crlf" is set on a path, that file will now have its
line endings normalized.  Previously, they would only be normalized if
"core.autocrlf" was also set.

Signed-off-by: Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com>
---
 Documentation/config.txt        |    4 +-
 Documentation/gitattributes.txt |   71 +++++++++++++++++++++++++++-----------
 convert.c                       |   34 ++++++++++++++++--
 t/t0025-crlf-auto.sh            |    4 +-
 4 files changed, 84 insertions(+), 29 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3956ff7..7bbf8a0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -215,8 +215,8 @@ core.eolStyle::
 	line ending conversion.  See linkgit:gitattributes[5].
 
 core.safecrlf::
-	If true, makes git check if converting `CRLF` as controlled by
-	`core.autocrlf` is reversible.  Git will verify if a command
+	If true, makes git check if converting `CRLF` is reversible when
+	end-of-line conversion is active.  Git will verify if a command
 	modifies a file in the work tree either directly or indirectly.
 	For example, committing a file followed by checking out the
 	same file should yield the original file in the work tree.  If
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index d892e64..18b07f6 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -92,17 +92,28 @@ such as 'git checkout' and 'git merge' run.  They also affect how
 git stores the contents you prepare in the working tree in the
 repository upon 'git add' and 'git commit'.
 
+
 `crlf`
 ^^^^^^
 
-This attribute controls the line-ending convention.
+This attribute enables and controls end-of-line normalization.  When
+normalization is enabled for a file, its line endings are converted to
+LF when it is checked in, and the configuration variable
+`core.eolStyle` decides whether to convert line endings to CRLF on
+checkout.
+
+Set to string value "auto"::
+
+	When `crlf` is set to "auto", the file is marked for automatic
+	end-of-line normalization.  If git detects that the file is
+	a text file, its line endings are normalized to LF on checkin.
 
 Set::
 
-	Setting the `crlf` attribute on a path is meant to mark
-	the path as a "text" file.  'core.autocrlf' conversion
-	takes place without guessing the content type by
-	inspection.
+	Setting the `crlf` attribute on a path enables end-of-line
+	normalization and marks the path as a text file.
+	End-of-line conversion takes place without guessing the
+	content type by inspection.
 
 Unset::
 
@@ -111,34 +122,52 @@ Unset::
 
 Unspecified::
 
-	Unspecified `crlf` attribute tells git to apply the
-	`core.autocrlf` conversion when the file content looks
-	like text.
+	Unspecified `crlf` attribute tells git to apply end-of-line
+	conversion only if the `core.autocrlf` configuration variable
+	is set.
 
 Set to string value "input"::
 
 	This is similar to setting the attribute to `true`, but
-	also forces git to act as if `core.autocrlf` is set to
-	`input` for the path.
+	prevents git from converting the line endings to CRLF when the
+	file is checked out.
 
 Any other value set to `crlf` attribute is ignored and git acts
 as if the attribute is left unspecified.
 
 
-The `core.autocrlf` conversion
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-If the configuration variable `core.autocrlf` is false, no
-conversion is done.
+End-of-line conversion
+^^^^^^^^^^^^^^^^^^^^^^
 
-When `core.autocrlf` is true, it means that the platform wants
-CRLF line endings for files in the working tree, and you want to
-convert them back to the normal LF line endings when checking
-in to the repository.
+While git normally leaves file contents alone, it can be configured to
+normalize line endings to LF in the repository and, optionally, to
+convert them to CRLF when files are checked out.  Binary files are
+detected automatically and will not be modified; this detection can be
+overridden with the `crlf` attribute.
+
+NOTE: This normalization requires the repository to be free of text
+files containing CRLFs.  When it is enabled on an existing repository,
+the index should be rebuilt to find any such files, and these files
+should either have their `crlf` attribute set to false ("-crlf"), or
+they should be checked in to the repository in normalized form.
+
+This example shows how to convert an existing repository with a mix of
+LF and CRLF contents into one with normalized line endings.  From a
+clean working directory:
+
+-------------------------------------------------
+$ echo "* crlf=auto" >.gitattributes
+$ rm .git/index     # Remove the index to force git to
+$ git reset         # re-scan the working directory
+$ git status        # Show files that will be normalized
+$ git add -u
+$ git add .gitattributes
+$ git commit -m "Introduce end-of-line normalization"
+-------------------------------------------------
+
+If any files that should not be normalized show up in 'git status',
+unset their `crlf` attribute in `.gitattributes` before 'git add -u'.
 
-When `core.autocrlf` is set to "input", line endings are
-converted to LF upon checkin, but there is no conversion done
-upon checkout.
 
 If `core.safecrlf` is set to "true" or "warn", git verifies if
 the conversion is reversible for the current setting of
diff --git a/convert.c b/convert.c
index 2eef2f6..6871896 100644
--- a/convert.c
+++ b/convert.c
@@ -15,6 +15,7 @@
 #define CRLF_BINARY	0
 #define CRLF_TEXT	1
 #define CRLF_INPUT	2
+#define CRLF_AUTO	3
 
 struct text_stat {
 	/* NUL, CR, LF and CRLF counts */
@@ -392,6 +393,17 @@ static void setup_convert_check(struct git_attr_check *check)
 	check[2].attr = attr_filter;
 }
 
+static int choose_eol_conversion(int auto_eol)
+{
+	if (auto_crlf)
+		return auto_crlf;
+
+	if (auto_eol)
+		return eol_style;
+
+	return 0;
+}
+
 static int count_ident(const char *cp, unsigned long size)
 {
 	/*
@@ -546,6 +558,8 @@ static int git_path_check_crlf(const char *path, struct git_attr_check *check)
 		;
 	else if (!strcmp(value, "input"))
 		return CRLF_INPUT;
+	else if (!strcmp(value, "auto"))
+		return CRLF_AUTO;
 	return CRLF_GUESS;
 }
 
@@ -575,7 +589,7 @@ int convert_to_git(const char *path, const char *src, size_t len,
 {
 	struct git_attr_check check[3];
 	int crlf = CRLF_GUESS;
-	int ident = 0, ret = 0;
+	int ident = 0, ret = 0, auto_eol = 0;
 	const char *filter = NULL;
 
 	setup_convert_check(check);
@@ -586,6 +600,11 @@ int convert_to_git(const char *path, const char *src, size_t len,
 		drv = git_path_check_convert(path, check + 2);
 		if (drv && drv->clean)
 			filter = drv->clean;
+		if (crlf > 0) {
+			auto_eol = 1;
+			if (crlf == CRLF_AUTO)
+				crlf = CRLF_GUESS;
+		}
 	}
 
 	ret |= apply_filter(path, src, len, dst, filter);
@@ -593,7 +612,8 @@ int convert_to_git(const char *path, const char *src, size_t len,
 		src = dst->buf;
 		len = dst->len;
 	}
-	ret |= crlf_to_git(path, src, len, dst, crlf, checksafe, auto_crlf);
+	ret |= crlf_to_git(path, src, len, dst, crlf, checksafe,
+		choose_eol_conversion(auto_eol));
 	if (ret) {
 		src = dst->buf;
 		len = dst->len;
@@ -605,7 +625,7 @@ int convert_to_working_tree(const char *path, const char *src, size_t len, struc
 {
 	struct git_attr_check check[3];
 	int crlf = CRLF_GUESS;
-	int ident = 0, ret = 0;
+	int ident = 0, ret = 0, auto_eol = 0;
 	const char *filter = NULL;
 
 	setup_convert_check(check);
@@ -616,6 +636,11 @@ int convert_to_working_tree(const char *path, const char *src, size_t len, struc
 		drv = git_path_check_convert(path, check + 2);
 		if (drv && drv->smudge)
 			filter = drv->smudge;
+		if (crlf > 0) {
+			auto_eol = 1;
+			if (crlf == CRLF_AUTO)
+				crlf = CRLF_GUESS;
+		}
 	}
 
 	ret |= ident_to_worktree(path, src, len, dst, ident);
@@ -623,7 +648,8 @@ int convert_to_working_tree(const char *path, const char *src, size_t len, struc
 		src = dst->buf;
 		len = dst->len;
 	}
-	ret |= crlf_to_worktree(path, src, len, dst, crlf, auto_crlf);
+	ret |= crlf_to_worktree(path, src, len, dst, crlf,
+		choose_eol_conversion(auto_eol));
 	if (ret) {
 		src = dst->buf;
 		len = dst->len;
diff --git a/t/t0025-crlf-auto.sh b/t/t0025-crlf-auto.sh
index 367b459..e8e76f5 100755
--- a/t/t0025-crlf-auto.sh
+++ b/t/t0025-crlf-auto.sh
@@ -60,7 +60,7 @@ test_expect_success 'no crlf=auto, explicit eolstyle=native causes no changes' '
 	test -z "$onediff" -a -z "$twodiff"
 '
 
-test_expect_failure 'crlf=auto, eolStyle=crlf <=> autocrlf=true' '
+test_expect_success 'crlf=auto, eolStyle=crlf <=> autocrlf=true' '
 
 	rm -f .gitattributes tmp one two &&
 	git config core.autocrlf false &&
@@ -81,7 +81,7 @@ test_expect_failure 'crlf=auto, eolStyle=crlf <=> autocrlf=true' '
 	test -z "$missing_cr"
 '
 
-test_expect_failure 'crlf=auto, eolStyle=lf <=> autocrlf=input' '
+test_expect_success 'crlf=auto, eolStyle=lf <=> autocrlf=input' '
 
 	rm -f .gitattributes tmp one two &&
 	git config core.autocrlf false &&
-- 
1.7.1.3.gb95c9