{"thread":{"id":"59322","subject":"[RFC PATCH 1/2] Add pull.mode","startedAt":"2023-02-28T14:02:45Z","lastAt":"2023-02-28T14:02:53Z","messageCount":3,"participants":["Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"472842","messageId":"20230228140236.4175835-2-felipe.contreras@gmail.com","threadId":"59322","inReplyTo":"20230228140236.4175835-1-felipe.contreras@gmail.com","subject":"[RFC PATCH 1/2] Add pull.mode","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-02-28T14:02:35Z","receivedAt":"2023-02-28T14:02:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The evolution of pull options has somewhat served most users, however,\nthey have been found lacking for a very needed trio: merge / rebase /\nfast-forward-only.\n\nTo avoid some of the problems of git pull Linus Torvalds suggested a\nconfiguration pull.merge [1], however, instead of having pull.merge and\npull.rebase, we can have pull.mode which works for both.\n\nMany people have argued that `git pull` should have a per-branch\nconfiguration for its mode of operation (e.g. Linus Torvalds [1] and\nTheodore Ts'o [2]).\n\nbranch.<name>.pullMode achieves that.\n\nSome discussions about pull.mode = fast-forward:\n\nhttps://lore.kernel.org/git/742df4c2-2bc5-8a4b-8de1-cd5e48718398@redhat.com/\nhttps://lore.kernel.org/git/20130522115042.GA20649@inner.h.apk.li\nhttps://lore.kernel.org/git/1377988690-23460-1-git-send-email-felipe.contreras@gmail.com\nhttps://lore.kernel.org/git/4ay6w9i74cygt6ii1b0db7wg.1398433713382@email.android.com\n\nThis is a squash of the following patches:\n\n * pull: reorganize mode conditionals\n * pull: improve default warning\n * pull: add pull.mode=fast-forward\n * pull: add per-branch mode configuration\n * pull: add pull.mode\n * pull: introduce --merge option\n * rebase: add REBASE_DEFAULT\n\n[1] https://lore.kernel.org/git/CA+55aFz2Uvq4vmyjJPao5tS-uuVvKm6mbP7Uz8sdq1VMxMGJCw@mail.gmail.com/\n[2] https://lore.kernel.org/git/20130312212027.GE14792@thunk.org/\n\nCc: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/config/branch.txt |   5 ++\n Documentation/config/pull.txt   |   6 ++\n Documentation/git-pull.txt      |  15 ++--\n builtin/pull.c                  | 147 +++++++++++++++++++++-----------\n rebase.c                        |  12 +++\n rebase.h                        |  13 ++-\n t/t7601-merge-pull-config.sh    |  30 +++----\n 7 files changed, 158 insertions(+), 70 deletions(-)\n\ndiff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt\nindex 445341a906..035e7c1fe4 100644\n--- a/Documentation/config/branch.txt\n+++ b/Documentation/config/branch.txt\n@@ -96,6 +96,11 @@ mode.\n it unless you understand the implications (see linkgit:git-rebase[1]\n for details).\n \n+branch.<name>.pullMode::\n+\tWhen `git pull` is run, this determines the mode of operation,\n+\tpossible values are 'fast-forward', 'merge', and 'rebase'.\n+\tSee `pull.mode` for doing this in a non branch-specific manner.\n+\n branch.<name>.description::\n \tBranch description, can be edited with\n \t`git branch --edit-description`. Branch description is\ndiff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt\nindex 9349e09261..596a9a504b 100644\n--- a/Documentation/config/pull.txt\n+++ b/Documentation/config/pull.txt\n@@ -25,6 +25,12 @@ mode.\n it unless you understand the implications (see linkgit:git-rebase[1]\n for details).\n \n+pull.mode::\n+\tWhen \"git pull\" is run, this determines if it would either merge or\n+\trebase the fetched branch. The possible values are 'merge',\n+\t'rebase', and 'fast-forward'. See \"branch.<name>.pullMode\" for setting\n+\tthis on a per-branch basis.\n+\n pull.octopus::\n \tThe default merge strategy to use when pulling multiple branches\n \tat once.\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex 0e14f8b5b2..e961439936 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -17,10 +17,10 @@ DESCRIPTION\n \n Incorporates changes from a remote repository into the current branch.\n If the current branch is behind the remote, then by default it will\n-fast-forward the current branch to match the remote.  If the current\n-branch and the remote have diverged, the user needs to specify how to\n-reconcile the divergent branches with `--rebase` or `--no-rebase` (or\n-the corresponding configuration option in `pull.rebase`).\n+fast-forward the current branch to match the remote.\n+\n+By default non-fast-forward merges fail, so you need to specify if you want to\n+do a merge or a rebase.\n \n More precisely, `git pull` runs `git fetch` with the given parameters\n and then depending on configuration options or command line flags,\n@@ -130,8 +130,11 @@ It rewrites history, which does not bode well when you\n published that history already.  Do *not* use this option\n unless you have read linkgit:git-rebase[1] carefully.\n \n---no-rebase::\n-\tThis is shorthand for --rebase=false.\n+-m::\n+--merge::\n+\tForce a merge.\n++\n+Previously this was --no-rebase, but that usage has been deprecated.\n \n Options related to fetching\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 1ab4de0005..0531328e2f 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -28,6 +28,8 @@\n #include \"sequencer.h\"\n #include \"packfile.h\"\n \n+static enum pull_mode_type mode;\n+\n /**\n  * Parses the value of --rebase. If value is a false value, returns\n  * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is\n@@ -49,6 +51,14 @@ static enum rebase_type parse_config_rebase(const char *key, const char *value,\n \treturn REBASE_INVALID;\n }\n \n+static enum pull_mode_type parse_config_pull_mode(const char *key, const char *value)\n+{\n+\tenum pull_mode_type v = pull_mode_parse_value(value);\n+\tif (v == PULL_MODE_INVALID)\n+\t\tdie(_(\"Invalid value for %s: %s\"), key, value);\n+\treturn v;\n+}\n+\n /**\n  * Callback for --rebase, which parses arg with parse_config_rebase().\n  */\n@@ -60,9 +70,21 @@ static int parse_opt_rebase(const struct option *opt, const char *arg, int unset\n \t\t*value = parse_config_rebase(\"--rebase\", arg, 0);\n \telse\n \t\t*value = unset ? REBASE_FALSE : REBASE_TRUE;\n+\n+\tif (*value > 0)\n+\t\tmode = *value >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;\n+\n \treturn *value == REBASE_INVALID ? -1 : 0;\n }\n \n+static int parse_opt_merge(const struct option *opt, const char *arg, int unset)\n+{\n+\tenum rebase_type *value = opt->value;\n+\tmode = PULL_MODE_MERGE;\n+\t*value = REBASE_FALSE;\n+\treturn 0;\n+}\n+\n static const char * const pull_usage[] = {\n \tN_(\"git pull [<options>] [<repository> [<refspec>...]]\"),\n \tNULL\n@@ -75,7 +97,7 @@ static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;\n \n /* Options passed to git-merge or git-rebase */\n-static enum rebase_type opt_rebase = -1;\n+static enum rebase_type opt_rebase;\n static char *opt_diffstat;\n static char *opt_log;\n static char *opt_signoff;\n@@ -131,6 +153,9 @@ static struct option pull_options[] = {\n \t\t\"(false|true|merges|interactive)\",\n \t\tN_(\"incorporate changes by rebasing rather than merging\"),\n \t\tPARSE_OPT_OPTARG, parse_opt_rebase),\n+\tOPT_CALLBACK_F('m', \"merge\", &opt_rebase, NULL,\n+\t\tN_(\"incorporate changes by merging\"),\n+\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_merge),\n \tOPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,\n \t\tN_(\"do not show a diffstat at the end of the merge\"),\n \t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG),\n@@ -329,7 +354,7 @@ static const char *config_get_ff(void)\n  * looks for the value of \"pull.rebase\". If both configuration keys do not\n  * exist, returns REBASE_FALSE.\n  */\n-static enum rebase_type config_get_rebase(int *rebase_unspecified)\n+static enum rebase_type config_get_rebase(void)\n {\n \tstruct branch *curr_branch = branch_get(\"HEAD\");\n \tconst char *value;\n@@ -349,9 +374,30 @@ static enum rebase_type config_get_rebase(int *rebase_unspecified)\n \tif (!git_config_get_value(\"pull.rebase\", &value))\n \t\treturn parse_config_rebase(\"pull.rebase\", value, 1);\n \n-\t*rebase_unspecified = 1;\n+\treturn REBASE_DEFAULT;\n+}\n+\n+static enum pull_mode_type config_get_pull_mode(const char *repo)\n+{\n+\tstruct branch *curr_branch = branch_get(\"HEAD\");\n+\tconst char *value;\n+\n+\tif (curr_branch) {\n+\t\tchar *key = xstrfmt(\"branch.%s.pullmode\", curr_branch->name);\n+\n+\t\tif (!git_config_get_value(key, &value)) {\n+\t\t\tenum pull_mode_type ret = parse_config_pull_mode(key, value);\n+\t\t\tfree(key);\n+\t\t\treturn ret;\n+\t\t}\n+\n+\t\tfree(key);\n+\t}\n+\n+\tif (!git_config_get_value(\"pull.mode\", &value))\n+\t\treturn parse_config_pull_mode(\"pull.mode\", value);\n \n-\treturn REBASE_FALSE;\n+\treturn PULL_MODE_DEFAULT;\n }\n \n /**\n@@ -446,7 +492,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n \tconst char *remote = curr_branch ? curr_branch->remote_name : NULL;\n \n \tif (*refspecs) {\n-\t\tif (opt_rebase)\n+\t\tif (opt_rebase >= REBASE_TRUE)\n \t\t\tfprintf_ln(stderr, _(\"There is no candidate for rebasing against among the refs that you just fetched.\"));\n \t\telse\n \t\t\tfprintf_ln(stderr, _(\"There are no candidates for merging among the refs that you just fetched.\"));\n@@ -459,7 +505,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n \t\t\trepo);\n \t} else if (!curr_branch) {\n \t\tfprintf_ln(stderr, _(\"You are not currently on a branch.\"));\n-\t\tif (opt_rebase)\n+\t\tif (opt_rebase >= REBASE_TRUE)\n \t\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n \t\telse\n \t\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n@@ -474,7 +520,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n \t\t\tremote_name = _(\"<remote>\");\n \n \t\tfprintf_ln(stderr, _(\"There is no tracking information for the current branch.\"));\n-\t\tif (opt_rebase)\n+\t\tif (opt_rebase >= REBASE_TRUE)\n \t\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n \t\telse\n \t\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n@@ -962,18 +1008,19 @@ static int already_up_to_date(struct object_id *orig_head,\n \n static void show_advice_pull_non_ff(void)\n {\n-\tadvise(_(\"You have divergent branches and need to specify how to reconcile them.\\n\"\n-\t\t \"You can do so by running one of the following commands sometime before\\n\"\n-\t\t \"your next pull:\\n\"\n+\tadvise(_(\"The pull wasn't a fast-forward, you have to choose between\\n\"\n+\t\t \"a merge or a rebase:\\n\"\n+\t\t \"\\n\"\n+\t\t \"  git pull --merge\\n\"\n+\t\t \"  git pull --rebase\\n\"\n \t\t \"\\n\"\n-\t\t \"  git config pull.rebase false  # merge\\n\"\n-\t\t \"  git config pull.rebase true   # rebase\\n\"\n-\t\t \"  git config pull.ff only       # fast-forward only\\n\"\n+\t\t \"You can quell this message by running one of the following commands:\\n\"\n \t\t \"\\n\"\n-\t\t \"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n-\t\t \"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n-\t\t \"or --ff-only on the command line to override the configured default per\\n\"\n-\t\t \"invocation.\\n\"));\n+\t\t \"  git config --global pull.mode merge\\n\"\n+\t\t \"  git config --global pull.mode rebase\\n\"\n+\t\t \"  git config --global pull.mode fast-forward\\n\"\n+\t\t \"\\n\"\n+\t\t \"For more information check \\\"git help fast-forward\\\".\"));\n }\n \n int cmd_pull(int argc, const char **argv, const char *prefix)\n@@ -982,11 +1029,13 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tstruct oid_array merge_heads = OID_ARRAY_INIT;\n \tstruct object_id orig_head, curr_head;\n \tstruct object_id rebase_fork_point;\n-\tint rebase_unspecified = 0;\n \tint can_ff;\n \tint divergent;\n \tint ret;\n \n+\topt_ff = xstrdup_or_null(config_get_ff());\n+\topt_rebase = config_get_rebase();\n+\n \tif (!getenv(\"GIT_REFLOG_ACTION\"))\n \t\tset_reflog_message(argc, argv);\n \n@@ -1010,25 +1059,31 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \n \tparse_repo_refspecs(argc, argv, &repo, &refspecs);\n \n-\tif (!opt_ff) {\n-\t\topt_ff = xstrdup_or_null(config_get_ff());\n-\t\t/*\n-\t\t * A subtle point: opt_ff was set on the line above via\n-\t\t * reading from config.  opt_rebase, in contrast, is set\n-\t\t * before this point via command line options.  The setting\n-\t\t * of opt_rebase via reading from config (using\n-\t\t * config_get_rebase()) does not happen until later.  We\n-\t\t * are relying on the next if-condition happening before\n-\t\t * the config_get_rebase() call so that an explicit\n-\t\t * \"--rebase\" can override a config setting of\n-\t\t * pull.ff=only.\n-\t\t */\n-\t\tif (opt_rebase >= 0 && opt_ff && !strcmp(opt_ff, \"--ff-only\"))\n-\t\t\topt_ff = \"--ff\";\n-\t}\n+\t/*\n+\t * If the user has not specified --merge or --rebase, fetch pull.mode to override\n+\t * pull.rename.\n+\t */\n+\tif (!mode) {\n+\t\tmode = config_get_pull_mode(repo);\n \n-\tif (opt_rebase < 0)\n-\t\topt_rebase = config_get_rebase(&rebase_unspecified);\n+\t\tswitch (mode) {\n+\t\tcase PULL_MODE_MERGE:\n+\t\tcase PULL_MODE_FAST_FORWARD:\n+\t\t\topt_rebase = REBASE_FALSE;\n+\t\t\tbreak;\n+\t\tcase PULL_MODE_REBASE:\n+\t\t\t/* Do not oeverride other rebase modes */\n+\t\t\tif (opt_rebase < REBASE_TRUE)\n+\t\t\t\topt_rebase = REBASE_TRUE;\n+\t\t\tbreak;\n+\t\tcase PULL_MODE_DEFAULT:\n+\t\t\tif (opt_rebase > 0)\n+\t\t\t\tmode = opt_rebase >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tbreak;\n+\t\t}\n+\t}\n \n \tif (repo_read_index_unmerged(the_repository))\n \t\tdie_resolve_conflict(\"pull\");\n@@ -1039,7 +1094,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (get_oid(\"HEAD\", &orig_head))\n \t\toidclr(&orig_head);\n \n-\tif (opt_rebase) {\n+\tif (opt_rebase >= REBASE_TRUE) {\n \t\tif (opt_autostash == -1)\n \t\t\topt_autostash = config_autostash;\n \n@@ -1100,7 +1155,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\tgoto cleanup;\n \t}\n \tif (merge_heads.nr > 1) {\n-\t\tif (opt_rebase)\n+\t\tif (opt_rebase >= REBASE_TRUE)\n \t\t\tdie(_(\"Cannot rebase onto multiple branches.\"));\n \t\tif (opt_ff && !strcmp(opt_ff, \"--ff-only\"))\n \t\t\tdie(_(\"Cannot fast-forward to multiple branches.\"));\n@@ -1109,19 +1164,15 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tcan_ff = get_can_ff(&orig_head, &merge_heads);\n \tdivergent = !can_ff && !already_up_to_date(&orig_head, &merge_heads);\n \n-\t/* ff-only takes precedence over rebase */\n-\tif (opt_ff && !strcmp(opt_ff, \"--ff-only\")) {\n-\t\tif (divergent)\n+\tif (divergent) {\n+\t\tif (!mode)\n+\t\t\tshow_advice_pull_non_ff();\n+\n+\t\tif (!mode || mode == PULL_MODE_FAST_FORWARD)\n \t\t\tdie_ff_impossible();\n-\t\topt_rebase = REBASE_FALSE;\n-\t}\n-\t/* If no action specified and we can't fast forward, then warn. */\n-\tif (!opt_ff && rebase_unspecified && divergent) {\n-\t\tshow_advice_pull_non_ff();\n-\t\tdie(_(\"Need to specify how to reconcile divergent branches.\"));\n \t}\n \n-\tif (opt_rebase) {\n+\tif (opt_rebase >= REBASE_TRUE) {\n \t\tstruct object_id newbase;\n \t\tstruct object_id upstream;\n \t\tget_rebase_newbase_and_upstream(&newbase, &upstream, &curr_head,\ndiff --git a/rebase.c b/rebase.c\nindex 6775cddb28..4b9a5e2066 100644\n--- a/rebase.c\n+++ b/rebase.c\n@@ -34,3 +34,15 @@ enum rebase_type rebase_parse_value(const char *value)\n \n \treturn REBASE_INVALID;\n }\n+\n+enum pull_mode_type pull_mode_parse_value(const char *value)\n+{\n+\tif (!strcmp(value, \"merge\") || !strcmp(value, \"m\"))\n+\t\treturn PULL_MODE_MERGE;\n+\telse if (!strcmp(value, \"rebase\") || !strcmp(value, \"r\"))\n+\t\treturn PULL_MODE_REBASE;\n+\telse if (!strcmp(value, \"fast-forward\") || !strcmp(value, \"f\"))\n+\t\treturn PULL_MODE_FAST_FORWARD;\n+\n+\treturn PULL_MODE_INVALID;\n+}\ndiff --git a/rebase.h b/rebase.h\nindex 203b437282..bc2cd75f52 100644\n--- a/rebase.h\n+++ b/rebase.h\n@@ -3,7 +3,8 @@\n \n enum rebase_type {\n \tREBASE_INVALID = -1,\n-\tREBASE_FALSE = 0,\n+\tREBASE_DEFAULT = 0,\n+\tREBASE_FALSE,\n \tREBASE_TRUE,\n \tREBASE_MERGES,\n \tREBASE_INTERACTIVE\n@@ -11,4 +12,14 @@ enum rebase_type {\n \n enum rebase_type rebase_parse_value(const char *value);\n \n+enum pull_mode_type {\n+\tPULL_MODE_INVALID = -1,\n+\tPULL_MODE_DEFAULT = 0,\n+\tPULL_MODE_MERGE,\n+\tPULL_MODE_REBASE,\n+\tPULL_MODE_FAST_FORWARD\n+};\n+\n+enum pull_mode_type pull_mode_parse_value(const char *value);\n+\n #endif /* REBASE */\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex bd238d89b0..d1e3ecb68c 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -95,22 +95,22 @@ test_expect_success 'pull.rebase not set (not-fast-forward)' '\n test_expect_success 'pull.rebase not set and pull.ff=true (not-fast-forward)' '\n \tgit reset --hard c2 &&\n \ttest_config pull.ff true &&\n-\tgit pull . c1 2>err &&\n-\ttest_i18ngrep ! \"You have divergent branches\" err\n+\ttest_must_fail git pull . c1 2>err &&\n+\ttest_i18ngrep \"You have divergent branches\" err\n '\n \n test_expect_success 'pull.rebase not set and pull.ff=false (not-fast-forward)' '\n \tgit reset --hard c2 &&\n \ttest_config pull.ff false &&\n-\tgit pull . c1 2>err &&\n-\ttest_i18ngrep ! \"You have divergent branches\" err\n+\ttest_must_fail git pull . c1 2>err &&\n+\ttest_i18ngrep \"You have divergent branches\" err\n '\n \n test_expect_success 'pull.rebase not set and pull.ff=only (not-fast-forward)' '\n \tgit reset --hard c2 &&\n \ttest_config pull.ff only &&\n \ttest_must_fail git pull . c1 2>err &&\n-\ttest_i18ngrep ! \"You have divergent branches\" err\n+\ttest_i18ngrep \"You have divergent branches\" err\n '\n \n test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)' '\n@@ -127,20 +127,20 @@ test_expect_success 'pull.rebase not set and --no-rebase given (not-fast-forward\n \n test_expect_success 'pull.rebase not set and --ff given (not-fast-forward)' '\n \tgit reset --hard c2 &&\n-\tgit pull --ff . c1 2>err &&\n-\ttest_i18ngrep ! \"You have divergent branches\" err\n+\ttest_must_fail git pull --ff . c1 2>err &&\n+\ttest_i18ngrep \"You have divergent branches\" err\n '\n \n test_expect_success 'pull.rebase not set and --no-ff given (not-fast-forward)' '\n \tgit reset --hard c2 &&\n-\tgit pull --no-ff . c1 2>err &&\n-\ttest_i18ngrep ! \"You have divergent branches\" err\n+\ttest_must_fail git pull --no-ff . c1 2>err &&\n+\ttest_i18ngrep \"You have divergent branches\" err\n '\n \n test_expect_success 'pull.rebase not set and --ff-only given (not-fast-forward)' '\n \tgit reset --hard c2 &&\n \ttest_must_fail git pull --ff-only . c1 2>err &&\n-\ttest_i18ngrep ! \"You have divergent branches\" err\n+\ttest_i18ngrep \"You have divergent branches\" err\n '\n \n test_does_rebase () {\n@@ -210,11 +210,11 @@ test_attempts_fast_forward () {\n # (And related interaction of pull.ff=only with pull.rebase)\n #\n test_expect_success '--ff-only overrides --rebase' '\n-\ttest_attempts_fast_forward pull --rebase --ff-only\n+\ttest_does_fast_forward pull --rebase --ff-only\n '\n \n test_expect_success '--ff-only overrides --rebase even if first' '\n-\ttest_attempts_fast_forward pull --ff-only --rebase\n+\ttest_does_fast_forward pull --ff-only --rebase\n '\n \n test_expect_success '--ff-only overrides --no-rebase' '\n@@ -222,7 +222,7 @@ test_expect_success '--ff-only overrides --no-rebase' '\n '\n \n test_expect_success 'pull.ff=only overrides pull.rebase=true' '\n-\ttest_attempts_fast_forward -c pull.ff=only -c pull.rebase=true pull\n+\ttest_does_fast_forward -c pull.ff=only -c pull.rebase=true pull\n '\n \n test_expect_success 'pull.ff=only overrides pull.rebase=false' '\n@@ -253,7 +253,7 @@ test_expect_success 'pull.rebase=true overrides pull.ff=true' '\n \n # Group 3: command line flags take precedence over config\n test_expect_success '--ff-only takes precedence over pull.rebase=true' '\n-\ttest_attempts_fast_forward -c pull.rebase=true pull --ff-only\n+\ttest_does_fast_forward -c pull.rebase=true pull --ff-only\n '\n \n test_expect_success '--ff-only takes precedence over pull.rebase=false' '\n@@ -261,7 +261,7 @@ test_expect_success '--ff-only takes precedence over pull.rebase=false' '\n '\n \n test_expect_success '--no-rebase takes precedence over pull.ff=only' '\n-\ttest_falls_back_to_full_merge -c pull.ff=only pull --no-rebase\n+\ttest_attempts_fast_forward -c pull.ff=only pull --no-rebase\n '\n \n test_expect_success '--rebase takes precedence over pull.ff=only' '\n-- \n2.39.2\n\n"},{"id":"472843","messageId":"20230228140236.4175835-3-felipe.contreras@gmail.com","threadId":"59322","inReplyTo":"20230228140236.4175835-1-felipe.contreras@gmail.com","subject":"[RFC PATCH 2/2] pull: improve --rebase and pull.rebase interaction","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-02-28T14:02:36Z","receivedAt":"2023-02-28T14:02:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Currently --rebase without argument overrides pull.rebase:\n\n  git config pull.rebase merges\n  git pull --rebase\n\nUp until now this hasn't been a big issue, since user has not been\nforced to specify a merge, or a rebase. But with the introduction of\n--merge and pull.mode, the user could in theory have the following\nconfiguration:\n\n  git config pull.mode merge\n  git config pull.rebase merges\n\nIn such case, the user would expect:\n\n  git pull --rebase\n\nTo be the same as:\n\n  git pull --rebase=merges\n\nIf the user wants to override the configuration, she can do:\n\n  git pull --rebase=true\n\nCc: Tao Klerks <tao@klerks.biz>\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/pull.c | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 0531328e2f..3c9e7f0861 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -68,8 +68,14 @@ static int parse_opt_rebase(const struct option *opt, const char *arg, int unset\n \n \tif (arg)\n \t\t*value = parse_config_rebase(\"--rebase\", arg, 0);\n-\telse\n-\t\t*value = unset ? REBASE_FALSE : REBASE_TRUE;\n+\telse {\n+\t\tif (!unset) {\n+\t\t\t/* --rebase shouldn't override pull.rebase=merges (and others) */\n+\t\t\tif (*value < REBASE_TRUE)\n+\t\t\t\t*value = REBASE_TRUE;\n+\t\t} else\n+\t\t\t*value = REBASE_FALSE;\n+\t}\n \n \tif (*value > 0)\n \t\tmode = *value >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;\n-- \n2.39.2\n\n"},{"id":"472844","messageId":"20230228140236.4175835-1-felipe.contreras@gmail.com","threadId":"59322","inReplyTo":null,"subject":"[RFC PATCH 0/2] Example of pull.mode","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-02-28T14:02:34Z","receivedAt":"2023-02-28T14:02:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The discussions about the default mode of pull never end, since there's\nyet another discussion [1] about the relationship with\npull.rebase=merges I'm sending a condensed version of my git update\npatches [2] which were not properly considered, but they solve the\nissue.\n\nThe two configurations in particular that are currently broken are:\n\n    git config pull.mode merge\n    git config pull.rebase merges\n    git pull --rebase # same as --rebase=merges\n\nAnd:\n\n    git config pull.rebase true\n    git pull --no-ff # no mode specified\n\nThese patches are *not* meant to be merged, they are just meant as\nreference for the new discussion.\n\nI have the patches properly split and with tests, which I avoided here\nfor the sake of simplicity.\n\n[1] https://lore.kernel.org/git/pull.1474.git.1675614276549.gitgitgadget@gmail.com/\n[2] https://lore.kernel.org/git/20210705123209.1808663-1-felipe.contreras@gmail.com/\n\nFelipe Contreras (2):\n  Add pull.mode\n  pull: improve --rebase and pull.rebase interaction\n\n Documentation/config/branch.txt |   5 +\n Documentation/config/pull.txt   |   6 ++\n Documentation/git-pull.txt      |  15 +--\n builtin/pull.c                  | 157 ++++++++++++++++++++++----------\n rebase.c                        |  12 +++\n rebase.h                        |  13 ++-\n t/t7601-merge-pull-config.sh    |  30 +++---\n 7 files changed, 166 insertions(+), 72 deletions(-)\n\n-- \n2.39.2\n\n"}]}