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

Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 8, 2016, 17:59 UTC
Message-ID
<xmqqwpkwko3w.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<xmqq1t34m47z.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Not yet.  As I called it "experiment", it was merely to demonstrate
> that there are less intrusive ways to kill the "safer crlf" we may
> want to consider first before passing an extra blob object name
> around.

Here is another approach, that probably is less intrusive. It contains the test updates from your 3/3, and you can apply it directly on top of 65237284 (convert: unify the "auto" handling of CRLF, 2016-06-28), which is the result of applying your 1/3 on top of v2.9.0-rc0~11^2 (convert.c: ident + core.autocrlf didn't work, 2016-04-25).

Instead of letting add_cacheinfo() to refresh individual cache entries as they are added, we just leave them as they are, which would bypass the make_cache_entry() -> refresh_cache_entry() -> ... -> ce_compare_data() -> "safer crlf" callchain. After we are done all (in builtin/merge.c that directly calls merge_recursive(), and also merge_recursive_generic()), we let refresh_cache() to take care of the refreshing of the stat information--by the time this happens, the stale cache entries that had CRLF in stage #2 that were carried over before the renormalizing merge started will all be gone and will not interfere.

 builtin/merge.c            |  3 ++-
 merge-recursive.c          | 20 +++++++++---------
 t/t6038-merge-text-auto.sh | 51 +++++++++++++++++++++++++---------------------
 3 files changed, 40 insertions(+), 34 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 101ffef..d5bf68d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -678,7 +678,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		hold_locked_index(&lock, 1);
 		clean = merge_recursive(&o, head,
-				remoteheads->item, reversed, &result);
+					remoteheads->item, reversed, &result);
+		refresh_cache(CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
 		if (active_cache_changed &&
 		    write_locked_index(&the_index, &lock, COMMIT_LOCK))
 			die (_("unable to write %s"), get_index_file());
diff --git a/merge-recursive.c b/merge-recursive.c
index b880ae5..8aaf1b5 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -199,14 +199,14 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)
 }
 
 static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
-		const char *path, int stage, int refresh, int options)
+			 const char *path, int stage, int options)
 {
 	struct cache_entry *ce;
-	ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage,
-			      (refresh ? (CE_MATCH_REFRESH |
-					  CE_MATCH_IGNORE_MISSING) : 0 ));
+
+	ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage, 0);
 	if (!ce)
 		return error(_("addinfo_cache failed for path '%s'"), path);
+
 	return add_cache_entry(ce, options);
 }
 
@@ -552,13 +552,13 @@ static int update_stages(const char *path, const struct diff_filespec *o,
 		if (remove_file_from_cache(path))
 			return -1;
 	if (o)
-		if (add_cacheinfo(o->mode, o->sha1, path, 1, 0, options))
+		if (add_cacheinfo(o->mode, o->sha1, path, 1, options))
 			return -1;
 	if (a)
-		if (add_cacheinfo(a->mode, a->sha1, path, 2, 0, options))
+		if (add_cacheinfo(a->mode, a->sha1, path, 2, options))
 			return -1;
 	if (b)
-		if (add_cacheinfo(b->mode, b->sha1, path, 3, 0, options))
+		if (add_cacheinfo(b->mode, b->sha1, path, 3, options))
 			return -1;
 	return 0;
 }
@@ -804,7 +804,7 @@ static void update_file_flags(struct merge_options *o,
 	}
  update_index:
 	if (update_cache)
-		add_cacheinfo(mode, sha, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);
+		add_cacheinfo(mode, sha, path, 0, ADD_CACHE_OK_TO_ADD);
 }
 
 static void update_file(struct merge_options *o,
@@ -1638,8 +1638,7 @@ static int merge_content(struct merge_options *o,
 		 */
 		path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
 		if (!path_renamed_outside_HEAD) {
-			add_cacheinfo(mfi.mode, mfi.sha, path,
-				      0, (!o->call_depth), 0);
+			add_cacheinfo(mfi.mode, mfi.sha, path, 0, 0);
 			return mfi.clean;
 		}
 	} else
@@ -2019,6 +2018,7 @@ int merge_recursive_generic(struct merge_options *o,
 	hold_locked_index(lock, 1);
 	clean = merge_recursive(o, head_commit, next_commit, ca,
 			result);
+	refresh_cache(CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
 	if (active_cache_changed &&
 	    write_locked_index(&the_index, lock, COMMIT_LOCK))
 		return error(_("Unable to write index."));
diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh
index 33b77ee..5e8d5fa 100755
--- a/t/t6038-merge-text-auto.sh
+++ b/t/t6038-merge-text-auto.sh
@@ -91,16 +91,13 @@ test_expect_success 'Merge after setting text=auto' '
 	compare_files expected file
 '
 
-test_expect_success 'Merge addition of text=auto' '
+test_expect_success 'Merge addition of text=auto eol=LF' '
+	git config core.eol lf &&
 	cat <<-\EOF >expected &&
 	first line
 	same line
 	EOF
 
-	if test_have_prereq NATIVE_CRLF; then
-		append_cr <expected >expected.temp &&
-		mv expected.temp expected
-	fi &&
 	git config merge.renormalize true &&
 	git rm -fr . &&
 	rm -f .gitattributes &&
@@ -109,17 +106,31 @@ test_expect_success 'Merge addition of text=auto' '
 	compare_files  expected file
 '
 
+test_expect_success 'Merge addition of text=auto eol=CRLF' '
+	git config core.eol crlf &&
+	cat <<-\EOF >expected &&
+	first line
+	same line
+	EOF
+
+	append_cr <expected >expected.temp &&
+	mv expected.temp expected &&
+	git config merge.renormalize true &&
+	git rm -fr . &&
+	rm -f .gitattributes &&
+	git reset --hard b &&
+	echo >&2 "After git reset --hard b" &&
+	git ls-files -s --eol >&2 &&
+	git merge a &&
+	compare_files  expected file
+'
+
 test_expect_success 'Detect CRLF/LF conflict after setting text=auto' '
+	git config core.eol native &&
 	echo "<<<<<<<" >expected &&
-	if test_have_prereq NATIVE_CRLF; then
-		echo first line | append_cr >>expected &&
-		echo same line | append_cr >>expected &&
-		echo ======= | append_cr >>expected
-	else
-		echo first line >>expected &&
-		echo same line >>expected &&
-		echo ======= >>expected
-	fi &&
+	echo first line >>expected &&
+	echo same line >>expected &&
+	echo ======= >>expected &&
 	echo first line | append_cr >>expected &&
 	echo same line | append_cr >>expected &&
 	echo ">>>>>>>" >>expected &&
@@ -135,15 +146,9 @@ test_expect_success 'Detect LF/CRLF conflict from addition of text=auto' '
 	echo "<<<<<<<" >expected &&
 	echo first line | append_cr >>expected &&
 	echo same line | append_cr >>expected &&
-	if test_have_prereq NATIVE_CRLF; then
-		echo ======= | append_cr >>expected &&
-		echo first line | append_cr >>expected &&
-		echo same line | append_cr >>expected
-	else
-		echo ======= >>expected &&
-		echo first line >>expected &&
-		echo same line >>expected
-	fi &&
+	echo ======= >>expected &&
+	echo first line >>expected &&
+	echo same line >>expected &&
 	echo ">>>>>>>" >>expected &&
 	git config merge.renormalize false &&
 	rm -f .gitattributes &&
Previous: Junio C HamanoNext: Junio C Hamano
Message 18 of 24 in “What's cooking in git.git (Jun 2016, #09; Mon, 27)”
  1. Junio C HamanoJun 27, 2016
  2. 0/3 unified auto CRLF handling, V3tboegi@web.de, Jun 28, 2016
  3. 1/3 convert: unify the "auto" handling of CRLFtboegi@web.de, Jun 28, 2016
  4. 2/3 read-cache: factor out get_sha1_from_index() helpertboegi@web.de, Jun 28, 2016
  5. 3/3 correct ce_compare_data() in a middle of a mergetboegi@web.de, Jun 28, 2016
  6. Junio C HamanoJun 29, 2016
  7. Torsten BögershausenJun 30, 2016
  8. Junio C HamanoJul 1, 2016
  9. Torsten BögershausenJul 2, 2016
  10. Junio C HamanoJul 6, 2016
  11. Torsten BögershausenJul 7, 2016
  12. Junio C HamanoJul 7, 2016
  13. Junio C HamanoJul 7, 2016
  14. Torsten BögershausenJul 8, 2016
  15. Junio C HamanoJul 8, 2016
  16. Torsten BögershausenJul 8, 2016
  17. Junio C HamanoJul 8, 2016
  18. Junio C HamanoJul 8, 2016
  19. Junio C HamanoJul 8, 2016
  20. Junio C HamanoJul 8, 2016
  21. Junio C HamanoJul 11, 2016
  22. Torsten BögershausenJul 12, 2016
  23. Junio C HamanoJul 12, 2016
  24. Torsten BögershausenJul 8, 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.