Re: [PATCH 1/1] builtin/pull.c: use config value of autostash
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 14, 2022, 00:00 UTC
- Message-ID
- <xmqqee5bfaof.fsf@gitster.g>
- In-Reply-To
- <xmqq35m0el28.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Here is how I would explain your change.
This topic is in "Expecting an ack or two" state for some time.
- Philippe, are you OK with the attached patch? If so throw your Signed-off-by to this thread.
- John, if you find Philippe's implementation a good idea (I think it is, as it is simpler and cleaner) after reading and understanding it, please throw an Acked-by or Reviewed-by to this thread.
----- >8 --------- >8 --------- >8 --------- >8 --------- >8 ----- From: Philippe Blain <levraiphilippeblain@gmail.com> Date: Tue, 4 Jan 2022 22:58:32 -0500 Subject: [PATCH] pull --rebase: honor rebase.autostash even 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" was invoked with "--[no-]autostash" command line option, so that "git merge" can honor merge.autostash in such a case).
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/ --- builtin/pull.c | 7 +++--- t/t5521-pull-options.sh | 52 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c index 1cfaf9f343..9f8a8dd716 100644 --- 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/t5521-pull-options.sh b/t/t5521-pull-options.sh index 66cfcb09c5..4046a74cad 100755 --- a/t/t5521-pull-options.sh +++ b/t/t5521-pull-options.sh @@ -252,4 +252,56 @@ test_expect_success 'git pull --no-verify --verify passed to merge' ' test_must_fail git -C dst pull --no-ff --no-verify --verify ' +test_expect_success 'git pull --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 && + git -C dst pull --rebase --autostash >actual 2>&1 && + grep -q "Fast-forward" actual && + grep -q "Applied autostash." actual +' + +test_expect_success 'git pull --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 'git pull --rebase --autostash succeeds on non-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 "src_content" file "src content\ncontent\n" && + test_commit -C dst --append "dst_content" file "dst content" && + echo "dirty" >>dst/file && + git -C dst pull --rebase --autostash >actual 2>&1 && + grep -q "Successfully rebased and updated refs/heads/main." actual && + grep -q "Applied autostash." actual +' + +test_expect_success 'git pull --rebase with rebase.autostash succeeds on non-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 "src_content" file "src content\ncontent\n" && + test_commit -C dst --append "dst_content" file "dst content" && + echo "dirty" >>dst/file && + test_config -C dst rebase.autostash true && + git -C dst pull --rebase >actual 2>&1 && + grep -q "Successfully rebased and updated refs/heads/main." actual && + grep -q "Applied autostash." actual +' + test_done
-- 2.35.0-rc0-182-gd99ee8e0c1