git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/3] git reset --hard gives clean working tree

From
tboegi@web.de <tboegi@web.de>
Date
Feb 11, 2016, 16:16 UTC
Message-ID
<1455207366-24892-1-git-send-email-tboegi@web.de>
In-Reply-To
<Message-Id=xmqqio26nqk8.fsf@gitster.mtv.corp.google.com>
From: Torsten Bögershausen <tboegi@web.de>
We define the working tree file is clean if either:
  * the result of running convert_to_git() on the working tree
    contents matches what is in the index (because that would mean
    doing another "git add" on the path is a no-op); OR
  * the result of running convert_to_working_tree() on the content
    in the index matches what is in the working tree (because that
    would mean doing another "git checkout -f" on the path is a
    no-op).

Add an extra check in ce_compare_data() in read_cache.c, and adjust the test cases in t0025: When a file has CRLF in the index, and is checked out into the working tree, but left unchabged, it is not normalized at the next commit. Whenever the file is changed in the working tree, a line is added/deleted or dos2unix is run, it may be normalized at the next commit, depending on .gitattributes.

This patch is a result of a longer discussion on the mailing list, how to fix the flaky t0025.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
This is my attempt to help to fix flaky t0025:
 - steal the code from Junio
 - Add test case, change the existing if needed.
 - Spice with some optimizations
The commit messages may be in the state "improvable".
 read-cache.c         | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t0025-crlf-auto.sh | 57 +++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 109 insertions(+), 6 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 394ce14..2948b8f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -156,17 +156,75 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)
 		ce_mark_uptodate(ce);
 }
 
+/*
+ * Compare the data in buf with the data in the file pointed by fd and
+ * return 0 if they are identical, and non-zero if they differ.
+ */
+static int compare_with_fd(const char *input, ssize_t len, int fd)
+{
+	for (;;) {
+		char buf[1024 * 16];
+		ssize_t chunk_len, read_len;
+
+		chunk_len = sizeof(buf) < len ? sizeof(buf) : len;
+		read_len = xread(fd, buf, chunk_len ? chunk_len : 1);
+
+		if (!read_len)
+			/* EOF on the working tree file */
+			return !len ? 0 : -1;
+
+		if (!len)
+			/* we expected there is nothing left */
+			return -1;
+
+		if (memcmp(buf, input, read_len))
+			return -1;
+		input += read_len;
+		len -= read_len;
+	}
+}
+
 static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
 {
 	int match = -1;
 	int fd = open(ce->name, O_RDONLY);
 
+	/*
+	 * Would another "git add" on the path change what is in the
+	 * index for the path?
+	 */
 	if (fd >= 0) {
 		unsigned char sha1[20];
 		if (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, 0))
 			match = hashcmp(sha1, ce->sha1);
 		/* index_fd() closed the file descriptor already */
 	}
+	if (!match)
+		return match;
+
+	/*
+	 * Would another "git checkout -f" out of the index change
+	 * what is in the working tree file?
+	 */
+	fd = open(ce->name, O_RDONLY);
+	if (fd >= 0) {
+		enum object_type type;
+		unsigned long size;
+		void *data = read_sha1_file(ce->sha1, &type, &size);
+
+		if (type == OBJ_BLOB) {
+			struct strbuf worktree = STRBUF_INIT;
+			if (convert_to_working_tree(ce->name, data, size,
+																	&worktree)) {
+				free(data);
+				data = strbuf_detach(&worktree, &size);
+			}
+			if (!compare_with_fd(data, size, fd))
+				match = 0;
+		}
+		free(data);
+		close(fd);
+	}
 	return match;
 }
 
diff --git a/t/t0025-crlf-auto.sh b/t/t0025-crlf-auto.sh
index c164b46..4a7e5c0 100755
--- a/t/t0025-crlf-auto.sh
+++ b/t/t0025-crlf-auto.sh
@@ -14,6 +14,8 @@ test_expect_success setup '
 
 	for w in Hello world how are you; do echo $w; done >LFonly &&
 	for w in I am very very fine thank you; do echo ${w}Q; done | q_to_cr >CRLFonly &&
+	cp CRLFonly CRLFonly1 &&
+	cp CRLFonly CRLFonly2 &&
 	for w in Oh here is a QNUL byte how alarming; do echo ${w}; done | q_to_nul >LFwithNUL &&
 	git add . &&
 
@@ -23,6 +25,28 @@ test_expect_success setup '
 	CRLFonly=$(git rev-parse HEAD:CRLFonly) &&
 	LFwithNUL=$(git rev-parse HEAD:LFwithNUL) &&
 
+	cp CRLFonly CRLFonly.orig &&
+	for w in I am very very fine thank YOU; do echo ${w}Q; done | q_to_cr >CRLFonly.changed &&
+	cat >expnorm  <<-EOF &&
+		MIQ
+		MamQ
+		MveryQ
+		MveryQ
+		MfineQ
+		MthankQ
+		MyouQ
+		PI
+		Pam
+		Pvery
+		Pvery
+		Pfine
+		Pthank
+		PYOU
+	EOF
+	cat >expunor  <<-EOF &&
+		MyouQ
+		PYOUQ
+	EOF
 	echo happy.
 '
 
@@ -39,7 +63,7 @@ test_expect_success 'default settings cause no changes' '
 	test -z "$LFonlydiff" -a -z "$CRLFonlydiff" -a -z "$LFwithNULdiff"
 '
 
-test_expect_success 'crlf=true causes a CRLF file to be normalized' '
+test_expect_success 'crlf=true causes an unchanged CRLF file not to be normalized' '
 
 	# Backwards compatibility check
 	rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
@@ -49,10 +73,10 @@ test_expect_success 'crlf=true causes a CRLF file to be normalized' '
 	# Note, "normalized" means that git will normalize it if added
 	has_cr CRLFonly &&
 	CRLFonlydiff=$(git diff CRLFonly) &&
-	test -n "$CRLFonlydiff"
+	test -z "$CRLFonlydiff"
 '
 
-test_expect_success 'text=true causes a CRLF file to be normalized' '
+test_expect_success 'text=true causes an unchanged CRLF file not to be normalized' '
 
 	rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
 	echo "CRLFonly text" > .gitattributes &&
@@ -61,7 +85,7 @@ test_expect_success 'text=true causes a CRLF file to be normalized' '
 	# Note, "normalized" means that git will normalize it if added
 	has_cr CRLFonly &&
 	CRLFonlydiff=$(git diff CRLFonly) &&
-	test -n "$CRLFonlydiff"
+	test -z "$CRLFonlydiff"
 '
 
 test_expect_success 'eol=crlf gives a normalized file CRLFs with autocrlf=false' '
@@ -114,7 +138,7 @@ test_expect_success 'autocrlf=true does not normalize CRLF files' '
 	test -z "$LFonlydiff" -a -z "$CRLFonlydiff" -a -z "$LFwithNULdiff"
 '
 
-test_expect_success 'text=auto, autocrlf=true _does_ normalize CRLF files' '
+test_expect_success 'text=auto, autocrlf=true  causes an unchanged CRLF file not to be normalized' '
 
 	rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
 	git config core.autocrlf true &&
@@ -126,7 +150,7 @@ test_expect_success 'text=auto, autocrlf=true _does_ normalize CRLF files' '
 	LFonlydiff=$(git diff LFonly) &&
 	CRLFonlydiff=$(git diff CRLFonly) &&
 	LFwithNULdiff=$(git diff LFwithNUL) &&
-	test -z "$LFonlydiff" -a -n "$CRLFonlydiff" -a -z "$LFwithNULdiff"
+	test -z "$LFonlydiff" -a -z "$CRLFonlydiff" -a -z "$LFwithNULdiff"
 '
 
 test_expect_success 'text=auto, autocrlf=true does not normalize binary files' '
@@ -152,4 +176,25 @@ test_expect_success 'eol=crlf _does_ normalize binary files' '
 	test -z "$LFwithNULdiff"
 '
 
+test_expect_success 'crlf=true causes a changed CRLF file to be normalized' '
+	git config core.autocrlf false &&
+	echo "CRLFonly1 crlf" > .gitattributes &&
+	# Note, "normalized" means that git will normalize it if added
+	cp CRLFonly.changed CRLFonly1 &&
+	git add CRLFonly1 &&
+	git diff --cached CRLFonly1 |
+	tr "\015" Q | sed -ne "/^[+-][a-zA-Z]/p" | tr "+-" PM >actual1 &&
+	test_cmp expnorm actual1
+'
+
+test_expect_success 'autocrlf=true causes a changed CRLF file not to be normalized' '
+	git config core.autocrlf true &&
+	echo > .gitattributes &&
+	# Note, "normalized" means that git will normalize it if added
+	cp CRLFonly.changed CRLFonly2 &&
+	git add CRLFonly2 &&
+	git diff --cached CRLFonly2 |
+	tr "\015" Q  | sed -ne "/^[+-][a-zA-Z]/p" |	tr "+-" PM >actual2 &&
+	test_cmp expunor actual2
+'
 test_done
-- 
2.7.0.303.g2c4f448.dirty
Next: Junio C Hamano
Message 1 of 126 in “git reset --hard gives clean working tree”
  1. 1/3 git reset --hard gives clean working treetboegi@web.de, Feb 11, 2016
  2. Junio C HamanoFeb 11, 2016
  3. Torsten BögershausenMar 5, 2016
  4. Junio C HamanoMar 5, 2016
  5. Torsten BögershausenMar 5, 2016
  6. Junio C HamanoMar 5, 2016
  7. Junio C HamanoMar 7, 2016
  8. Junio C HamanoMar 7, 2016
  9. Torsten BögershausenMar 7, 2016
  10. Junio C HamanoMar 7, 2016
  11. 1/7 Make it possible to get sha1 for a path from the indextboegi@web.de, Mar 29, 2016
  12. Duy NguyenMar 29, 2016
  13. Duy NguyenMar 29, 2016
  14. Torsten BögershausenMar 29, 2016
  15. Eric SunshineMar 29, 2016
  16. 2/7 convert.c: stream and early outtboegi@web.de, Mar 29, 2016
  17. 3/7 Allow core.autocrlf=input and core.eol=crlftboegi@web.de, Mar 29, 2016
  18. 4/7 t0027: TC for combined attributestboegi@web.de, Mar 29, 2016
  19. 5/7 CRLF: unify the "auto" handlingtboegi@web.de, Mar 29, 2016
  20. Eric SunshineMar 29, 2016
  21. 6/7 correct blame for files commited with CRLFtboegi@web.de, Mar 29, 2016
  22. Junio C HamanoMar 29, 2016
  23. Torsten BögershausenMar 29, 2016
  24. Junio C HamanoMar 29, 2016
  25. Junio C HamanoMar 29, 2016
  26. Junio C HamanoMar 29, 2016
  27. Junio C HamanoMar 29, 2016
  28. Torsten BögershausenMar 30, 2016
  29. 7/7 convert.c: more safer crlf handling with text attributetboegi@web.de, Mar 29, 2016
  30. Junio C HamanoMar 29, 2016
  31. 1/7 Make it possible to get sha1 for a path from the indextboegi@web.de, Apr 1, 2016
  32. 2/7 convert.c: stream and early outtboegi@web.de, Apr 1, 2016
  33. 3/7 Allow core.autocrlf=input and core.eol=crlftboegi@web.de, Apr 1, 2016
  34. Junio C HamanoApr 1, 2016
  35. 4/7 t0027: TC for combined attributestboegi@web.de, Apr 1, 2016
  36. Junio C HamanoApr 1, 2016
  37. 5/7 CRLF: unify the "auto" handlingtboegi@web.de, Apr 1, 2016
  38. Junio C HamanoApr 1, 2016
  39. 6/7 correct blame for files commited with CRLFtboegi@web.de, Apr 1, 2016
  40. Junio C HamanoApr 1, 2016
  41. Torsten BögershausenApr 3, 2016
  42. 7/7 convert.c: more safer crlf handling with text attributetboegi@web.de, Apr 1, 2016
  43. correct blame for files commited with CRLFtboegi@web.de, Apr 5, 2016
  44. Junio C HamanoApr 5, 2016
  45. Junio C HamanoApr 5, 2016
  46. Torsten BögershausenApr 6, 2016
  47. 1/4 t0027: Make more reliabletboegi@web.de, Apr 19, 2016
  48. 2/4 convert: allow core.autocrlf=input and core.eol=crlftboegi@web.de, Apr 19, 2016
  49. 3/4 t0027: test cases for combined attributestboegi@web.de, Apr 19, 2016
  50. Junio C HamanoApr 19, 2016
  51. Torsten BögershausenApr 20, 2016
  52. 4/4 convert.c: ident + core.autocrlf didn't worktboegi@web.de, Apr 19, 2016
  53. Junio C HamanoApr 20, 2016
  54. 01/10 t0027: Make more reliabletboegi@web.de, Apr 22, 2016
  55. Junio C HamanoApr 22, 2016
  56. Torsten BögershausenApr 24, 2016
  57. 02/10 convert: allow core.autocrlf=input and core.eol=crlftboegi@web.de, Apr 22, 2016
  58. 03/10 t0027: test cases for combined attributestboegi@web.de, Apr 22, 2016
  59. 04/10 convert.c: ident + core.autocrlf didn't worktboegi@web.de, Apr 22, 2016
  60. 05/10 read-cache: factor out get_sha1_from_index() helpertboegi@web.de, Apr 22, 2016
  61. 06/10 convert.c: stream and early outtboegi@web.de, Apr 22, 2016
  62. 07/10 convert: unify the "auto" handling of CRLFtboegi@web.de, Apr 22, 2016
  63. 08/10 convert.c: more safer crlf handling with text attributetboegi@web.de, Apr 22, 2016
  64. 09/10 t6038; use crlf on all platformstboegi@web.de, Apr 22, 2016
  65. 10/10 ce_compare_data() did not respect conversiontboegi@web.de, Apr 22, 2016
  66. 01/10 t0027: Make commit_chk_wrnNNO() reliabletboegi@web.de, Apr 24, 2016
  67. 02/10 convert: allow core.autocrlf=input and core.eol=crlftboegi@web.de, Apr 24, 2016
  68. 03/10 t0027: test cases for combined attributestboegi@web.de, Apr 24, 2016
  69. 04/10 convert.c: ident + core.autocrlf didn't worktboegi@web.de, Apr 24, 2016
  70. 05/10 read-cache: factor out get_sha1_from_index() helpertboegi@web.de, Apr 24, 2016
  71. 06/10 convert.c: stream and early outtboegi@web.de, Apr 24, 2016
  72. 07/10 convert: unify the "auto" handling of CRLFtboegi@web.de, Apr 24, 2016
  73. 08/10 convert.c: more safer crlf handling with text attributetboegi@web.de, Apr 24, 2016
  74. 09/10 t6038; use crlf on all platformstboegi@web.de, Apr 24, 2016
  75. 10/10 ce_compare_data() did not respect conversiontboegi@web.de, Apr 24, 2016
  76. 01/10 t0027: Make commit_chk_wrnNNO() reliabletboegi@web.de, Apr 25, 2016
  77. Junio C HamanoApr 25, 2016
  78. 02/10 convert: allow core.autocrlf=input and core.eol=crlftboegi@web.de, Apr 25, 2016
  79. 03/10 t0027: test cases for combined attributestboegi@web.de, Apr 25, 2016
  80. 04/10 convert.c: ident + core.autocrlf didn't worktboegi@web.de, Apr 25, 2016
  81. 05/10 read-cache: factor out get_sha1_from_index() helpertboegi@web.de, Apr 25, 2016
  82. 06/10 convert.c: stream and early outtboegi@web.de, Apr 25, 2016
  83. 07/10 convert: unify the "auto" handling of CRLFtboegi@web.de, Apr 25, 2016
  84. Junio C HamanoApr 25, 2016
  85. Torsten BögershausenApr 26, 2016
  86. Junio C HamanoApr 26, 2016
  87. 08/10 convert.c: more safer crlf handling with text attributetboegi@web.de, Apr 25, 2016
  88. 09/10 t6038; use crlf on all platformstboegi@web.de, Apr 25, 2016
  89. 10/10 ce_compare_data() did not respect conversiontboegi@web.de, Apr 25, 2016
  90. 01/10 t0027: make commit_chk_wrnNNO() reliabletboegi@web.de, Apr 29, 2016
  91. 02/10 convert: allow core.autocrlf=input and core.eol=crlftboegi@web.de, Apr 29, 2016
  92. 03/10 t0027: test cases for combined attributestboegi@web.de, Apr 29, 2016
  93. 04/10 convert.c: ident + core.autocrlf didn't worktboegi@web.de, Apr 29, 2016
  94. 05/10 read-cache: factor out get_sha1_from_index() helpertboegi@web.de, Apr 29, 2016
  95. 06/10 convert.c: stream and early outtboegi@web.de, Apr 29, 2016
  96. 07/10 convert: unify the "auto" handling of CRLFtboegi@web.de, Apr 29, 2016
  97. Torsten BögershausenNov 25, 2016
  98. 1/1 New way to normalize the line endingstboegi@web.de, Nov 27, 2016
  99. Junio C HamanoNov 29, 2016
  100. 1/1 Document how to normalize the line endingstboegi@web.de, Apr 12, 2017
  101. 08/10 convert.c: more safer crlf handling with text attributetboegi@web.de, Apr 29, 2016
  102. 09/10 t6038; use crlf on all platformstboegi@web.de, Apr 29, 2016
  103. 10/10 ce_compare_data() did not respect conversiontboegi@web.de, Apr 29, 2016
  104. Junio C HamanoApr 29, 2016
  105. Junio C HamanoApr 29, 2016
  106. Torsten BögershausenMay 1, 2016
  107. Junio C HamanoMay 2, 2016
  108. Junio C HamanoMay 2, 2016
  109. Torsten BögershausenMay 3, 2016
  110. Junio C HamanoMay 3, 2016
  111. Torsten BögershausenMay 4, 2016
  112. Junio C HamanoMay 4, 2016
  113. Torsten BögershausenMay 6, 2016
  114. Junio C HamanoMay 6, 2016
  115. 0/6 convert-eol-autocrlf, old 5..10 now 1..6tboegi@web.de, May 7, 2016
  116. 1/6 read-cache: factor out get_sha1_from_index() helpertboegi@web.de, May 7, 2016
  117. Junio C HamanoMay 9, 2016
  118. 2/6 convert.c: stream and early outtboegi@web.de, May 7, 2016
  119. Junio C HamanoMay 9, 2016
  120. Torsten BögershausenMay 11, 2016
  121. 3/6 convert: unify the "auto" handling of CRLFtboegi@web.de, May 7, 2016
  122. 4/6 convert.c: more safer crlf handling with text attributetboegi@web.de, May 7, 2016
  123. 5/6 t6038; use crlf on all platformstboegi@web.de, May 7, 2016
  124. 6/6 convert: ce_compare_data() checks for a sha1 of a pathtboegi@web.de, May 7, 2016
  125. 2/3 Factor out convert_cmp_checkout() into convert.ctboegi@web.de, Feb 11, 2016
  126. 3/3 convert.c: Optimize convert_cmp_checkout() for changed file lentboegi@web.de, Feb 11, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.