[PATCH/RFC] blame: CRLF in the working tree and LF in the repo

Subsystems: the rest

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH/RFC] blame: CRLF in the working tree and LF in the repo

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:04:33

A typicall setup under Windows:
core.eol is CRLF and a file is marked as "text" in .gitattributes.

After 4d4813a5 "git blame" no longer works as expected,
every line is annotated as "Not Committed Yet",
even though the working directory is clean.

commit 4d4813a5 removed the conversion in blame.c for all files,
with or without CRLF in the repo.

Having files with CRLF in the repo and core.autocrlf=input is a temporary
situation, the files should be normalized in the repo.
Blaming them with "Not Committed Yet" is OK.

The solution is to revert commit 4d4813a5.

Reported-By: Stepan Kasal <redacted>
Signed-off-by: Torsten Bögershausen <redacted>
---
Reference:
https://github.com/git-for-windows/git/issues/105
Although the intention of 4d4813a5 is good, it breaks
the usual EOL-handling for Windows.
Until we have a better solution, we suggest to revert it.

 builtin/blame.c               |  1 +
 t/t8003-blame-corner-cases.sh | 26 +++++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 06484c2..8d70623 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2348,6 +2348,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
 		if (strbuf_read(&buf, 0, 0) < 0)
 			die_errno("failed to read from stdin");
 	}
+	convert_to_git(path, buf.buf, buf.len, &buf, 0);
 	origin->file.ptr = buf.buf;
 	origin->file.size = buf.len;
 	pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);
diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 32895e5..dcc9827 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -191,7 +191,7 @@ test_expect_success 'indent of line numbers, ten lines' '
 	test $(grep -c "  " actual) = 9
 '
 
-test_expect_success 'blaming files with CRLF newlines' '
+test_expect_failure 'blaming files with CRLF newlines in repo, core.autoclrf=input' '
 	git config core.autocrlf false &&
 	printf "testcase\r\n" >crlffile &&
 	git add crlffile &&
@@ -199,5 +199,29 @@ test_expect_success 'blaming files with CRLF newlines' '
 	git -c core.autocrlf=input blame crlffile >actual &&
 	grep "A U Thor" actual
 '
+test_expect_success 'blaming files with CRLF newlines core.autocrlf=true' '
+	test_create_repo blamerepo &&
+	(
+		cd blamerepo &&
+		git config core.autocrlf true &&
+		printf "testcase\r\n" >crlffile &&
+		git add crlffile &&
+		git commit -m TRUE &&
+		git blame crlffile >actual &&
+		grep "A U Thor" actual
+	)
+'
+
+test_expect_success 'blaming files with CRLF newlines core.autocrlf=false' '
+	(
+		cd blamerepo &&
+		git config core.autocrlf false &&
+		printf ".gitattributes text\r\n" >.gitattributes &&
+		git add .gitattributes &&
+		git commit -m FALSE &&
+		git blame .gitattributes >actual &&
+		grep "A U Thor" actual
+	)
+'
 
 test_done
-- 
2.2.0.rc1.790.ge19fcd2

Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:04:33

On Sun, Apr 26, 2015 at 8:02 AM, Torsten Bögershausen [off-list ref] wrote:
A typicall setup under Windows:
s/typicall/typical/
core.eol is CRLF and a file is marked as "text" in .gitattributes.

After 4d4813a5 "git blame" no longer works as expected,
every line is annotated as "Not Committed Yet",
even though the working directory is clean.

commit 4d4813a5 removed the conversion in blame.c for all files,
with or without CRLF in the repo.

Having files with CRLF in the repo and core.autocrlf=input is a temporary
situation, the files should be normalized in the repo.
Blaming them with "Not Committed Yet" is OK.

The solution is to revert commit 4d4813a5.

Reported-By: Stepan Kasal <redacted>
Signed-off-by: Torsten Bögershausen <redacted>

Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo

From: Stepan Kasal <hidden>
Date: 2016-06-15 23:04:33

Hello,

thank you Torsten for the patch [I'm the reporter, but could not do
it myself]
-test_expect_success 'blaming files with CRLF newlines' '
+test_expect_failure 'blaming files with CRLF newlines in repo, core.autoclrf=input' '
Shouldn't the old test be rather removed?
It deals with an invalid situation.

I thought that having crlf in the repo is incorrect, so no wonder
that it fails if the files in the working tree are changed to LF.

And changing the autocrlf transformation is effectively the same,
no matter that the files _physically_ are the same as the files in
the repo.

Have a nice day,
    Stepan Kasal
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help