pull --rebase: honor rebase.autostash when fast-forwarding

"pull --rebase" internally uses the merge machinery when the other
history is a descendant of ours (i.e. perform fast-forward).  This
came from [1], where the discussion was started from a feature
request to do so.  It is a bit hard to read the rationale behind it
in the discussion, but it seems that it was an established fact for
everybody involved that does not even need to be mentioned that
fast-forwarding done with "rebase" was much undesirable than done
with "merge", and more importantly, the result left by "merge" is as
good as (or better than) that by "rebase".

Except for one thing.  Because "git merge" does not (and should not)
honor rebase.autostash, "git pull" needs to read it and forward it
when we use "git merge" as a (hopefully better) substitute for "git
rebase" during the fast-forwarding.  But we forgot to do so (we only
add "--[no-]autostash" to the "git merge" command when "git pull" itself
was invoked with "--[no-]autostash" command line option.

Make sure "git merge" is run with "--autostash" when
rebase.autostash is set and used to fast-forward the history on
behalf of "git rebase".  Incidentally this change also takes care of
the case where

 - "git pull --rebase" (without other command line options) is run
 - "rebase.autostash" is not set
 - The history fast-forwards

In such a case, "git merge" is run with an explicit "--no-autostash"
to prevent it from honoring merge.autostash configuration, which is
what we want.  After all, we want the "git merge" to pretend as if
it is "git rebase" while being used for this purpose.

[1] https://lore.kernel.org/git/xmqqa8cfbkeq.fsf_-_@gitster.mtv.corp.google.com/

Reported-by: Tilman Vogel <tilman.vogel@web.de>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/pull.c b/builtin/pull.c
index 1cfaf9f3436b2b2d5f4823088d6f69295ab64985..9f8a8dd7165bd089825c4ba5e6e64a6d06693f4f 100644 (file)
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1036,14 +1036,13 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
                oidclr(&orig_head);
 
        if (opt_rebase) {
-               int autostash = config_autostash;
-               if (opt_autostash != -1)
-                       autostash = opt_autostash;
+               if (opt_autostash == -1)
+                       opt_autostash = config_autostash;
 
                if (is_null_oid(&orig_head) && !is_cache_unborn())
                        die(_("Updating an unborn branch with changes added to the index."));
 
-               if (!autostash)
+               if (!opt_autostash)
                        require_clean_work_tree(the_repository,
                                N_("pull with rebase"),
                                _("please commit or stash them."), 1, 0);

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 93ecfcdd245b4ed86dd71fd6d3bc69fbf2505934..081808009b2e74aa4cb81a768a15701dd4eca9d5 100755 (executable)
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -330,6 +330,19 @@ test_expect_success '--rebase --autostash fast forward' '
        test_cmp_rev HEAD to-rebase-ff
 '
 
+test_expect_success '--rebase with rebase.autostash succeeds on ff' '
+       test_when_finished "rm -fr src dst actual" &&
+       git init src &&
+       test_commit -C src "initial" file "content" &&
+       git clone src dst &&
+       test_commit -C src --printf "more_content" file "more content\ncontent\n" &&
+       echo "dirty" >>dst/file &&
+       test_config -C dst rebase.autostash true &&
+       git -C dst pull --rebase >actual 2>&1 &&
+       grep -q "Fast-forward" actual &&
+       grep -q "Applied autostash." actual
+'
+
 test_expect_success '--rebase with conflicts shows advice' '
        test_when_finished "git rebase --abort; git checkout -f to-rebase" &&
        git checkout -b seq &&