[PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
From: D. Ben Knoble <hidden>
Date: 2025-02-05 03:11:41
Subsystem:
the rest · Maintainer:
Linus Torvalds
When running "git pull" with the following configuration options, we
fail to merge divergent branches:
- pull.ff=only
- pull.rebase (unset)
- branch.<current_branch>.rebase=true
Yet it seems that the user intended to make rebase the default for the
current branch while using --ff-only for non-rebase pulls. Since this
case appears uncovered by existing tests, changing the behavior here
might be safe: it makes what was an error into a successful rebase.
Add a test for the behavior and make it pass: this requires knowing from
where the rebase was requested. Previous commits (e4dc25ed49 (pull:
since --ff-only overrides, handle it first, 2021-07-22), adc27d6a93
(pull: make --rebase and --no-rebase override pull.ff=only, 2021-07-22))
took care to differentiate that --rebase overrides pull.ff=only, but
don't distinguish which config setting requests the rebase. Split
config_get_rebase into 2 parts so that we know where the rebase comes
from, since we only want to allow branch-config to override pull.ff=only
(like --rebase does); pull.rebase should still be overridden by
pull.ff=only or --ff-only.
Signed-off-by: D. Ben Knoble <redacted>
---
Notes:
- I also looked at ea1954af77 (pull: should be noop when already-up-to-date,
2021-11-17) when trying to understand how some options override others,
but it didnt' seem germane to the final version.
- I think I've got the right test script, since it's the one that started
failing before I added the "else" branch to the new code (which also
confirms that it's necessary to preserve current behavior); the only new
behavior should be the one mentioned by the new test.
- A possible #leftoverbits: it would be good to document more clearly the
interplay of --ff[-only], --rebase, pull.ff, pull.rebase, and
branch.<name>.rebase, particularly when they override each other.
Confusingly, branch.<name>.merge has nothing to do with whether pull will
merge or rebase ;) lest you think I'd forgotten something that _looks_
parallel to pull.rebase.
builtin/pull.c | 39 ++++++++++++++++++++++++++++--------
t/t7601-merge-pull-config.sh | 8 ++++++++
2 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 9c4a00620a..c30f233dcc 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c@@ -326,13 +326,13 @@ static const char *config_get_ff(void) } /** - * Returns the default configured value for --rebase. It first looks for the + * Returns the default configured value for --rebase. It looks for the * value of "branch.$curr_branch.rebase", where $curr_branch is the current * branch, and if HEAD is detached or the configuration key does not exist, - * looks for the value of "pull.rebase". If both configuration keys do not - * exist, returns REBASE_FALSE. + * considers the result unspecified. Follow up by checking + * config_get_rebase_pull. */ -static enum rebase_type config_get_rebase(int *rebase_unspecified) +static enum rebase_type config_get_rebase_branch(int *rebase_unspecified) { struct branch *curr_branch = branch_get("HEAD"); const char *value;
@@ -349,11 +349,22 @@ static enum rebase_type config_get_rebase(int *rebase_unspecified) free(key); } + *rebase_unspecified = 1; + return REBASE_INVALID; +} + +/* + * Looks for the value of "pull.rebase". If it does not exist, returns + * REBASE_FALSE. + */ +static enum rebase_type config_get_rebase_pull(int *rebase_unspecified) +{ + const char *value; + if (!git_config_get_value("pull.rebase", &value)) return parse_config_rebase("pull.rebase", value, 1); *rebase_unspecified = 1; - return REBASE_FALSE; }
@@ -1026,7 +1037,7 @@ int cmd_pull(int argc, * are relying on the next if-condition happening before * the config_get_rebase() call so that an explicit * "--rebase" can override a config setting of - * pull.ff=only. + * pull.ff=only. [continued…] */ if (opt_rebase >= 0 && opt_ff && !strcmp(opt_ff, "--ff-only")) { free(opt_ff);
@@ -1034,8 +1045,20 @@ int cmd_pull(int argc, } } - if (opt_rebase < 0) - opt_rebase = config_get_rebase(&rebase_unspecified); + if (opt_rebase < 0) { + /* + * […continued] But, if the config requests rebase *for this + * branch*, override --ff-only, which otherwise takes precedence + * over pull.rebase=true. + */ + opt_rebase = config_get_rebase_branch(&rebase_unspecified); + if (opt_rebase >= 0 && opt_ff && !strcmp(opt_ff, "--ff-only")) { + free(opt_ff); + opt_ff = xstrdup("--ff"); + } else { + opt_rebase = config_get_rebase_pull(&rebase_unspecified); + } + } if (repo_read_index_unmerged(the_repository)) die_resolve_conflict("pull");
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 199a1d5db3..fd99f46aad 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh@@ -113,6 +113,14 @@ test_grep ! "You have divergent branches" err ' +test_expect_success 'pull.rebase not set and pull.ff=only and branch.<name>.rebase=true (not-fast-forward)' ' + git reset --hard c2 && + test_config pull.ff only && + git switch -c bc2 && + test_config branch.bc2.rebase true && + git pull . c1 +' + test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)' ' git reset --hard c2 && git pull --rebase . c1 2>err &&
--
2.47.0