From e037c2e41869489c1e17701a9f1c429ad9c18f9b Mon Sep 17 00:00:00 2001 From: Elijah Newren <newren@gmail.com> Date: Wed, 4 Aug 2021 23:50:45 +0000 Subject: [PATCH 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges When --rebase-merges was first introduced, it only worked with the `recursive` strategy. Some time later, it gained support for merges using the `octopus` strategy. The limitation of only supporting these two strategies was documented in 25cff9f109 ("rebase -i --rebase-merges: add a section to the man page", 2018-04-25) and lifted in e145d99347 ("rebase -r: support merge strategies other than `recursive`", 2019-07-31). However, when the limitation was lifted, the documentation was not updated. Update it now. Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Acked-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/git-rebase.txt | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 55af6fd24e..8a67227846 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -1219,12 +1219,16 @@ successful merge so that the user can edit the message. If a `merge` command fails for any reason other than merge conflicts (i.e. when the merge operation did not even start), it is rescheduled immediately. -At this time, the `merge` command will *always* use the `recursive` -merge strategy for regular merges, and `octopus` for octopus merges, -with no way to choose a different one. To work around -this, an `exec` command can be used to call `git merge` explicitly, -using the fact that the labels are worktree-local refs (the ref -`refs/rewritten/onto` would correspond to the label `onto`, for example). +By default, the `merge` command will use the `recursive` merge +strategy for regular merges, and `octopus` for octopus merges. One +can specify a default strategy for all merges using the `--strategy` +argument when invoking rebase, or can override specific merges in the +interactive list of commands by using an `exec` command to call `git +merge` explicitly with a `--strategy` argument. Note that when +calling `git merge` explicitly like this, you can make use of the fact +that the labels are worktree-local refs (the ref `refs/rewritten/onto` +would correspond to the label `onto`, for example) in order to refer +to the branches you want to merge. Note: the first command (`label onto`) labels the revision onto which the commits are rebased; The name `onto` is just a convention, as a nod From b378df72ed147d346503341db06771c52cd2dbb8 Mon Sep 17 00:00:00 2001 From: Elijah Newren <newren@gmail.com> Date: Wed, 4 Aug 2021 23:50:46 +0000 Subject: [PATCH 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations In commit 0c4fd732f0 ("Move computation of dir_rename_count from merge-ort to diffcore-rename", 2021-02-27), much of the logic for computing directory renames moved into diffcore-rename. directory-rename-detection.txt had claims that all of that logic was found in merge-recursive. Update the documentation. Acked-by: Derrick Stolee <dstolee@microsoft.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- .../technical/directory-rename-detection.txt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Documentation/technical/directory-rename-detection.txt b/Documentation/technical/directory-rename-detection.txt index 49b83ef3cc..029ee2cedc 100644 --- a/Documentation/technical/directory-rename-detection.txt +++ b/Documentation/technical/directory-rename-detection.txt @@ -2,9 +2,9 @@ Directory rename detection ========================== Rename detection logic in diffcore-rename that checks for renames of -individual files is aggregated and analyzed in merge-recursive for cases -where combinations of renames indicate that a full directory has been -renamed. +individual files is also aggregated there and then analyzed in either +merge-ort or merge-recursive for cases where combinations of renames +indicate that a full directory has been renamed. Scope of abilities ------------------ @@ -88,9 +88,11 @@ directory rename detection support in: Folks have requested in the past that `git diff` detect directory renames and somehow simplify its output. It is not clear whether this would be desirable or how the output should be simplified, so this was - simply not implemented. Further, to implement this, directory rename - detection logic would need to move from merge-recursive to - diffcore-rename. + simply not implemented. Also, while diffcore-rename has most of the + logic for detecting directory renames, some of the logic is still found + within merge-ort and merge-recursive. Fully supporting directory + rename detection in diffs would require copying or moving the remaining + bits of logic to the diff machinery. * am From e80178eac6f3513bed7ec7ead9d5b0ebfa429241 Mon Sep 17 00:00:00 2001 From: Elijah Newren <newren@gmail.com> Date: Wed, 4 Aug 2021 23:50:47 +0000 Subject: [PATCH 03/10] Documentation: edit awkward references to `git merge-recursive` A few places in the documentation referred to the "`recursive` strategy" using the phrase "`git merge-recursive`", suggesting that it was forking subprocesses to call a toplevel builtin. Perhaps that was relevant to when rebase was a shell script, but it seems like a rather indirect way to refer to the `recursive` strategy. Simplify the references. Acked-by: Derrick Stolee <dstolee@microsoft.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/git-rebase.txt | 5 ++--- Documentation/merge-options.txt | 4 ++-- Documentation/merge-strategies.txt | 9 +++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 8a67227846..c3edcb07e3 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -354,9 +354,8 @@ See also INCOMPATIBLE OPTIONS below. -s <strategy>:: --strategy=<strategy>:: - Use the given merge strategy. - If there is no `-s` option 'git merge-recursive' is used - instead. This implies --merge. + Use the given merge strategy, instead of the default + `recursive`. This implies `--merge`. + Because 'git rebase' replays each commit from the working branch on top of the <upstream> branch using the given strategy, using diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index eb0aabd396..f819bd8dd6 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -112,8 +112,8 @@ With --squash, --commit is not allowed, and will fail. Use the given merge strategy; can be supplied more than once to specify them in the order they should be tried. If there is no `-s` option, a built-in list of strategies - is used instead ('git merge-recursive' when merging a single - head, 'git merge-octopus' otherwise). + is used instead (`recursive` when merging a single head, + `octopus` otherwise). -X <option>:: --strategy-option=<option>:: diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 2912de706b..5d707e952a 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -51,10 +51,11 @@ patience;; See also linkgit:git-diff[1] `--patience`. diff-algorithm=[patience|minimal|histogram|myers];; - Tells 'merge-recursive' to use a different diff algorithm, which - can help avoid mismerges that occur due to unimportant matching - lines (such as braces from distinct functions). See also - linkgit:git-diff[1] `--diff-algorithm`. + Use a different diff algorithm while merging, which can help + avoid mismerges that occur due to unimportant matching lines + (such as braces from distinct functions). See also + linkgit:git-diff[1] `--diff-algorithm`. Defaults to the + `diff.algorithm` config setting. ignore-space-change;; ignore-all-space;; From 510415ecc9d6ff4a99991dc1b70a1ac50818a539 Mon Sep 17 00:00:00 2001 From: Elijah Newren <newren@gmail.com> Date: Wed, 4 Aug 2021 23:50:48 +0000 Subject: [PATCH 04/10] merge-strategies.txt: update wording for the resolve strategy It is probably helpful to cover the default merge strategy first, so move the text for the resolve strategy to later in the document. Further, the wording for "resolve" claimed that it was "considered generally safe and fast", which might imply in some readers minds that the same is not true of other strategies. Rather than adding this text to all the strategies, just remove it from this one. Acked-by: Derrick Stolee <dstolee@microsoft.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/merge-strategies.txt | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 5d707e952a..f100fad1e4 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -6,13 +6,6 @@ backend 'merge strategies' to be chosen with `-s` option. Some strategies can also take their own options, which can be passed by giving `-X<option>` arguments to `git merge` and/or `git pull`. -resolve:: - This can only resolve two heads (i.e. the current branch - and another branch you pulled from) using a 3-way merge - algorithm. It tries to carefully detect criss-cross - merge ambiguities and is considered generally safe and - fast. - recursive:: This can only resolve two heads using a 3-way merge algorithm. When there is more than one common @@ -106,6 +99,12 @@ subtree[=<path>];; is prefixed (or stripped from the beginning) to make the shape of two trees to match. +resolve:: + This can only resolve two heads (i.e. the current branch + and another branch you pulled from) using a 3-way merge + algorithm. It tries to carefully detect criss-cross + merge ambiguities. It does not handle renames. + octopus:: This resolves cases with more than two heads, but refuses to do a complex merge that needs manual resolution. It is From 002a6dfc7c4642524d1e82f8cf994fc9ea5786f9 Mon Sep 17 00:00:00 2001 From: Elijah Newren <newren@gmail.com> Date: Wed, 4 Aug 2021 23:50:49 +0000 Subject: [PATCH 05/10] merge-strategies.txt: do not imply using copy detection is desired Stating that the recursive strategy "currently cannot make use of detected copies" implies that this is a technical shortcoming of the current algorithm. I disagree with that. I don't see how copies could possibly be used in a sane fashion in a merge algorithm -- would we propagate changes in one file on one side of history to each copy of that file when merging? That makes no sense to me. I cannot think of anything else that would make sense either. Change the wording to simply state that we ignore any copies. Acked-by: Derrick Stolee <dstolee@microsoft.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/merge-strategies.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index f100fad1e4..e298812458 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -16,9 +16,9 @@ recursive:: causing mismerges by tests done on actual merge commits taken from Linux 2.6 kernel development history. Additionally this can detect and handle merges involving - renames, but currently cannot make use of detected - copies. This is the default merge strategy when pulling - or merging one branch. + renames. It does not make use of detected copies. This + is the default merge strategy when pulling or merging one + branch. + The 'recursive' strategy can take the following options: From 4d15c85556891bfdddfb90b6be056b08079696bb Mon Sep 17 00:00:00 2001 From: Elijah Newren <newren@gmail.com> Date: Wed, 4 Aug 2021 23:50:50 +0000 Subject: [PATCH 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm We already have diff-algorithm that explains why there are special diff algorithms, so we do not need to re-explain patience. patience exists as its own toplevel option for historical reasons, but there's no reason to give it special preference or document it again and suggest it's more important than other diff algorithms, so just refer to it as a deprecated shorthand for `diff-algorithm=patience`. Acked-by: Derrick Stolee <dstolee@microsoft.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/merge-strategies.txt | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index e298812458..b54bcf68f2 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -37,11 +37,7 @@ theirs;; no 'theirs' merge strategy to confuse this merge option with. patience;; - With this option, 'merge-recursive' spends a little extra time - to avoid mismerges that sometimes occur due to unimportant - matching lines (e.g., braces from distinct functions). Use - this when the branches to be merged have diverged wildly. - See also linkgit:git-diff[1] `--patience`. + Deprecated synonym for `diff-algorithm=patience`. diff-algorithm=[patience|minimal|histogram|myers];; Use a different diff algorithm while merging, which can help From b36ade216c4a72dda6b8e3006cd3ded44ece1c66 Mon Sep 17 00:00:00 2001 From: Elijah Newren <newren@gmail.com> Date: Wed, 4 Aug 2021 23:50:51 +0000 Subject: [PATCH 07/10] merge-strategies.txt: fix simple capitalization error Acked-by: Derrick Stolee <dstolee@microsoft.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/git-rebase.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index c3edcb07e3..cecdcfff47 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -529,7 +529,7 @@ The `--rebase-merges` mode is similar in spirit to the deprecated where commits can be reordered, inserted and dropped at will. + It is currently only possible to recreate the merge commits using the -`recursive` merge strategy; Different merge strategies can be used only via +`recursive` merge strategy; different merge strategies can be used only via explicit `exec git merge -s <strategy> [...]` commands. + See also REBASING MERGES and INCOMPATIBLE OPTIONS below. From 6320813bc0daf03f087072a498b5aaa8bbba36bd Mon Sep 17 00:00:00 2001 From: Elijah Newren <newren@gmail.com> Date: Wed, 4 Aug 2021 23:50:52 +0000 Subject: [PATCH 08/10] git-rebase.txt: correct out-of-date and misleading text about renames Commit 58634dbff8 ("rebase: Allow merge strategies to be used when rebasing", 2006-06-21) added the --merge option to git-rebase so that renames could be detected (at least when using the `recursive` merge backend). However, git-am -3 gained that same ability in commit 579c9bb198 ("Use merge-recursive in git-am -3.", 2006-12-28). As such, the comment about being able to detect renames is not particularly noteworthy. Remove it. Acked-by: Derrick Stolee <dstolee@microsoft.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/git-rebase.txt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index cecdcfff47..73d49ec8d9 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -340,9 +340,7 @@ See also INCOMPATIBLE OPTIONS below. -m:: --merge:: - Use merging strategies to rebase. When the recursive (default) merge - strategy is used, this allows rebase to be aware of renames on the - upstream side. This is the default. + Using merging strategies to rebase (default). + Note that a rebase merge works by replaying each commit from the working branch on top of the <upstream> branch. Because of this, when a merge From 67feccd3ba4566392774b8ed9492d7169031d4bb Mon Sep 17 00:00:00 2001 From: Elijah Newren <newren@gmail.com> Date: Wed, 4 Aug 2021 23:50:53 +0000 Subject: [PATCH 09/10] merge-strategies.txt: add coverage of the `ort` merge strategy Acked-by: Derrick Stolee <dstolee@microsoft.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/merge-strategies.txt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index b54bcf68f2..210f0f850b 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -95,6 +95,20 @@ subtree[=<path>];; is prefixed (or stripped from the beginning) to make the shape of two trees to match. +ort:: + This is meant as a drop-in replacement for the `recursive` + algorithm (as reflected in its acronym -- "Ostensibly + Recursive's Twin"), and will likely replace it in the future. + It fixes corner cases that the `recursive` strategy handles + suboptimally, and is significantly faster in large + repositories -- especially when many renames are involved. ++ +The `ort` strategy takes all the same options as `recursive`. +However, it ignores three of those options: `no-renames`, +`patience` and `diff-algorithm`. It always runs with rename +detection (it handles it much faster than `recursive` does), and +it specifically uses `diff-algorithm=histogram`. + resolve:: This can only resolve two heads (i.e. the current branch and another branch you pulled from) using a 3-way merge From 81483fe613de70e17913167876676528cb37cbcd Mon Sep 17 00:00:00 2001 From: Elijah Newren <newren@gmail.com> Date: Wed, 4 Aug 2021 23:50:54 +0000 Subject: [PATCH 10/10] Update error message and code comment There were two locations in the code that referred to 'merge-recursive' but which were also applicable to 'merge-ort'. Update them to more general wording. Acked-by: Derrick Stolee <dstolee@microsoft.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/merge.c | 2 +- sequencer.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index a8a843b1f5..d7b14bf4a7 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -738,7 +738,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common, for (x = 0; x < xopts_nr; x++) if (parse_merge_opt(&o, xopts[x])) - die(_("Unknown option for merge-recursive: -X%s"), xopts[x]); + die(_("unknown strategy option: -X%s"), xopts[x]); o.branch1 = head_arg; o.branch2 = merge_remote_util(remoteheads->item)->name; diff --git a/sequencer.c b/sequencer.c index 7f07cd00f3..a4e5c43fcf 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2065,7 +2065,7 @@ static int do_pick_commit(struct repository *r, /* * We do not intend to commit immediately. We just want to * merge the differences in, so let's compute the tree - * that represents the "current" state for merge-recursive + * that represents the "current" state for the merge machinery * to work on. */ if (write_index_as_tree(&head, r->index, r->index_file, 0, NULL))