rebase: change the default backend from "am" to "merge"

The am-backend drops information and thus limits what we can do:

  * lack of full tree information from the original commits means we
    cannot do directory rename detection and warn users that they might
    want to move some of their new files that they placed in old
    directories to prevent their becoming orphaned.[1]
  * reduction in context from only having a few lines beyond those
    changed means that when context lines are non-unique we can apply
    patches incorrectly.[2]
  * lack of access to original commits means that conflict marker
    annotation has less information available.
  * the am backend has safety problems with an ill-timed interrupt.

Also, the merge/interactive backend have far more abilities, appear to
currently have a slight performance advantage[3] and have room for more
optimizations than the am backend[4] (and work is underway to take
advantage of some of those possibilities).

[1] https://lore.kernel.org/git/xmqqh8jeh1id.fsf@gitster-ct.c.googlers.com/
[2] https://lore.kernel.org/git/CABPp-BGiu2nVMQY_t-rnFR5GQUz_ipyEE8oDocKeO+h+t4Mn4A@mail.gmail.com/
[3] https://public-inbox.org/git/CABPp-BF=ev03WgODk6TMQmuNoatg2kiEe5DR__gJ0OTVqHSnfQ@mail.gmail.com/
[4] https://lore.kernel.org/git/CABPp-BGh7yW69QwxQb13K0HM38NKmQif3A6C6UULEKYnkEJ5vA@mail.gmail.com/

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 93092da3bf6ab215c9c141629f6c14a17843b890..fbac1cf38dd251299d8dde601bdff1e9548a170f 100644 (file)
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -315,7 +315,7 @@ See also INCOMPATIBLE OPTIONS below.
 --merge::
        Use merging strategies to rebase.  When the recursive (default) merge
        strategy is used, this allows rebase to be aware of renames on the
-       upstream side.
+       upstream side.  This is the default.
 +
 Note that a rebase merge works by replaying each commit from the working
 branch on top of the <upstream> branch.  Because of this, when a merge
@@ -683,6 +683,17 @@ accident of implementation rather than by design.  Both backends
 should have the same behavior, though it is not clear which one is
 correct.
 
+Interruptability
+~~~~~~~~~~~~~~~~
+
+The am backend has safety problems with an ill-timed interrupt; if the
+user presses Ctrl-C at the wrong time to try to abort the rebase, the
+rebase can enter a state where it cannot be aborted with a subsequent
+`git rebase --abort`.  The interactive backend does not appear to
+suffer from the same shortcoming.  (See
+https://lore.kernel.org/git/20200207132152.GC2868@szeder.dev/ for
+details.)
+
 Miscellaneous differences
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 

diff --git a/builtin/rebase.c b/builtin/rebase.c
index f0a862f41b5c4c3500a7282219fdc6ae7f2bf1b7..db8fd699d79c51dfbfdb6063704619741a47de6d 100644 (file)
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -101,7 +101,7 @@ struct rebase_options {
 #define REBASE_OPTIONS_INIT {                          \
                .type = REBASE_UNSPECIFIED,             \
                .empty = EMPTY_UNSPECIFIED,             \
-               .default_backend = "am",                \
+               .default_backend = "merge",             \
                .flags = REBASE_NO_QUIET,               \
                .git_am_opts = ARGV_ARRAY_INIT,         \
                .git_format_patch_opt = STRBUF_INIT     \
@@ -1917,7 +1917,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 
        if (options.type == REBASE_UNSPECIFIED) {
                if (!strcmp(options.default_backend, "merge"))
-                       options.type = REBASE_MERGE;
+                       imply_interactive(&options, "--merge");
                else if (!strcmp(options.default_backend, "am"))
                        options.type = REBASE_AM;
                else

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 3fff6a06fa8fed5e19bb961c23e77627abd6b547..4f9e7f7ff6b022beb233766453f59a4ca29adc85 100755 (executable)
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -340,7 +340,7 @@ test_expect_success '--rebase with conflicts shows advice' '
        test_tick &&
        git commit -m "Create conflict" seq.txt &&
        test_must_fail git pull --rebase . seq 2>err >out &&
-       test_i18ngrep "Resolve all conflicts manually" out
+       test_i18ngrep "Resolve all conflicts manually" err
 '
 
 test_expect_success 'failed --rebase shows advice' '
@@ -354,7 +354,7 @@ test_expect_success 'failed --rebase shows advice' '
        git checkout -f -b fails-to-rebase HEAD^ &&
        test_commit v2-without-cr file "2" file2-lf &&
        test_must_fail git pull --rebase . diverging 2>err >out &&
-       test_i18ngrep "Resolve all conflicts manually" out
+       test_i18ngrep "Resolve all conflicts manually" err
 '
 
 test_expect_success '--rebase fails with multiple branches' '
@@ -774,8 +774,10 @@ test_expect_success 'git pull --rebase does not reapply old patches' '
        (
                cd dst &&
                test_must_fail git pull --rebase &&
-               find .git/rebase-apply -name "000*" >patches &&
-               test_line_count = 1 patches
+               cat .git/rebase-merge/done .git/rebase-merge/git-rebase-todo >work &&
+               grep -v -e \# -e ^$ work >patches &&
+               test_line_count = 1 patches &&
+               rm -f work
        )
 '
 

diff --git a/t/t9106-git-svn-commit-diff-clobber.sh b/t/t9106-git-svn-commit-diff-clobber.sh
index dbe8deac0d2f28099241957e8944e3f714ee2fde..aec45bca3b7b6fc4561e02ff975f77a5331cb7ce 100755 (executable)
--- a/t/t9106-git-svn-commit-diff-clobber.sh
+++ b/t/t9106-git-svn-commit-diff-clobber.sh
@@ -92,7 +92,8 @@ test_expect_success 'multiple dcommit from git svn will not clobber svn' "
 
 
 test_expect_success 'check that rebase really failed' '
-       test -d .git/rebase-apply
+       git status >output &&
+       grep currently.rebasing output
 '
 
 test_expect_success 'resolve, continue the rebase and dcommit' "