{"thread":{"id":"56211","subject":"[PATCH 0/2] Switch default merge backend from recursive to ort","startedAt":"2021-08-04T05:38:07Z","lastAt":"2021-08-09T21:01:51Z","messageCount":7,"participants":["Elijah Newren via GitGitGadget","Derrick Stolee","Phillip Wood","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"431935","messageId":"pull.1011.git.1628055482.gitgitgadget@gmail.com","threadId":"56211","inReplyTo":null,"subject":"[PATCH 0/2] Switch default merge backend from recursive to ort","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-04T05:38:00Z","receivedAt":"2021-08-04T05:38:07Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This is a reroll of my RFC series[1] to switch the default merge backend\nfrom recursive to ort. The consensus seems to be that we should do this\nimmediately after the 2.33 release.\n\nNote that folks who want to get the old merge backend after this series can\nsimply set pull.twohead=recursive. (And, similarly, before this series,\nthose who want to try out ort in Git 2.32 or Git 2.33 can set\npull.twohead=ort.)\n\nChanges since the RFC version of this series:\n\n * Now depends on the reroll of en/merge-strategy-docs I just submitted[2].\n * Made tweaks to code and documentation suggested by Stolee, Dscho, and\n   Ævar\n\n[1]\nhttps://lore.kernel.org/git/pull.1055.git.git.1627776461.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/pull.1059.git.git.1628004920.gitgitgadget@gmail.com/\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       | 28 ++++-----\n Documentation/gitfaq.txt           |  2 +-\n Documentation/merge-options.txt    |  2 +-\n Documentation/merge-strategies.txt | 93 ++++++++++++++++--------------\n Documentation/user-manual.txt      |  2 +-\n builtin/merge.c                    | 10 +++-\n builtin/rebase.c                   |  2 +-\n sequencer.c                        |  4 +-\n 8 files changed, 78 insertions(+), 65 deletions(-)\n\n\nbase-commit: 4a78ac53424525d7099867d5a4377b8afb9bf18f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1011%2Fnewren%2Fort-default-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1011/newren/ort-default-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1011\n-- \ngitgitgadget\n"},{"id":"431936","messageId":"845d5bf510c5dea2bf6d96d6bc21f356d00fc562.1628055482.git.gitgitgadget@gmail.com","threadId":"56211","inReplyTo":"pull.1011.git.1628055482.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-04T05:38:02Z","receivedAt":"2021-08-04T05:38:08Z","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 it clear that `ort` is the default merge strategy now rather than\n`recursive`, including moving `ort` to the front of the list of merge\nstrategies.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n Documentation/git-rebase.txt       | 28 ++++-----\n Documentation/gitfaq.txt           |  2 +-\n Documentation/merge-options.txt    |  2 +-\n Documentation/merge-strategies.txt | 93 ++++++++++++++++--------------\n Documentation/user-manual.txt      |  2 +-\n 5 files changed, 67 insertions(+), 60 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 73d49ec8d91..3f1030df70e 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -352,8 +352,8 @@ See also INCOMPATIBLE OPTIONS below.\n \n -s <strategy>::\n --strategy=<strategy>::\n-\tUse the given merge strategy, instead of the default\n-\t`recursive`.  This implies `--merge`.\n+\tUse the given merge strategy, instead of the default `ort`.\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@@ -366,7 +366,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@@ -527,7 +527,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@@ -1216,16 +1216,16 @@ 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-By default, the `merge` command will use the `recursive` merge\n-strategy for regular merges, and `octopus` for octopus merges.  One\n-can specify a default strategy for all merges using the `--strategy`\n-argument when invoking rebase, or can override specific merges in the\n-interactive list of commands by using an `exec` command to call `git\n-merge` explicitly with a `--strategy` argument.  Note that when\n-calling `git merge` explicitly like this, you can make use of the fact\n-that the labels are worktree-local refs (the ref `refs/rewritten/onto`\n-would correspond to the label `onto`, for example) in order to refer\n-to the branches you want to merge.\n+By default, the `merge` command will use the `ort` merge strategy for\n+regular merges, and `octopus` for octopus merges.  One can specify a\n+default strategy for all merges using the `--strategy` argument when\n+invoking rebase, or can override specific merges in the interactive\n+list of commands by using an `exec` command to call `git merge`\n+explicitly with a `--strategy` argument.  Note that when calling `git\n+merge` explicitly like this, you can make use of the fact that the\n+labels are worktree-local refs (the ref `refs/rewritten/onto` would\n+correspond to the label `onto`, for example) in order to refer to the\n+branches you want to merge.\n \n Note: the first command (`label onto`) labels the revision onto which\n the commits are rebased; The name `onto` is just a convention, as a nod\ndiff --git a/Documentation/gitfaq.txt b/Documentation/gitfaq.txt\nindex afdaeab8503..8c1f2d56751 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 f819bd8dd68..72b53188504 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -112,7 +112,7 @@ 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 (`recursive` when merging a single head,\n+\tis used instead (`ort` when merging a single head,\n \t`octopus` otherwise).\n \n -X <option>::\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 210f0f850b2..34c6c31d8e7 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -6,21 +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-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.  This\n-\tis the default merge strategy when pulling or merging one\n-\tbranch.\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@@ -36,16 +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-\tDeprecated synonym 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`.  Defaults to the\n-\t`diff.algorithm` config setting.\n-\n ignore-space-change;;\n ignore-all-space;;\n ignore-space-at-eol;;\n@@ -74,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@@ -95,19 +82,39 @@ subtree[=<path>];;\n \tis prefixed (or stripped from the beginning) to make the shape of\n \ttwo trees to match.\n \n-ort::\n-\tThis is meant as a drop-in replacement for the `recursive`\n-\talgorithm (as reflected in its acronym -- \"Ostensibly\n-\tRecursive's Twin\"), and will likely replace it in the future.\n-\tIt fixes corner cases that the `recursive` strategy handles\n-\tsuboptimally, and is significantly faster in large\n-\trepositories -- especially when many renames are involved.\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.  This was\n+\tthe default strategy for resolving two heads from Git v0.99.9k\n+\tuntil v2.33.0.\n +\n-The `ort` strategy takes all the same options as `recursive`.\n-However, it ignores three of those options: `no-renames`,\n-`patience` and `diff-algorithm`.  It always runs with rename\n-detection (it handles it much faster than `recursive` does), and\n-it specifically uses `diff-algorithm=histogram`.\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 synonym 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. This overrides the `merge.renames`\n+\tconfiguration variable.\n+\tSee also linkgit:git-diff[1] `--no-renames`.\n \n resolve::\n \tThis can only resolve two heads (i.e. the current branch\n@@ -131,13 +138,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/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":"431937","messageId":"4a0f088f3669a95c7f75e885d06c0a3bdaf31f42.1628055482.git.gitgitgadget@gmail.com","threadId":"56211","inReplyTo":"pull.1011.git.1628055482.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-04T05:38:01Z","receivedAt":"2021-08-04T05:38:12Z","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 d7b14bf4a7f..fbe21d413c1 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 a4e5c43fcf2..d08ddae52fa 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, \"ort\")) {\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@@ -3988,7 +3988,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, \"ort\")) {\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":"431957","messageId":"fcaa2322-6ef4-9db0-4c0f-3050d37bb875@gmail.com","threadId":"56211","inReplyTo":"845d5bf510c5dea2bf6d96d6bc21f356d00fc562.1628055482.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-04T14:26:01Z","receivedAt":"2021-08-04T14:26:08Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/4/2021 1:38 AM, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren <newren@gmail.com>\n\nThese updates look good to me. Mostly whole paragraphs moving and some\nword replacements.\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\nHere is one more case where the old docs could have used quotes around\n\"recursive\" and the new ones could use quotes around \"ort\". Not worth\na re-roll on its own.\n\nThanks,\n-Stolee\n"},{"id":"431958","messageId":"19187114-4a91-19b8-7c41-407a4a661f41@gmail.com","threadId":"56211","inReplyTo":"pull.1011.git.1628055482.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Switch default merge backend from recursive to ort","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-04T14:27:17Z","receivedAt":"2021-08-04T14:27:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/4/2021 1:38 AM, Elijah Newren via GitGitGadget wrote:\n> This is a reroll of my RFC series[1] to switch the default merge backend\n> from recursive to ort. The consensus seems to be that we should do this\n> immediately after the 2.33 release.\n> \n> Note that folks who want to get the old merge backend after this series can\n> simply set pull.twohead=recursive. (And, similarly, before this series,\n> those who want to try out ort in Git 2.32 or Git 2.33 can set\n> pull.twohead=ort.)\n> \n> Changes since the RFC version of this series:\n> \n>  * Now depends on the reroll of en/merge-strategy-docs I just submitted[2].\n>  * Made tweaks to code and documentation suggested by Stolee, Dscho, and\n>    Ævar\n\nI'm happy with this version (modulo a super tiny nitpick that doesn't merit\na re-roll) and the docs that preceded it. I'm going to adapt my version for\nthe microsoft/git fork to take this version and put it in our v2.33.0\nrelease.\n\nThanks,\n-Stolee\n"},{"id":"432315","messageId":"f6e79035-1412-4f8b-5949-8e9cc7215785@gmail.com","threadId":"56211","inReplyTo":"4a0f088f3669a95c7f75e885d06c0a3bdaf31f42.1628055482.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] Change default merge backend from recursive to ort","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-08-09T17:38:28Z","receivedAt":"2021-08-09T17:38:34Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Elijah\n\nOn 04/08/2021 06:38, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren <newren@gmail.com>\n> \n> There are a few reasons to switch the default:\n>    * Correctness\n>    * Extensibility\n>    * Performance\n> \n> I'll provide some summaries about each.\n> \n> === Correctness ===\n> \n> The original impetus for a new merge backend was to fix issues that were\n> difficult to fix within recursive's design.  The success with this goal\n> is 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> \n> In 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\nI've not followed your patches too closely but I did notice you seem to \nhave put a lot of effort into improving the test coverage which is \nfantastic.\n\n> === Extensibility ===\n> \n> Being able to perform merges without touching the working tree or index\n> makes it possible to create new features that were difficult with the\n> old backend:\n> \n>    * Merging, cherry-picking, rebasing, reverting in bare repositories...\n>      or just on branches that aren't checked out.\n\nRebasing without updating the worktree for each commit is exciting.\n\nI agree with others that this should be merged into next sooner rather \nthan later. I also agree with Peff's point about moving it into master \nto get more people using it rather than sitting in next for too long.\n\nI think the sequencer changes below are easier to follow in this \nversion. One thing I did wonder is whether there needs to be any change \nto the CI scripts to ensure we keep testing both merge implementations.\n\nBest wishes and congratulations on an impressive achievement\n\nPhillip\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> \n> The last three have been implemented already (though only one has been\n> submitted upstream so far; the others were waiting for performance work\n> to complete), and I still plan to implement the first one.\n> \n> === Performance ===\n> \n> I'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>                  ----------   ---------   -----------   ---------\n> few-renames:      18.912 s    18.030 s     11.699 s     198.3 ms\n> mega-renames:   5964.031 s   361.281 s    203.886 s     661.8 ms\n> just-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>                  ----------   ---------   -----------   ---------\n> few-renames:        1           1.05         1.6           95\n> mega-renames:       1          16.5         29           9012\n> just-one-mega:      1          13.6         20            565\n> \n> And, 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>                  ----------   ---------   -----------   ---------\n> mega-renames:       1            1            1          181.3\n> \n> Signed-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> \n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index d7b14bf4a7f..fbe21d413c1 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 */\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 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++)\n> diff --git a/sequencer.c b/sequencer.c\n> index a4e5c43fcf2..d08ddae52fa 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, \"ort\")) {\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> @@ -3988,7 +3988,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, \"ort\")) {\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\n"},{"id":"432334","messageId":"CABPp-BEtkFkr=dw+A4K=jOY5wCDFtZicsShwwjmM1-aFRS1bSA@mail.gmail.com","threadId":"56211","inReplyTo":"f6e79035-1412-4f8b-5949-8e9cc7215785@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-09T21:01:37Z","receivedAt":"2021-08-09T21:01:51Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Aug 9, 2021 at 10:38 AM Phillip Wood <phillip.wood123@gmail.com> wrote:\n...\n> > === Extensibility ===\n> >\n> > Being able to perform merges without touching the working tree or index\n> > makes it possible to create new features that were difficult with the\n> > old backend:\n> >\n> >    * Merging, cherry-picking, rebasing, reverting in bare repositories...\n> >      or just on branches that aren't checked out.\n>\n> Rebasing without updating the worktree for each commit is exciting.\n\nYeah, there appear to be some interesting twists from my initial\ninvestigations so far.  I may need to start some interesting\ndiscussions in the next cycle or two about what we should do here (and\nalso for merges in bare repositories.)\n\n> I agree with others that this should be merged into next sooner rather\n> than later. I also agree with Peff's point about moving it into master\n> to get more people using it rather than sitting in next for too long.\n\n:-)\n\n> I think the sequencer changes below are easier to follow in this\n> version.\n\nThanks for taking a look.\n\n> One thing I did wonder is whether there needs to be any change\n> to the CI scripts to ensure we keep testing both merge implementations.\n\nThere did need to be such a change, but it was made previously in\ncommit f3b964a07e (\"Add testing with merge-ort merge strategy\",\n2021-03-20).  That commit changed the default merge backend *for\ntesting* to be merge-ort, and modified the linux-gcc job to explicitly\nspecify recursive.  This commit doesn't change the testing story, so\nthe recursive backend will still continue to be tested with the\nlinux-gcc tests, or whenever someone requests it with\nGIT_TEST_MERGE_ALGORITHM=recursive, and otherwise merge-ort will be\nused.\n\n> Best wishes and congratulations on an impressive achievement\n\nThanks!\n"}]}