@@ -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");
@@ -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 &&
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 <ben.knoble+github@gmail.com> --- 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(-)