On Thu, Jan 06, 2022 at 10:02:26AM -0500, John Cai wrote:
> On a git pull --rebase, if fast forward is possible we run merge.
> However, merge will not honor rebase.autostash if it is configured. This
> has the unfortunate result of
>
> $ git config rebase.autostash true
> $ git pull --rebase
>
> to ignore the rebase.autostash value.
>
> Allow run_merge() to honor rebase.autostash by passing in
> config_autostash if --autostash or --no-autostash flags are not
> explicitly set.
>
> Reported-by: "Tilman Vogel" <tilman.vogel@xxxxxx>
> Co-authored-by: "Junio C Hamano" <gitster@xxxxxxxxx>
> Signed-off-by: "John Cai" <johncai86@xxxxxxxxx>
> ---
>
> Notes:
> Fix a bug that prevents git pull --rebase from honoring the rebase.autostash
> config value.
>
> Changes since V2:
> - fixed Junio's email in trailer
>
> Changes since V1:
> - used simpler fix as proposed by Junio
> - removed redundant test cases
>
> builtin/pull.c | 9 ++++++++-
> t/t5521-pull-options.sh | 12 ++++++++++++
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 100cbf9fb8..8423e420ee 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -1133,7 +1133,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
> die(_("cannot rebase with locally recorded submodule modifications"));
>
> if (can_ff) {
> - /* we can fast-forward this without invoking rebase */
> + /*
> + * We can fast-forward without invoking
> + * rebase, by calling run_merge(). But we
> + * have to allow rebase.autostash=true to kick
> + * in.
> + */
> + if (opt_autostash < 0)
> + opt_autostash = config_autostash;
This looks OK, and prefers the value of autostash given over options
over the configured one. But it may be a little clearer to construct it
that way explicitly (see the conditional "if (opt_rebase)" inside of
cmd_pull()).
> opt_ff = "--ff-only";
> ret = run_merge();
> } else {
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> index 66cfcb09c5..66fac99d2b 100755
> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -252,4 +252,16 @@ 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" &&
A handful of these arguments do not need extra quotes. It would be fine
to instead write:
test_commit -C src initial file content &&
but if you don't care about the commit message being "content", you can
shorten this to just:
test_commit -C src content file &&
since test_commit defaults all of its arguments to the value of
"<message>".
> + git clone src dst &&
> + test_commit -C src --printf "more_content" file "more content\ncontent\n" &&
Same note here, but I think the `--printf` is unnecessary. Running
`echo` with "\n" characters in its argument is fine, so this could be
shortened to:
test_commit -C src blah file "more\ncontent"
Unfortunately using `--append` isn't an option here, since the "content"
anchor needs to be at the end of the file in order to apply the stash
without conflict.
Thanks,
Taylor
