{"thread":{"id":"56190","subject":"[PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","startedAt":"2021-08-01T00:07:47Z","lastAt":"2021-08-03T22:48:15Z","messageCount":21,"participants":["Elijah Newren via GitGitGadget","Derrick Stolee","Ævar Arnfjörð Bjarmason","Elijah Newren","Johannes Schindelin","Philippe Blain","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"431641","messageId":"pull.1055.git.git.1627776461.gitgitgadget@gmail.com","threadId":"56190","inReplyTo":null,"subject":"[PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-01T00:07:39Z","receivedAt":"2021-08-01T00:07:47Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This is an RFC series designed to spur feedback about switching the default\nmerge backend (reviewing the patches is of secondary importance at this\npoint). Some questions:\n\n * Are there things others want before this series is considered for\n   inclusion?\n * What kind of timeline do others think is reasonable?\n * Would it be beneficial to let this series sit in 'next' for an extended\n   duration to gain more feedback?\n\nSome potentially useful context in relation to the above:\n\n * I've personally used the ort backend for well over a year\n * I have ~50 testers using ort as the default merge backend since Nov.\n   2020.\n * ort fixes known bugs in recursive, and there are no known regressions\n   (more testers may change that)\n * ort is significantly faster than recursive\n * ort provides one new feature already, and enables more that are on the\n   way\n * The commit message of patch 1 has more details about the last three items\n   above\n\nSo...thoughts?\n\nElijah Newren (2):\n  Change default merge backend from recursive to ort\n  Update docs for change of default merge backend\n\n Documentation/git-rebase.txt                  | 17 ++--\n Documentation/gitfaq.txt                      |  2 +-\n Documentation/merge-options.txt               |  4 +-\n Documentation/merge-strategies.txt            | 98 +++++++++++--------\n .../technical/directory-rename-detection.txt  | 14 +--\n Documentation/user-manual.txt                 |  2 +-\n builtin/merge.c                               | 10 +-\n builtin/rebase.c                              |  2 +-\n sequencer.c                                   |  4 +-\n 9 files changed, 89 insertions(+), 64 deletions(-)\n\n\nbase-commit: eb27b338a3e71c7c4079fbac8aeae3f8fbb5c687\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1055%2Fnewren%2Fort-default-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1055/newren/ort-default-v1\nPull-Request: https://github.com/git/git/pull/1055\n-- \ngitgitgadget\n"},{"id":"431642","messageId":"8f6af8d494e0924aef4ae6963b8dca2228dad9b1.1627776462.git.gitgitgadget@gmail.com","threadId":"56190","inReplyTo":"pull.1055.git.git.1627776461.gitgitgadget@gmail.com","subject":"[PATCH 1/2] Change default merge backend from recursive to ort","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-01T00:07:40Z","receivedAt":"2021-08-01T00:08:31Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren <newren@gmail.com>\n\nThere are a few reasons to switch the default:\n  * Correctness\n  * Extensibility\n  * Performance\n\nI'll provide some summaries about each.\n\n=== Correctness ===\n\nThe original impetus for a new merge backend was to fix issues that were\ndifficult to fix within recursive's design.  The success with this goal\nis perhaps most easily demonstrated by running the following:\n\n  $ git grep -2 KNOWN_FAILURE t/ | grep -A 4 GIT_TEST_MERGE_ALGORITHM\n  $ git grep test_expect_merge_algorithm.failure.success t/\n  $ git grep test_expect_merge_algorithm.success.failure t/\n\nIn order, these greps show:\n\n  * Seven sets of submodule tests (10 total tests) that fail with\n    recursive but succeed with ort\n  * 22 other tests that fail with recursive, but succeed with ort\n  * 0 tests that pass with recursive, but fail with ort\n\n=== Extensibility ===\n\nBeing able to perform merges without touching the working tree or index\nmakes it possible to create new features that were difficult with the\nold backend:\n\n  * Merging, cherry-picking, rebasing, reverting in bare repositories...\n    or just on branches that aren't checked out.\n\n  * `git diff AUTO_MERGE` -- ability to see what changes the user has\n    made to resolve conflicts so far (see commit 5291828df8 (\"merge-ort:\n    write $GIT_DIR/AUTO_MERGE whenever we hit a conflict\", 2021-03-20)\n\n  * A --remerge-diff option for log/show, used to show diffs for merges\n    that display the difference between what an automatic merge would\n    have created and what was recorded in the merge.  (This option will\n    often result in an empty diff because many merges are clean, but for\n    the non-clean ones it will show how conflicts were fixed including\n    the removal of conflict markers, and also show additional changes\n    made outside of conflict regions to e.g. fix semantic conflicts.)\n\n  * A --remerge-diff-only option for log/show, similar to --remerge-diff\n    but also showing how cherry-picks or reverts differed from what an\n    automatic cherry-pick or revert would provide.\n\nThe last three have been implemented already (though only one has been\nsubmitted upstream so far; the others were waiting for performance work\nto complete), and I still plan to implement the first one.\n\n=== Performance ===\n\nI'll quote from the summary of my final optimization for merge-ort\n(while fixing the testcase name from 'no-renames' to 'few-renames'):\n\n                               Timings\n\n                                          Infinite\n                 merge-       merge-     Parallelism\n                recursive    recursive    of rename    merge-ort\n                 v2.30.0      current     detection     current\n                ----------   ---------   -----------   ---------\nfew-renames:      18.912 s    18.030 s     11.699 s     198.3 ms\nmega-renames:   5964.031 s   361.281 s    203.886 s     661.8 ms\njust-one-mega:   149.583 s    11.009 s      7.553 s     264.6 ms\n\n                           Speedup factors\n\n                                          Infinite\n                 merge-       merge-     Parallelism\n                recursive    recursive    of rename\n                 v2.30.0      current     detection    merge-ort\n                ----------   ---------   -----------   ---------\nfew-renames:        1           1.05         1.6           95\nmega-renames:       1          16.5         29           9012\njust-one-mega:      1          13.6         20            565\n\nAnd, for partial clone users:\n\n             Factor reduction in number of objects needed\n\n                                          Infinite\n                 merge-       merge-     Parallelism\n                recursive    recursive    of rename\n                 v2.30.0      current     detection    merge-ort\n                ----------   ---------   -----------   ---------\nmega-renames:       1            1            1          181.3\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n builtin/merge.c  | 10 ++++++++--\n builtin/rebase.c |  2 +-\n sequencer.c      |  4 ++--\n 3 files changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex a8a843b1f54..217c5061eb6 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -88,9 +88,9 @@ static int autostash;\n static int no_verify;\n \n static struct strategy all_strategy[] = {\n-\t{ \"recursive\",  DEFAULT_TWOHEAD | NO_TRIVIAL },\n+\t{ \"recursive\",  NO_TRIVIAL },\n \t{ \"octopus\",    DEFAULT_OCTOPUS },\n-\t{ \"ort\",        NO_TRIVIAL },\n+\t{ \"ort\",        DEFAULT_TWOHEAD | NO_TRIVIAL },\n \t{ \"resolve\",    0 },\n \t{ \"ours\",       NO_FAST_FORWARD | NO_TRIVIAL },\n \t{ \"subtree\",    NO_FAST_FORWARD | NO_TRIVIAL },\n@@ -1484,6 +1484,12 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t\tfast_forward = FF_NO;\n \t}\n \n+\tif (!use_strategies && !pull_twohead &&\n+\t    remoteheads && !remoteheads->next) {\n+\t\tchar *default_strategy = getenv(\"GIT_TEST_MERGE_ALGORITHM\");\n+\t\tif (default_strategy)\n+\t\t\tappend_strategy(get_strategy(default_strategy));\n+\t}\n \tif (!use_strategies) {\n \t\tif (!remoteheads)\n \t\t\t; /* already up-to-date */\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 12f093121d9..587a2f1d299 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1713,7 +1713,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tint i;\n \n \t\tif (!options.strategy)\n-\t\t\toptions.strategy = \"recursive\";\n+\t\t\toptions.strategy = \"ort\";\n \n \t\tstrbuf_reset(&buf);\n \t\tfor (i = 0; i < strategy_options.nr; i++)\ndiff --git a/sequencer.c b/sequencer.c\nindex 0bec01cf38e..a98de9a8d15 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -636,7 +636,7 @@ static int do_recursive_merge(struct repository *r,\n \tfor (i = 0; i < opts->xopts_nr; i++)\n \t\tparse_merge_opt(&o, opts->xopts[i]);\n \n-\tif (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n+\tif (!opts->strategy || strcmp(opts->strategy, \"recursive\")) {\n \t\tmemset(&result, 0, sizeof(result));\n \t\tmerge_incore_nonrecursive(&o, base_tree, head_tree, next_tree,\n \t\t\t\t\t    &result);\n@@ -3968,7 +3968,7 @@ static int do_merge(struct repository *r,\n \to.branch2 = ref_name.buf;\n \to.buffer_output = 2;\n \n-\tif (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n+\tif (!opts->strategy || strcmp(opts->strategy, \"recursive\")) {\n \t\t/*\n \t\t * TODO: Should use merge_incore_recursive() and\n \t\t * merge_switch_to_result(), skipping the call to\n-- \ngitgitgadget\n\n"},{"id":"431643","messageId":"35490397590ae4d39ba98c7eb206bfadc22ddf35.1627776462.git.gitgitgadget@gmail.com","threadId":"56190","inReplyTo":"pull.1055.git.git.1627776461.gitgitgadget@gmail.com","subject":"[PATCH 2/2] Update docs for change of default merge backend","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-01T00:07:41Z","receivedAt":"2021-08-01T00:08:31Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren <newren@gmail.com>\n\nMake multiple documentation updates to bring things up to date as we\nchange the default merge backend...\n\nAdd a section for `ort` within merge-strategies.txt; while it accepts\nthe same flags as `recursive` (even if it ignores three of them) and is\nmeant as a drop in replacement, it still makes sense to explicitly cover\nit.\n\nChange several locations in the docs that referred to `recursive` as the\ndefault merge backend, and fix a few that said or implied that only\n`recursive` had certain abilities (such as rename detection).\n\nFix up some wording in directory-rename-detection.txt due to some\nrestructurings performed while optimizing both the ort backend and the\nrename detection machinery.\n\nDrop the \"is considered generally safe and fast\" from the description of\nthe `resolve` strategy, since that implies the other strategies are not.\nWhile such an implication may have been true in 2005 when written, it\nmay well be that `ort` is faster today (since it does not need to\nrecurse into all directories).  Also, since `resolve` was the default\nfor less than a year while `recursive` has been the default for a decade\nand a half, I think `recursive` is more battle-tested than `resolve` is.\nMove the description of `resolve` near `octopus` and `ours` while at it\nsince it is no longer the default merge algorithm and hasn't been for a\nvery long time.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n Documentation/git-rebase.txt                  | 17 ++--\n Documentation/gitfaq.txt                      |  2 +-\n Documentation/merge-options.txt               |  4 +-\n Documentation/merge-strategies.txt            | 98 +++++++++++--------\n .../technical/directory-rename-detection.txt  | 14 +--\n Documentation/user-manual.txt                 |  2 +-\n 6 files changed, 78 insertions(+), 59 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 55af6fd24e2..41b68423444 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -340,9 +340,10 @@ See also INCOMPATIBLE OPTIONS below.\n \n -m::\n --merge::\n-\tUse merging strategies to rebase.  When the recursive (default) merge\n-\tstrategy is used, this allows rebase to be aware of renames on the\n-\tupstream side.  This is the default.\n+\tUse merging strategies to rebase.  When either the `ort`\n+\t(default) or `recursive` merge strategy is used, this allows\n+\trebase to be aware of renames on the upstream side.  This is the\n+\tdefault.\n +\n Note that a rebase merge works by replaying each commit from the working\n branch on top of the <upstream> branch.  Because of this, when a merge\n@@ -355,8 +356,8 @@ See also INCOMPATIBLE OPTIONS below.\n -s <strategy>::\n --strategy=<strategy>::\n \tUse the given merge strategy.\n-\tIf there is no `-s` option 'git merge-recursive' is used\n-\tinstead.  This implies --merge.\n+\tIf there is no `-s` option the `ort` strategy is the default.\n+\tThis implies --merge.\n +\n Because 'git rebase' replays each commit from the working branch\n on top of the <upstream> branch using the given strategy, using\n@@ -369,7 +370,7 @@ See also INCOMPATIBLE OPTIONS below.\n --strategy-option=<strategy-option>::\n \tPass the <strategy-option> through to the merge strategy.\n \tThis implies `--merge` and, if no strategy has been\n-\tspecified, `-s recursive`.  Note the reversal of 'ours' and\n+\tspecified, `-s ort`.  Note the reversal of 'ours' and\n \t'theirs' as noted above for the `-m` option.\n +\n See also INCOMPATIBLE OPTIONS below.\n@@ -530,7 +531,7 @@ The `--rebase-merges` mode is similar in spirit to the deprecated\n where commits can be reordered, inserted and dropped at will.\n +\n It is currently only possible to recreate the merge commits using the\n-`recursive` merge strategy; Different merge strategies can be used only via\n+`ort` merge strategy; different merge strategies can be used only via\n explicit `exec git merge -s <strategy> [...]` commands.\n +\n See also REBASING MERGES and INCOMPATIBLE OPTIONS below.\n@@ -1219,7 +1220,7 @@ successful merge so that the user can edit the message.\n If a `merge` command fails for any reason other than merge conflicts (i.e.\n when the merge operation did not even start), it is rescheduled immediately.\n \n-At this time, the `merge` command will *always* use the `recursive`\n+At this time, the `merge` command will *always* use the `ort`\n merge strategy for regular merges, and `octopus` for octopus merges,\n with no way to choose a different one. To work around\n this, an `exec` command can be used to call `git merge` explicitly,\ndiff --git a/Documentation/gitfaq.txt b/Documentation/gitfaq.txt\nindex afdaeab8503..072bf84fa8a 100644\n--- a/Documentation/gitfaq.txt\n+++ b/Documentation/gitfaq.txt\n@@ -275,7 +275,7 @@ best to always use a regular merge commit.\n \n [[merge-two-revert-one]]\n If I make a change on two branches but revert it on one, why does the merge of those branches include the change?::\n-\tBy default, when Git does a merge, it uses a strategy called the recursive\n+\tBy default, when Git does a merge, it uses a strategy called the ort\n \tstrategy, which does a fancy three-way merge.  In such a case, when Git\n \tperforms the merge, it considers exactly three points: the two heads and a\n \tthird point, called the _merge base_, which is usually the common ancestor of\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex eb0aabd396f..72b53188504 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -112,8 +112,8 @@ With --squash, --commit is not allowed, and will fail.\n \tUse the given merge strategy; can be supplied more than\n \tonce to specify them in the order they should be tried.\n \tIf there is no `-s` option, a built-in list of strategies\n-\tis used instead ('git merge-recursive' when merging a single\n-\thead, 'git merge-octopus' otherwise).\n+\tis used instead (`ort` when merging a single head,\n+\t`octopus` otherwise).\n \n -X <option>::\n --strategy-option=<option>::\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 2912de706bf..6caf3fd6486 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -6,28 +6,23 @@ backend 'merge strategies' to be chosen with `-s` option.  Some strategies\n can also take their own options, which can be passed by giving `-X<option>`\n arguments to `git merge` and/or `git pull`.\n \n-resolve::\n-\tThis can only resolve two heads (i.e. the current branch\n-\tand another branch you pulled from) using a 3-way merge\n-\talgorithm.  It tries to carefully detect criss-cross\n-\tmerge ambiguities and is considered generally safe and\n-\tfast.\n-\n-recursive::\n-\tThis can only resolve two heads using a 3-way merge\n-\talgorithm.  When there is more than one common\n-\tancestor that can be used for 3-way merge, it creates a\n-\tmerged tree of the common ancestors and uses that as\n-\tthe reference tree for the 3-way merge.  This has been\n-\treported to result in fewer merge conflicts without\n-\tcausing mismerges by tests done on actual merge commits\n-\ttaken from Linux 2.6 kernel development history.\n-\tAdditionally this can detect and handle merges involving\n-\trenames, but currently cannot make use of detected\n-\tcopies.  This is the default merge strategy when pulling\n-\tor merging one branch.\n+ort::\n+\tThis is the default merge strategy when pulling or merging one\n+\tbranch.  This strategy can only resolve two heads using a\n+\t3-way merge algorithm.  When there is more than one common\n+\tancestor that can be used for 3-way merge, it creates a merged\n+\ttree of the common ancestors and uses that as the reference\n+\ttree for the 3-way merge.  This has been reported to result in\n+\tfewer merge conflicts without causing mismerges by tests done\n+\ton actual merge commits taken from Linux 2.6 kernel\n+\tdevelopment history.  Additionally this strategy can detect\n+\tand handle merges involving renames.  It does not make use of\n+\tdetected copies.  The name for this algorithm is an acronym\n+\t(\"Ostensibly Recursive's Twin\") and came from the fact that it\n+\twas written as a replacement for the previous default\n+\talgorithm, recursive.\n +\n-The 'recursive' strategy can take the following options:\n+The 'ort' strategy can take the following options:\n \n ours;;\n \tThis option forces conflicting hunks to be auto-resolved cleanly by\n@@ -43,19 +38,6 @@ theirs;;\n \tThis is the opposite of 'ours'; note that, unlike 'ours', there is\n \tno 'theirs' merge strategy to confuse this merge option with.\n \n-patience;;\n-\tWith this option, 'merge-recursive' spends a little extra time\n-\tto avoid mismerges that sometimes occur due to unimportant\n-\tmatching lines (e.g., braces from distinct functions).  Use\n-\tthis when the branches to be merged have diverged wildly.\n-\tSee also linkgit:git-diff[1] `--patience`.\n-\n-diff-algorithm=[patience|minimal|histogram|myers];;\n-\tTells 'merge-recursive' to use a different diff algorithm, which\n-\tcan help avoid mismerges that occur due to unimportant matching\n-\tlines (such as braces from distinct functions).  See also\n-\tlinkgit:git-diff[1] `--diff-algorithm`.\n-\n ignore-space-change;;\n ignore-all-space;;\n ignore-space-at-eol;;\n@@ -84,11 +66,6 @@ no-renormalize;;\n \tDisables the `renormalize` option.  This overrides the\n \t`merge.renormalize` configuration variable.\n \n-no-renames;;\n-\tTurn off rename detection. This overrides the `merge.renames`\n-\tconfiguration variable.\n-\tSee also linkgit:git-diff[1] `--no-renames`.\n-\n find-renames[=<n>];;\n \tTurn on rename detection, optionally setting the similarity\n \tthreshold.  This is the default. This overrides the\n@@ -105,6 +82,45 @@ subtree[=<path>];;\n \tis prefixed (or stripped from the beginning) to make the shape of\n \ttwo trees to match.\n \n+recursive::\n+\tThis can only resolve two heads using a 3-way merge\n+\talgorithm.  When there is more than one common\n+\tancestor that can be used for 3-way merge, it creates a\n+\tmerged tree of the common ancestors and uses that as\n+\tthe reference tree for the 3-way merge.  This has been\n+\treported to result in fewer merge conflicts without\n+\tcausing mismerges by tests done on actual merge commits\n+\ttaken from Linux 2.6 kernel development history.\n+\tAdditionally this can detect and handle merges involving\n+\trenames.  It does not make use of detected copies.\n++\n+The 'recursive' strategy takes the same options as 'ort'.  However,\n+there are three additional options that 'ort' ignores (not documented\n+above) that are potentially useful with the 'recursive' strategy:\n+\n+patience;;\n+\tDeprecated shorthand for diff-algorithm=patience.\n+\n+diff-algorithm=[patience|minimal|histogram|myers];;\n+\tUse a different diff algorithm while merging, which can help\n+\tavoid mismerges that occur due to unimportant matching lines\n+\t(such as braces from distinct functions).  See also\n+\tlinkgit:git-diff[1] `--diff-algorithm`.  Note that `ort`\n+\tspecifically uses diff-algorithm=histogram, while `recursive`\n+\tdefaults to the `diff.algorithm` config setting.\n+\n+no-renames;;\n+\tTurn off rename detection, which can be computationally much\n+\tmore expensive for the `recursive` strategy than for `ort`.\n+\tThis overrides the `merge.renames` configuration variable.  See\n+\talso linkgit:git-diff[1] `--no-renames`.\n+\n+resolve::\n+\tThis can only resolve two heads (i.e. the current branch\n+\tand another branch you pulled from) using a 3-way merge\n+\talgorithm.  It tries to carefully detect criss-cross\n+\tmerge ambiguities.  It cannot handle renames.\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\n@@ -121,13 +137,13 @@ ours::\n \tthe 'recursive' merge strategy.\n \n subtree::\n-\tThis is a modified recursive strategy. When merging trees A and\n+\tThis is a modified ort strategy. When merging trees A and\n \tB, if B corresponds to a subtree of A, B is first adjusted to\n \tmatch the tree structure of A, instead of reading the trees at\n \tthe same level. This adjustment is also done to the common\n \tancestor tree.\n \n-With the strategies that use 3-way merge (including the default, 'recursive'),\n+With the strategies that use 3-way merge (including the default, 'ort'),\n if a change is made on both branches, but later reverted on one of the\n branches, that change will be present in the merged result; some people find\n this behavior confusing.  It occurs because only the heads and the merge base\ndiff --git a/Documentation/technical/directory-rename-detection.txt b/Documentation/technical/directory-rename-detection.txt\nindex 49b83ef3cc4..029ee2cedc5 100644\n--- a/Documentation/technical/directory-rename-detection.txt\n+++ b/Documentation/technical/directory-rename-detection.txt\n@@ -2,9 +2,9 @@ Directory rename detection\n ==========================\n \n Rename detection logic in diffcore-rename that checks for renames of\n-individual files is aggregated and analyzed in merge-recursive for cases\n-where combinations of renames indicate that a full directory has been\n-renamed.\n+individual files is also aggregated there and then analyzed in either\n+merge-ort or merge-recursive for cases where combinations of renames\n+indicate that a full directory has been renamed.\n \n Scope of abilities\n ------------------\n@@ -88,9 +88,11 @@ directory rename detection support in:\n     Folks have requested in the past that `git diff` detect directory\n     renames and somehow simplify its output.  It is not clear whether this\n     would be desirable or how the output should be simplified, so this was\n-    simply not implemented.  Further, to implement this, directory rename\n-    detection logic would need to move from merge-recursive to\n-    diffcore-rename.\n+    simply not implemented.  Also, while diffcore-rename has most of the\n+    logic for detecting directory renames, some of the logic is still found\n+    within merge-ort and merge-recursive.  Fully supporting directory\n+    rename detection in diffs would require copying or moving the remaining\n+    bits of logic to the diff machinery.\n \n   * am\n \ndiff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt\nindex 96240598e3f..865074bed4e 100644\n--- a/Documentation/user-manual.txt\n+++ b/Documentation/user-manual.txt\n@@ -3190,7 +3190,7 @@ that *updated* thing--the old state that you added originally ends up\n not being pointed to by any commit or tree, so it's now a dangling blob\n object.\n \n-Similarly, when the \"recursive\" merge strategy runs, and finds that\n+Similarly, when the \"ort\" merge strategy runs, and finds that\n there are criss-cross merges and thus more than one merge base (which is\n fairly unusual, but it does happen), it will generate one temporary\n midway tree (or possibly even more, if you had lots of criss-crossing\n-- \ngitgitgadget\n"},{"id":"431695","messageId":"4f223333-7cbb-2304-2e81-dc91a3281241@gmail.com","threadId":"56190","inReplyTo":"35490397590ae4d39ba98c7eb206bfadc22ddf35.1627776462.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] Update docs for change of default merge backend","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-02T14:59:12Z","receivedAt":"2021-08-02T14:59:17Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/31/2021 8:07 PM, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren <newren@gmail.com>\n...\n> diff --git a/Documentation/gitfaq.txt b/Documentation/gitfaq.txt\n> index afdaeab8503..072bf84fa8a 100644\n> --- a/Documentation/gitfaq.txt\n> +++ b/Documentation/gitfaq.txt\n> @@ -275,7 +275,7 @@ best to always use a regular merge commit.\n>  \n>  [[merge-two-revert-one]]\n>  If I make a change on two branches but revert it on one, why does the merge of those branches include the change?::\n> -\tBy default, when Git does a merge, it uses a strategy called the recursive\n> +\tBy default, when Git does a merge, it uses a strategy called the ort\n>  \tstrategy, which does a fancy three-way merge.  In such a case, when Git\n\nnit: I feel like quotes around \"ort\" would be beneficial here. It would have\nalso helped the previous version, too, in my opinion.\n\n>  \tperforms the merge, it considers exactly three points: the two heads and a\n>  \tthird point, called the _merge base_, which is usually the common ancestor of\n> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\n> index eb0aabd396f..72b53188504 100644\n> --- a/Documentation/merge-options.txt\n> +++ b/Documentation/merge-options.txt\n> @@ -112,8 +112,8 @@ With --squash, --commit is not allowed, and will fail.\n>  \tUse the given merge strategy; can be supplied more than\n>  \tonce to specify them in the order they should be tried.\n>  \tIf there is no `-s` option, a built-in list of strategies\n> -\tis used instead ('git merge-recursive' when merging a single\n> -\thead, 'git merge-octopus' otherwise).\n> +\tis used instead (`ort` when merging a single head,\n> +\t`octopus` otherwise).\n>  \n>  -X <option>::\n>  --strategy-option=<option>::\n> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\n> index 2912de706bf..6caf3fd6486 100644\n> --- a/Documentation/merge-strategies.txt\n> +++ b/Documentation/merge-strategies.txt\n> @@ -6,28 +6,23 @@ backend 'merge strategies' to be chosen with `-s` option.  Some strategies\n>  can also take their own options, which can be passed by giving `-X<option>`\n>  arguments to `git merge` and/or `git pull`.\n>  \n> -resolve::\n> -\tThis can only resolve two heads (i.e. the current branch\n> -\tand another branch you pulled from) using a 3-way merge\n> -\talgorithm.  It tries to carefully detect criss-cross\n> -\tmerge ambiguities and is considered generally safe and\n> -\tfast.\n> -\n> -recursive::\n> -\tThis can only resolve two heads using a 3-way merge\n> -\talgorithm.  When there is more than one common\n> -\tancestor that can be used for 3-way merge, it creates a\n> -\tmerged tree of the common ancestors and uses that as\n> -\tthe reference tree for the 3-way merge.  This has been\n> -\treported to result in fewer merge conflicts without\n> -\tcausing mismerges by tests done on actual merge commits\n> -\ttaken from Linux 2.6 kernel development history.\n> -\tAdditionally this can detect and handle merges involving\n> -\trenames, but currently cannot make use of detected\n> -\tcopies.  This is the default merge strategy when pulling\n> -\tor merging one branch.\n> +ort::\n> +\tThis is the default merge strategy when pulling or merging one\n> +\tbranch.  This strategy can only resolve two heads using a\n> +\t3-way merge algorithm.  When there is more than one common\n> +\tancestor that can be used for 3-way merge, it creates a merged\n> +\ttree of the common ancestors and uses that as the reference\n> +\ttree for the 3-way merge.  This has been reported to result in\n> +\tfewer merge conflicts without causing mismerges by tests done\n> +\ton actual merge commits taken from Linux 2.6 kernel\n> +\tdevelopment history.  Additionally this strategy can detect\n> +\tand handle merges involving renames.  It does not make use of\n> +\tdetected copies.  The name for this algorithm is an acronym\n> +\t(\"Ostensibly Recursive's Twin\") and came from the fact that it\n> +\twas written as a replacement for the previous default\n> +\talgorithm, recursive.\n\nnit: Quotes around \"recursive\" might be useful here, too.\n\n>  +\n> -The 'recursive' strategy can take the following options:\n> +The 'ort' strategy can take the following options:\n\n(Like these quotes.)\n\nOther than my nits, these doc updates are solid.\n\nThanks,\n-Stolee\n"},{"id":"431696","messageId":"ec1bb449-347a-924d-7462-3cef6facc0e9@gmail.com","threadId":"56190","inReplyTo":"pull.1055.git.git.1627776461.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-02T15:05:44Z","receivedAt":"2021-08-02T15:05:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/31/2021 8:07 PM, Elijah Newren via GitGitGadget wrote:\n> This is an RFC series designed to spur feedback about switching the default\n> merge backend (reviewing the patches is of secondary importance at this\n> point). Some questions:\n> \n>  * Are there things others want before this series is considered for\n>    inclusion?\n>  * What kind of timeline do others think is reasonable?\n>  * Would it be beneficial to let this series sit in 'next' for an extended\n>    duration to gain more feedback?\n> \n> Some potentially useful context in relation to the above:\n> \n>  * I've personally used the ort backend for well over a year\n>  * I have ~50 testers using ort as the default merge backend since Nov.\n>    2020.\n>  * ort fixes known bugs in recursive, and there are no known regressions\n>    (more testers may change that)\n>  * ort is significantly faster than recursive\n>  * ort provides one new feature already, and enables more that are on the\n>    way\n>  * The commit message of patch 1 has more details about the last three items\n>    above\n> \n> So...thoughts?\n\nI fully endorse this change as soon as possible. I've applied the patches\nyou supplied here and submitted a PR to microsoft/git [1] to take them.\n\n[1] https://github.com/microsoft/git/pull/404\n\nI've done my own share of testing on some of our private monorepos to see\nhow ORT compares to the recursive algorithm. My data is not perhaps as\nrigorous as yours, but I did notice that the ORT algorithm was consistently\nwithin the 5-6 second range while the recursive algorithm would vary within\nthe 7-20 second range (and some outliers near 30s).\n\nOf course, I'm particularly excited about the benefits to the sparse index\nwork. I also have a prototype of a 'git merge' integration with sparse\nindex which was not very hard because the ORT strategy does not use the\nindex as a data structure. With that change, my tests dropped to between\n0.5s and 1.5 seconds. (This shows just how much my earlier timings were\nstuck on index reads and writes.) Now, the largest indicator of time is\nhow long it takes to resolve text conflicts.\n\nThanks! What a monumental effort.\n\n-Stolee\n"},{"id":"431701","messageId":"87h7g7on6q.fsf@evledraar.gmail.com","threadId":"56190","inReplyTo":"8f6af8d494e0924aef4ae6963b8dca2228dad9b1.1627776462.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] Change default merge backend from recursive to ort","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-08-02T15:55:18Z","receivedAt":"2021-08-02T15:56:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Aug 01 2021, Elijah Newren via GitGitGadget wrote:\n\n> From: Elijah Newren <newren@gmail.com>\n> [...]\n> @@ -3968,7 +3968,7 @@ static int do_merge(struct repository *r,\n>  \to.branch2 = ref_name.buf;\n>  \to.buffer_output = 2;\n>  \n> -\tif (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n> +\tif (!opts->strategy || strcmp(opts->strategy, \"recursive\")) {\n>  \t\t/*\n>  \t\t * TODO: Should use merge_incore_recursive() and\n>  \t\t * merge_switch_to_result(), skipping the call to\n\nI might spot more tiny issues, but it looks like our error messaging\nneeds updating for 14c4586c2df (merge,rebase,revert: select ort or\nrecursive by config or environment, 2020-11-02).\n\nI.e. we die on \"Unknown option for merge-recursive\", presumably that\nshould be updated to indicate that we might call one of\nmerge_recursive() or merge_ort_recursive() now.\n\nAnd perhaps this in sequencer.c:\n\n    that represents the \"current\" state for merge-recursive[...]\n"},{"id":"431708","messageId":"CABPp-BHNYPzJc=oksWfkUGqsrmK_nhr_24WcaqjZoK_+4wG=tA@mail.gmail.com","threadId":"56190","inReplyTo":"ec1bb449-347a-924d-7462-3cef6facc0e9@gmail.com","subject":"Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-08-02T16:27:31Z","receivedAt":"2021-08-02T16:27:44Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Aug 2, 2021 at 9:05 AM Derrick Stolee <stolee@gmail.com> wrote:\n>\n> On 7/31/2021 8:07 PM, Elijah Newren via GitGitGadget wrote:\n> > This is an RFC series designed to spur feedback about switching the default\n> > merge backend (reviewing the patches is of secondary importance at this\n> > point). Some questions:\n> >\n> >  * Are there things others want before this series is considered for\n> >    inclusion?\n> >  * What kind of timeline do others think is reasonable?\n> >  * Would it be beneficial to let this series sit in 'next' for an extended\n> >    duration to gain more feedback?\n> >\n> > Some potentially useful context in relation to the above:\n> >\n> >  * I've personally used the ort backend for well over a year\n> >  * I have ~50 testers using ort as the default merge backend since Nov.\n> >    2020.\n> >  * ort fixes known bugs in recursive, and there are no known regressions\n> >    (more testers may change that)\n> >  * ort is significantly faster than recursive\n> >  * ort provides one new feature already, and enables more that are on the\n> >    way\n> >  * The commit message of patch 1 has more details about the last three items\n> >    above\n> >\n> > So...thoughts?\n>\n> I fully endorse this change as soon as possible. I've applied the patches\n> you supplied here and submitted a PR to microsoft/git [1] to take them.\n>\n> [1] https://github.com/microsoft/git/pull/404\n>\n> I've done my own share of testing on some of our private monorepos to see\n> how ORT compares to the recursive algorithm. My data is not perhaps as\n> rigorous as yours, but I did notice that the ORT algorithm was consistently\n> within the 5-6 second range while the recursive algorithm would vary within\n> the 7-20 second range (and some outliers near 30s).\n>\n> Of course, I'm particularly excited about the benefits to the sparse index\n> work. I also have a prototype of a 'git merge' integration with sparse\n> index which was not very hard because the ORT strategy does not use the\n> index as a data structure. With that change, my tests dropped to between\n> 0.5s and 1.5 seconds. (This shows just how much my earlier timings were\n> stuck on index reads and writes.) Now, the largest indicator of time is\n> how long it takes to resolve text conflicts.\n>\n> Thanks! What a monumental effort.\n\nIt's certainly been a long haul.  And you jumped after all my\npreparatory patches when I started submitting actual merge-ort\npatches.  Since that time about a year ago, you have reviewed nearly\nevery single one of these patches, and there have been well over a\nhundred of them and they've been far from trivial.  The code is better\ntoday because of your comments and suggestions.  Thanks for the\nmonumental review effort.\n\nOne quick question on your timings here, though: in the past, when you\ncompared timings, you compared merge-recursive *without* rename\ndetection to merge-ort *with* rename detection.  Are you doing the\nsame here (just so others have a better sense of what is being\ncompared)?\n\nThanks again!\nElijah\n"},{"id":"431711","messageId":"CABPp-BFD4wMyaWwcfjWAUQSV2fTwCKRviengSRsf=LV5u39anw@mail.gmail.com","threadId":"56190","inReplyTo":"87h7g7on6q.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/2] Change default merge backend from recursive to ort","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-08-02T16:33:10Z","receivedAt":"2021-08-02T16:33:25Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Aug 2, 2021 at 9:56 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>\n> On Sun, Aug 01 2021, Elijah Newren via GitGitGadget wrote:\n>\n> > From: Elijah Newren <newren@gmail.com>\n> > [...]\n> > @@ -3968,7 +3968,7 @@ static int do_merge(struct repository *r,\n> >       o.branch2 = ref_name.buf;\n> >       o.buffer_output = 2;\n> >\n> > -     if (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n> > +     if (!opts->strategy || strcmp(opts->strategy, \"recursive\")) {\n> >               /*\n> >                * TODO: Should use merge_incore_recursive() and\n> >                * merge_switch_to_result(), skipping the call to\n>\n> I might spot more tiny issues, but it looks like our error messaging\n> needs updating for 14c4586c2df (merge,rebase,revert: select ort or\n> recursive by config or environment, 2020-11-02).\n>\n> I.e. we die on \"Unknown option for merge-recursive\", presumably that\n> should be updated to indicate that we might call one of\n> merge_recursive() or merge_ort_recursive() now.\n\nOoh, good catch.  I think I'd prefer to reword this to \"Unknown\nstrategy option: -X%s\"\n\n> And perhaps this in sequencer.c:\n>\n>     that represents the \"current\" state for merge-recursive[...]\n\nYeah, it's just a comment but it should still be updated.  I'll\ns/merge-recursive/the merge machinery/ for this one.\n\nI tried to look for other error messages or comments similar to these\ntwo but didn't find anything.  I might have missed something, though.\n\nI'll get these fixed up with the next submission.\n"},{"id":"431738","messageId":"c27894f0-b0d2-a1a6-bac3-31e82b0283df@gmail.com","threadId":"56190","inReplyTo":"CABPp-BHNYPzJc=oksWfkUGqsrmK_nhr_24WcaqjZoK_+4wG=tA@mail.gmail.com","subject":"Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-02T18:03:16Z","receivedAt":"2021-08-02T18:03:23Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/2/2021 12:27 PM, Elijah Newren wrote:\n> On Mon, Aug 2, 2021 at 9:05 AM Derrick Stolee <stolee@gmail.com> wrote:\n>> I've done my own share of testing on some of our private monorepos to see\n>> how ORT compares to the recursive algorithm. My data is not perhaps as\n>> rigorous as yours, but I did notice that the ORT algorithm was consistently\n>> within the 5-6 second range while the recursive algorithm would vary within\n>> the 7-20 second range (and some outliers near 30s).\n...\n> One quick question on your timings here, though: in the past, when you\n> compared timings, you compared merge-recursive *without* rename\n> detection to merge-ort *with* rename detection.  Are you doing the\n> same here (just so others have a better sense of what is being\n> compared)?\n\nYes, I continue to test with merge.renames=0 in the config. That makes\nthe data more impressive.\n\nThanks,\n-Stolee\n"},{"id":"431778","messageId":"nycvar.QRO.7.76.6.2108030030000.55@tvgsbejvaqbjf.bet","threadId":"56190","inReplyTo":"8f6af8d494e0924aef4ae6963b8dca2228dad9b1.1627776462.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] Change default merge backend from recursive to ort","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-02T22:46:02Z","receivedAt":"2021-08-02T22:46:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Sun, 1 Aug 2021, Elijah Newren via GitGitGadget wrote:\n\n> From: Elijah Newren <newren@gmail.com>\n>\n> There are a few reasons to switch the default:\n> [...]\n\nI think it would be really fantastic to change to the new default right\nafter v2.33.0.\n\nAs to the patch, I only struggled slightly with the changes to\n`sequencer.c`:\n\n> diff --git a/sequencer.c b/sequencer.c\n> index 0bec01cf38e..a98de9a8d15 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -636,7 +636,7 @@ static int do_recursive_merge(struct repository *r,\n>  \tfor (i = 0; i < opts->xopts_nr; i++)\n>  \t\tparse_merge_opt(&o, opts->xopts[i]);\n>\n> -\tif (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n> +\tif (!opts->strategy || strcmp(opts->strategy, \"recursive\")) {\n\nAt this stage, we're in `do_recursive_merge()`, and there is only one\ncaller, `do_pick_commit()`, and the caller is guarded by the following\ncondition:\n\n        else if (!opts->strategy ||\n                 !strcmp(opts->strategy, \"recursive\") ||\n                 !strcmp(opts->strategy, \"ort\") ||\n                 command == TODO_REVERT) {\n\nThe issue I see is with `git revert` allowing custom merge strategies. I\n_think_ we need a slightly different patch here, something like this:\n\n-\tif (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n+\tif (!opts->strategy || !strcmp(opts->strategy, \"ort\")) {\n\n>  \t\tmemset(&result, 0, sizeof(result));\n>  \t\tmerge_incore_nonrecursive(&o, base_tree, head_tree, next_tree,\n>  \t\t\t\t\t    &result);\n> @@ -3968,7 +3968,7 @@ static int do_merge(struct repository *r,\n>  \to.branch2 = ref_name.buf;\n>  \to.buffer_output = 2;\n>\n> -\tif (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n> +\tif (!opts->strategy || strcmp(opts->strategy, \"recursive\")) {\n\nIt took me a while to convince myself that this is correct. At least now I\n_think_ it is correct: `do_merge()` defines:\n\n        const char *strategy = !opts->xopts_nr &&\n                (!opts->strategy ||\n                 !strcmp(opts->strategy, \"recursive\") ||\n                 !strcmp(opts->strategy, \"ort\")) ?\n                NULL : opts->strategy;\n\nand then hands off to `git merge -s <strategy>` if `strategy` is set,\n_before_ this hunk. Therefore we can be pretty certain that\n`opts->strategy` is either not set, or \"ort\", or \"recursive\" at that\nstage.\n\nHowever, I think we could use the same idea I outlined in the previous\nhunk, to make things more obvious:\n\n-\tif (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n+\tif (!opts->strategy || !strcmp(opts->strategy, \"ort\")) {\n\nThank you,\nDscho\n\n>  \t\t/*\n>  \t\t * TODO: Should use merge_incore_recursive() and\n>  \t\t * merge_switch_to_result(), skipping the call to\n> --\n> gitgitgadget\n>\n>\n"},{"id":"431789","messageId":"CABPp-BHCy-CpiJQ7LVmrRXgZNj3sYAxu0WwJ2JLzvY-qrwDiaQ@mail.gmail.com","threadId":"56190","inReplyTo":"nycvar.QRO.7.76.6.2108030030000.55@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/2] Change default merge backend from recursive to ort","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-08-03T01:04:54Z","receivedAt":"2021-08-03T01:05:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Aug 2, 2021 at 4:46 PM Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>\n> Hi Elijah,\n>\n> On Sun, 1 Aug 2021, Elijah Newren via GitGitGadget wrote:\n>\n> > From: Elijah Newren <newren@gmail.com>\n> >\n> > There are a few reasons to switch the default:\n> > [...]\n>\n> I think it would be really fantastic to change to the new default right\n> after v2.33.0.\n>\n> As to the patch, I only struggled slightly with the changes to\n> `sequencer.c`:\n>\n> > diff --git a/sequencer.c b/sequencer.c\n> > index 0bec01cf38e..a98de9a8d15 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -636,7 +636,7 @@ static int do_recursive_merge(struct repository *r,\n> >       for (i = 0; i < opts->xopts_nr; i++)\n> >               parse_merge_opt(&o, opts->xopts[i]);\n> >\n> > -     if (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n> > +     if (!opts->strategy || strcmp(opts->strategy, \"recursive\")) {\n>\n> At this stage, we're in `do_recursive_merge()`, and there is only one\n> caller, `do_pick_commit()`, and the caller is guarded by the following\n> condition:\n>\n>         else if (!opts->strategy ||\n>                  !strcmp(opts->strategy, \"recursive\") ||\n>                  !strcmp(opts->strategy, \"ort\") ||\n>                  command == TODO_REVERT) {\n>\n> The issue I see is with `git revert` allowing custom merge strategies. I\n> _think_ we need a slightly different patch here, something like this:\n>\n> -       if (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n> +       if (!opts->strategy || !strcmp(opts->strategy, \"ort\")) {\n>\n> >               memset(&result, 0, sizeof(result));\n> >               merge_incore_nonrecursive(&o, base_tree, head_tree, next_tree,\n> >                                           &result);\n> > @@ -3968,7 +3968,7 @@ static int do_merge(struct repository *r,\n> >       o.branch2 = ref_name.buf;\n> >       o.buffer_output = 2;\n> >\n> > -     if (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n> > +     if (!opts->strategy || strcmp(opts->strategy, \"recursive\")) {\n>\n> It took me a while to convince myself that this is correct. At least now I\n> _think_ it is correct: `do_merge()` defines:\n>\n>         const char *strategy = !opts->xopts_nr &&\n>                 (!opts->strategy ||\n>                  !strcmp(opts->strategy, \"recursive\") ||\n>                  !strcmp(opts->strategy, \"ort\")) ?\n>                 NULL : opts->strategy;\n>\n> and then hands off to `git merge -s <strategy>` if `strategy` is set,\n> _before_ this hunk. Therefore we can be pretty certain that\n> `opts->strategy` is either not set, or \"ort\", or \"recursive\" at that\n> stage.\n>\n> However, I think we could use the same idea I outlined in the previous\n> hunk, to make things more obvious:\n>\n> -       if (opts->strategy && !strcmp(opts->strategy, \"ort\")) {\n> +       if (!opts->strategy || !strcmp(opts->strategy, \"ort\")) {\n>\n> Thank you,\n> Dscho\n>\n> >               /*\n> >                * TODO: Should use merge_incore_recursive() and\n> >                * merge_switch_to_result(), skipping the call to\n> > --\n> > gitgitgadget\n\nI'll include both suggestions in my next re-roll.  Thanks for the feedback!\n"},{"id":"431796","messageId":"49b61447-06b7-9c43-fdd6-25adc45f2a88@gmail.com","threadId":"56190","inReplyTo":"8f6af8d494e0924aef4ae6963b8dca2228dad9b1.1627776462.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] Change default merge backend from recursive to ort","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-08-03T02:56:02Z","receivedAt":"2021-08-03T02:57:10Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Elijah,\n\nLe 2021-07-31 à 20:07, Elijah Newren via GitGitGadget a écrit :\n> From: Elijah Newren <newren@gmail.com>\n> \n> \n>    * `git diff AUTO_MERGE` -- ability to see what changes the user has\n>      made to resolve conflicts so far (see commit 5291828df8 (\"merge-ort:\n>      write $GIT_DIR/AUTO_MERGE whenever we hit a conflict\", 2021-03-20)\n> \n>\n> The last three have been implemented already (though only one has been\n> submitted upstream so far; \n\n From what I could find this indeed only refers to your 5291828df8 (merge-ort:\nwrite $GIT_DIR/AUTO_MERGE whenever we hit a conflict, 2021-03-20).\nThis is a very nice improvement, but I noticed it is not mentioned in the doc.\nDo you plan to update the 'git diff' doc to mention that special ref ?\n(And maybe also gitrevisions(5), where most of the special refs are listed ?)\n\nDo you plan to implement a new '--auto-merge' option to 'git diff' as a shortcut\nto 'git diff AUTO_MERGE', in order to hide a bit the special ref from users ?\n\nThanks a lot for your work,\n\nPhilippe.\n"},{"id":"431799","messageId":"CABPp-BGJ_ZfOPRdsjxnT8q9r9YPZUsnSmPoT0xuwJRJ3TN4wWA@mail.gmail.com","threadId":"56190","inReplyTo":"49b61447-06b7-9c43-fdd6-25adc45f2a88@gmail.com","subject":"Re: [PATCH 1/2] Change default merge backend from recursive to ort","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-08-03T03:39:51Z","receivedAt":"2021-08-03T03:40:06Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Philippe,\n\nOn Mon, Aug 2, 2021 at 8:56 PM Philippe Blain\n<levraiphilippeblain@gmail.com> wrote:\n>\n> Hi Elijah,\n>\n> Le 2021-07-31 à 20:07, Elijah Newren via GitGitGadget a écrit :\n> > From: Elijah Newren <newren@gmail.com>\n> >\n> >\n> >    * `git diff AUTO_MERGE` -- ability to see what changes the user has\n> >      made to resolve conflicts so far (see commit 5291828df8 (\"merge-ort:\n> >      write $GIT_DIR/AUTO_MERGE whenever we hit a conflict\", 2021-03-20)\n> >\n> >\n> > The last three have been implemented already (though only one has been\n> > submitted upstream so far;\n>\n>  From what I could find this indeed only refers to your 5291828df8 (merge-ort:\n> write $GIT_DIR/AUTO_MERGE whenever we hit a conflict, 2021-03-20).\n> This is a very nice improvement, but I noticed it is not mentioned in the doc.\n> Do you plan to update the 'git diff' doc to mention that special ref ?\n> (And maybe also gitrevisions(5), where most of the special refs are listed ?)\n>\n> Do you plan to implement a new '--auto-merge' option to 'git diff' as a shortcut\n> to 'git diff AUTO_MERGE', in order to hide a bit the special ref from users ?\n\nFair point, it probably does deserve to be documented somewhere, at\nleast once ort is the default merge algorithm.\n\nI don't think it'd make sense to include a reference to it in\ngitrevisions(5), since $GIT_DIR/AUTO_MERGE is a reference to a tree\nrather than to a revision.  But documenting that special ref in\nDocumentation/git-diff.txt, and perhaps linking to it from other\nconflict-related options (e.g. --base, --ours, --theirs) may make\nsense.  Your --auto-merge idea may also make sense, and it'd be\nsomewhat similar to how git-rebase has a --show-current-patch option\nthat is shorthand for `git show REBASE_HEAD` and is documented as\nsuch.  However, it might be confusing to users how to combine\n--auto-merge with paths, whereas `git diff AUTO_MERGE -- pathname` is\npretty clear once you know that AUTO_MERGE is a tree you can diff\nagainst.  Hmmm....\n"},{"id":"431822","messageId":"CABPp-BGmnOUy3d2Ey5Pxr+3x-icBDkd7ZHWG52OymJKGuOw7ZQ@mail.gmail.com","threadId":"56190","inReplyTo":"35490397590ae4d39ba98c7eb206bfadc22ddf35.1627776462.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] Update docs for change of default merge backend","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-08-03T14:39:10Z","receivedAt":"2021-08-03T14:39:23Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sat, Jul 31, 2021 at 6:07 PM Elijah Newren via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> From: Elijah Newren <newren@gmail.com>\n>\n> Make multiple documentation updates to bring things up to date as we\n> change the default merge backend...\n\nSome of these changes are specific to the change in default backend,\nbut many of them are probably useful updates prior to such a switch.\nI'm going to break those out into several small patches and submit\nthem separately so they can be included in Git v2.33.0 if wanted.\nI'll then take the remaining changes and resubmit them after 2.33.0\nalong with the switch of default.\n"},{"id":"431836","messageId":"YQlnNXgk3qcGb5Bk@coredump.intra.peff.net","threadId":"56190","inReplyTo":"pull.1055.git.git.1627776461.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-03T15:56:37Z","receivedAt":"2021-08-03T15:56:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 01, 2021 at 12:07:39AM +0000, Elijah Newren via GitGitGadget wrote:\n\n> This is an RFC series designed to spur feedback about switching the default\n> merge backend (reviewing the patches is of secondary importance at this\n> point). Some questions:\n> \n>  * Are there things others want before this series is considered for\n>    inclusion?\n>  * What kind of timeline do others think is reasonable?\n>  * Would it be beneficial to let this series sit in 'next' for an extended\n>    duration to gain more feedback?\n\nIt looks like others gave some more specific review on the patches, but\non the meta-topic of \"do we switch, and when\", my response is: yes, and\nsoon. :)\n\nHaving watched the development of merge-ort, plus all of the weird\ncorner cases in merge-recursive we've seen over the years (many of which\nyou found and added tests for while working on merge-ort!), my gut\nfeeling is that the switch is _much_ more likely to fix problems people\nmight see in the wild rather than cause them.\n\nIt would make sense to me to do the switch in 'next' early in the\npost-v2.33 cycle. It can cook there for a bit, but I think we have found\nthat it's much more likely to see actual use once it hits 'master'. So I\ndon't see a particular reason to have it sit in 'next' for a long time.\nWe should get as much exposure in 'master' during the v2.34 cycle as\npossible.\n\nThe nice thing is that the two strategies can co-exist. So if it does\nturn out to have any regressions, it's an easy revert to switch back,\nand even post-release users can switch at runtime. We have pull.twohead,\nbut I don't think we have an equivalent that would impact a bare \"git\nmerge\" or \"git rebase -m\". Maybe it would be worth adding those as an\nescape hatch?\n\n-Peff\n"},{"id":"431837","messageId":"CABPp-BGgcpMbFrEjaDxpfb8=Wr_5O76qCDq1OSbpemrnYDcsqw@mail.gmail.com","threadId":"56190","inReplyTo":"YQlnNXgk3qcGb5Bk@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-08-03T16:57:12Z","receivedAt":"2021-08-03T17:01:37Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Aug 3, 2021 at 9:56 AM Jeff King <peff@peff.net> wrote:\n>\n> On Sun, Aug 01, 2021 at 12:07:39AM +0000, Elijah Newren via GitGitGadget wrote:\n>\n> > This is an RFC series designed to spur feedback about switching the default\n> > merge backend (reviewing the patches is of secondary importance at this\n> > point). Some questions:\n> >\n> >  * Are there things others want before this series is considered for\n> >    inclusion?\n> >  * What kind of timeline do others think is reasonable?\n> >  * Would it be beneficial to let this series sit in 'next' for an extended\n> >    duration to gain more feedback?\n>\n> It looks like others gave some more specific review on the patches, but\n> on the meta-topic of \"do we switch, and when\", my response is: yes, and\n> soon. :)\n>\n> Having watched the development of merge-ort, plus all of the weird\n> corner cases in merge-recursive we've seen over the years (many of which\n> you found and added tests for while working on merge-ort!), my gut\n> feeling is that the switch is _much_ more likely to fix problems people\n> might see in the wild rather than cause them.\n>\n> It would make sense to me to do the switch in 'next' early in the\n> post-v2.33 cycle. It can cook there for a bit, but I think we have found\n> that it's much more likely to see actual use once it hits 'master'. So I\n> don't see a particular reason to have it sit in 'next' for a long time.\n> We should get as much exposure in 'master' during the v2.34 cycle as\n> possible.\n\n> The nice thing is that the two strategies can co-exist. So if it does\n> turn out to have any regressions, it's an easy revert to switch back,\n> and even post-release users can switch at runtime. We have pull.twohead,\n> but I don't think we have an equivalent that would impact a bare \"git\n> merge\" or \"git rebase -m\". Maybe it would be worth adding those as an\n> escape hatch?\n\nActually, pull.twohead is not pull specific; it already affects merge,\nrebase (-m is the default for rebase, btw), cherry-pick, and revert.\npull.twohead has affected a bare \"git merge\" since 1c7b76be7d (\"Build\nin merge\", 2008-07-07).  I thought it was weird that \"merge strategy\"\nfor the merge command was specified via a config option under \"pull\",\nand included my misgivings about it in the commit message of\n14c4586c2d (\"merge,rebase,revert: select ort or recursive by config or\nenvironment\", 2020-11-02) when I made sequencer.c pay attention to\nthat config option as well:\n\n\"\"\"\n    Also, allow folks to pick the new algorithm via config setting.  It\n    turns out builtin/merge.c already had a way to allow users to specify a\n    different default merge algorithm: pull.twohead.  Rather odd\n    configuration name (especially to be in the 'pull' namespace rather than\n    'merge') but it's there.  Add that same configuration to rebase,\n    cherry-pick, and revert.\n\"\"\"\n\nBut no one had an alternate suggestion or opinion on attempting to\nmigrate the configuration to a different name, so it has just stuck.\nAnyway, if folks want to try out 'ort' with the 2.32 or 2.33 releases,\nthey can set pull.twohead=ort.  Once we switch the default, they can\nset pull.twohead=recursive to get the old default.\n"},{"id":"431839","messageId":"YQl5SlmJsFbQ3Kw+@coredump.intra.peff.net","threadId":"56190","inReplyTo":"CABPp-BGgcpMbFrEjaDxpfb8=Wr_5O76qCDq1OSbpemrnYDcsqw@mail.gmail.com","subject":"Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-03T17:13:46Z","receivedAt":"2021-08-03T17:14:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 03, 2021 at 10:57:12AM -0600, Elijah Newren wrote:\n\n> > The nice thing is that the two strategies can co-exist. So if it does\n> > turn out to have any regressions, it's an easy revert to switch back,\n> > and even post-release users can switch at runtime. We have pull.twohead,\n> > but I don't think we have an equivalent that would impact a bare \"git\n> > merge\" or \"git rebase -m\". Maybe it would be worth adding those as an\n> > escape hatch?\n> \n> Actually, pull.twohead is not pull specific; it already affects merge,\n> rebase (-m is the default for rebase, btw), cherry-pick, and revert.\n> pull.twohead has affected a bare \"git merge\" since 1c7b76be7d (\"Build\n> in merge\", 2008-07-07).  I thought it was weird that \"merge strategy\"\n> for the merge command was specified via a config option under \"pull\",\n> and included my misgivings about it in the commit message of\n> 14c4586c2d (\"merge,rebase,revert: select ort or recursive by config or\n> environment\", 2020-11-02) when I made sequencer.c pay attention to\n> that config option as well:\n> \n> \"\"\"\n>     Also, allow folks to pick the new algorithm via config setting.  It\n>     turns out builtin/merge.c already had a way to allow users to specify a\n>     different default merge algorithm: pull.twohead.  Rather odd\n>     configuration name (especially to be in the 'pull' namespace rather than\n>     'merge') but it's there.  Add that same configuration to rebase,\n>     cherry-pick, and revert.\n> \"\"\"\n>\n> But no one had an alternate suggestion or opinion on attempting to\n> migrate the configuration to a different name, so it has just stuck.\n> Anyway, if folks want to try out 'ort' with the 2.32 or 2.33 releases,\n> they can set pull.twohead=ort.  Once we switch the default, they can\n> set pull.twohead=recursive to get the old default.\n\nAh, thanks for clarifying. I think we are in good shape, then. We could\npossibly introduce merge.twohead as a synonym, but given that most\npeople would probably not ever even look at this, it may not be worth\nworrying about.\n\n-Peff\n"},{"id":"431889","messageId":"xmqqim0mtc3u.fsf@gitster.g","threadId":"56190","inReplyTo":"YQlnNXgk3qcGb5Bk@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-03T22:08:53Z","receivedAt":"2021-08-03T22:09:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> It would make sense to me to do the switch in 'next' early in the\n> post-v2.33 cycle. It can cook there for a bit, but I think we have found\n> that it's much more likely to see actual use once it hits 'master'. So I\n> don't see a particular reason to have it sit in 'next' for a long time.\n> We should get as much exposure in 'master' during the v2.34 cycle as\n> possible.\n\nI do not mind queuing what is available today to 'next' to gain 2\nmore weeks of dogfood time during the pre-release freeze.  If an\nsimple escape hatch that lets us say \"anytime we ask ort, use\nrecursive instead as an emergency measure\" can be added with a\ntrivially obvious small patch, that would be a plus.\n\n> The nice thing is that the two strategies can co-exist. So if it does\n> turn out to have any regressions, it's an easy revert to switch back,\n> and even post-release users can switch at runtime. We have pull.twohead,\n> but I don't think we have an equivalent that would impact a bare \"git\n> merge\" or \"git rebase -m\". Maybe it would be worth adding those as an\n> escape hatch?\n\n"},{"id":"431891","messageId":"YQnFMmyd744Oolep@coredump.intra.peff.net","threadId":"56190","inReplyTo":"xmqqim0mtc3u.fsf@gitster.g","subject":"Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-03T22:37:38Z","receivedAt":"2021-08-03T22:38:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 03, 2021 at 03:08:53PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > It would make sense to me to do the switch in 'next' early in the\n> > post-v2.33 cycle. It can cook there for a bit, but I think we have found\n> > that it's much more likely to see actual use once it hits 'master'. So I\n> > don't see a particular reason to have it sit in 'next' for a long time.\n> > We should get as much exposure in 'master' during the v2.34 cycle as\n> > possible.\n> \n> I do not mind queuing what is available today to 'next' to gain 2\n> more weeks of dogfood time during the pre-release freeze.  If an\n> simple escape hatch that lets us say \"anytime we ask ort, use\n> recursive instead as an emergency measure\" can be added with a\n> trivially obvious small patch, that would be a plus.\n\nYeah, I am happy even starting the 'next' portion sooner than the\nrelease. :) As Elijah explained, we already have that emergency measure\nin place (pull.merge).\n\n-Peff\n"},{"id":"431893","messageId":"CABPp-BH=+d1xQaaBkLpRu91FJtQwh7GyGuve_ZwSwCUz5VreNQ@mail.gmail.com","threadId":"56190","inReplyTo":"xmqqim0mtc3u.fsf@gitster.g","subject":"Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-08-03T22:46:13Z","receivedAt":"2021-08-03T22:46:28Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Aug 3, 2021 at 4:08 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Jeff King <peff@peff.net> writes:\n>\n> > It would make sense to me to do the switch in 'next' early in the\n> > post-v2.33 cycle. It can cook there for a bit, but I think we have found\n> > that it's much more likely to see actual use once it hits 'master'. So I\n> > don't see a particular reason to have it sit in 'next' for a long time.\n> > We should get as much exposure in 'master' during the v2.34 cycle as\n> > possible.\n>\n> I do not mind queuing what is available today to 'next' to gain 2\n> more weeks of dogfood time during the pre-release freeze.\n\nSo I think I made a small mess here.  Much of the second patch wasn't\nspecific to the change of default, so I decided to split it up into a\nbunch of tiny documentation fixes that could be beneficial before 2.33\n(https://lore.kernel.org/git/pull.1059.git.git.1628004920.gitgitgadget@gmail.com/)\n\nSo, perhaps I should submit a v2 of this series which is based on that\nother one; then the second patch will only have the pieces specific to\nthe change of default left.\n\n> If an\n> simple escape hatch that lets us say \"anytime we ask ort, use\n> recursive instead as an emergency measure\" can be added with a\n> trivially obvious small patch, that would be a plus.\n\nI think what you're asking for here is the `pull.twohead` config\noption that has existed for a while (despite its name, it affects\nmerge, rebase, cherry-pick, and revert).\n"},{"id":"431894","messageId":"CABPp-BFyjDzwLgdpWZC59y4=t5=QZ8e8XFZ8HiPzFK=vSJkc4g@mail.gmail.com","threadId":"56190","inReplyTo":"YQnFMmyd744Oolep@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-08-03T22:48:02Z","receivedAt":"2021-08-03T22:48:15Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Aug 3, 2021 at 4:37 PM Jeff King <peff@peff.net> wrote:\n>\n> On Tue, Aug 03, 2021 at 03:08:53PM -0700, Junio C Hamano wrote:\n>\n> > Jeff King <peff@peff.net> writes:\n> >\n> > > It would make sense to me to do the switch in 'next' early in the\n> > > post-v2.33 cycle. It can cook there for a bit, but I think we have found\n> > > that it's much more likely to see actual use once it hits 'master'. So I\n> > > don't see a particular reason to have it sit in 'next' for a long time.\n> > > We should get as much exposure in 'master' during the v2.34 cycle as\n> > > possible.\n> >\n> > I do not mind queuing what is available today to 'next' to gain 2\n> > more weeks of dogfood time during the pre-release freeze.  If an\n> > simple escape hatch that lets us say \"anytime we ask ort, use\n> > recursive instead as an emergency measure\" can be added with a\n> > trivially obvious small patch, that would be a plus.\n>\n> Yeah, I am happy even starting the 'next' portion sooner than the\n> release. :) As Elijah explained, we already have that emergency measure\n> in place (pull.merge).\n\ns/pull.merge/pull.twohead/  Relevant commits:\n\n1c7b76be7d (\"Build in merge\", 2008-07-07)\n\n14c4586c2d (\"merge,rebase,revert: select ort or recursive by config or\nenvironment\", 2020-11-02)\n"}]}