Skip to content

Commit

Permalink
rebase: change the default backend from "am" to "merge"
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
newren authored and gitster committed Feb 16, 2020
1 parent 8295ed6 commit 2ac0d62
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 8 deletions.
13 changes: 12 additions & 1 deletion Documentation/git-rebase.txt
Expand Up @@ -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
Expand Down Expand Up @@ -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
~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
4 changes: 2 additions & 2 deletions builtin/rebase.c
Expand Up @@ -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 \
Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions t/t5520-pull.sh
Expand Up @@ -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' '
Expand All @@ -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' '
Expand Down Expand Up @@ -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
)
'

Expand Down
3 changes: 2 additions & 1 deletion t/t9106-git-svn-commit-diff-clobber.sh
Expand Up @@ -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' "
Expand Down

0 comments on commit 2ac0d62

Please sign in to comment.