"Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Elijah Newren <newren@xxxxxxxxx>
>
> Fix the last few precedence tests failing in t7601 by now implementing
> the logic to have --[no-]rebase override a pull.ff=only config setting.
Because we only have opt_rebase and opt_ff, unless we are in a very
early part of the code, we cannot tell if "opt_rebase >= 0" we see
came from the command line or from the configuration, and because
pull.rebase, pull.ff, --rebase=<kind>, and <--ff|--ff-only|--no-ff>
have intricate interactions among them, we'd probably need to behave
differently depending on where "opt_rebase >= 0" came from.
The new code works only because we haven't called config_get_rebase()
yet in this part. If we swapped the order of "if ff is not set,
then figure it out from the configuration" (the code you are
extending) and "if rebase is not set, read from the configuration",
then the logic would not work.
The resulting code looks correct, but it is subtle and may deserve
an in-code comment.
> Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> ---
> builtin/pull.c | 5 ++++-
> t/t7601-merge-pull-config.sh | 4 ++--
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 5ba376a7487..7e7c90f6a10 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -966,8 +966,11 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
>
> parse_repo_refspecs(argc, argv, &repo, &refspecs);
>
> - if (!opt_ff)
> + if (!opt_ff) {
> opt_ff = xstrdup_or_null(config_get_ff());
/*
* Do we have --rebase=<kind> or --no-rebase from the
* command line (note that we haven't read pull.rebase yet)?
* If so, defeat the configured pull.ff=only; instead of
* failing to integrate divergent histories, we want the
* rebase or merge to happen.
*/
> + if (opt_rebase >= 0 && opt_ff && !strcmp(opt_ff, "--ff-only"))
> + opt_ff = "--ff";
> + }
Thanks.
