Cannot checkout after setting the eol attribute

Hi,

I specified the `eol` attribute on some files recently and the behavior
of Git is very strange.

Here is the set of commands to set up the repository used for the
discussion:

    git init
    echo $'dos\r' > dos
    git add dos
    git commit -m "dos newlines"
    echo "dos -crlf" > .gitattributes
    git add .gitattributes
    git commit -m "add attributes"
    echo "dos eol=crlf" > .gitattributes
    git add .gitattributes
    git commit -m "set eol attribute instead"

The following behaviors are observed:

  - `git reset --hard` does not make the working directory clean; and
  - `git rebase` gets *very* confused about the diffs in the working
    tree because `git stash` can't reset the working tree;

There are probably other oddities lingering about as well. If I commit
what Git thinks is the difference, the diff (with invisibles made
visible) is:

    % git diff | cat -A
    diff --git a/dos b/dos$
    index fde2310..4723a1b 100644$
    --- a/dos$
    +++ b/dos$
    @@ -1 +1 @@$
    -dos^M$
    +dos$

Seen in 2.9.5 and 2.14.0.rc1.

--Ben

On Tue, Aug 22, 2017 at 01:49:18PM -0400, Ben Boeckel wrote:

Hi,

I specified the `eol` attribute on some files recently and the behavior
of Git is very strange.

Here is the set of commands to set up the repository used for the
discussion:

    git init
    echo $'dos\r' > dos
    git add dos
    git commit -m "dos newlines"
    echo "dos -crlf" > .gitattributes
    git add .gitattributes
    git commit -m "add attributes"
    echo "dos eol=crlf" > .gitattributes
    git add .gitattributes
    git commit -m "set eol attribute instead"

The following behaviors are observed:

  - `git reset --hard` does not make the working directory clean; and
  - `git rebase` gets *very* confused about the diffs in the working
    tree because `git stash` can't reset the working tree;

There are probably other oddities lingering about as well. If I commit
what Git thinks is the difference, the diff (with invisibles made
visible) is:

    % git diff | cat -A
    diff --git a/dos b/dos$
    index fde2310..4723a1b 100644$
    --- a/dos$
    +++ b/dos$
    @@ -1 +1 @@$
    -dos^M$
    +dos$

Thanks for the goos example!
This is by design.

When you set the text attribute (in your case "eol=crlf" implies text)
then the file(s) -must- be nomalized and commited so that they have LF
in the repo (technically speaking the index)


This is what is written about the "eol=crlf" attribute:
	This setting forces Git to normalize line endings for this
	file on checkin and convert them to CRLF when the file is
	checked out.
And this is what is implemented in Git.

Long story short:

The following would solve your problem:
   git init
   echo $'dos\r' > dos
   git add dos
   git commit -m "dos newlines"
   echo "dos -crlf" > .gitattributes
   git add .gitattributes
   git commit -m "add attributes"
   echo "dos eol=crlf" > .gitattributes
   git read-tree --empty   # Clean index, force re-scan of working directory
   git add dos
   git add .gitattributes
   git commit -m "set eol attribute instead"
   git ls-files --eol

Seen in 2.9.5 and 2.14.0.rc1.

--Ben

On Tue, Aug 22, 2017 at 21:13:18 +0200, Torsten Bögershausen wrote:

When you set the text attribute (in your case "eol=crlf" implies text)
then the file(s) -must- be nomalized and commited so that they have LF
in the repo (technically speaking the index)

This seems like a special case that Git could detect and message about
somehow.

This is what is written about the "eol=crlf" attribute:
	This setting forces Git to normalize line endings for this
	file on checkin and convert them to CRLF when the file is
	checked out.
And this is what is implemented in Git.

Yeah, I read the docs, but the oddities of reset not doing its job
wasn't clear from this sentence :) .

Long story short:

The following would solve your problem:
   git init
   echo $'dos\r' > dos
   git add dos
   git commit -m "dos newlines"
   echo "dos -crlf" > .gitattributes
   git add .gitattributes
   git commit -m "add attributes"
   echo "dos eol=crlf" > .gitattributes
   git read-tree --empty   # Clean index, force re-scan of working directory

The fact that plumbing is necessary to dig yourself out of a hole of the
`eol` attribute changes points to something needing to be changed, even
if it's only documentation. Could Git detect this and message about it
somehow when `git reset` cannot fix the working tree? Or maybe it could
at least exit with failure instead of success?

--Ben

On Tue, Aug 22, 2017 at 03:44:41PM -0400, Ben Boeckel wrote:

On Tue, Aug 22, 2017 at 21:13:18 +0200, Torsten Bögershausen wrote:

quoted

When you set the text attribute (in your case "eol=crlf" implies text)
then the file(s) -must- be nomalized and commited so that they have LF
in the repo (technically speaking the index)

This seems like a special case that Git could detect and message about
somehow.

quoted

This is what is written about the "eol=crlf" attribute:
	This setting forces Git to normalize line endings for this
	file on checkin and convert them to CRLF when the file is
	checked out.
And this is what is implemented in Git.

Yeah, I read the docs, but the oddities of reset not doing its job
wasn't clear from this sentence :) .

git reset does it's job - please see below.

The problem is that we need a "git commit" here.
After applying .gitattributes, it may be neccessary to "normalize" the
files. If there is something in the documentation, that can be
improved, please let us know.

quoted

Long story short:

The following would solve your problem:
   git init
   echo $'dos\r' > dos
   git add dos
   git commit -m "dos newlines"
   echo "dos -crlf" > .gitattributes
   git add .gitattributes
   git commit -m "add attributes"
   echo "dos eol=crlf" > .gitattributes
   git read-tree --empty   # Clean index, force re-scan of working directory

The fact that plumbing is necessary to dig yourself out of a hole of the
`eol` attribute changes points to something needing to be changed, even
if it's only documentation. Could Git detect this and message about it
somehow when `git reset` cannot fix the working tree?

The thing is, that the working tree is "in a good state":
We want "dos" with CRLF, and that is what we have.
There is nothing that can be improved in the working tree.
What needs to be fixed, is the index. And that needs to be done with
"git add" "git commit."
As Junio pointed out, the read-tree is not ideal
(to fix a single file in a possible dirty working tree)

In your case it looks like this:

    echo "dos eol=crlf" > .gitattributes
    git add .gitattributes &&
    git rm --cached dos && git add dos &&
    git commit

Or maybe it could at least exit with failure instead of success?

I don't know.
It -may- be possible to add a warning in "git reset".
I can have a look at that...

On Wed, Aug 23, 2017 at 21:43:15 +0200, Torsten Bögershausen wrote:

git reset does it's job - please see below.

The problem is that we need a "git commit" here.
After applying .gitattributes, it may be neccessary to "normalize" the
files. If there is something in the documentation, that can be
improved, please let us know.

I'll have a patch up shortly.

On Tue, Aug 22, 2017 at 03:44:41PM -0400, Ben Boeckel wrote:

quoted

The fact that plumbing is necessary to dig yourself out of a hole of the
`eol` attribute changes points to something needing to be changed, even
if it's only documentation. Could Git detect this and message about it
somehow when `git reset` cannot fix the working tree?

The thing is, that the working tree is "in a good state":
We want "dos" with CRLF, and that is what we have.
There is nothing that can be improved in the working tree.
What needs to be fixed, is the index. And that needs to be done with
"git add" "git commit."
As Junio pointed out, the read-tree is not ideal
(to fix a single file in a possible dirty working tree)

In your case it looks like this:

    echo "dos eol=crlf" > .gitattributes
    git add .gitattributes &&
    git rm --cached dos && git add dos &&
    git commit

In this case, just adding the file should work: the file is on-disk as
intended and adding the file should normalize the line endings when
adding it into the index (basically, just `git add dos` should be
required to make the index look like it should).

quoted

Or maybe it could at least exit with failure instead of success?

I don't know.
It -may- be possible to add a warning in "git reset".
I can have a look at that...

Thanks.

--Ben

When setting the `eol` attribute, paths can change their dirty status
without any change in the working directory. This can cause confusion
and should at least be mentioned with a remedy.

Signed-off-by: Ben Boeckel <redacted>
---
 Documentation/gitattributes.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index c4f2be2..3044b71 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt

@@ -151,7 +151,11 @@ unspecified.
 
 This attribute sets a specific line-ending style to be used in the
 working directory.  It enables end-of-line conversion without any
-content checks, effectively setting the `text` attribute.
+content checks, effectively setting the `text` attribute.  Note that
+setting this attribute on paths which are in the index with different
+line endings than the attribute indicates the paths to be considered
+dirty.  Adding the path to the index again will normalize the line
+endings in the index.
 
 Set to string value "crlf"::
 

-- 
2.9.5

On Wed, Aug 23, 2017 at 17:17:41 -0400, Ben Boeckel wrote:

Oops, missed a "causes" in here. -----------^

I'll wait on uploading a new patch until after feedback on this one.

+dirty.  Adding the path to the index again will normalize the line
+endings in the index.

--Ben

On Wed, Aug 23, 2017 at 05:17:41PM -0400, Ben Boeckel wrote:

There is one minor comment:
The problem is when files had been commited with CRLF (regardless what your
eol= attribute says)

How about something like this :

  This attribute sets a specific line-ending style to be used in the
  working directory.  It enables end-of-line conversion without any
 -content checks, effectively setting the `text` attribute.
 +content checks, effectively setting the `text` attribute.  Note that
 +setting this attribute on paths which are in the index with CRLF
 +line endings makes the paths to be considered dirty.
 +Adding the path to the index again will normalize the line
 +endings in the index.

On Thu, Aug 24, 2017 at 07:50:54 +0200, Torsten Bögershausen wrote:

  This attribute sets a specific line-ending style to be used in the
  working directory.  It enables end-of-line conversion without any
 -content checks, effectively setting the `text` attribute.
 +content checks, effectively setting the `text` attribute.  Note that
 +setting this attribute on paths which are in the index with CRLF
 +line endings makes the paths to be considered dirty.
 +Adding the path to the index again will normalize the line
 +endings in the index.

Yes, that sounds better. Will resubmit the patch.

--Ben

When setting the `eol` attribute, paths can change their dirty status
without any change in the working directory. This can cause confusion
and should at least be mentioned with a remedy.

Reviewed-by: Torsten Bögershausen <redacted>
Signed-off-by: Ben Boeckel <redacted>
---
 Documentation/gitattributes.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index c4f2be2..b940d37 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt

@@ -151,7 +151,10 @@ unspecified.
 
 This attribute sets a specific line-ending style to be used in the
 working directory.  It enables end-of-line conversion without any
-content checks, effectively setting the `text` attribute.
+content checks, effectively setting the `text` attribute.  Note that
+setting this attribute on paths which are in the index with CRLF line
+endings makes the paths to be considered dirty.  Adding the path to the
+index again will normalize the line endings in the index.
 
 Set to string value "crlf"::
 

-- 
2.9.5

When setting the `eol` attribute, paths can change their dirty status
without any change in the working directory. This can cause confusion
and should at least be mentioned with a remedy.

Reviewed-by: Torsten Bögershausen <redacted>
Reviewed-by: Junio C Hamano <redacted>
Signed-off-by: Ben Boeckel <redacted>
---
 Documentation/gitattributes.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index c4f2be2..4c68bc1 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt

@@ -151,7 +151,10 @@ unspecified.
 
 This attribute sets a specific line-ending style to be used in the
 working directory.  It enables end-of-line conversion without any
-content checks, effectively setting the `text` attribute.
+content checks, effectively setting the `text` attribute.  Note that
+setting this attribute on paths which are in the index with CRLF line
+endings may make the paths to be considered dirty.  Adding the path to
+the index again will normalize the line endings in the index.
 
 Set to string value "crlf"::
 

-- 
2.9.5

On 2017-08-31 15:19, Ben Boeckel wrote:

 This attribute sets a specific line-ending style to be used in the
 working directory.  It enables end-of-line conversion without any
-content checks, effectively setting the `text` attribute.
+content checks, effectively setting the `text` attribute.  Note that
+setting this attribute on paths which are in the index with CRLF line
+endings may make the paths to be considered dirty.  Adding the path to

OK - and this leads to another question:

Would it make sense if we allow files with "text eol=CRLF" to continue
to be stored with CRLF in the index?

Skip the normalization for existing files, since the user asked
for CRLF in the working tree (and the internal storage is less interesting)

Should I consider a patch ?