Skip to contentSkip to conversation

Cannot checkout after setting the eol attribute

PATCH Superseded Ben Boeckel 3 participants 15 messages Aug 22, 2017 – Aug 31, 2017 Documentation

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 31 lines · Ben Boeckel

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:

Quoted 8 lines
    % 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

Quoted 4 lines

Seen in 2.9.5 and 2.14.0.rc1.

--Ben

#2 lore raw
Quoted 3 lines · Torsten Bögershausen

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 5 lines

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 :) .

Quoted 12 lines

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

#3 lore raw
Quoted 8 lines · Ben Boeckel
Quoted 3 lines · Torsten Bögershausen

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 8 lines
Quoted 5 lines

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 14 lines
Quoted 13 lines

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

Quoted 4 lines

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
Quoted 1 line

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...

#4 lore raw
Quoted 6 lines · Torsten Bögershausen

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.

Quoted 14 lines · Ben Boeckel
Quoted 4 lines

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)

Quoted 6 lines

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 5 lines
Quoted 1 line

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

#5 lore raw
In reply to Torsten Bögershausen
Quoted 10 lines · Torsten Bögershausen

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

Quoted 2 lines

git read-tree --empty # Clean index, force re-scan of working directory
git add dos

This is not advisable as a general suggestion, as in a real life (as opposed to a toy example) there will be paths other than this 'dos' thing in the working tree. Perhaps replace "git read-tree --empty" with "git rm -f --cached dos" or something like that that limits its damage would prevent mistakes better.

Quoted 3 lines

git add .gitattributes
git commit -m "set eol attribute instead"
git ls-files --eol

#6 lore raw

PATCH 1/1 Documentation: mention that `eol` can change the dirty status of paths

In reply to Ben Boeckel

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.

---
 Documentation/gitattributes.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
Documentation/gitattributes.txt +5 -1
index c4f2be2..3044b71 100644
@@ -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"::
 
Signature · 1 line
2.9.5
#7 lore raw

Re: [PATCH] Documentation: mention that `eol` can change the dirty status of paths

Documentation/gitattributes.txt unspecified. · line 156

Quoted 12 lines · Ben Boeckel
Documentation/gitattributes.txt +3 -1
index c4f2be2..3044b71 100644
@@ -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

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

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

Documentation/gitattributes.txt unspecified. · line 158

Quoted 2 lines
Documentation/gitattributes.txt +2 -0
+dirty.  Adding the path to the index again will normalize the line
+endings in the index.

--Ben

#8 lore raw

Re: [PATCH] Documentation: mention that `eol` can change the dirty status of paths

In reply to Ben Boeckel
Quoted 14 lines · Ben Boeckel

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.

---
 Documentation/gitattributes.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
Documentation/gitattributes.txt +0 -0
index c4f2be2..3044b71 100644

Documentation/gitattributes.txt unspecified. · line 159

Quoted 10 lines
Documentation/gitattributes.txt +5 -1
@@ -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.
 

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.

#9 lore raw

Re: [PATCH] Documentation: mention that `eol` can change the dirty status of paths

Quoted 8 lines · Torsten Bögershausen

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

#10 lore raw

Re: [PATCH] Documentation: mention that `eol` can change the dirty status of paths

In reply to Torsten Bögershausen
Quoted 8 lines · Torsten Bögershausen

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.

Is this "always makes them dirty" or "may make them dirty depending on the situation"?

If the latter, I'd prefer to see s/makes/may make/ here.

#11 lore raw

Re: [PATCH] Documentation: mention that `eol` can change the dirty status of paths

Quoted 4 lines · Junio C Hamano

Is this "always makes them dirty" or "may make them dirty depending on the situation"?

If the latter, I'd prefer to see s/makes/may make/ here.

Yes, a coworker reported that some files slipped through this. New patch incoming.

--Ben

#12 lore raw

PATCH v2 1/1 Documentation: mention that `eol` can change the dirty status of paths

In reply to Ben Boeckel

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.

---
 Documentation/gitattributes.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Documentation/gitattributes.txt +4 -1
index c4f2be2..b940d37 100644
@@ -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"::
 
Signature · 1 line
2.9.5
#13 lore raw

PATCH v3 1/1 Documentation: mention that `eol` can change the dirty status of paths

In reply to Ben Boeckel

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.

---
 Documentation/gitattributes.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Documentation/gitattributes.txt +4 -1
index c4f2be2..4c68bc1 100644
@@ -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"::
 
Signature · 1 line
2.9.5
#14 lore raw

Re: [PATCH v3] Documentation: mention that `eol` can change the dirty status of paths

Documentation/gitattributes.txt unspecified. · line 156

Quoted 6 lines · Ben Boeckel

This attribute sets a specific line-ending style to be used in the working directory. It enables end-of-line conversion without any

Documentation/gitattributes.txt +3 -1
-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 ?