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

[Junio C Hamano <gitster@pobox.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 &&