Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge
flat view
From: Junio C Hamano <hidden>
Date: 2016-07-08 17:59:23
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-07-12 · Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge · Junio C Hamano <hidden>
- 2016-07-12 · Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge · Torsten Bögershausen <hidden>
- 2016-07-11 · Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge · Junio C Hamano <hidden>
- 2016-07-08 · Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge · Junio C Hamano <hidden>
- 2016-07-08 · Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge · Junio C Hamano <hidden>
Junio C Hamano [off-list ref] 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 &&