{"thread":{"id":"37502","subject":"[PATCH v3 1/8] merge-recursive: remove dead conditional in update_stages()","startedAt":"2014-09-06T17:56:58Z","lastAt":"2014-09-11T19:37:45Z","messageCount":18,"participants":["Thomas Rast","Junio C Hamano","Jens Lehmann"],"isPatch":true,"patchVersion":3,"patchTotal":8},"messages":[{"id":"248982","messageId":"cover.1409860234.git.tr@thomasrast.ch","threadId":"37502","inReplyTo":null,"subject":"[PATCH v3 0/8] --remerge-diff","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-09-06T17:56:58Z","receivedAt":"2014-09-06T17:56:58Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"This is a resend of the remerge-diff patch series, previously posted\nhere:\n\n http://thread.gmane.org/gmane.comp.version-control.git/242514\n\nDifferences to the previous version:\n\n- Rebased onto the new {name,dir}_hash maps (7/8 looks very different\n now). This also allows freeing index entries that we no longer need\n (in 8/8); previously, the insert-only name-hash kept them alive.\n\n- Adaptations to match Duy's changes to cache_tree handling (in 8/8).\n Please review the cache_tree handling extra carefully, as I'm not\n 100% convinced the dance there is all that is needed.\n\n\n\nThomas Rast (8):\n merge-recursive: remove dead conditional in update_stages()\n merge-recursive: internal flag to avoid touching the worktree\n merge-recursive: -Xindex-only to leave worktree unchanged\n combine-diff: do not pass revs->dense_combined_merges redundantly\n Fold all merge diff variants into an enum\n merge-recursive: allow storing conflict hunks in index\n name-hash: allow dir hashing even when !ignore_case\n log --remerge-diff: show what the conflict resolution changed\n\n Documentation/merge-strategies.txt | 9 ++\n Documentation/rev-list-options.txt | 7 +\n builtin/diff-files.c | 5 +-\n builtin/diff-tree.c | 2 +-\n builtin/diff.c | 12 +-\n builtin/fmt-merge-msg.c | 2 +-\n builtin/log.c | 9 +-\n builtin/merge.c | 1 -\n cache.h | 2 +\n combine-diff.c | 13 +-\n diff-lib.c | 13 +-\n diff.h | 6 +-\n log-tree.c | 303 ++++++++++++++++++++++++++++++++++++-\n merge-recursive.c | 52 ++++---\n merge-recursive.h | 3 +\n name-hash.c | 13 +-\n revision.c | 15 +-\n revision.h | 24 ++-\n submodule.c | 3 +-\n t/t3030-merge-recursive.sh | 33 ++++\n t/t4213-log-remerge-diff.sh | 222 +++++++++++++++++++++++++++\n 21 files changed, 673 insertions(+), 76 deletions(-)\n create mode 100755 t/t4213-log-remerge-diff.sh\n\n-- \n2.1.0.72.g9b94086\n"},{"id":"248981","messageId":"407c0fe475316267949c3043d8e6457ae793835c.1409860234.git.tr@thomasrast.ch","threadId":"37502","inReplyTo":"cover.1409860234.git.tr@thomasrast.ch","subject":"[PATCH v3 1/8] merge-recursive: remove dead conditional in update_stages()","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-09-06T17:56:59Z","receivedAt":"2014-09-06T17:56:59Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"From: Thomas Rast \n\n650467c (merge-recursive: Consolidate different update_stages\nfunctions, 2011-08-11) changed the former argument 'clear' to always\nbe true. Remove the useless conditional.\n\nSigned-off-by: Thomas Rast \nSigned-off-by: Junio C Hamano \n---\n merge-recursive.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 1d332b8..4459607 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -547,11 +547,9 @@ static int update_stages(const char *path, const struct diff_filespec *o,\n \t * would_lose_untracked). Instead, reverse the order of the calls\n \t * (executing update_file first and then update_stages).\n \t */\n-\tint clear = 1;\n \tint options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK;\n-\tif (clear)\n-\t\tif (remove_file_from_cache(path))\n-\t\t\treturn -1;\n+\tif (remove_file_from_cache(path))\n+\t\treturn -1;\n \tif (o)\n \t\tif (add_cacheinfo(o->mode, o->sha1, path, 1, 0, options))\n \t\t\treturn -1;\n-- \n2.1.0.72.g9b94086\n"},{"id":"248983","messageId":"5bd5960659d85943477d2a5fbca3dd5ccd0da686.1409860234.git.tr@thomasrast.ch","threadId":"37502","inReplyTo":"cover.1409860234.git.tr@thomasrast.ch","subject":"[PATCH v3 2/8] merge-recursive: internal flag to avoid touching the worktree","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-09-06T17:57:00Z","receivedAt":"2014-09-06T17:57:00Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"From: Thomas Rast \n\no->call_depth has a double function: a nonzero call_depth means we\nwant to construct virtual merge bases, but it also means we want to\navoid touching the worktree. Introduce a new flag o->no_worktree to\ntrigger only the latter.\n\nSigned-off-by: Thomas Rast \nSigned-off-by: Junio C Hamano \n---\n merge-recursive.c | 37 +++++++++++++++++++++----------------\n merge-recursive.h | 1 +\n 2 files changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 4459607..059ad03 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -410,10 +410,10 @@ static void record_df_conflict_files(struct merge_options *o,\n \tint i;\n \n \t/*\n-\t * If we're merging merge-bases, we don't want to bother with\n-\t * any working directory changes.\n+\t * If we're working in-core only (e.g., merging merge-bases),\n+\t * we don't want to bother with any working directory changes.\n \t */\n-\tif (o->call_depth)\n+\tif (o->call_depth || o->no_worktree)\n \t\treturn;\n \n \t/* Ensure D/F conflicts are adjacent in the entries list. */\n@@ -743,7 +743,7 @@ static void update_file_flags(struct merge_options *o,\n \t\t\t int update_cache,\n \t\t\t int update_wd)\n {\n-\tif (o->call_depth)\n+\tif (o->call_depth || o->no_worktree)\n \t\tupdate_wd = 0;\n \n \tif (update_wd) {\n@@ -950,7 +950,8 @@ static struct merge_file_info merge_file_1(struct merge_options *o,\n \t\t\tresult.clean = merge_submodule(result.sha,\n \t\t\t\t\t\t one->path, one->sha1,\n \t\t\t\t\t\t a->sha1, b->sha1,\n-\t\t\t\t\t\t !o->call_depth);\n+\t\t\t\t\t\t !(o->call_depth ||\n+\t\t\t\t\t\t\t o->no_worktree));\n \t\t} else if (S_ISLNK(a->mode)) {\n \t\t\thashcpy(result.sha, a->sha1);\n \n@@ -1018,7 +1019,7 @@ static void handle_change_delete(struct merge_options *o,\n \t\t\t\t const char *change, const char *change_past)\n {\n \tchar *renamed = NULL;\n-\tif (dir_in_way(path, !o->call_depth)) {\n+\tif (dir_in_way(path, !(o->call_depth || o->no_worktree))) {\n \t\trenamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);\n \t}\n \n@@ -1143,10 +1144,10 @@ static void handle_file(struct merge_options *o,\n \t\tchar *add_name = unique_path(o, rename->path, other_branch);\n \t\tupdate_file(o, 0, add->sha1, add->mode, add_name);\n \n-\t\tremove_file(o, 0, rename->path, 0);\n+\t\tremove_file(o, 0, rename->path, o->call_depth || o->no_worktree);\n \t\tdst_name = unique_path(o, rename->path, cur_branch);\n \t} else {\n-\t\tif (dir_in_way(rename->path, !o->call_depth)) {\n+\t\tif (dir_in_way(rename->path, !(o->call_depth || o->no_worktree))) {\n \t\t\tdst_name = unique_path(o, rename->path, cur_branch);\n \t\t\toutput(o, 1, _(\"%s is a directory in %s adding as %s instead\"),\n \t\t\t rename->path, other_branch, dst_name);\n@@ -1253,7 +1254,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o,\n \t\t * merge base just undo the renames; they can be detected\n \t\t * again later for the non-recursive merge.\n \t\t */\n-\t\tremove_file(o, 0, path, 0);\n+\t\tremove_file(o, 0, path, o->call_depth || o->no_worktree);\n \t\tupdate_file(o, 0, mfi_c1.sha, mfi_c1.mode, a->path);\n \t\tupdate_file(o, 0, mfi_c2.sha, mfi_c2.mode, b->path);\n \t} else {\n@@ -1261,7 +1262,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o,\n \t\tchar *new_path2 = unique_path(o, path, ci->branch2);\n \t\toutput(o, 1, _(\"Renaming %s to %s and %s to %s instead\"),\n \t\t a->path, new_path1, b->path, new_path2);\n-\t\tremove_file(o, 0, path, 0);\n+\t\tremove_file(o, 0, path, o->call_depth || o->no_worktree);\n \t\tupdate_file(o, 0, mfi_c1.sha, mfi_c1.mode, new_path1);\n \t\tupdate_file(o, 0, mfi_c2.sha, mfi_c2.mode, new_path2);\n \t\tfree(new_path2);\n@@ -1420,6 +1421,7 @@ static int process_renames(struct merge_options *o,\n \t\t\t * add-source case).\n \t\t\t */\n \t\t\tremove_file(o, 1, ren1_src,\n+\t\t\t\t o->call_depth || o->no_worktree ||\n \t\t\t\t renamed_stage == 2 || !was_tracked(ren1_src));\n \n \t\t\thashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha);\n@@ -1616,7 +1618,7 @@ static int merge_content(struct merge_options *o,\n \t\t\t o->branch2 == rename_conflict_info->branch1) ?\n \t\t\tpair1->two->path : pair1->one->path;\n \n-\t\tif (dir_in_way(path, !o->call_depth))\n+\t\tif (dir_in_way(path, !(o->call_depth || o->no_worktree)))\n \t\t\tdf_conflict_remains = 1;\n \t}\n \tmfi = merge_file_special_markers(o, &one, &a, &b,\n@@ -1636,7 +1638,7 @@ static int merge_content(struct merge_options *o,\n \t\tpath_renamed_outside_HEAD = !path2 || !strcmp(path, path2);\n \t\tif (!path_renamed_outside_HEAD) {\n \t\t\tadd_cacheinfo(mfi.mode, mfi.sha, path,\n-\t\t\t\t 0, (!o->call_depth), 0);\n+\t\t\t\t 0, !(o->call_depth || o->no_worktree), 0);\n \t\t\treturn mfi.clean;\n \t\t}\n \t} else\n@@ -1737,7 +1739,8 @@ static int process_entry(struct merge_options *o,\n \t\t\tif (a_sha)\n \t\t\t\toutput(o, 2, _(\"Removing %s\"), path);\n \t\t\t/* do not touch working file if it did not exist */\n-\t\t\tremove_file(o, 1, path, !a_sha);\n+\t\t\tremove_file(o, 1, path,\n+\t\t\t\t o->call_depth || o->no_worktree || !a_sha);\n \t\t} else {\n \t\t\t/* Modify/delete; deleted side may have put a directory in the way */\n \t\t\tclean_merge = 0;\n@@ -1768,7 +1771,7 @@ static int process_entry(struct merge_options *o,\n \t\t\tsha = b_sha;\n \t\t\tconf = _(\"directory/file\");\n \t\t}\n-\t\tif (dir_in_way(path, !o->call_depth)) {\n+\t\tif (dir_in_way(path, !(o->call_depth || o->no_worktree))) {\n \t\t\tchar *new_path = unique_path(o, path, add_branch);\n \t\t\tclean_merge = 0;\n \t\t\toutput(o, 1, _(\"CONFLICT (%s): There is a directory with name %s in %s. \"\n@@ -1796,7 +1799,8 @@ static int process_entry(struct merge_options *o,\n \t\t * this entry was deleted altogether. a_mode == 0 means\n \t\t * we had that path and want to actively remove it.\n \t\t */\n-\t\tremove_file(o, 1, path, !a_mode);\n+\t\tremove_file(o, 1, path,\n+\t\t\t o->call_depth || o->no_worktree || !a_mode);\n \t} else\n \t\tdie(_(\"Fatal merge failure, shouldn't happen.\"));\n \n@@ -1822,7 +1826,8 @@ int merge_trees(struct merge_options *o,\n \t\treturn 1;\n \t}\n \n-\tcode = git_merge_trees(o->call_depth, common, head, merge);\n+\tcode = git_merge_trees(o->call_depth || o->no_worktree,\n+\t\t\t common, head, merge);\n \n \tif (code != 0) {\n \t\tif (show(o, 4) || o->call_depth)\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 9e090a3..d8dd7a1 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -15,6 +15,7 @@ struct merge_options {\n \tconst char *subtree_shift;\n \tunsigned buffer_output : 1;\n \tunsigned renormalize : 1;\n+\tunsigned no_worktree : 1; /* do not touch worktree */\n \tlong xdl_opts;\n \tint verbosity;\n \tint diff_rename_limit;\n-- \n2.1.0.72.g9b94086\n"},{"id":"248984","messageId":"5d5715639af05dfd640d1a9827d10480e1313905.1409860234.git.tr@thomasrast.ch","threadId":"37502","inReplyTo":"cover.1409860234.git.tr@thomasrast.ch","subject":"[PATCH v3 3/8] merge-recursive: -Xindex-only to leave worktree unchanged","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-09-06T17:57:01Z","receivedAt":"2014-09-06T17:57:01Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"From: Thomas Rast \n\nUsing the new no_worktree flag from the previous commit, we can teach\nmerge-recursive to leave the worktree untouched. Expose this with a\nnew strategy option so that scripts can use it.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/merge-strategies.txt | 4 ++++\n merge-recursive.c | 2 ++\n t/t3030-merge-recursive.sh | 13 +++++++++++++\n 3 files changed, 19 insertions(+)\n\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 7bbd19b..7716fda 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -92,6 +92,10 @@ subtree[=];;\n \tis prefixed (or stripped from the beginning) to make the shape of\n \ttwo trees to match.\n \n+index-only;;\n+\tWrite the merge result only to the index; do not touch the\n+\tworktree.\n+\n octopus::\n \tThis resolves cases with more than two heads, but refuses to do\n \ta complex merge that needs manual resolution. It is\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 059ad03..d54bac2 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -2108,6 +2108,8 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n \t\tif ((o->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)\n \t\t\treturn -1;\n \t}\n+\telse if (!strcmp(s, \"index-only\"))\n+\t\to->no_worktree = 1;\n \telse\n \t\treturn -1;\n \treturn 0;\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex 82e1854..be07705 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -297,6 +297,19 @@ test_expect_success 'merge-recursive result' '\n \n '\n \n+test_expect_success 'merge-recursive --index-only' '\n+\n+\trm -fr [abcd] &&\n+\tgit checkout -f \"$c2\" &&\n+\ttest_expect_code 1 git merge-recursive --index-only \"$c0\" -- \"$c2\" \"$c1\" &&\n+\tgit ls-files -s >actual &&\n+\t# reuses \"expected\" from previous test!\n+\ttest_cmp expected actual &&\n+\tgit diff HEAD >actual-diff &&\n+\t: >expected-diff &&\n+\ttest_cmp expected-diff actual-diff\n+'\n+\n test_expect_success 'fail if the index has unresolved entries' '\n \n \trm -fr [abcd] &&\n-- \n2.1.0.72.g9b94086\n"},{"id":"248987","messageId":"33951a1d4be8ec15eec569e1a36c0a620b9edaa6.1409860234.git.tr@thomasrast.ch","threadId":"37502","inReplyTo":"cover.1409860234.git.tr@thomasrast.ch","subject":"[PATCH v3 4/8] combine-diff: do not pass revs->dense_combined_merges redundantly","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-09-06T17:57:02Z","receivedAt":"2014-09-06T17:57:02Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The existing code passed revs->dense_combined_merges along revs itself\ninto the combine-diff functions, which is rather redundant. Remove\nthe 'dense' argument until much further down the callchain to simplify\ncallers.\n\nNote that while the caller in submodule.c needs to do extra work now,\nthe next commit will simplify this to a single setting again.\n\nSigned-off-by: Thomas Rast \n---\n builtin/diff.c | 3 +--\n combine-diff.c | 13 ++++++-------\n diff-lib.c | 6 ++----\n diff.h | 6 +++---\n log-tree.c | 2 +-\n submodule.c | 5 ++++-\n 6 files changed, 17 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 0f247d2..47f663b 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -196,8 +196,7 @@ static int builtin_diff_combined(struct rev_info *revs,\n \t\trevs->dense_combined_merges = revs->combine_merges = 1;\n \tfor (i = 1; i < ents; i++)\n \t\tsha1_array_append(&parents, ent[i].item->sha1);\n-\tdiff_tree_combined(ent[0].item->sha1, &parents,\n-\t\t\t revs->dense_combined_merges, revs);\n+\tdiff_tree_combined(ent[0].item->sha1, &parents, revs);\n \tsha1_array_clear(&parents);\n \treturn 0;\n }\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 91edce5..221ab22 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -966,7 +966,7 @@ static void show_combined_header(struct combine_diff_path *elem,\n }\n \n static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n-\t\t\t int dense, int working_tree_file,\n+\t\t\t int working_tree_file,\n \t\t\t struct rev_info *rev)\n {\n \tstruct diff_options *opt = &rev->diffopt;\n@@ -981,6 +981,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \tstruct userdiff_driver *textconv = NULL;\n \tint is_binary;\n \tconst char *line_prefix = diff_line_prefix(opt);\n+\tint dense = rev->dense_combined_merges;\n \n \tcontext = opt->context;\n \tuserdiff = userdiff_find_by_path(elem->path);\n@@ -1228,7 +1229,6 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re\n */\n void show_combined_diff(struct combine_diff_path *p,\n \t\t int num_parent,\n-\t\t int dense,\n \t\t struct rev_info *rev)\n {\n \tstruct diff_options *opt = &rev->diffopt;\n@@ -1238,7 +1238,7 @@ void show_combined_diff(struct combine_diff_path *p,\n \t\t\t\t DIFF_FORMAT_NAME_STATUS))\n \t\tshow_raw_diff(p, num_parent, rev);\n \telse if (opt->output_format & DIFF_FORMAT_PATCH)\n-\t\tshow_patch_diff(p, num_parent, dense, 1, rev);\n+\t\tshow_patch_diff(p, num_parent, 1, rev);\n }\n \n static void free_combined_pair(struct diff_filepair *pair)\n@@ -1388,7 +1388,6 @@ static struct combine_diff_path *find_paths_multitree(\n \n void diff_tree_combined(const unsigned char *sha1,\n \t\t\tconst struct sha1_array *parents,\n-\t\t\tint dense,\n \t\t\tstruct rev_info *rev)\n {\n \tstruct diff_options *opt = &rev->diffopt;\n@@ -1516,7 +1515,7 @@ void diff_tree_combined(const unsigned char *sha1,\n \t\t\t\tprintf(\"%s%c\", diff_line_prefix(opt),\n \t\t\t\t opt->line_termination);\n \t\t\tfor (p = paths; p; p = p->next)\n-\t\t\t\tshow_patch_diff(p, num_parent, dense,\n+\t\t\t\tshow_patch_diff(p, num_parent,\n \t\t\t\t\t\t0, rev);\n \t\t}\n \t}\n@@ -1531,7 +1530,7 @@ void diff_tree_combined(const unsigned char *sha1,\n \tfree_pathspec(&diffopts.pathspec);\n }\n \n-void diff_tree_combined_merge(const struct commit *commit, int dense,\n+void diff_tree_combined_merge(const struct commit *commit,\n \t\t\t struct rev_info *rev)\n {\n \tstruct commit_list *parent = get_saved_parents(rev, commit);\n@@ -1541,6 +1540,6 @@ void diff_tree_combined_merge(const struct commit *commit, int dense,\n \t\tsha1_array_append(&parents, parent->item->object.sha1);\n \t\tparent = parent->next;\n \t}\n-\tdiff_tree_combined(commit->object.sha1, &parents, dense, rev);\n+\tdiff_tree_combined(commit->object.sha1, &parents, rev);\n \tsha1_array_clear(&parents);\n }\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 875aff8..3e533d2 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -171,9 +171,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\ti--;\n \n \t\t\tif (revs->combine_merges && num_compare_stages == 2) {\n-\t\t\t\tshow_combined_diff(dpath, 2,\n-\t\t\t\t\t\t revs->dense_combined_merges,\n-\t\t\t\t\t\t revs);\n+\t\t\t\tshow_combined_diff(dpath, 2, revs);\n \t\t\t\tfree(dpath);\n \t\t\t\tcontinue;\n \t\t\t}\n@@ -343,7 +341,7 @@ static int show_modified(struct rev_info *revs,\n \t\tp->parent[1].status = DIFF_STATUS_MODIFIED;\n \t\tp->parent[1].mode = old->ce_mode;\n \t\thashcpy(p->parent[1].sha1, old->sha1);\n-\t\tshow_combined_diff(p, 2, revs->dense_combined_merges, revs);\n+\t\tshow_combined_diff(p, 2, revs);\n \t\tfree(p);\n \t\treturn 0;\n \t}\ndiff --git a/diff.h b/diff.h\nindex b4a624d..3b1b54e 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -219,11 +219,11 @@ struct combine_diff_path {\n \t sizeof(struct combine_diff_parent) * (n) + (l) + 1)\n \n extern void show_combined_diff(struct combine_diff_path *elem, int num_parent,\n-\t\t\t int dense, struct rev_info *);\n+\t\t\t struct rev_info *);\n \n-extern void diff_tree_combined(const unsigned char *sha1, const struct sha1_array *parents, int dense, struct rev_info *rev);\n+extern void diff_tree_combined(const unsigned char *sha1, const struct sha1_array *parents, struct rev_info *rev);\n \n-extern void diff_tree_combined_merge(const struct commit *commit, int dense, struct rev_info *rev);\n+extern void diff_tree_combined_merge(const struct commit *commit, struct rev_info *rev);\n \n void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);\n \ndiff --git a/log-tree.c b/log-tree.c\nindex 95e9b1d..40a9db1 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -714,7 +714,7 @@ int log_tree_diff_flush(struct rev_info *opt)\n \n static int do_diff_combined(struct rev_info *opt, struct commit *commit)\n {\n-\tdiff_tree_combined_merge(commit, opt->dense_combined_merges, opt);\n+\tdiff_tree_combined_merge(commit, opt);\n \treturn !opt->loginfo;\n }\n \ndiff --git a/submodule.c b/submodule.c\nindex c3a61e7..0499de6 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -482,10 +482,13 @@ static void find_unpushed_submodule_commits(struct commit *commit,\n \tstruct rev_info rev;\n \n \tinit_revisions(&rev, NULL);\n+\trev.ignore_merges = 0;\n+\trev.combined_merges = 1;\n+\trev.dense_combined_merges = 1;\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = collect_submodules_from_diff;\n \trev.diffopt.format_callback_data = needs_pushing;\n-\tdiff_tree_combined_merge(commit, 1, &rev);\n+\tdiff_tree_combined_merge(commit, &rev);\n }\n \n int find_unpushed_submodules(unsigned char new_sha1[20],\n-- \n2.1.0.72.g9b94086\n"},{"id":"248986","messageId":"e95adf985efac162da72ac27220b904659dbb02d.1409860234.git.tr@thomasrast.ch","threadId":"37502","inReplyTo":"cover.1409860234.git.tr@thomasrast.ch","subject":"[PATCH v3 5/8] Fold all merge diff variants into an enum","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-09-06T17:57:03Z","receivedAt":"2014-09-06T17:57:03Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The four ways of displaying merge diffs,\n\n* none: no diff\n* -m: against each parent\n* -c: combined\n* --cc: combined-condensed\n\nwere encoded in three flag bits in struct rev_info. Fold them all\ninto a single enum field that captures the variants.\n\nThis makes it easier to add new merge diff variants without yet more\nspecial casing. It should also be slightly easier to read because one\ndoes not have to ensure that the flag bits are set in an expected\ncombination.\n\nSigned-off-by: Thomas Rast \n---\n builtin/diff-files.c | 5 +++--\n builtin/diff-tree.c | 2 +-\n builtin/diff.c | 9 +++++----\n builtin/fmt-merge-msg.c | 2 +-\n builtin/log.c | 9 ++++-----\n builtin/merge.c | 1 -\n combine-diff.c | 2 +-\n diff-lib.c | 7 ++++---\n log-tree.c | 4 ++--\n revision.c | 13 +++----------\n revision.h | 22 +++++++++++++++++++---\n submodule.c | 4 +---\n 12 files changed, 44 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/diff-files.c b/builtin/diff-files.c\nindex 9200069..172b50d 100644\n--- a/builtin/diff-files.c\n+++ b/builtin/diff-files.c\n@@ -57,9 +57,10 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n \t * was not asked to. \"diff-files -c -p\" should not densify\n \t * (the user should ask with \"diff-files --cc\" explicitly).\n \t */\n-\tif (rev.max_count == -1 && !rev.combine_merges &&\n+\tif (rev.max_count == -1 &&\n+\t !merge_diff_mode_is_any_combined(&rev) &&\n \t (rev.diffopt.output_format & DIFF_FORMAT_PATCH))\n-\t\trev.combine_merges = rev.dense_combined_merges = 1;\n+\t\trev.merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;\n \n \tif (read_cache_preload(&rev.diffopt.pathspec) < 0) {\n \t\tperror(\"read_cache_preload\");\ndiff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\nindex 1c4ad62..1a4bcf1 100644\n--- a/builtin/diff-tree.c\n+++ b/builtin/diff-tree.c\n@@ -90,7 +90,7 @@ COMMON_DIFF_OPTIONS_HELP;\n static void diff_tree_tweak_rev(struct rev_info *rev, struct setup_revision_opt *opt)\n {\n \tif (!rev->diffopt.output_format) {\n-\t\tif (rev->dense_combined_merges)\n+\t\tif (rev->merge_diff_mode == MERGE_DIFF_COMBINED_CONDENSED)\n \t\t\trev->diffopt.output_format = DIFF_FORMAT_PATCH;\n \t\telse\n \t\t\trev->diffopt.output_format = DIFF_FORMAT_RAW;\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 47f663b..fd4c75f 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -192,8 +192,8 @@ static int builtin_diff_combined(struct rev_info *revs,\n \tif (argc > 1)\n \t\tusage(builtin_diff_usage);\n \n-\tif (!revs->dense_combined_merges && !revs->combine_merges)\n-\t\trevs->dense_combined_merges = revs->combine_merges = 1;\n+\tif (!merge_diff_mode_is_any_combined(revs))\n+\t\trevs->merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;\n \tfor (i = 1; i < ents; i++)\n \t\tsha1_array_append(&parents, ent[i].item->sha1);\n \tdiff_tree_combined(ent[0].item->sha1, &parents, revs);\n@@ -242,9 +242,10 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv\n \t * dense one, --cc can be explicitly asked for, or just rely\n \t * on the default).\n \t */\n-\tif (revs->max_count == -1 && !revs->combine_merges &&\n+\tif (revs->max_count == -1 &&\n+\t !merge_diff_mode_is_any_combined(revs) &&\n \t (revs->diffopt.output_format & DIFF_FORMAT_PATCH))\n-\t\trevs->combine_merges = revs->dense_combined_merges = 1;\n+\t\trevs->merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;\n \n \tsetup_work_tree();\n \tif (read_cache_preload(&revs->diffopt.pathspec) < 0) {\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 79df05e..db23626 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -637,7 +637,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n \t\thead = lookup_commit_or_die(head_sha1, \"HEAD\");\n \t\tinit_revisions(&rev, NULL);\n \t\trev.commit_format = CMIT_FMT_ONELINE;\n-\t\trev.ignore_merges = 1;\n+\t\trev.merge_diff_mode = MERGE_DIFF_IGNORE;\n \t\trev.limited = 1;\n \n \t\tstrbuf_complete_line(out);\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 4389722..ba057d5 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -496,13 +496,12 @@ static int show_tree_object(const unsigned char *sha1,\n \n static void show_rev_tweak_rev(struct rev_info *rev, struct setup_revision_opt *opt)\n {\n-\tif (rev->ignore_merges) {\n+\tif (!rev->merge_diff_mode) {\n \t\t/* There was no \"-m\" on the command line */\n-\t\trev->ignore_merges = 0;\n-\t\tif (!rev->first_parent_only && !rev->combine_merges) {\n+\t\trev->merge_diff_mode = MERGE_DIFF_EACH;\n+\t\tif (!rev->first_parent_only) {\n \t\t\t/* No \"--first-parent\", \"-c\", or \"--cc\" */\n-\t\t\trev->combine_merges = 1;\n-\t\t\trev->dense_combined_merges = 1;\n+\t\t\trev->merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;\n \t\t}\n \t}\n \tif (!rev->diffopt.output_format)\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex ce82eb2..f3e568a 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -343,7 +343,6 @@ static void squash_message(struct commit *commit, struct commit_list *remotehead\n \t\tdie_errno(_(\"Could not write to '%s'\"), filename);\n \n \tinit_revisions(&rev, NULL);\n-\trev.ignore_merges = 1;\n \trev.commit_format = CMIT_FMT_MEDIUM;\n \n \tcommit->object.flags |= UNINTERESTING;\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 221ab22..d590485 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -981,7 +981,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \tstruct userdiff_driver *textconv = NULL;\n \tint is_binary;\n \tconst char *line_prefix = diff_line_prefix(opt);\n-\tint dense = rev->dense_combined_merges;\n+\tint dense = (rev->merge_diff_mode == MERGE_DIFF_COMBINED_CONDENSED);\n \n \tcontext = opt->context;\n \tuserdiff = userdiff_find_by_path(elem->path);\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 3e533d2..683bb44 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -170,7 +170,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t */\n \t\t\ti--;\n \n-\t\t\tif (revs->combine_merges && num_compare_stages == 2) {\n+\t\t\tif (merge_diff_mode_is_any_combined(revs) &&\n+\t\t\t num_compare_stages == 2) {\n \t\t\t\tshow_combined_diff(dpath, 2, revs);\n \t\t\t\tfree(dpath);\n \t\t\t\tcontinue;\n@@ -322,7 +323,7 @@ static int show_modified(struct rev_info *revs,\n \t\treturn -1;\n \t}\n \n-\tif (revs->combine_merges && !cached &&\n+\tif (merge_diff_mode_is_any_combined(revs) && !cached &&\n \t (hashcmp(sha1, old->sha1) || hashcmp(old->sha1, new->sha1))) {\n \t\tstruct combine_diff_path *p;\n \t\tint pathlen = ce_namelen(new);\n@@ -380,7 +381,7 @@ static void do_oneway_diff(struct unpack_trees_options *o,\n \t * But with the revision flag parsing, that's found in\n \t * \"!revs->ignore_merges\".\n \t */\n-\tmatch_missing = !revs->ignore_merges;\n+\tmatch_missing = (revs->merge_diff_mode == MERGE_DIFF_EACH);\n \n \tif (cached && idx && ce_stage(idx)) {\n \t\tstruct diff_filepair *pair;\ndiff --git a/log-tree.c b/log-tree.c\nindex 40a9db1..8f57651 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -747,9 +747,9 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log\n \n \t/* More than one parent? */\n \tif (parents && parents->next) {\n-\t\tif (opt->ignore_merges)\n+\t\tif (opt->merge_diff_mode == MERGE_DIFF_IGNORE)\n \t\t\treturn 0;\n-\t\telse if (opt->combine_merges)\n+\t\telse if (merge_diff_mode_is_any_combined(opt))\n \t\t\treturn do_diff_combined(opt, commit);\n \t\telse if (opt->first_parent_only) {\n \t\t\t/*\ndiff --git a/revision.c b/revision.c\nindex 615535c..7a9a141 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1324,7 +1324,6 @@ void init_revisions(struct rev_info *revs, const char *prefix)\n \tmemset(revs, 0, sizeof(*revs));\n \n \trevs->abbrev = DEFAULT_ABBREV;\n-\trevs->ignore_merges = 1;\n \trevs->simplify_history = 1;\n \tDIFF_OPT_SET(&revs->pruning, RECURSIVE);\n \tDIFF_OPT_SET(&revs->pruning, QUICK);\n@@ -1811,15 +1810,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\tDIFF_OPT_SET(&revs->diffopt, RECURSIVE);\n \t\tDIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);\n \t} else if (!strcmp(arg, \"-m\")) {\n-\t\trevs->ignore_merges = 0;\n+\t\trevs->merge_diff_mode = MERGE_DIFF_EACH;\n \t} else if (!strcmp(arg, \"-c\")) {\n-\t\trevs->diff = 1;\n-\t\trevs->dense_combined_merges = 0;\n-\t\trevs->combine_merges = 1;\n+\t\trevs->merge_diff_mode = MERGE_DIFF_COMBINED;\n \t} else if (!strcmp(arg, \"--cc\")) {\n-\t\trevs->diff = 1;\n-\t\trevs->dense_combined_merges = 1;\n-\t\trevs->combine_merges = 1;\n+\t\trevs->merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;\n \t} else if (!strcmp(arg, \"-v\")) {\n \t\trevs->verbose_header = 1;\n \t} else if (!strcmp(arg, \"--pretty\")) {\n@@ -2242,8 +2237,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\tcopy_pathspec(&revs->diffopt.pathspec,\n \t\t\t\t &revs->prune_data);\n \t}\n-\tif (revs->combine_merges)\n-\t\trevs->ignore_merges = 0;\n \trevs->diffopt.abbrev = revs->abbrev;\n \n \tif (revs->line_level_traverse) {\ndiff --git a/revision.h b/revision.h\nindex a620530..0eb34c2 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -52,6 +52,17 @@ struct rev_cmdline_info {\n #define REVISION_WALK_NO_WALK_SORTED 1\n #define REVISION_WALK_NO_WALK_UNSORTED 2\n \n+enum merge_diff_mode {\n+\t/* default: do not show diffs for merge */\n+\tMERGE_DIFF_IGNORE = 0,\n+\t/* diff against each side (-m) */\n+\tMERGE_DIFF_EACH,\n+\t/* combined format (-c) */\n+\tMERGE_DIFF_COMBINED,\n+\t/* combined-condensed format (-cc) */\n+\tMERGE_DIFF_COMBINED_CONDENSED\n+};\n+\n struct rev_info {\n \t/* Starting list */\n \tstruct commit_list *commits;\n@@ -119,11 +130,10 @@ struct rev_info {\n \t\t\tshow_root_diff:1,\n \t\t\tno_commit_id:1,\n \t\t\tverbose_header:1,\n-\t\t\tignore_merges:1,\n-\t\t\tcombine_merges:1,\n-\t\t\tdense_combined_merges:1,\n \t\t\talways_show_header:1;\n \n+\tenum merge_diff_mode merge_diff_mode;\n+\n \t/* Format info */\n \tunsigned int\tshown_one:1,\n \t\t\tshown_dashes:1,\n@@ -209,6 +219,12 @@ struct rev_info {\n \tconst char *break_bar;\n };\n \n+static inline int merge_diff_mode_is_any_combined(struct rev_info *revs)\n+{\n+\treturn (revs->merge_diff_mode == MERGE_DIFF_COMBINED ||\n+\t\trevs->merge_diff_mode == MERGE_DIFF_COMBINED_CONDENSED);\n+}\n+\n extern int ref_excluded(struct string_list *, const char *path);\n void clear_ref_exclusion(struct string_list **);\n void add_ref_exclusion(struct string_list **, const char *exclude);\ndiff --git a/submodule.c b/submodule.c\nindex 0499de6..aa5eac3 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -482,9 +482,7 @@ static void find_unpushed_submodule_commits(struct commit *commit,\n \tstruct rev_info rev;\n \n \tinit_revisions(&rev, NULL);\n-\trev.ignore_merges = 0;\n-\trev.combined_merges = 1;\n-\trev.dense_combined_merges = 1;\n+\trev.merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = collect_submodules_from_diff;\n \trev.diffopt.format_callback_data = needs_pushing;\n-- \n2.1.0.72.g9b94086\n"},{"id":"248985","messageId":"781fa77328ef04600c9ef7ad4f682e157c3aeefe.1409860234.git.tr@thomasrast.ch","threadId":"37502","inReplyTo":"cover.1409860234.git.tr@thomasrast.ch","subject":"[PATCH v3 6/8] merge-recursive: allow storing conflict hunks in index","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-09-06T17:57:04Z","receivedAt":"2014-09-06T17:57:04Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Add a --conflicts-in-index option to merge-recursive, which instructs\nit to always store the 3-way merged result in the index. (Normally it\nonly does so in recursive invocations, but not for the final result.)\n\nThis serves as a building block for the \"remerge diff\" feature coming\nup in a subsequent patch. The external option lets us easily use it\nfrom tests, where we'd otherwise need a new test-* helper to access\nthe feature.\n\nFurthermore, it might occasionally be useful for scripts that want to\nlook at the result of invoking git-merge without tampering with the\nworktree. They could already get the _conflicts_ with --index-only,\nbut not (conveniently) the conflict-hunk formatted files that would\nnormally be written to the worktree.\n\nSigned-off-by: Thomas Rast \n---\n Documentation/merge-strategies.txt | 5 +++++\n merge-recursive.c | 4 ++++\n merge-recursive.h | 1 +\n t/t3030-merge-recursive.sh | 20 ++++++++++++++++++++\n 4 files changed, 30 insertions(+)\n\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 7716fda..19f3a5d 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -96,6 +96,11 @@ index-only;;\n \tWrite the merge result only to the index; do not touch the\n \tworktree.\n \n+conflicts-in-index;;\n+\tFor conflicted files, write 3-way merged contents with\n+\tconflict hunks to the index, instead of leaving their entries\n+\tunresolved.\n+\n octopus::\n \tThis resolves cases with more than two heads, but refuses to do\n \ta complex merge that needs manual resolution. It is\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex d54bac2..20d3051 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -743,6 +743,8 @@ static void update_file_flags(struct merge_options *o,\n \t\t\t int update_cache,\n \t\t\t int update_wd)\n {\n+\tif (o->conflicts_in_index)\n+\t\tupdate_cache = 1;\n \tif (o->call_depth || o->no_worktree)\n \t\tupdate_wd = 0;\n \n@@ -2110,6 +2112,8 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n \t}\n \telse if (!strcmp(s, \"index-only\"))\n \t\to->no_worktree = 1;\n+\telse if (!strcmp(s, \"conflicts-in-index\"))\n+\t\to->conflicts_in_index = 1;\n \telse\n \t\treturn -1;\n \treturn 0;\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex d8dd7a1..9b8e20b 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -16,6 +16,7 @@ struct merge_options {\n \tunsigned buffer_output : 1;\n \tunsigned renormalize : 1;\n \tunsigned no_worktree : 1; /* do not touch worktree */\n+\tunsigned conflicts_in_index : 1; /* index will contain conflict hunks */\n \tlong xdl_opts;\n \tint verbosity;\n \tint diff_rename_limit;\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex be07705..39841a9 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -310,6 +310,26 @@ test_expect_success 'merge-recursive --index-only' '\n \ttest_cmp expected-diff actual-diff\n '\n \n+test_expect_success 'merge-recursive --index-only --conflicts-in-index' '\n+\t# first pass: do a merge as usual to obtain \"expected\"\n+\trm -fr [abcd] &&\n+\tgit checkout -f \"$c2\" &&\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c2\" \"$c1\" &&\n+\tgit add [abcd] &&\n+\tgit ls-files -s >expected &&\n+\t# second pass: actual test\n+\trm -fr [abcd] &&\n+\tgit checkout -f \"$c2\" &&\n+\ttest_expect_code 1 \\\n+\t\tgit merge-recursive --index-only --conflicts-in-index \\\n+\t\t\"$c0\" -- \"$c2\" \"$c1\" &&\n+\tgit ls-files -s >actual &&\n+\ttest_cmp expected actual &&\n+\tgit diff HEAD >actual-diff &&\n+\t: >expected-diff &&\n+\ttest_cmp expected-diff actual-diff\n+'\n+\n test_expect_success 'fail if the index has unresolved entries' '\n \n \trm -fr [abcd] &&\n-- \n2.1.0.72.g9b94086\n"},{"id":"248989","messageId":"4d43fca586637dcb02b2b19c8d8a6dcfe368e059.1409860234.git.tr@thomasrast.ch","threadId":"37502","inReplyTo":"cover.1409860234.git.tr@thomasrast.ch","subject":"[PATCH v3 7/8] name-hash: allow dir hashing even when !ignore_case","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-09-06T17:57:05Z","receivedAt":"2014-09-06T17:57:05Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The directory hash (for fast checks if the index already has a\ndirectory) was only used in ignore_case mode and so depended on that\nflag.\n\nMake it generally available on request.\n\nSigned-off-by: Thomas Rast \n---\n cache.h | 2 ++\n name-hash.c | 13 ++++++++-----\n 2 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4d5b76c..c54b2e1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -306,6 +306,7 @@ struct index_state {\n \tstruct split_index *split_index;\n \tstruct cache_time timestamp;\n \tunsigned name_hash_initialized : 1,\n+\t\t has_dir_hash : 1,\n \t\t initialized : 1;\n \tstruct hashmap name_hash;\n \tstruct hashmap dir_hash;\n@@ -315,6 +316,7 @@ struct index_state {\n extern struct index_state the_index;\n \n /* Name hashing */\n+extern void init_name_hash(struct index_state *istate, int force_dir_hash);\n extern void add_name_hash(struct index_state *istate, struct cache_entry *ce);\n extern void remove_name_hash(struct index_state *istate, struct cache_entry *ce);\n extern void free_name_hash(struct index_state *istate);\ndiff --git a/name-hash.c b/name-hash.c\nindex 702cd05..22e3ec6 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -106,7 +106,7 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)\n \thashmap_entry_init(ce, memihash(ce->name, ce_namelen(ce)));\n \thashmap_add(&istate->name_hash, ce);\n \n-\tif (ignore_case)\n+\tif (istate->has_dir_hash)\n \t\tadd_dir_entry(istate, ce);\n }\n \n@@ -121,7 +121,7 @@ static int cache_entry_cmp(const struct cache_entry *ce1,\n \treturn remove ? !(ce1 == ce2) : 0;\n }\n \n-static void lazy_init_name_hash(struct index_state *istate)\n+void init_name_hash(struct index_state *istate, int force_dir_hash)\n {\n \tint nr;\n \n@@ -130,6 +130,9 @@ static void lazy_init_name_hash(struct index_state *istate)\n \thashmap_init(&istate->name_hash, (hashmap_cmp_fn) cache_entry_cmp,\n \t\t\tistate->cache_nr);\n \thashmap_init(&istate->dir_hash, (hashmap_cmp_fn) dir_entry_cmp, 0);\n+\n+\tistate->has_dir_hash = force_dir_hash || ignore_case;\n+\n \tfor (nr = 0; nr < istate->cache_nr; nr++)\n \t\thash_index_entry(istate, istate->cache[nr]);\n \tistate->name_hash_initialized = 1;\n@@ -148,7 +151,7 @@ void remove_name_hash(struct index_state *istate, struct cache_entry *ce)\n \tce->ce_flags &= ~CE_HASHED;\n \thashmap_remove(&istate->name_hash, ce, ce);\n \n-\tif (ignore_case)\n+\tif (istate->has_dir_hash)\n \t\tremove_dir_entry(istate, ce);\n }\n \n@@ -193,7 +196,7 @@ struct cache_entry *index_dir_exists(struct index_state *istate, const char *nam\n \tstruct cache_entry *ce;\n \tstruct dir_entry *dir;\n \n-\tlazy_init_name_hash(istate);\n+\tinit_name_hash(istate, 0);\n \tdir = find_dir_entry(istate, name, namelen);\n \tif (dir && dir->nr)\n \t\treturn dir->ce;\n@@ -214,7 +217,7 @@ struct cache_entry *index_file_exists(struct index_state *istate, const char *na\n {\n \tstruct cache_entry *ce;\n \n-\tlazy_init_name_hash(istate);\n+\tinit_name_hash(istate, 0);\n \n \tce = hashmap_get_from_hash(&istate->name_hash,\n \t\t\t\t memihash(name, namelen), NULL);\n-- \n2.1.0.72.g9b94086\n"},{"id":"248988","messageId":"29c15efe998630143eaa75ec7155a31ce17bd433.1409860234.git.tr@thomasrast.ch","threadId":"37502","inReplyTo":"cover.1409860234.git.tr@thomasrast.ch","subject":"[PATCH v3 8/8] log --remerge-diff: show what the conflict resolution changed","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-09-06T17:57:06Z","receivedAt":"2014-09-06T17:57:06Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Git has --cc as a very fast inspection tool that shows a brief summary\nof what a conflicted merge \"looks like\", and -c/-m as \"give me the\nfull information\" data dumps.\n\nBut --cc actually loses information: if the merge lost(!) some changes\nfrom one side, that hunk would fully agree with the other side, and\ntherefore be elided. So --cc cannot be used to investigate mismerges.\nIndeed it is rather hard to find a merge that has lost changes, unless\none knows where to look.\n\nThe new option --remerge-diff is an attempt at filling this gap,\nadmittedly at the cost of a lot of CPU cycles. For each merge commit,\nit diffs the merge result against a recursive merge of the merge's\nparents.\n\nFor files that can be auto-merged cleanly, it will typically show\nnothing. However, it will make it obvious when the merge introduces\nextra changes.\n\nFor files that result in merge conflicts, we diff against the\nrepresentation with conflict hunks (what the user would usually see in\nthe worktree). So the diff will show what was changed in the conflict\nhunks to resolve the conflict.\n\nIt still takes a bit of staring to tell an evil from a regular merge.\nBut at least the information is there, unlike with --cc; and the\noutput is usually much shorter than with -c.\n\nSigned-off-by: Thomas Rast \n---\n Documentation/rev-list-options.txt | 7 +\n log-tree.c | 297 +++++++++++++++++++++++++++++++++++++\n merge-recursive.c | 3 +-\n merge-recursive.h | 1 +\n revision.c | 2 +\n revision.h | 4 +-\n t/t4213-log-remerge-diff.sh | 222 +++++++++++++++++++++++++++\n 7 files changed, 534 insertions(+), 2 deletions(-)\n create mode 100755 t/t4213-log-remerge-diff.sh\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex deb8cca..7128350 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -805,6 +805,13 @@ options may be given. See linkgit:git-diff-files[1] for more options.\n \tin that case, the output represents the changes the merge\n \tbrought _into_ the then-current branch.\n \n+--remerge-diff::\n+\tDiff merge commits against a recursive merge of their parents,\n+\twith conflict hunks. Intuitively speaking, this shows what\n+\tthe author of the merge changed to resolve the merge. It\n+\tassumes that all (or most) merges are recursive merges; other\n+\tstrategies are not supported.\n+\n -r::\n \tShow recursive diffs.\n \ndiff --git a/log-tree.c b/log-tree.c\nindex 8f57651..4db1385 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -11,6 +11,8 @@\n #include \"gpg-interface.h\"\n #include \"sequencer.h\"\n #include \"line-log.h\"\n+#include \"cache-tree.h\"\n+#include \"merge-recursive.h\"\n \n struct decoration name_decoration = { \"object names\" };\n \n@@ -719,6 +721,299 @@ static int do_diff_combined(struct rev_info *opt, struct commit *commit)\n }\n \n /*\n+ * Helpers for make_asymmetric_conflict_entries() below.\n+ */\n+static char *load_cache_entry_blob(struct cache_entry *entry,\n+\t\t\t\t unsigned long *size)\n+{\n+\tenum object_type type;\n+\tvoid *data;\n+\n+\tif (!entry)\n+\t\treturn NULL;\n+\n+\tdata = read_sha1_file(entry->sha1, &type, size);\n+\tif (type != OBJ_BLOB)\n+\t\tdie(\"BUG: load_cache_entry_blob for non-blob\");\n+\n+\treturn data;\n+}\n+\n+static void strbuf_append_cache_entry_blob(struct strbuf *sb,\n+\t\t\t\t\t struct cache_entry *entry)\n+{\n+\tunsigned long size;\n+\tchar *data = load_cache_entry_blob(entry, &size);;\n+\n+\tif (!data)\n+\t\treturn;\n+\n+\tstrbuf_add(sb, data, size);\n+\tfree(data);\n+}\n+\n+static void assemble_conflict_entry(struct strbuf *sb,\n+\t\t\t\t const char *branch1,\n+\t\t\t\t const char *branch2,\n+\t\t\t\t struct cache_entry *entry1,\n+\t\t\t\t struct cache_entry *entry2)\n+{\n+\tstrbuf_addf(sb, \"<<<<<<< %s\\n\", branch1);\n+\tstrbuf_append_cache_entry_blob(sb, entry1);\n+\tstrbuf_addstr(sb, \"=======\\n\");\n+\tstrbuf_append_cache_entry_blob(sb, entry2);\n+\tstrbuf_addf(sb, \">>>>>>> %s\\n\", branch2);\n+}\n+\n+/*\n+ * For --remerge-diff, we need conflicted (<<<<<<< ... >>>>>>>)\n+ * representations of as many conflicts as possible. Default conflict\n+ * generation only applies to files that have all three stages.\n+ *\n+ * This function generates conflict hunk representations for files\n+ * that have only one of stage 2 or 3. The corresponding side in the\n+ * conflict hunk format will be empty. A stage 1, if any, will be\n+ * dropped in the process.\n+ */\n+static void make_asymmetric_conflict_entries(const char *branch1,\n+\t\t\t\t\t const char *branch2)\n+{\n+\tint o = 0, i = 0;\n+\n+\t/*\n+\t * NEEDSWORK: we trample all over the cache below, so we need\n+\t * to set up the name hash early, before modifying it. And\n+\t * after that we cannot free any cache entries, because they\n+\t * remain hashed even if deleted. Sigh.\n+\t */\n+\tinit_name_hash(&the_index, 1);\n+\n+\t/*\n+\t * The loop always starts with 'i' pointing at the first entry\n+\t * for a pathname.\n+\t */\n+\twhile (i < active_nr) {\n+\t\tstruct cache_entry *ce;\n+\t\tstruct cache_entry *stage1 = NULL;\n+\t\tstruct cache_entry *stage2 = NULL;\n+\t\tstruct cache_entry *stage3 = NULL;\n+\t\tstruct cache_entry *new_ce = NULL;\n+\t\tstruct strbuf content = STRBUF_INIT;\n+\t\tunsigned char sha1[20];\n+\n+\t\tassert(o <= i);\n+\n+\t\tce = active_cache[i];\n+\n+\t\t/*\n+\t\t * Pass through stage 0 and submodules unchanged, we\n+\t\t * don't know how to handle them.\n+\t\t */\n+\t\tif (ce_stage(ce) == 0\n+\t\t || S_ISGITLINK(ce->ce_mode)) {\n+\t\t\tactive_cache[o++] = ce;\n+\t\t\ti++;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Collect the stages we have. Point 'i' past the\n+\t\t * entry.\n+\t\t */\n+\t\tif (/* i < active_nr && */\n+\t\t !strcmp(ce->name, active_cache[i]->name) &&\n+\t\t ce_stage(active_cache[i]) == 1)\n+\t\t\tstage1 = active_cache[i++];\n+\n+\t\tif (i < active_nr &&\n+\t\t !strcmp(ce->name, active_cache[i]->name) &&\n+\t\t ce_stage(active_cache[i]) == 2)\n+\t\t\tstage2 = active_cache[i++];\n+\n+\t\tif (i < active_nr &&\n+\t\t !strcmp(ce->name, active_cache[i]->name) &&\n+\t\t ce_stage(active_cache[i]) == 3)\n+\t\t\tstage3 = active_cache[i++];\n+\n+\t\tif (!stage2 && !stage3)\n+\t\t\tdie(\"BUG: merging resulted in conflict with neither \"\n+\t\t\t \"stage 2 nor 3\");\n+\n+\t\tif (cache_dir_exists(ce->name, ce->ce_namelen)) {\n+\t\t\t/*\n+\t\t\t * If a conflicting directory for this entry exists,\n+\t\t\t * we can drop it:\n+\t\t\t *\n+\t\t\t * In the face of a file/directory conflict,\n+\t\t\t * merge-recursive\n+\t\t\t * - puts the file at stage >0 as usual\n+\t\t\t * - also attempts to check out the file as\n+\t\t\t * 'file~side' where side is the sha1 of the commit\n+\t\t\t * the file came from, to avoid colliding with the\n+\t\t\t * directory\n+\t\t\t *\n+\t\t\t * But we have requested that files go to the index\n+\t\t\t * instead of the worktree, so by the time we get\n+\t\t\t * here, we have both stage>0 'file' from ordinary\n+\t\t\t * merging and a stage=0 'file~side' from the \"write\n+\t\t\t * all files to index\".\n+\t\t\t *\n+\t\t\t * We need to remove one of them. Currently we ditch\n+\t\t\t * the 'file' entry because it's easier to detect.\n+\t\t\t * This amounts to always renaming the file to make\n+\t\t\t * room for the directory.\n+\t\t\t *\n+\t\t\t * NEEDSWORK: Two options:\n+\t\t\t *\n+\t\t\t * - If the merge result kept the file, it would be\n+\t\t\t * better to rename the _directory_ to make room for\n+\t\t\t * the file, so that filenames match between the\n+\t\t\t * result and the re-merge.\n+\t\t\t *\n+\t\t\t * - Or we could avoid going through a tree, since the\n+\t\t\t * index can represent (though it's not \"legal\") a\n+\t\t\t * file/directory collision just fine.\n+\t\t\t */\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * Otherwise, there is room for a file entry\n+\t\t\t * at stage 0. It has fake-conflict content,\n+\t\t\t * but its mode is the same.\n+\t\t\t */\n+\n+\t\t\tassemble_conflict_entry(&content,\n+\t\t\t\t\t\tbranch1, branch2,\n+\t\t\t\t\t\tstage2, stage3);\n+\t\t\tif (write_sha1_file(content.buf, content.len,\n+\t\t\t\t\t typename(OBJ_BLOB), sha1))\n+\t\t\t\tdie(\"write_sha1_file failed\");\n+\t\t\tstrbuf_release(&content);\n+\n+\t\t\tnew_ce = xcalloc(1, cache_entry_size(ce->ce_namelen));\n+\t\t\tnew_ce->ce_mode = ce->ce_mode;\n+\t\t\tnew_ce->ce_flags = ce->ce_flags & ~CE_STAGEMASK;\n+\t\t\tnew_ce->ce_namelen = ce->ce_namelen;\n+\t\t\thashcpy(new_ce->sha1, sha1);\n+\t\t\tmemcpy(new_ce->name, ce->name, ce->ce_namelen+1);\n+\t\t\tactive_cache[o++] = new_ce;\n+\t\t\tadd_name_hash(&the_index, new_ce);\n+\t\t}\n+\n+\t\tif (stage1)\n+\t\t\tremove_name_hash(&the_index, stage1);\n+\t\tif (stage2)\n+\t\t\tremove_name_hash(&the_index, stage2);\n+\t\tif (stage3)\n+\t\t\tremove_name_hash(&the_index, stage3);\n+\t\tfree(stage1);\n+\t\tfree(stage2);\n+\t\tfree(stage3);\n+\t}\n+\n+\tactive_nr = o;\n+}\n+\n+/*\n+ * --remerge-diff doesn't currently handle entries that cannot be\n+ * turned into a stage0 conflicted-file format blob. So this routine\n+ * clears the corresponding entries from the index. This is\n+ * suboptimal; we should eventually handle them _somehow_.\n+*/\n+static void drop_non_stage0()\n+{\n+\tint o = 0, i = 0;\n+\n+\twhile (i < active_nr) {\n+\t\tstruct cache_entry *ce = active_cache[i];\n+\t\tconst char *name;\n+\n+\t\tif (!ce_stage(ce)) {\n+\t\t\tactive_cache[o++] = active_cache[i++];\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tname = ce->name;\n+\n+\t\tprintf(\"Cannot handle stage %d entry '%s', skipping\\n\",\n+\t\t ce_stage(ce), name);\n+\t\ti++;\n+\n+\t\twhile (i < active_nr && !strcmp(name, active_cache[i]->name))\n+\t\t\tfree(active_cache[i++]);\n+\n+\t\tfree(ce);\n+\t}\n+\n+\tactive_nr = o;\n+}\n+\n+static int do_diff_remerge(struct rev_info *opt, struct commit *commit)\n+{\n+\tstruct commit_list *merge_bases;\n+\tstruct commit *result, *parent1, *parent2;\n+\tstruct merge_options o;\n+\tchar *branch1, *branch2;\n+\tstruct cache_tree *orig_cache_tree;\n+\n+\t/*\n+\t * We show the log message early to avoid headaches later. In\n+\t * general we need to run this before printing anything in\n+\t * this routine.\n+\t */\n+\tif (opt->loginfo && !opt->no_commit_id) {\n+\t\tshow_log(opt);\n+\n+\t\tif (opt->verbose_header && opt->diffopt.output_format)\n+\t\t\tprintf(\"%s%c\", diff_line_prefix(&opt->diffopt),\n+\t\t\t opt->diffopt.line_termination);\n+\t}\n+\n+\tif (commit->parents->next->next) {\n+\t\tprintf(\"--remerge-diff not supported for octopus merges.\\n\");\n+\t\treturn !opt->loginfo;\n+\t}\n+\n+\tparent1 = commit->parents->item;\n+\tparent2 = commit->parents->next->item;\n+\tparse_commit(parent1);\n+\tparse_commit(parent2);\n+\tbranch1 = xstrdup(sha1_to_hex(parent1->object.sha1));\n+\tbranch2 = xstrdup(sha1_to_hex(parent2->object.sha1));\n+\n+\tmerge_bases = get_octopus_merge_bases(commit->parents);\n+\tinit_merge_options(&o);\n+\to.verbosity = -1;\n+\to.no_worktree = 1;\n+\to.conflicts_in_index = 1;\n+\to.use_ondisk_index = 0;\n+\to.branch1 = branch1;\n+\to.branch2 = branch2;\n+\tmerge_recursive(&o, parent1, parent2, merge_bases, &result);\n+\n+\tmake_asymmetric_conflict_entries(branch1, branch2);\n+\tdrop_non_stage0();\n+\n+\tfree(branch1);\n+\tfree(branch2);\n+\n+\torig_cache_tree = the_index.cache_tree;\n+\tthe_index.cache_tree = cache_tree();\n+\tif (cache_tree_update(&the_index, WRITE_TREE_SILENT) < 0) {\n+\t\tprintf(\"BUG: merge conflicts not fully folded, cannot diff.\\n\");\n+\t\treturn !opt->loginfo;\n+\t}\n+\n+\tdiff_tree_sha1(the_index.cache_tree->sha1, commit->tree->object.sha1,\n+\t\t \"\", &opt->diffopt);\n+\tlog_tree_diff_flush(opt);\n+\n+\tcache_tree_free(&the_index.cache_tree);\n+\tthe_index.cache_tree = orig_cache_tree;\n+\n+\treturn !opt->loginfo;\n+}\n+\n+/*\n * Show the diff of a commit.\n *\n * Return true if we printed any log info messages\n@@ -751,6 +1046,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log\n \t\t\treturn 0;\n \t\telse if (merge_diff_mode_is_any_combined(opt))\n \t\t\treturn do_diff_combined(opt, commit);\n+\t\telse if (opt->merge_diff_mode == MERGE_DIFF_REMERGE)\n+\t\t\treturn do_diff_remerge(opt, commit);\n \t\telse if (opt->first_parent_only) {\n \t\t\t/*\n \t\t\t * Generate merge log entry only for the first\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 20d3051..c5d0bb3 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1962,7 +1962,7 @@ int merge_recursive(struct merge_options *o,\n \t}\n \n \tdiscard_cache();\n-\tif (!o->call_depth)\n+\tif (!o->call_depth && o->use_ondisk_index)\n \t\tread_cache();\n \n \to->ancestor = \"merged common ancestors\";\n@@ -2057,6 +2057,7 @@ void init_merge_options(struct merge_options *o)\n \to->diff_rename_limit = -1;\n \to->merge_rename_limit = -1;\n \to->renormalize = 0;\n+\to->use_ondisk_index = 1;\n \tgit_config(merge_recursive_config, o);\n \tif (getenv(\"GIT_MERGE_VERBOSITY\"))\n \t\to->verbosity =\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 9b8e20b..d7466c7 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -17,6 +17,7 @@ struct merge_options {\n \tunsigned renormalize : 1;\n \tunsigned no_worktree : 1; /* do not touch worktree */\n \tunsigned conflicts_in_index : 1; /* index will contain conflict hunks */\n+\tunsigned use_ondisk_index : 1; /* tree-level merge loads .git/index */\n \tlong xdl_opts;\n \tint verbosity;\n \tint diff_rename_limit;\ndiff --git a/revision.c b/revision.c\nindex 7a9a141..5ccf225 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1815,6 +1815,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->merge_diff_mode = MERGE_DIFF_COMBINED;\n \t} else if (!strcmp(arg, \"--cc\")) {\n \t\trevs->merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;\n+\t} else if (!strcmp(arg, \"--remerge-diff\")) {\n+\t\trevs->merge_diff_mode = MERGE_DIFF_REMERGE;\n \t} else if (!strcmp(arg, \"-v\")) {\n \t\trevs->verbose_header = 1;\n \t} else if (!strcmp(arg, \"--pretty\")) {\ndiff --git a/revision.h b/revision.h\nindex 0eb34c2..38544ea 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -60,7 +60,9 @@ enum merge_diff_mode {\n \t/* combined format (-c) */\n \tMERGE_DIFF_COMBINED,\n \t/* combined-condensed format (-cc) */\n-\tMERGE_DIFF_COMBINED_CONDENSED\n+\tMERGE_DIFF_COMBINED_CONDENSED,\n+\t/* --remerge-diff */\n+\tMERGE_DIFF_REMERGE\n };\n \n struct rev_info {\ndiff --git a/t/t4213-log-remerge-diff.sh b/t/t4213-log-remerge-diff.sh\nnew file mode 100755\nindex 0000000..36ef17a\n--- /dev/null\n+++ b/t/t4213-log-remerge-diff.sh\n@@ -0,0 +1,222 @@\n+#!/bin/sh\n+\n+test_description='test log --remerge-diff'\n+. ./test-lib.sh\n+\n+# A -----------------+----\n+# | \\ \\ \\ | \\\n+# | C \\ \\ | |\n+# B |\\ \\ | | |\n+# | | | D U dir file\n+# |\\ | |__|__ | | \\ /|\n+# | X |_ | \\ | | X |\n+# |/ \\/ \\| \\ | | / \\|\n+# M1 M2 M3 M4 M5 M6\n+# ^ ^ ^ ^ ^ ^\n+# | | | | | filedir\n+# | | | | dirfile\n+# | | dm unrelated\n+# | evil\n+# benign\n+#\n+#\n+# M1 has a \"benign\" conflict\n+# M2 has an \"evil\" conflict: it ignores the changes in D\n+# M3 has a delete/modify conflict, resolved in favor of a modification\n+# M4 is a merge of an unrelated change, without conflicts\n+# M5 has a file/directory conflict, resolved in favor of the directory\n+# M6 has a file/directory conflict, resolved in favor of the file\n+\n+test_expect_success 'setup' '\n+\ttest_commit A file original &&\n+\ttest_commit B file change &&\n+\tgit checkout -b side A &&\n+\ttest_commit C file side &&\n+\tgit checkout -b delete A &&\n+\tgit rm file &&\n+\ttest_commit D &&\n+\tgit checkout -b benign master &&\n+\ttest_must_fail git merge C &&\n+\ttest_commit M1 file merged &&\n+\tgit checkout -b evil B &&\n+\ttest_must_fail git merge C &&\n+\ttest_commit M2 file change &&\n+\tgit checkout -b dm C &&\n+\ttest_must_fail git merge D &&\n+\ttest_commit M3 file resolved &&\n+\tgit checkout -b unrelated A &&\n+\ttest_commit unrelated_file &&\n+\tgit merge C &&\n+\ttest_tick &&\n+\tgit tag M4 &&\n+\tgit checkout -b dir A &&\n+\tmkdir sub &&\n+\ttest_commit dir sub/file &&\n+\tgit checkout -b file A &&\n+\ttest_commit file sub &&\n+\tgit checkout -b dirfile tags/dir &&\n+\ttest_must_fail git merge tags/file &&\n+\tgit rm --cached sub &&\n+\ttest_commit M5 sub/file resolved &&\n+\tgit checkout -b filedir tags/file &&\n+\ttest_must_fail git merge tags/dir &&\n+\tgit rm --cached sub/file &&\n+\trm -rf sub &&\n+\ttest_commit M6 sub resolved &&\n+\tgit branch -D master side delete dir file\n+'\n+\n+test_expect_success 'unrelated merge: without conflicts' '\n+\tgit log -p --cc unrelated >expected &&\n+\tgit log -p --remerge-diff unrelated >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+clean_output () {\n+\tgit name-rev --name-only --stdin |\n+\t# strip away bits that aren't treated by the above\n+\tsed -e 's/^\\(index\\|Merge:\\|Date:\\).*/\\1/'\n+}\n+\n+cat >expected <\n+Date:\n+\n+ M1\n+\n+diff --git a/file b/file\n+index\n+--- a/file\n++++ b/file\n+@@ -1,5 +1 @@\n+-<<<<<<< tags/B\n+-change\n+-=======\n+-side\n+->>>>>>> tags/C\n++merged\n+EOF\n+\n+test_expect_success 'benign merge: conflicts resolved' '\n+\tgit log -1 -p --remerge-diff benign >output &&\n+\tclean_output actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <\n+Date:\n+\n+ M2\n+\n+diff --git a/file b/file\n+index\n+--- a/file\n++++ b/file\n+@@ -1,5 +1 @@\n+-<<<<<<< tags/B\n+ change\n+-=======\n+-side\n+->>>>>>> tags/C\n+EOF\n+\n+test_expect_success 'evil merge: changes ignored' '\n+\tgit log -1 --remerge-diff -p evil >output &&\n+\tclean_output actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <\n+Date:\n+\n+ M3\n+\n+diff --git a/file b/file\n+index\n+--- a/file\n++++ b/file\n+@@ -1,4 +1 @@\n+-<<<<<<< tags/C\n+-side\n+-=======\n+->>>>>>> tags/D\n++resolved\n+EOF\n+\n+test_expect_success 'delete/modify conflict' '\n+\tgit log -1 --remerge-diff -p dm >output &&\n+\tclean_output actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <\n+Date:\n+\n+ M5\n+\n+diff --git a/sub/file b/sub/file\n+index\n+--- a/sub/file\n++++ b/sub/file\n+@@ -1 +1 @@\n+-dir\n++resolved\n+diff --git a/sub~tags/file b/sub~tags/file\n+deleted file mode 100644\n+index\n+--- a/sub~tags/file\n++++ /dev/null\n+@@ -1 +0,0 @@\n+-file\n+EOF\n+\n+test_expect_success 'file/directory conflict resulting in directory' '\n+\tgit log -1 --remerge-diff -p dirfile >output &&\n+\tclean_output actual &&\n+\ttest_cmp expected actual\n+'\n+\n+# This is wishful thinking, see the NEEDSWORK in\n+# make_asymmetric_conflict_entries().\n+cat >expected <\n+Date:\n+\n+ M6\n+\n+diff --git a/sub b/sub\n+index\n+--- a/sub\n++++ b/sub\n+@@ -1 +1 @@\n+-file\n++resolved\n+diff --git a/sub/file b/sub/file\n+deleted file mode 100644\n+index\n+--- a/sub/file\n++++ /dev/null\n+@@ -1 +0,0 @@\n+-dir\n+EOF\n+\n+test_expect_failure 'file/directory conflict resulting in file' '\n+\tgit log -1 --remerge-diff -p filedir >output &&\n+\tclean_output actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \n2.1.0.72.g9b94086\n"},{"id":"249045","messageId":"xmqqvboynhq2.fsf@gitster.dls.corp.google.com","threadId":"37502","inReplyTo":"33951a1d4be8ec15eec569e1a36c0a620b9edaa6.1409860234.git.tr@thomasrast.ch","subject":"Re: [PATCH v3 4/8] combine-diff: do not pass revs->dense_combined_merges redundantly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-08T17:29:41Z","receivedAt":"2014-09-08T17:29:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> The existing code passed revs->dense_combined_merges along revs itself\n> into the combine-diff functions, which is rather redundant. Remove\n> the 'dense' argument until much further down the callchain to simplify\n> callers.\n\nIt was not apparent that the changes to diff_tree_combined_merge()\nwas correct without looking at both of its callsites, but one passes\nthe .dense_combined_merges member, and the other in submodules\nalways gives true, which you covered here:\n\n> Note that while the caller in submodule.c needs to do extra work now,\n> the next commit will simplify this to a single setting again.\n\n> diff --git a/submodule.c b/submodule.c\n> index c3a61e7..0499de6 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -482,10 +482,13 @@ static void find_unpushed_submodule_commits(struct commit *commit,\n> \tstruct rev_info rev;\n> \n> \tinit_revisions(&rev, NULL);\n> +\trev.ignore_merges = 0;\n> +\trev.combined_merges = 1;\n> +\trev.dense_combined_merges = 1;\n> \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n> \trev.diffopt.format_callback = collect_submodules_from_diff;\n> \trev.diffopt.format_callback_data = needs_pushing;\n> -\tdiff_tree_combined_merge(commit, 1, &rev);\n> +\tdiff_tree_combined_merge(commit, &rev);\n> }\n\nI briefly wondered if there can be any unwanted side effects in this\nparticular codepath that is caused by setting rev.combined_merges\nwhich was not set in the original code, but seeing that this &rev is\nnot used for anything other than diff_tree_combined_merge(), it\nshould be OK.\n\nAlso I wondered if this is leaking whatever in the &rev structure,\nbut in this call I think rev is used only for its embedded diffopt\nin a way that does not leak anything, so it seems to be OK, but I'd\nappreciate if submodule folks can double check.\n\nThanks.\n"},{"id":"249047","messageId":"xmqqr3zmnhev.fsf@gitster.dls.corp.google.com","threadId":"37502","inReplyTo":"e95adf985efac162da72ac27220b904659dbb02d.1409860234.git.tr@thomasrast.ch","subject":"Re: [PATCH v3 5/8] Fold all merge diff variants into an enum","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-08T17:36:24Z","receivedAt":"2014-09-08T17:36:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> The four ways of displaying merge diffs,\n>\n> * none: no diff\n> * -m: against each parent\n> * -c: combined\n> * --cc: combined-condensed\n>\n> were encoded in three flag bits in struct rev_info. Fold them all\n> into a single enum field that captures the variants.\n\nNice. It also has a good side effect to spell \"condensed\" out,\ninstead of using a shorter \"dense\", and the end result matches what\nthe command line option calls it ;-).\n"},{"id":"249048","messageId":"xmqqk35enhcu.fsf@gitster.dls.corp.google.com","threadId":"37502","inReplyTo":"5bd5960659d85943477d2a5fbca3dd5ccd0da686.1409860234.git.tr@thomasrast.ch","subject":"Re: [PATCH v3 2/8] merge-recursive: internal flag to avoid touching the worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-08T17:37:37Z","receivedAt":"2014-09-08T17:37:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> From: Thomas Rast \n>\n> o->call_depth has a double function: a nonzero call_depth means we\n> want to construct virtual merge bases, but it also means we want to\n> avoid touching the worktree. Introduce a new flag o->no_worktree to\n> trigger only the latter.\n>\n> Signed-off-by: Thomas Rast \n> Signed-off-by: Junio C Hamano \n> ---\n\nI notice that many hits from\n\n $ git grep -e '->call_depth' --and --not -e '->no_worktree'\n\nare about how the progress is reported during recursive operations\nor setting up ll_opts suitable for ancestor merges (both of which\nare perfectly fine not to pay any attention to no_worktree), but\nsome others look iffy. For example, function remove_file() decides\nto update the in-core index only when call_depth is set (i.e. we are\ndoing a virtual parent) or clean (clean merge at the content level,\ni.e. \"both removed\"), and decides to update the working tree only at\nthe top-level of the recursion and no_wd is passed.\n\n - As to \"update_cache\", if you do not update it while you are\n operating in the cache-only mode (aka ->no_worktree), I wonder\n where the result goes. Shouldn't it be done for in-core merge as\n well?\n\n - As to \"update_working_tree\", there are few places where the\n function is called with no_wd that is not true, even when\n ->no_worktree is set. Do you want to allow working tree to be\n modified in such a call?\n"},{"id":"249053","messageId":"xmqq38c2nezv.fsf@gitster.dls.corp.google.com","threadId":"37502","inReplyTo":"29c15efe998630143eaa75ec7155a31ce17bd433.1409860234.git.tr@thomasrast.ch","subject":"Re: [PATCH v3 8/8] log --remerge-diff: show what the conflict resolution changed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-08T18:28:35Z","receivedAt":"2014-09-08T18:28:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> +static void assemble_conflict_entry(struct strbuf *sb,\n> +\t\t\t\t const char *branch1,\n> +\t\t\t\t const char *branch2,\n> +\t\t\t\t struct cache_entry *entry1,\n> +\t\t\t\t struct cache_entry *entry2)\n> +{\n> +\tstrbuf_addf(sb, \"<<<<<<< %s\\n\", branch1);\n> +\tstrbuf_append_cache_entry_blob(sb, entry1);\n> +\tstrbuf_addstr(sb, \"=======\\n\");\n> +\tstrbuf_append_cache_entry_blob(sb, entry2);\n> +\tstrbuf_addf(sb, \">>>>>>> %s\\n\", branch2);\n> +}\n\nI didn't read 6 thru 8 as carefully as I did the earlier ones, but\nthis part stood out. How does the above hardcoded markers interact\nwith the conflict-marker-size attribute set to the path?\n"},{"id":"249081","messageId":"xmqqd2b4lm96.fsf@gitster.dls.corp.google.com","threadId":"37502","inReplyTo":"781fa77328ef04600c9ef7ad4f682e157c3aeefe.1409860234.git.tr@thomasrast.ch","subject":"Re: [PATCH v3 6/8] merge-recursive: allow storing conflict hunks in index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-09T17:47:01Z","receivedAt":"2014-09-09T17:47:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\n> index be07705..39841a9 100755\n> --- a/t/t3030-merge-recursive.sh\n> +++ b/t/t3030-merge-recursive.sh\n> @@ -310,6 +310,26 @@ test_expect_success 'merge-recursive --index-only' '\n> \ttest_cmp expected-diff actual-diff\n> '\n> \n> +test_expect_success 'merge-recursive --index-only --conflicts-in-index' '\n> +\t# first pass: do a merge as usual to obtain \"expected\"\n> +\trm -fr [abcd] &&\n> +\tgit checkout -f \"$c2\" &&\n> +\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c2\" \"$c1\" &&\n> +\tgit add [abcd] &&\n> +\tgit ls-files -s >expected &&\n> +\t# second pass: actual test\n> +\trm -fr [abcd] &&\n> +\tgit checkout -f \"$c2\" &&\n> +\ttest_expect_code 1 \\\n> +\t\tgit merge-recursive --index-only --conflicts-in-index \\\n> +\t\t\"$c0\" -- \"$c2\" \"$c1\" &&\n> +\tgit ls-files -s >actual &&\n> +\ttest_cmp expected actual &&\n\nAt this point what is the expected output from \"git diff-files\"?\n\n> +\tgit diff HEAD >actual-diff &&\n> +\t: >expected-diff &&\n> +\ttest_cmp expected-diff actual-diff\n> +'\n> +\n> test_expect_success 'fail if the index has unresolved entries' '\n> \n> \trm -fr [abcd] &&\n"},{"id":"249082","messageId":"xmqq8ulslm4n.fsf@gitster.dls.corp.google.com","threadId":"37502","inReplyTo":"4d43fca586637dcb02b2b19c8d8a6dcfe368e059.1409860234.git.tr@thomasrast.ch","subject":"Re: [PATCH v3 7/8] name-hash: allow dir hashing even when !ignore_case","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-09T17:49:44Z","receivedAt":"2014-09-09T17:49:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> The directory hash (for fast checks if the index already has a\n> directory) was only used in ignore_case mode and so depended on that\n> flag.\n>\n> Make it generally available on request.\n>\n> Signed-off-by: Thomas Rast \n> ---\n> cache.h | 2 ++\n> name-hash.c | 13 ++++++++-----\n> 2 files changed, 10 insertions(+), 5 deletions(-)\n>\n> diff --git a/cache.h b/cache.h\n> index 4d5b76c..c54b2e1 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -306,6 +306,7 @@ struct index_state {\n> \tstruct split_index *split_index;\n> \tstruct cache_time timestamp;\n> \tunsigned name_hash_initialized : 1,\n> +\t\t has_dir_hash : 1,\n> \t\t initialized : 1;\n> \tstruct hashmap name_hash;\n> \tstruct hashmap dir_hash;\n> @@ -315,6 +316,7 @@ struct index_state {\n> extern struct index_state the_index;\n> \n> /* Name hashing */\n> +extern void init_name_hash(struct index_state *istate, int force_dir_hash);\n> extern void add_name_hash(struct index_state *istate, struct cache_entry *ce);\n> extern void remove_name_hash(struct index_state *istate, struct cache_entry *ce);\n> extern void free_name_hash(struct index_state *istate);\n> diff --git a/name-hash.c b/name-hash.c\n> index 702cd05..22e3ec6 100644\n> --- a/name-hash.c\n> +++ b/name-hash.c\n> @@ -106,7 +106,7 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)\n> \thashmap_entry_init(ce, memihash(ce->name, ce_namelen(ce)));\n> \thashmap_add(&istate->name_hash, ce);\n> \n> -\tif (ignore_case)\n> +\tif (istate->has_dir_hash)\n> \t\tadd_dir_entry(istate, ce);\n\nThis smells more like needs_dir_hash than has_dir_hash to me. For\nignore-case, we need dir_hash to make sure we do not end up adding\ntwo entries that cannot be represented on the filesystem.\n"},{"id":"249086","messageId":"xmqqy4tsk4cz.fsf@gitster.dls.corp.google.com","threadId":"37502","inReplyTo":"29c15efe998630143eaa75ec7155a31ce17bd433.1409860234.git.tr@thomasrast.ch","subject":"Re: [PATCH v3 8/8] log --remerge-diff: show what the conflict resolution changed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-09T18:58:52Z","receivedAt":"2014-09-09T18:58:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\nThomas Rast writes:\n\n> Git has --cc as a very fast inspection tool that shows a brief summary\n> of what a conflicted merge \"looks like\", and -c/-m as \"give me the\n> full information\" data dumps.\n>\n> But --cc actually loses information: if the merge lost(!) some changes\n> from one side, that hunk would fully agree with the other side, and\n> therefore be elided. So --cc cannot be used to investigate mismerges.\n> Indeed it is rather hard to find a merge that has lost changes, unless\n> one knows where to look.\n>\n> The new option --remerge-diff is an attempt at filling this gap,\n> admittedly at the cost of a lot of CPU cycles. For each merge commit,\n> it diffs the merge result against a recursive merge of the merge's\n> parents.\n>\n> For files that can be auto-merged cleanly, it will typically show\n> nothing. However, it will make it obvious when the merge introduces\n> extra changes.\n>\n> For files that result in merge conflicts, we diff against the\n> representation with conflict hunks (what the user would usually see in\n> the worktree). So the diff will show what was changed in the conflict\n> hunks to resolve the conflict.\n>\n> It still takes a bit of staring to tell an evil from a regular merge.\n> But at least the information is there, unlike with --cc; and the\n> output is usually much shorter than with -c.\n>\n> Signed-off-by: Thomas Rast \n> ---\n> Documentation/rev-list-options.txt | 7 +\n> log-tree.c | 297 +++++++++++++++++++++++++++++++++++++\n> merge-recursive.c | 3 +-\n> merge-recursive.h | 1 +\n> revision.c | 2 +\n> revision.h | 4 +-\n> t/t4213-log-remerge-diff.sh | 222 +++++++++++++++++++++++++++\n> 7 files changed, 534 insertions(+), 2 deletions(-)\n> create mode 100755 t/t4213-log-remerge-diff.sh\n>\n> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n> index deb8cca..7128350 100644\n> --- a/Documentation/rev-list-options.txt\n> +++ b/Documentation/rev-list-options.txt\n> @@ -805,6 +805,13 @@ options may be given. See linkgit:git-diff-files[1] for more options.\n> \tin that case, the output represents the changes the merge\n> \tbrought _into_ the then-current branch.\n> \n> +--remerge-diff::\n> +\tDiff merge commits against a recursive merge of their parents,\n> +\twith conflict hunks. Intuitively speaking, this shows what\n> +\tthe author of the merge changed to resolve the merge. It\n> +\tassumes that all (or most) merges are recursive merges; other\n> +\tstrategies are not supported.\n> +\n> -r::\n> \tShow recursive diffs.\n> \n> diff --git a/log-tree.c b/log-tree.c\n> index 8f57651..4db1385 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -11,6 +11,8 @@\n> #include \"gpg-interface.h\"\n> #include \"sequencer.h\"\n> #include \"line-log.h\"\n> +#include \"cache-tree.h\"\n> +#include \"merge-recursive.h\"\n> \n> struct decoration name_decoration = { \"object names\" };\n> \n> @@ -719,6 +721,299 @@ static int do_diff_combined(struct rev_info *opt, struct commit *commit)\n> }\n> \n> /*\n> + * Helpers for make_asymmetric_conflict_entries() below.\n> + */\n> +static char *load_cache_entry_blob(struct cache_entry *entry,\n> +\t\t\t\t unsigned long *size)\n> +{\n> +\tenum object_type type;\n> +\tvoid *data;\n> +\n> +\tif (!entry)\n> +\t\treturn NULL;\n> +\n> +\tdata = read_sha1_file(entry->sha1, &type, size);\n> +\tif (type != OBJ_BLOB)\n> +\t\tdie(\"BUG: load_cache_entry_blob for non-blob\");\n> +\n> +\treturn data;\n> +}\n> +\n> +static void strbuf_append_cache_entry_blob(struct strbuf *sb,\n> +\t\t\t\t\t struct cache_entry *entry)\n> +{\n> +\tunsigned long size;\n> +\tchar *data = load_cache_entry_blob(entry, &size);;\n> +\n> +\tif (!data)\n> +\t\treturn;\n> +\n> +\tstrbuf_add(sb, data, size);\n> +\tfree(data);\n> +}\n> +\n> +static void assemble_conflict_entry(struct strbuf *sb,\n> +\t\t\t\t const char *branch1,\n> +\t\t\t\t const char *branch2,\n> +\t\t\t\t struct cache_entry *entry1,\n> +\t\t\t\t struct cache_entry *entry2)\n> +{\n> +\tstrbuf_addf(sb, \"<<<<<<< %s\\n\", branch1);\n> +\tstrbuf_append_cache_entry_blob(sb, entry1);\n> +\tstrbuf_addstr(sb, \"=======\\n\");\n> +\tstrbuf_append_cache_entry_blob(sb, entry2);\n> +\tstrbuf_addf(sb, \">>>>>>> %s\\n\", branch2);\n> +}\n\nHmm, what is this one doing? I would have expected that you would\ngive file-level three-way merge using ll_merge machinery here. For\na conflicted path with two entries, using an empty buffer as the\ncommon ancestor would give you a reasonable-looking two-way\nno-parent merge.\n\n> +/*\n> + * For --remerge-diff, we need conflicted (<<<<<<< ... >>>>>>>)\n> + * representations of as many conflicts as possible. Default conflict\n> + * generation only applies to files that have all three stages.\n> + *\n> + * This function generates conflict hunk representations for files\n> + * that have only one of stage 2 or 3. The corresponding side in the\n> + * conflict hunk format will be empty. A stage 1, if any, will be\n> + * dropped in the process.\n> + */\n> +static void make_asymmetric_conflict_entries(const char *branch1,\n> +\t\t\t\t\t const char *branch2)\n> +{\n> +\tint o = 0, i = 0;\n> +\n> +\t/*\n> +\t * NEEDSWORK: we trample all over the cache below, so we need\n> +\t * to set up the name hash early, before modifying it. And\n> +\t * after that we cannot free any cache entries, because they\n> +\t * remain hashed even if deleted. Sigh.\n> +\t */\n> +\tinit_name_hash(&the_index, 1);\n> +\n> +\t/*\n> +\t * The loop always starts with 'i' pointing at the first entry\n> +\t * for a pathname.\n> +\t */\n> +\twhile (i < active_nr) {\n> +\t\tstruct cache_entry *ce;\n> +\t\tstruct cache_entry *stage1 = NULL;\n> +\t\tstruct cache_entry *stage2 = NULL;\n> +\t\tstruct cache_entry *stage3 = NULL;\n> +\t\tstruct cache_entry *new_ce = NULL;\n> +\t\tstruct strbuf content = STRBUF_INIT;\n> +\t\tunsigned char sha1[20];\n> +\n> +\t\tassert(o <= i);\n> +\n> +\t\tce = active_cache[i];\n> +\n> +\t\t/*\n> +\t\t * Pass through stage 0 and submodules unchanged, we\n> +\t\t * don't know how to handle them.\n> +\t\t */\n> +\t\tif (ce_stage(ce) == 0\n> +\t\t || S_ISGITLINK(ce->ce_mode)) {\n\nunnecessary line folding?\n\n> +\t\t\tactive_cache[o++] = ce;\n> +\t\t\ti++;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * Collect the stages we have. Point 'i' past the\n> +\t\t * entry.\n> +\t\t */\n> +\t\tif (/* i < active_nr && */\n\nWhy is this commented out?\n\nIs it because ce === active_cache[i] and i that was i < active_nr at\nthe top has not been incremented yet? If so, shouldn't the strcmp()\non the next line also be unnecessary?\n\n> +\t\t !strcmp(ce->name, active_cache[i]->name) &&\n> +\t\t ce_stage(active_cache[i]) == 1)\n> +\t\t\tstage1 = active_cache[i++];\n> +\n> +\t\tif (i < active_nr &&\n> +\t\t !strcmp(ce->name, active_cache[i]->name) &&\n> +\t\t ce_stage(active_cache[i]) == 2)\n> +\t\t\tstage2 = active_cache[i++];\n> +\n> +\t\tif (i < active_nr &&\n> +\t\t !strcmp(ce->name, active_cache[i]->name) &&\n> +\t\t ce_stage(active_cache[i]) == 3)\n> +\t\t\tstage3 = active_cache[i++];\n> +\n> +\t\tif (!stage2 && !stage3)\n> +\t\t\tdie(\"BUG: merging resulted in conflict with neither \"\n> +\t\t\t \"stage 2 nor 3\");\n\n... which would mean that the caller did not resolve \"both deleted\"\ninto \"delete from the result\". Sensible.\n\n> +\t\tif (cache_dir_exists(ce->name, ce->ce_namelen)) {\n> +\t\t\t/*\n> +\t\t\t * If a conflicting directory for this entry exists,\n> +\t\t\t * we can drop it:\n> +\t\t\t *\n> +\t\t\t * In the face of a file/directory conflict,\n> +\t\t\t * merge-recursive\n> +\t\t\t * - puts the file at stage >0 as usual\n> +\t\t\t * - also attempts to check out the file as\n> +\t\t\t * 'file~side' where side is the sha1 of the commit\n> +\t\t\t * the file came from, to avoid colliding with the\n> +\t\t\t * directory\n> +\t\t\t *\n> +\t\t\t * But we have requested that files go to the index\n> +\t\t\t * instead of the worktree, so by the time we get\n> +\t\t\t * here, we have both stage>0 'file' from ordinary\n> +\t\t\t * merging and a stage=0 'file~side' from the \"write\n> +\t\t\t * all files to index\".\n> +\t\t\t *\n> +\t\t\t * We need to remove one of them. Currently we ditch\n> +\t\t\t * the 'file' entry because it's easier to detect.\n> +\t\t\t * This amounts to always renaming the file to make\n> +\t\t\t * room for the directory.\n\nGood explanation.\n\n> +\t\t\t * NEEDSWORK: Two options:\n> +\t\t\t *\n> +\t\t\t * - If the merge result kept the file, it would be\n> +\t\t\t * better to rename the _directory_ to make room for\n> +\t\t\t * the file, so that filenames match between the\n> +\t\t\t * result and the re-merge.\n> +\t\t\t *\n> +\t\t\t * - Or we could avoid going through a tree, since the\n> +\t\t\t * index can represent (though it's not \"legal\") a\n> +\t\t\t * file/directory collision just fine.\n\nIf you are going to compare the resulting garbage with an existing\ntree, it is not \"just fine even though illegal\", I suspect. It is\njust a garbage.\n\n> +\t\t\t */\n> +\t\t} else {\n> +\t\t\t/*\n> +\t\t\t * Otherwise, there is room for a file entry\n> +\t\t\t * at stage 0. It has fake-conflict content,\n> +\t\t\t * but its mode is the same.\n> +\t\t\t */\n> +\n> +\t\t\tassemble_conflict_entry(&content,\n> +\t\t\t\t\t\tbranch1, branch2,\n> +\t\t\t\t\t\tstage2, stage3);\n\nNeed to ask the conflict-marker-size for the ce->name path and tell\nthe callee to use it, perhaps?\n\n> +\t\t\tif (write_sha1_file(content.buf, content.len,\n> +\t\t\t\t\t typename(OBJ_BLOB), sha1))\n> +\t\t\t\tdie(\"write_sha1_file failed\");\n> +\t\t\tstrbuf_release(&content);\n> +\n> +\t\t\tnew_ce = xcalloc(1, cache_entry_size(ce->ce_namelen));\n> +\t\t\tnew_ce->ce_mode = ce->ce_mode;\n> +\t\t\tnew_ce->ce_flags = ce->ce_flags & ~CE_STAGEMASK;\n> +\t\t\tnew_ce->ce_namelen = ce->ce_namelen;\n> +\t\t\thashcpy(new_ce->sha1, sha1);\n> +\t\t\tmemcpy(new_ce->name, ce->name, ce->ce_namelen+1);\n> +\t\t\tactive_cache[o++] = new_ce;\n> +\t\t\tadd_name_hash(&the_index, new_ce);\n\nInvalidate the cache-tree entry here?\n\nAn easier alternative may be to drop the cache-tree at the very\nbeginning and not worry about it, as you are not going to write this\nout as a tree (or save it as an on-disk index for somebody else to\nlater write a tree out of), but there is a rumor that since a0919ce\n\"diff-index --cached\" sees a large performance gain with cache-tree,\nso dropping the cache-tree entirely may be too huge a hammer. It\nneeds to be measured.\n\nI haven't read the series enough to know who calls this with an\nindex in what state, so all of the above might be a moot point. For\nexample, three-way merge would drop the cache-tree at the very\nbeginning anyway, so if the caller is doing such a merge and giving\nus such an index, then just dropping the cache-tree at the beginning\nto protect yourself from future changes in the caller may turn out\nto be the simplest. The same comment applies to drop-non-stage0.\n\n> +/*\n> + * --remerge-diff doesn't currently handle entries that cannot be\n> + * turned into a stage0 conflicted-file format blob. So this routine\n> + * clears the corresponding entries from the index. This is\n> + * suboptimal; we should eventually handle them _somehow_.\n> +*/\n> +static void drop_non_stage0()\n\n\"static void drop_unmerged_entries(void)\", perhaps?\n\n> +static int do_diff_remerge(struct rev_info *opt, struct commit *commit)\n> +{\n> +\tstruct commit_list *merge_bases;\n> +\tstruct commit *result, *parent1, *parent2;\n> +\tstruct merge_options o;\n> +\tchar *branch1, *branch2;\n> +\tstruct cache_tree *orig_cache_tree;\n> +\n> +\t/*\n> +\t * We show the log message early to avoid headaches later. In\n> +\t * general we need to run this before printing anything in\n> +\t * this routine.\n> +\t */\n\nHuh? What if the merge turns out to be a no-op, in which case you\nshould not print anything here?\n\n> +\tif (opt->loginfo && !opt->no_commit_id) {\n> +\t\tshow_log(opt);\n> +\n> +\t\tif (opt->verbose_header && opt->diffopt.output_format)\n> +\t\t\tprintf(\"%s%c\", diff_line_prefix(&opt->diffopt),\n> +\t\t\t opt->diffopt.line_termination);\n> +\t}\n> +\n> +\tif (commit->parents->next->next) {\n> +\t\tprintf(\"--remerge-diff not supported for octopus merges.\\n\");\n> +\t\treturn !opt->loginfo;\n> +\t}\n> +\n> +\tparent1 = commit->parents->item;\n> +\tparent2 = commit->parents->next->item;\n> +\tparse_commit(parent1);\n> +\tparse_commit(parent2);\n> +\tbranch1 = xstrdup(sha1_to_hex(parent1->object.sha1));\n> +\tbranch2 = xstrdup(sha1_to_hex(parent2->object.sha1));\n> +\n> +\tmerge_bases = get_octopus_merge_bases(commit->parents);\n\nHmm, why not get_merge_bases()?\n\n> +\tinit_merge_options(&o);\n> +\to.verbosity = -1;\n> +\to.no_worktree = 1;\n> +\to.conflicts_in_index = 1;\n> +\to.use_ondisk_index = 0;\n> +\to.branch1 = branch1;\n> +\to.branch2 = branch2;\n\nHmm. Usual merge_recursive() does in-core virtual merges and then\nbuilds the final one on top of the current index which represents\nwhat would have come from parent1. Here you are (correctly)\nrefusing the on-disk index, which does not have anything to do with\nparent1, to be used. Now at this point, what does the_index have?\nIdeally, you wouldn't have made any read_cache() call, as you are\nnot interested in the current working tree state at all in \"git\nlog\", and the call to merge_recursive() will discard_cache() anyway,\nso you will start from an empty in-core index.\n\n> +\tmerge_recursive(&o, parent1, parent2, merge_bases, &result);\n\n> +\tmake_asymmetric_conflict_entries(branch1, branch2);\n> +\tdrop_non_stage0();\n> +\n> +\tfree(branch1);\n> +\tfree(branch2);\n> +\n> +\torig_cache_tree = the_index.cache_tree;\n\n... which makes me wonder what you are saving here, and ...\n\n> +\tthe_index.cache_tree = cache_tree();\n> +\tif (cache_tree_update(&the_index, WRITE_TREE_SILENT) < 0) {\n> +\t\tprintf(\"BUG: merge conflicts not fully folded, cannot diff.\\n\");\n> +\t\treturn !opt->loginfo;\n> +\t}\n\n... why you run cache-tree-update here.\n\nAhh, this is a \"write_tree()\", because you want to run\ndiff-tree-sha1?\n\nI wondered why you are not doing diff_cache() from diff-lib.c\ninstead, but I think that is because it punts on unmerged entries\nwith \"* Unmerged path $pathname\". Teaching \"diff-index --cached -p\"\nan option to give a remerge diff may give us a useful tool that can\nbe used _during_ a merge resolution.\n\n $ git merge other\n ... results in conflicts\n $ git diff\n ... shows the working tree content and the initial conflict\n $ edit\n $ git diff\n ... shows your partial resolution in the working tree and the\n ... initial conflict\n $ git diff HEAD\n ... shows your partial resolution in the working tree and the HEAD\n $ git diff --cached HEAD\n ... shows the initial conflict and the HEAD, may be useful when\n ... you are lost\n\nOn second thought, that may not be so useful after all. I dunno\nuntil I see one ;-).\n\n> +\tdiff_tree_sha1(the_index.cache_tree->sha1, commit->tree->object.sha1,\n> +\t\t \"\", &opt->diffopt);\n> +\tlog_tree_diff_flush(opt);\n> +\n> +\tcache_tree_free(&the_index.cache_tree);\n> +\tthe_index.cache_tree = orig_cache_tree;\n\nIn any case, I do not understand what we are trying to save and\nrestore here.\n"},{"id":"249087","messageId":"xmqqr3zkk3x9.fsf@gitster.dls.corp.google.com","threadId":"37502","inReplyTo":"29c15efe998630143eaa75ec7155a31ce17bd433.1409860234.git.tr@thomasrast.ch","subject":"Re: [PATCH v3 8/8] log --remerge-diff: show what the conflict resolution changed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-09T19:08:18Z","receivedAt":"2014-09-09T19:08:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> +\t\t\tassemble_conflict_entry(&content,\n> +\t\t\t\t\t\tbranch1, branch2,\n> +\t\t\t\t\t\tstage2, stage3);\n> +\t\t\tif (write_sha1_file(content.buf, content.len,\n> +\t\t\t\t\t typename(OBJ_BLOB), sha1))\n> +\t\t\t\tdie(\"write_sha1_file failed\");\n> +...\n> +\tif (cache_tree_update(&the_index, WRITE_TREE_SILENT) < 0) {\n> +\t\tprintf(\"BUG: merge conflicts not fully folded, cannot diff.\\n\");\n> +\t\treturn !opt->loginfo;\n> +\t}\n\nAnother worry I have on this change is that it breaks the\nexpectation that \"log [-p]\" is a read-only operation. I do not know\nhow big a breakage this will be viewed as by those uninitiated who\ndo not know how --remerge-diff (or Git in general) works internally.\n"},{"id":"249251","messageId":"5411FA09.3050303@web.de","threadId":"37502","inReplyTo":"xmqqvboynhq2.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 4/8] combine-diff: do not pass revs->dense_combined_merges redundantly","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2014-09-11T19:37:45Z","receivedAt":"2014-09-11T19:37:45Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 08.09.2014 um 19:29 schrieb Junio C Hamano:\n> Thomas Rast writes:\n>\n>> The existing code passed revs->dense_combined_merges along revs itself\n>> into the combine-diff functions, which is rather redundant. Remove\n>> the 'dense' argument until much further down the callchain to simplify\n>> callers.\n>\n> It was not apparent that the changes to diff_tree_combined_merge()\n> was correct without looking at both of its callsites, but one passes\n> the .dense_combined_merges member, and the other in submodules\n> always gives true, which you covered here:\n>\n>> Note that while the caller in submodule.c needs to do extra work now,\n>> the next commit will simplify this to a single setting again.\n>\n>> diff --git a/submodule.c b/submodule.c\n>> index c3a61e7..0499de6 100644\n>> --- a/submodule.c\n>> +++ b/submodule.c\n>> @@ -482,10 +482,13 @@ static void find_unpushed_submodule_commits(struct commit *commit,\n>> \tstruct rev_info rev;\n>>\n>> \tinit_revisions(&rev, NULL);\n>> +\trev.ignore_merges = 0;\n>> +\trev.combined_merges = 1;\n>> +\trev.dense_combined_merges = 1;\n>> \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n>> \trev.diffopt.format_callback = collect_submodules_from_diff;\n>> \trev.diffopt.format_callback_data = needs_pushing;\n>> -\tdiff_tree_combined_merge(commit, 1, &rev);\n>> +\tdiff_tree_combined_merge(commit, &rev);\n>> }\n>\n> I briefly wondered if there can be any unwanted side effects in this\n> particular codepath that is caused by setting rev.combined_merges\n> which was not set in the original code, but seeing that this &rev is\n> not used for anything other than diff_tree_combined_merge(), it\n> should be OK.\n>\n> Also I wondered if this is leaking whatever in the &rev structure,\n> but in this call I think rev is used only for its embedded diffopt\n> in a way that does not leak anything, so it seems to be OK, but I'd\n> appreciate if submodule folks can double check.\n\nThe only thing the collect_submodules_from_diff() callback does\nis to collect the to-be-pushed submodules in the needs_pushing\nstring_list initialized with STRING_LIST_INIT_DUP which is cleared\nat the end of push_unpushed_submodules(), so I think we should be\nok here.\n"}]}