diff --git a/Documentation/merge-strategies.adoc b/Documentation/merge-strategies.adoc index 93822ebc4e83b0..59f5ae36ccb1dd 100644 --- a/Documentation/merge-strategies.adoc +++ b/Documentation/merge-strategies.adoc @@ -82,6 +82,11 @@ find-renames[=];; rename-threshold=;; Deprecated synonym for `find-renames=`. +no-renames;; + Turn off rename detection. This overrides the `merge.renames` + configuration variable. + See also linkgit:git-diff[1] `--no-renames`. + subtree[=];; This option is a more advanced form of 'subtree' strategy, where the strategy makes a guess on how two trees must be shifted to @@ -107,7 +112,7 @@ For a path that is a submodule, the same caution as 'ort' applies to this strategy. + The 'recursive' strategy takes the same options as 'ort'. However, -there are three additional options that 'ort' ignores (not documented +there are two additional options that 'ort' ignores (not documented above) that are potentially useful with the 'recursive' strategy: patience;; @@ -121,11 +126,6 @@ diff-algorithm=[patience|minimal|histogram|myers];; specifically uses `diff-algorithm=histogram`, while `recursive` defaults to the `diff.algorithm` config setting. -no-renames;; - Turn off rename detection. This overrides the `merge.renames` - configuration variable. - See also linkgit:git-diff[1] `--no-renames`. - resolve:: This can only resolve two heads (i.e. the current branch and another branch you pulled from) using a 3-way merge diff --git a/builtin/am.c b/builtin/am.c index 2921bb89ef16e6..3b61bd4c333c4b 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -31,7 +31,7 @@ #include "preload-index.h" #include "sequencer.h" #include "revision.h" -#include "merge-recursive.h" +#include "merge-ort-wrappers.h" #include "log-tree.h" #include "notes-utils.h" #include "rerere.h" @@ -1638,12 +1638,13 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa o.branch1 = "HEAD"; their_tree_name = xstrfmt("%.*s", linelen(state->msg), state->msg); o.branch2 = their_tree_name; + o.ancestor = "constructed fake ancestor"; o.detect_directory_renames = MERGE_DIRECTORY_RENAMES_NONE; if (state->quiet) o.verbosity = 0; - if (merge_recursive_generic(&o, &our_tree, &their_tree, 1, bases, &result)) { + if (merge_ort_generic(&o, &our_tree, &their_tree, 1, bases, &result)) { repo_rerere(the_repository, state->allow_rerere_autoupdate); free(their_tree_name); return error(_("Failed to merge in the changes.")); diff --git a/merge-ort-wrappers.c b/merge-ort-wrappers.c index d6f61359965b49..c54d56b34465bf 100644 --- a/merge-ort-wrappers.c +++ b/merge-ort-wrappers.c @@ -1,9 +1,13 @@ #include "git-compat-util.h" #include "gettext.h" #include "hash.h" +#include "hex.h" +#include "lockfile.h" #include "merge-ort.h" #include "merge-ort-wrappers.h" #include "read-cache-ll.h" +#include "repository.h" +#include "tag.h" #include "tree.h" #include "commit.h" @@ -29,6 +33,7 @@ int merge_ort_nonrecursive(struct merge_options *opt, struct tree *merge_base) { struct merge_result result; + int show_msgs; if (unclean(opt, head)) return -1; @@ -38,9 +43,10 @@ int merge_ort_nonrecursive(struct merge_options *opt, return 1; } + show_msgs = !!opt->verbosity; memset(&result, 0, sizeof(result)); merge_incore_nonrecursive(opt, merge_base, head, merge, &result); - merge_switch_to_result(opt, head, &result, 1, 1); + merge_switch_to_result(opt, head, &result, 1, show_msgs); return result.clean; } @@ -53,14 +59,76 @@ int merge_ort_recursive(struct merge_options *opt, { struct tree *head = repo_get_commit_tree(opt->repo, side1); struct merge_result tmp; + int show_msgs; if (unclean(opt, head)) return -1; + show_msgs = !!opt->verbosity; memset(&tmp, 0, sizeof(tmp)); merge_incore_recursive(opt, merge_bases, side1, side2, &tmp); - merge_switch_to_result(opt, head, &tmp, 1, 1); + merge_switch_to_result(opt, head, &tmp, 1, show_msgs); *result = NULL; return tmp.clean; } + +static struct commit *get_ref(struct repository *repo, + const struct object_id *oid, + const char *name) +{ + struct object *object; + + object = deref_tag(repo, parse_object(repo, oid), + name, strlen(name)); + if (!object) + return NULL; + if (object->type == OBJ_TREE) + return make_virtual_commit(repo, (struct tree*)object, name); + if (object->type != OBJ_COMMIT) + return NULL; + if (repo_parse_commit(repo, (struct commit *)object)) + return NULL; + return (struct commit *)object; +} + +int merge_ort_generic(struct merge_options *opt, + const struct object_id *head, + const struct object_id *merge, + int num_merge_bases, + const struct object_id *merge_bases, + struct commit **result) +{ + int clean; + struct lock_file lock = LOCK_INIT; + struct commit *head_commit = get_ref(opt->repo, head, opt->branch1); + struct commit *next_commit = get_ref(opt->repo, merge, opt->branch2); + struct commit_list *ca = NULL; + + if (merge_bases) { + int i; + for (i = 0; i < num_merge_bases; ++i) { + struct commit *base; + if (!(base = get_ref(opt->repo, &merge_bases[i], + oid_to_hex(&merge_bases[i])))) + return error(_("Could not parse object '%s'"), + oid_to_hex(&merge_bases[i])); + commit_list_insert(base, &ca); + } + } + + repo_hold_locked_index(opt->repo, &lock, LOCK_DIE_ON_ERROR); + clean = merge_ort_recursive(opt, head_commit, next_commit, ca, + result); + free_commit_list(ca); + if (clean < 0) { + rollback_lock_file(&lock); + return clean; + } + + if (write_locked_index(opt->repo->index, &lock, + COMMIT_LOCK | SKIP_IF_UNCHANGED)) + return error(_("Unable to write index.")); + + return clean ? 0 : 1; +} diff --git a/merge-ort-wrappers.h b/merge-ort-wrappers.h index 90af1f69c55038..aeffa1c87b4f60 100644 --- a/merge-ort-wrappers.h +++ b/merge-ort-wrappers.h @@ -22,4 +22,16 @@ int merge_ort_recursive(struct merge_options *opt, const struct commit_list *ancestors, struct commit **result); +/* + * rename-detecting three-way merge. num_merge_bases must be at least 1. + * Recursive ancestor consolidation will be performed if num_merge_bases > 1. + * Wrapper mimicking the old merge_recursive_generic() function. + */ +int merge_ort_generic(struct merge_options *opt, + const struct object_id *head, + const struct object_id *merge, + int num_merge_bases, + const struct object_id *merge_bases, + struct commit **result); + #endif diff --git a/merge-ort.c b/merge-ort.c index 46e78c3ffa68e6..785e5c6f24ad91 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -3404,6 +3404,11 @@ static int collect_renames(struct merge_options *opt, pool_diff_free_filepair(&opt->priv->pool, p); continue; } + if (opt->detect_directory_renames == MERGE_DIRECTORY_RENAMES_NONE && + p->status == 'R' && 1) { + possibly_cache_new_pair(renames, p, side_index, NULL); + goto skip_directory_renames; + } new_path = check_for_directory_rename(opt, p->two->path, side_index, @@ -3421,6 +3426,7 @@ static int collect_renames(struct merge_options *opt, if (new_path) apply_directory_rename_modifications(opt, p, new_path); +skip_directory_renames: /* * p->score comes back from diffcore_rename_extended() with * the similarity of the renamed file. The similarity is @@ -3448,6 +3454,11 @@ static int detect_and_process_renames(struct merge_options *opt) if (!possible_renames(renames)) goto cleanup; + if (!opt->detect_renames) { + renames->redo_after_renames = 0; + renames->cached_pairs_valid_side = 0; + goto cleanup; + } trace2_region_enter("merge", "regular renames", opt->repo); detection_run |= detect_regular_renames(opt, MERGE_SIDE1); @@ -4878,9 +4889,9 @@ static inline void set_commit_tree(struct commit *c, struct tree *t) c->maybe_tree = t; } -static struct commit *make_virtual_commit(struct repository *repo, - struct tree *tree, - const char *comment) +struct commit *make_virtual_commit(struct repository *repo, + struct tree *tree, + const char *comment) { struct commit *commit = alloc_commit_node(repo); @@ -5020,7 +5031,8 @@ static void merge_start(struct merge_options *opt, struct merge_result *result) trace2_region_leave("merge", "allocate/init", opt->repo); } -static void merge_check_renames_reusable(struct merge_result *result, +static void merge_check_renames_reusable(struct merge_options *opt, + struct merge_result *result, struct tree *merge_base, struct tree *side1, struct tree *side2) @@ -5045,6 +5057,26 @@ static void merge_check_renames_reusable(struct merge_result *result, return; } + /* + * Avoid using cached renames when directory rename detection is + * turned off. Cached renames are far less important in that case, + * and they lead to testcases with an interesting intersection of + * effects from relevant renames optimization, trivial directory + * resolution optimization, and cached renames all converging when + * the target of a cached rename is in a directory that + * collect_merge_info() does not recurse into. To avoid such + * problems, simply disable cached renames for this case (similar + * to the rename/rename(1to1) case; see the "disabling the + * optimization" comment near that case). + * + * This could be revisited in the future; see the commit message + * where this comment was added for some possible pointers. + */ + if (opt->detect_directory_renames == MERGE_DIRECTORY_RENAMES_NONE) { + renames->cached_pairs_valid_side = 0; /* neither side valid */ + return; + } + /* * Handle other cases; note that merge_trees[0..2] will only * be NULL if opti is, or if all three were manually set to @@ -5186,6 +5218,8 @@ static void merge_ort_internal(struct merge_options *opt, ancestor_name = "empty tree"; } else if (merge_bases) { ancestor_name = "merged common ancestors"; + } else if (opt->ancestor) { + ancestor_name = opt->ancestor; } else { strbuf_add_unique_abbrev(&merge_base_abbrev, &merged_merge_bases->object.oid, @@ -5251,7 +5285,7 @@ void merge_incore_nonrecursive(struct merge_options *opt, trace2_region_enter("merge", "merge_start", opt->repo); assert(opt->ancestor != NULL); - merge_check_renames_reusable(result, merge_base, side1, side2); + merge_check_renames_reusable(opt, result, merge_base, side1, side2); merge_start(opt, result); /* * Record the trees used in this merge, so if there's a next merge in @@ -5275,8 +5309,13 @@ void merge_incore_recursive(struct merge_options *opt, { trace2_region_enter("merge", "incore_recursive", opt->repo); - /* We set the ancestor label based on the merge_bases */ - assert(opt->ancestor == NULL); + /* + * We set the ancestor label based on the merge_bases...but we + * allow one exception through so that builtin/am can override + * with its constructed fake ancestor. + */ + assert(opt->ancestor == NULL || + (merge_bases && !merge_bases->next)); trace2_region_enter("merge", "merge_start", opt->repo); merge_start(opt, result); diff --git a/merge-ort.h b/merge-ort.h index 82f2b3222d2fbe..b63bc5424e7459 100644 --- a/merge-ort.h +++ b/merge-ort.h @@ -44,6 +44,11 @@ struct merge_result { unsigned _properly_initialized; }; +/* Mostly internal function also used by merge-ort-wrappers.c */ +struct commit *make_virtual_commit(struct repository *repo, + struct tree *tree, + const char *comment); + /* * rename-detecting three-way merge with recursive ancestor consolidation. * working tree and index are untouched. diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh index 389670262e458e..58b37599357827 100755 --- a/t/t3650-replay-basics.sh +++ b/t/t3650-replay-basics.sh @@ -195,4 +195,26 @@ test_expect_success 'using replay on bare repo to rebase multiple divergent bran done ' +test_expect_success 'merge.directoryRenames=false' ' + # create a test case that stress-tests the rename caching + git switch -c rename-onto && + + mkdir -p to-rename && + test_commit to-rename/move && + + mkdir -p renamed-directory && + git mv to-rename/move* renamed-directory/ && + test_tick && + git commit -m renamed-directory && + + git switch -c rename-from HEAD^ && + test_commit to-rename/add-a-file && + echo modified >to-rename/add-a-file.t && + test_tick && + git commit -m modified to-rename/add-a-file.t && + + git -c merge.directoryRenames=false replay \ + --onto rename-onto rename-onto..rename-from +' + test_done diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index edb38da7010d33..8e1ecf8a68546c 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -112,7 +112,7 @@ test_expect_success 'am --abort will keep dirty index intact' ' test_expect_success 'am -3 stops on conflict on unborn branch' ' git checkout -f --orphan orphan && git reset && - rm -f otherfile-4 && + rm -f file-1 otherfile-4 && test_must_fail git am -3 0003-*.patch && test 2 -eq $(git ls-files -u | wc -l) && test 4 = "$(cat otherfile-4)" diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh index a7ba08f728c0b8..e6679a01b4413a 100755 --- a/t/t4255-am-submodule.sh +++ b/t/t4255-am-submodule.sh @@ -19,7 +19,6 @@ am_3way () { $2 git am --3way patch } -KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1 test_submodule_switch_func "am_3way" test_expect_success 'setup diff.submodule' ' diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh index eea19907b550c4..44f7d0775933f2 100755 --- a/t/t4301-merge-tree-write-tree.sh +++ b/t/t4301-merge-tree-write-tree.sh @@ -73,6 +73,12 @@ test_expect_success 'Clean merge' ' test_cmp expect actual ' +# Repeat the previous test, but turn off rename detection +test_expect_success 'Failed merge without rename detection' ' + test_must_fail git -c diff.renames=false merge-tree --write-tree side1 side3 >out && + grep "CONFLICT (modify/delete): numbers deleted" out +' + test_expect_success 'Content merge and a few conflicts' ' git checkout side1^0 && test_must_fail git merge side2 && diff --git a/t/t6427-diff3-conflict-markers.sh b/t/t6427-diff3-conflict-markers.sh index dd5fe6a4021962..57569c4f4bd2ff 100755 --- a/t/t6427-diff3-conflict-markers.sh +++ b/t/t6427-diff3-conflict-markers.sh @@ -207,7 +207,7 @@ test_expect_success 'rebase --apply describes fake ancestor base' ' cd rebase && git rebase --abort && test_must_fail git -c merge.conflictstyle=diff3 rebase --apply main && - grep "||||||| constructed merge base" file + grep "||||||| constructed fake ancestor" file ) '