{"thread":{"id":"54712","subject":"[PATCH v8 00/10] Reject non-ff pulls by default","startedAt":"2020-11-25T03:30:02Z","lastAt":"2020-12-02T05:05:40Z","messageCount":13,"participants":["Felipe Contreras","Philip Oakley"],"isPatch":true,"patchVersion":8,"patchTotal":10},"messages":[{"id":"410750","messageId":"20201125032938.786393-1-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":null,"subject":"[PATCH v8 00/10] Reject non-ff pulls by default","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:28Z","receivedAt":"2020-11-25T03:30:02Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This is an attempt to revive the old patch series [1].\n\nSince v7 of the series the pull.mode configuration is added, and the warning has been updated to\nmake use of that.\n\nWhat is missing from the old series is:\n  * Add git pull --merge\n  * Actually change the default to ff-only\n\nCheers.\n\n[1] https://lore.kernel.org/git/1398988808-29678-1-git-send-email-felipe.contreras@gmail.com/\n\n\nFelipe Contreras (10):\n  pull: refactor fast-forward check\n  pull: cleanup autostash check\n  pull: trivial cleanup\n  pull: move default warning\n  pull: display default warning only when non-ff\n  test: pull-options: revert unnecessary changes\n  rebase: add REBASE_DEFAULT\n  pull: add pull.mode\n  pull: add pull.mode=ff-only\n  pull: improve default warning\n\n Documentation/config/branch.txt |   6 ++\n Documentation/config/pull.txt   |   6 ++\n Documentation/git-pull.txt      |  17 ++++\n builtin/pull.c                  | 146 ++++++++++++++++++++++----------\n builtin/remote.c                |  22 ++++-\n rebase.c                        |  12 +++\n rebase.h                        |  13 ++-\n t/t5520-pull.sh                 |  92 ++++++++++++++++++++\n t/t5521-pull-options.sh         |  22 ++---\n t/t7601-merge-pull-config.sh    |  49 ++++-------\n 10 files changed, 297 insertions(+), 88 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"410751","messageId":"20201125032938.786393-3-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":"20201125032938.786393-1-felipe.contreras@gmail.com","subject":"[PATCH v8 02/10] pull: cleanup autostash check","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:30Z","receivedAt":"2020-11-25T03:30:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This essentially reverts commit f15e7cf5cc.\n\nOnce commit d9f15d37f1 introduced the autostash option for the merge\nmode, it's not necessary to skip the fast-forward run_merge() when\nautostash is set.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/pull.c | 15 ++++++---------\n 1 file changed, 6 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex e2de0d4c91..eaa268c559 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -926,7 +926,6 @@ 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 autostash;\n \n \tif (!getenv(\"GIT_REFLOG_ACTION\"))\n \t\tset_reflog_message(argc, argv);\n@@ -959,8 +958,8 @@ 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-\tautostash = config_autostash;\n \tif (opt_rebase) {\n+\t\tint autostash = config_autostash;\n \t\tif (opt_autostash != -1)\n \t\t\tautostash = opt_autostash;\n \n@@ -1029,13 +1028,11 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) &&\n \t\t    submodule_touches_in_range(the_repository, &rebase_fork_point, &curr_head))\n \t\t\tdie(_(\"cannot rebase with locally recorded submodule modifications\"));\n-\t\tif (!autostash) {\n-\t\t\tif (get_can_ff(&orig_head, &merge_heads.oid[0])) {\n-\t\t\t\t/* we can fast-forward this without invoking rebase */\n-\t\t\t\topt_ff = \"--ff-only\";\n-\t\t\t\tran_ff = 1;\n-\t\t\t\tret = run_merge();\n-\t\t\t}\n+\t\tif (get_can_ff(&orig_head, &merge_heads.oid[0])) {\n+\t\t\t/* we can fast-forward this without invoking rebase */\n+\t\t\topt_ff = \"--ff-only\";\n+\t\t\tran_ff = 1;\n+\t\t\tret = run_merge();\n \t\t}\n \t\tif (!ran_ff)\n \t\t\tret = run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);\n-- \n2.29.2\n\n"},{"id":"410752","messageId":"20201125032938.786393-4-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":"20201125032938.786393-1-felipe.contreras@gmail.com","subject":"[PATCH v8 03/10] pull: trivial cleanup","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:31Z","receivedAt":"2020-11-25T03:30:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no need to store ran_ff. Now it's obvious from the conditionals.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/pull.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex eaa268c559..121b9fb0e1 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1023,19 +1023,18 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \n \tif (opt_rebase) {\n \t\tint ret = 0;\n-\t\tint ran_ff = 0;\n \t\tif ((recurse_submodules == RECURSE_SUBMODULES_ON ||\n \t\t     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) &&\n \t\t    submodule_touches_in_range(the_repository, &rebase_fork_point, &curr_head))\n \t\t\tdie(_(\"cannot rebase with locally recorded submodule modifications\"));\n+\n \t\tif (get_can_ff(&orig_head, &merge_heads.oid[0])) {\n \t\t\t/* we can fast-forward this without invoking rebase */\n \t\t\topt_ff = \"--ff-only\";\n-\t\t\tran_ff = 1;\n \t\t\tret = run_merge();\n-\t\t}\n-\t\tif (!ran_ff)\n+\t\t} else {\n \t\t\tret = run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);\n+\t\t}\n \n \t\tif (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||\n \t\t\t     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))\n-- \n2.29.2\n\n"},{"id":"410753","messageId":"20201125032938.786393-6-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":"20201125032938.786393-1-felipe.contreras@gmail.com","subject":"[PATCH v8 05/10] pull: display default warning only when non-ff","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:33Z","receivedAt":"2020-11-25T03:30:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no need to display the annoying warning on every pull... only\nthe ones that are not fast-forward.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/pull.c               |  2 +-\n t/t7601-merge-pull-config.sh | 28 +++++++++++++++++-----------\n 2 files changed, 18 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 9a0f7937c2..479bdaf0ee 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1013,7 +1013,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \n \tcan_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);\n \n-\tif (default_mode && opt_verbosity >= 0 && !opt_ff) {\n+\tif (default_mode && !can_ff && opt_verbosity >= 0 && !opt_ff) {\n \t\twarning(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n \t\t\t\"discouraged. You can squelch this message by running one of the following\\n\"\n \t\t\t\"commands sometime before your next pull:\\n\"\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex c5c4ea5fc0..02e2a9b2c6 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -28,59 +28,65 @@ test_expect_success 'setup' '\n '\n \n test_expect_success 'pull.rebase not set' '\n-\tgit reset --hard c0 &&\n+\tgit reset --hard c2 &&\n \tgit pull . c1 2>err &&\n \ttest_i18ngrep \"Pulling without specifying how to reconcile\" err\n '\n \n-test_expect_success 'pull.rebase not set and pull.ff=true' '\n+test_expect_success 'pull.rebase not set (fast-forward)' '\n \tgit reset --hard c0 &&\n+\tgit pull . c1 2>err &&\n+\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n+'\n+\n+test_expect_success 'pull.rebase not set and pull.ff=true' '\n+\tgit reset --hard c2 &&\n \ttest_config pull.ff true &&\n \tgit pull . c1 2>err &&\n \ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n '\n \n test_expect_success 'pull.rebase not set and pull.ff=false' '\n-\tgit reset --hard c0 &&\n+\tgit reset --hard c2 &&\n \ttest_config pull.ff false &&\n \tgit pull . c1 2>err &&\n \ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n '\n \n test_expect_success 'pull.rebase not set and pull.ff=only' '\n-\tgit reset --hard c0 &&\n+\tgit reset --hard c2 &&\n \ttest_config pull.ff only &&\n-\tgit pull . c1 2>err &&\n+\ttest_must_fail git pull . c1 2>err &&\n \ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n '\n \n test_expect_success 'pull.rebase not set and --rebase given' '\n-\tgit reset --hard c0 &&\n+\tgit reset --hard c2 &&\n \tgit pull --rebase . c1 2>err &&\n \ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n '\n \n test_expect_success 'pull.rebase not set and --no-rebase given' '\n-\tgit reset --hard c0 &&\n+\tgit reset --hard c2 &&\n \tgit pull --no-rebase . c1 2>err &&\n \ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n '\n \n test_expect_success 'pull.rebase not set and --ff given' '\n-\tgit reset --hard c0 &&\n+\tgit reset --hard c2 &&\n \tgit pull --ff . c1 2>err &&\n \ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n '\n \n test_expect_success 'pull.rebase not set and --no-ff given' '\n-\tgit reset --hard c0 &&\n+\tgit reset --hard c2 &&\n \tgit pull --no-ff . c1 2>err &&\n \ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n '\n \n test_expect_success 'pull.rebase not set and --ff-only given' '\n-\tgit reset --hard c0 &&\n-\tgit pull --ff-only . c1 2>err &&\n+\tgit reset --hard c2 &&\n+\ttest_must_fail git pull --ff-only . c1 2>err &&\n \ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n '\n \n-- \n2.29.2\n\n"},{"id":"410755","messageId":"20201125032938.786393-5-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":"20201125032938.786393-1-felipe.contreras@gmail.com","subject":"[PATCH v8 04/10] pull: move default warning","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:32Z","receivedAt":"2020-11-25T03:30:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Up to the point where can check if we can fast-forward or not.\n\nNo functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/pull.c | 37 ++++++++++++++++++++++---------------\n 1 file changed, 22 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 121b9fb0e1..9a0f7937c2 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -27,6 +27,8 @@\n #include \"commit-reach.h\"\n #include \"sequencer.h\"\n \n+static int default_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@@ -344,20 +346,7 @@ static enum rebase_type config_get_rebase(void)\n \tif (!git_config_get_value(\"pull.rebase\", &value))\n \t\treturn parse_config_rebase(\"pull.rebase\", value, 1);\n \n-\tif (opt_verbosity >= 0 && !opt_ff) {\n-\t\twarning(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n-\t\t\t\"discouraged. You can squelch this message by running one of the following\\n\"\n-\t\t\t\"commands sometime before your next pull:\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"  git config pull.rebase false  # merge (the default strategy)\\n\"\n-\t\t\t\"  git config pull.rebase true   # rebase\\n\"\n-\t\t\t\"  git config pull.ff only       # fast-forward only\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n-\t\t\t\"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n-\t\t\t\"or --ff-only on the command line to override the configured default per\\n\"\n-\t\t\t\"invocation.\\n\"));\n-\t}\n+\tdefault_mode = 1;\n \n \treturn REBASE_FALSE;\n }\n@@ -926,6 +915,7 @@ 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 can_ff;\n \n \tif (!getenv(\"GIT_REFLOG_ACTION\"))\n \t\tset_reflog_message(argc, argv);\n@@ -1021,6 +1011,23 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (opt_rebase && merge_heads.nr > 1)\n \t\tdie(_(\"Cannot rebase onto multiple branches.\"));\n \n+\tcan_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);\n+\n+\tif (default_mode && opt_verbosity >= 0 && !opt_ff) {\n+\t\twarning(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n+\t\t\t\"discouraged. You can squelch this message by running one of the following\\n\"\n+\t\t\t\"commands sometime before your next pull:\\n\"\n+\t\t\t\"\\n\"\n+\t\t\t\"  git config pull.rebase false  # merge (the default strategy)\\n\"\n+\t\t\t\"  git config pull.rebase true   # rebase\\n\"\n+\t\t\t\"  git config pull.ff only       # fast-forward only\\n\"\n+\t\t\t\"\\n\"\n+\t\t\t\"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n+\t\t\t\"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n+\t\t\t\"or --ff-only on the command line to override the configured default per\\n\"\n+\t\t\t\"invocation.\\n\"));\n+\t}\n+\n \tif (opt_rebase) {\n \t\tint ret = 0;\n \t\tif ((recurse_submodules == RECURSE_SUBMODULES_ON ||\n@@ -1028,7 +1035,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t    submodule_touches_in_range(the_repository, &rebase_fork_point, &curr_head))\n \t\t\tdie(_(\"cannot rebase with locally recorded submodule modifications\"));\n \n-\t\tif (get_can_ff(&orig_head, &merge_heads.oid[0])) {\n+\t\tif (can_ff) {\n \t\t\t/* we can fast-forward this without invoking rebase */\n \t\t\topt_ff = \"--ff-only\";\n \t\t\tret = run_merge();\n-- \n2.29.2\n\n"},{"id":"410754","messageId":"20201125032938.786393-8-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":"20201125032938.786393-1-felipe.contreras@gmail.com","subject":"[PATCH v8 07/10] rebase: add REBASE_DEFAULT","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:35Z","receivedAt":"2020-11-25T03:30:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"So that we can distinguish when the user has forced an option.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/pull.c | 20 ++++++++------------\n rebase.h       |  3 ++-\n 2 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 479bdaf0ee..0df7d27343 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -27,8 +27,6 @@\n #include \"commit-reach.h\"\n #include \"sequencer.h\"\n \n-static int default_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@@ -346,9 +344,7 @@ static enum rebase_type config_get_rebase(void)\n \tif (!git_config_get_value(\"pull.rebase\", &value))\n \t\treturn parse_config_rebase(\"pull.rebase\", value, 1);\n \n-\tdefault_mode = 1;\n-\n-\treturn REBASE_FALSE;\n+\treturn REBASE_DEFAULT;\n }\n \n /**\n@@ -443,7 +439,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@@ -456,7 +452,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@@ -471,7 +467,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@@ -948,7 +944,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\tint autostash = config_autostash;\n \t\tif (opt_autostash != -1)\n \t\t\tautostash = opt_autostash;\n@@ -1008,12 +1004,12 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"Cannot merge multiple branches into empty head.\"));\n \t\treturn pull_into_void(merge_heads.oid, &curr_head);\n \t}\n-\tif (opt_rebase && merge_heads.nr > 1)\n+\tif (opt_rebase >= REBASE_TRUE && merge_heads.nr > 1)\n \t\tdie(_(\"Cannot rebase onto multiple branches.\"));\n \n \tcan_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);\n \n-\tif (default_mode && !can_ff && opt_verbosity >= 0 && !opt_ff) {\n+\tif (!opt_rebase && !can_ff && opt_verbosity >= 0 && !opt_ff) {\n \t\twarning(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n \t\t\t\"discouraged. You can squelch this message by running one of the following\\n\"\n \t\t\t\"commands sometime before your next pull:\\n\"\n@@ -1028,7 +1024,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\t\"invocation.\\n\"));\n \t}\n \n-\tif (opt_rebase) {\n+\tif (opt_rebase >= REBASE_TRUE) {\n \t\tint ret = 0;\n \t\tif ((recurse_submodules == RECURSE_SUBMODULES_ON ||\n \t\t     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) &&\ndiff --git a/rebase.h b/rebase.h\nindex cc723d4748..34d4acfd74 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_PRESERVE,\n \tREBASE_MERGES,\n-- \n2.29.2\n\n"},{"id":"410756","messageId":"20201125032938.786393-7-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":"20201125032938.786393-1-felipe.contreras@gmail.com","subject":"[PATCH v8 06/10] test: pull-options: revert unnecessary changes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:34Z","receivedAt":"2020-11-25T03:30:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Commit d18c950a69 changed these tests, but it's unclear why. Probably\nbecause earlier versions of the patch series died instead of printing a\nwarning.\n\nCc: Alex Henrie <alexhenrie24@gmail.com>\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t5521-pull-options.sh | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex db1a381cd9..1a4fe2583a 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -11,10 +11,10 @@ test_expect_success 'setup' '\n \t git commit -m one)\n '\n \n-test_expect_success 'git pull -q --no-rebase' '\n+test_expect_success 'git pull -q' '\n \tmkdir clonedq &&\n \t(cd clonedq && git init &&\n-\tgit pull -q --no-rebase \"../parent\" >out 2>err &&\n+\tgit pull -q \"../parent\" >out 2>err &&\n \ttest_must_be_empty err &&\n \ttest_must_be_empty out)\n '\n@@ -30,10 +30,10 @@ test_expect_success 'git pull -q --rebase' '\n \ttest_must_be_empty out)\n '\n \n-test_expect_success 'git pull --no-rebase' '\n+test_expect_success 'git pull' '\n \tmkdir cloned &&\n \t(cd cloned && git init &&\n-\tgit pull --no-rebase \"../parent\" >out 2>err &&\n+\tgit pull \"../parent\" >out 2>err &&\n \ttest -s err &&\n \ttest_must_be_empty out)\n '\n@@ -46,10 +46,10 @@ test_expect_success 'git pull --rebase' '\n \ttest_must_be_empty out)\n '\n \n-test_expect_success 'git pull -v --no-rebase' '\n+test_expect_success 'git pull -v' '\n \tmkdir clonedv &&\n \t(cd clonedv && git init &&\n-\tgit pull -v --no-rebase \"../parent\" >out 2>err &&\n+\tgit pull -v \"../parent\" >out 2>err &&\n \ttest -s err &&\n \ttest_must_be_empty out)\n '\n@@ -62,25 +62,25 @@ test_expect_success 'git pull -v --rebase' '\n \ttest_must_be_empty out)\n '\n \n-test_expect_success 'git pull -v -q --no-rebase' '\n+test_expect_success 'git pull -v -q' '\n \tmkdir clonedvq &&\n \t(cd clonedvq && git init &&\n-\tgit pull -v -q --no-rebase \"../parent\" >out 2>err &&\n+\tgit pull -v -q \"../parent\" >out 2>err &&\n \ttest_must_be_empty out &&\n \ttest_must_be_empty err)\n '\n \n-test_expect_success 'git pull -q -v --no-rebase' '\n+test_expect_success 'git pull -q -v' '\n \tmkdir clonedqv &&\n \t(cd clonedqv && git init &&\n-\tgit pull -q -v --no-rebase \"../parent\" >out 2>err &&\n+\tgit pull -q -v \"../parent\" >out 2>err &&\n \ttest_must_be_empty out &&\n \ttest -s err)\n '\n test_expect_success 'git pull --cleanup errors early on invalid argument' '\n \tmkdir clonedcleanup &&\n \t(cd clonedcleanup && git init &&\n-\ttest_must_fail git pull --no-rebase --cleanup invalid \"../parent\" >out 2>err &&\n+\ttest_must_fail git pull --cleanup invalid \"../parent\" >out 2>err &&\n \ttest_must_be_empty out &&\n \ttest -s err)\n '\n-- \n2.29.2\n\n"},{"id":"410757","messageId":"20201125032938.786393-9-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":"20201125032938.786393-1-felipe.contreras@gmail.com","subject":"[PATCH v8 08/10] pull: add pull.mode","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:36Z","receivedAt":"2020-11-25T03:30:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This way we can add more modes and the default can be something else,\nnamely it can be set to \"ff-only\", so eventually we can reject\nnon-fast-forward merges by default.\n\nAlso 'branch.<name>.pullmode'.\n\nFor now we simply override the rebase mode.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/config/branch.txt |  6 ++++\n Documentation/config/pull.txt   |  6 ++++\n builtin/pull.c                  | 57 ++++++++++++++++++++++++++++++---\n builtin/remote.c                | 19 ++++++++++-\n rebase.c                        | 10 ++++++\n rebase.h                        |  9 ++++++\n t/t5520-pull.sh                 | 42 ++++++++++++++++++++++++\n t/t7601-merge-pull-config.sh    |  7 ++++\n 8 files changed, 151 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt\nindex cc5f3249fc..94510a5184 100644\n--- a/Documentation/config/branch.txt\n+++ b/Documentation/config/branch.txt\n@@ -96,6 +96,12 @@ 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 if it would either merge or\n+\trebase the fetched branch. The possible values are 'merge',\n+\t'rebase'. See \"pull.mode\" for doing this in a non\n+\tbranch-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 5404830609..2606515fe4 100644\n--- a/Documentation/config/pull.txt\n+++ b/Documentation/config/pull.txt\n@@ -29,6 +29,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+\tand 'rebase'. See \"branch.<name>.pullmode\" for doing\n+\tthis in a non branch-specific manner.\n+\n pull.octopus::\n \tThe default merge strategy to use when pulling multiple branches\n \tat once.\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 0df7d27343..feb9a7f6ee 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -49,6 +49,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@@ -75,6 +83,7 @@ static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n \n /* Options passed to git-merge or git-rebase */\n static enum rebase_type opt_rebase = -1;\n+static enum pull_mode_type mode;\n static char *opt_diffstat;\n static char *opt_log;\n static char *opt_signoff;\n@@ -347,6 +356,29 @@ static enum rebase_type config_get_rebase(void)\n \treturn REBASE_DEFAULT;\n }\n \n+static enum pull_mode_type config_get_pull_mode(void)\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 PULL_MODE_DEFAULT;\n+}\n+\n /**\n  * Read config variables.\n  */\n@@ -932,8 +964,25 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (!opt_ff)\n \t\topt_ff = xstrdup_or_null(config_get_ff());\n \n-\tif (opt_rebase < 0)\n-\t\topt_rebase = config_get_rebase();\n+\tif (opt_rebase < 0) {\n+\t\tmode = config_get_pull_mode();\n+\t\tswitch (mode) {\n+\t\tcase PULL_MODE_MERGE:\n+\t\t\topt_rebase = REBASE_FALSE;\n+\t\t\tbreak;\n+\t\tcase PULL_MODE_REBASE:\n+\t\t\topt_rebase = config_get_rebase();\n+\t\t\tif (!opt_rebase) opt_rebase = REBASE_TRUE;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\topt_rebase = config_get_rebase();\n+\t\t}\n+\t}\n+\n+\tif (!mode) {\n+\t\tif (opt_rebase)\n+\t\t\tmode = opt_rebase >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;\n+\t}\n \n \tif (read_cache_unmerged())\n \t\tdie_resolve_conflict(\"pull\");\n@@ -1014,8 +1063,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\t\"discouraged. You can squelch this message by running one of the following\\n\"\n \t\t\t\"commands sometime before your next pull:\\n\"\n \t\t\t\"\\n\"\n-\t\t\t\"  git config pull.rebase false  # merge (the default strategy)\\n\"\n-\t\t\t\"  git config pull.rebase true   # rebase\\n\"\n+\t\t\t\"  git config pull.mode merge    # (the default strategy)\\n\"\n+\t\t\t\"  git config pull.mode rebase\\n\"\n \t\t\t\"  git config pull.ff only       # fast-forward only\\n\"\n \t\t\t\"\\n\"\n \t\t\t\"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex c1b211b272..51b1e675e3 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -269,7 +269,7 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \tchar *name;\n \tstruct string_list_item *item;\n \tstruct branch_info *info;\n-\tenum { REMOTE, MERGE, REBASE, PUSH_REMOTE } type;\n+\tenum { REMOTE, MERGE, REBASE, PUSH_REMOTE, PULL_MODE } type;\n \tsize_t key_len;\n \n \tif (!starts_with(key, \"branch.\"))\n@@ -284,6 +284,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \t\ttype = REBASE;\n \telse if (strip_suffix(key, \".pushremote\", &key_len))\n \t\ttype = PUSH_REMOTE;\n+\telse if (strip_suffix(key, \".pullmode\", &key_len))\n+\t\ttype = PULL_MODE;\n \telse\n \t\treturn 0;\n \tname = xmemdupz(key, key_len);\n@@ -324,6 +326,21 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \t\t\twarning(_(\"more than one %s\"), orig_key);\n \t\tinfo->push_remote_name = xstrdup(value);\n \t\tbreak;\n+\tcase PULL_MODE: {\n+\t\tint mode = pull_mode_parse_value(value);\n+\t\tswitch (mode) {\n+\t\tcase PULL_MODE_MERGE:\n+\t\t\tinfo->rebase = REBASE_FALSE;\n+\t\t\tbreak;\n+\t\tcase PULL_MODE_REBASE:\n+\t\t\tinfo->rebase = REBASE_TRUE;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tinfo->rebase = REBASE_INVALID;\n+\t\t\tbreak;\n+\t\t}\n+\t\tbreak;\n+\t}\n \tdefault:\n \t\tBUG(\"unexpected type=%d\", type);\n \t}\ndiff --git a/rebase.c b/rebase.c\nindex f8137d859b..bdfca49886 100644\n--- a/rebase.c\n+++ b/rebase.c\n@@ -33,3 +33,13 @@ 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+\n+\treturn PULL_MODE_INVALID;\n+}\ndiff --git a/rebase.h b/rebase.h\nindex 34d4acfd74..5ab8f4ddd5 100644\n--- a/rebase.h\n+++ b/rebase.h\n@@ -13,4 +13,13 @@ 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+};\n+\n+enum pull_mode_type pull_mode_parse_value(const char *value);\n+\n #endif /* REBASE */\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 9fae07cdfa..eb0086bd1c 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -449,6 +449,16 @@ test_expect_success 'pull.rebase' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'pull.mode rebase' '\n+\tgit reset --hard before-rebase &&\n+\ttest_config pull.mode rebase &&\n+\tgit pull . copy &&\n+\ttest_cmp_rev HEAD^ copy &&\n+\techo new >expect &&\n+\tgit show HEAD:file2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'pull --autostash & pull.rebase=true' '\n \ttest_config pull.rebase true &&\n \ttest_pull_autostash 1 --autostash\n@@ -523,6 +533,17 @@ test_expect_success 'pull.rebase=false create a new merge commit' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'pull.mode=merge create a new merge commit' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.mode merge &&\n+\tgit pull . copy &&\n+\ttest_cmp_rev HEAD^1 before-preserve-rebase &&\n+\ttest_cmp_rev HEAD^2 copy &&\n+\techo file3 >expect &&\n+\tgit show HEAD:file3.t >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'pull.rebase=true flattens keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase true &&\n@@ -552,6 +573,16 @@ test_expect_success REBASE_P \\\n \ttest_cmp_rev HEAD^2 keep-merge\n '\n \n+test_expect_success REBASE_P \\\n+\t'pull.rebase=preserve rebases and merges keep-merge with pull.mode' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.mode rebase &&\n+\ttest_config pull.rebase preserve &&\n+\tgit pull . copy &&\n+\ttest_cmp_rev HEAD^^ copy &&\n+\ttest_cmp_rev HEAD^2 keep-merge\n+'\n+\n test_expect_success 'pull.rebase=interactive' '\n \twrite_script \"$TRASH_DIRECTORY/fake-editor\" <<-\\EOF &&\n \techo I was here >fake.out &&\n@@ -593,6 +624,17 @@ test_expect_success '--rebase=false create a new merge commit' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--rebase=false create a new merge commit with pull.mode' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.mode rebase &&\n+\tgit pull --rebase=false . copy &&\n+\ttest_cmp_rev HEAD^1 before-preserve-rebase &&\n+\ttest_cmp_rev HEAD^2 copy &&\n+\techo file3 >expect &&\n+\tgit show HEAD:file3.t >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '--rebase=true rebases and flattens keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase preserve &&\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex 02e2a9b2c6..4a36ad30e2 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -39,6 +39,13 @@ test_expect_success 'pull.rebase not set (fast-forward)' '\n \ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n '\n \n+test_expect_success 'pull.mode set' '\n+\tgit reset --hard c2 &&\n+\ttest_config pull.mode merge &&\n+\tgit pull . c1 2>err &&\n+\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n+'\n+\n test_expect_success 'pull.rebase not set and pull.ff=true' '\n \tgit reset --hard c2 &&\n \ttest_config pull.ff true &&\n-- \n2.29.2\n\n"},{"id":"410758","messageId":"20201125032938.786393-10-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":"20201125032938.786393-1-felipe.contreras@gmail.com","subject":"[PATCH v8 09/10] pull: add pull.mode=ff-only","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:37Z","receivedAt":"2020-11-25T03:30:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It is very typical for Git newcomers to inadvertently create merges and\nworse; inadvertently pushing them. This is one of the reasons many\nexperienced users prefer to avoid 'git pull', and recommend newcomers to\navoid it as well.\n\nTo avoid these problems, and keep 'git pull' useful, it has been\nsuggested that 'git pull' barfs by default if the merge is\nnon-fast-forward, which unfortunately would break backwards\ncompatibility.\n\nThis patch leaves everything in place to enable this new mode, but it\nonly gets enabled if the user specifically configures it;\n\n  pull.mode = ff-only.\n\nLater on this mode can be enabled by default.\n\nFor the full discussion you can read:\n\nhttps://lore.kernel.org/git/5363BB9F.40102@xiplink.com/\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/config/branch.txt |  2 +-\n Documentation/config/pull.txt   |  2 +-\n builtin/pull.c                  | 10 +++++++--\n builtin/remote.c                |  3 +++\n rebase.c                        |  2 ++\n rebase.h                        |  3 ++-\n t/t5520-pull.sh                 | 36 +++++++++++++++++++++++++++++++++\n 7 files changed, 53 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt\nindex 94510a5184..39f60cd8f7 100644\n--- a/Documentation/config/branch.txt\n+++ b/Documentation/config/branch.txt\n@@ -99,7 +99,7 @@ for details).\n branch.<name>.pullmode::\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'. See \"pull.mode\" for doing this in a non\n+\t'rebase', and 'ff-only'. See \"pull.mode\" for doing this in a non\n \tbranch-specific manner.\n \n branch.<name>.description::\ndiff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt\nindex 2606515fe4..daa14f1170 100644\n--- a/Documentation/config/pull.txt\n+++ b/Documentation/config/pull.txt\n@@ -32,7 +32,7 @@ for details).\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-\tand 'rebase'. See \"branch.<name>.pullmode\" for doing\n+\t'rebase', and 'ff-only'. See \"branch.<name>.pullmode\" for doing\n \tthis in a non branch-specific manner.\n \n pull.octopus::\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex feb9a7f6ee..3aa7f56142 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -980,7 +980,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (!mode) {\n-\t\tif (opt_rebase)\n+\t\tif (opt_ff && !strcmp(opt_ff, \"--ff-only\"))\n+\t\t\tmode = PULL_MODE_FF_ONLY;\n+\t\telse if (opt_rebase)\n \t\t\tmode = opt_rebase >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;\n \t}\n \n@@ -1065,7 +1067,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\t\"\\n\"\n \t\t\t\"  git config pull.mode merge    # (the default strategy)\\n\"\n \t\t\t\"  git config pull.mode rebase\\n\"\n-\t\t\t\"  git config pull.ff only       # fast-forward only\\n\"\n+\t\t\t\"  git config pull.mode ff-only  # fast-forward only\\n\"\n \t\t\t\"\\n\"\n \t\t\t\"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n \t\t\t\"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n@@ -1073,6 +1075,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\t\"invocation.\\n\"));\n \t}\n \n+\tif (mode == PULL_MODE_FF_ONLY && !can_ff)\n+\t\tdie(_(\"The pull was not fast-forward, please either merge or rebase.\\n\"\n+\t\t\t\"If unsure, run 'git pull --no-rebase'.\"));\n+\n \tif (opt_rebase >= REBASE_TRUE) {\n \t\tint ret = 0;\n \t\tif ((recurse_submodules == RECURSE_SUBMODULES_ON ||\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 51b1e675e3..34d3ea9d38 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -335,6 +335,9 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \t\tcase PULL_MODE_REBASE:\n \t\t\tinfo->rebase = REBASE_TRUE;\n \t\t\tbreak;\n+\t\tcase PULL_MODE_FF_ONLY:\n+\t\t\tinfo->rebase = REBASE_TRUE;\n+\t\t\tbreak;\n \t\tdefault:\n \t\t\tinfo->rebase = REBASE_INVALID;\n \t\t\tbreak;\ndiff --git a/rebase.c b/rebase.c\nindex bdfca49886..c6233e888f 100644\n--- a/rebase.c\n+++ b/rebase.c\n@@ -40,6 +40,8 @@ enum pull_mode_type pull_mode_parse_value(const char *value)\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, \"ff-only\") || !strcmp(value, \"f\"))\n+\t\treturn PULL_MODE_FF_ONLY;\n \n \treturn PULL_MODE_INVALID;\n }\ndiff --git a/rebase.h b/rebase.h\nindex 5ab8f4ddd5..432bcb55c4 100644\n--- a/rebase.h\n+++ b/rebase.h\n@@ -17,7 +17,8 @@ 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_REBASE,\n+\tPULL_MODE_FF_ONLY\n };\n \n enum pull_mode_type pull_mode_parse_value(const char *value);\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex eb0086bd1c..ba78c16d73 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -861,4 +861,40 @@ test_expect_success 'git pull --rebase against local branch' '\n \ttest_cmp expect file2\n '\n \n+test_expect_success 'git pull fast-forward (ff-only)' '\n+\ttest_when_finished \"git checkout master && git branch -D other test\" &&\n+\ttest_config pull.mode ff-only &&\n+\tgit checkout -b other master &&\n+\t>new &&\n+\tgit add new &&\n+\tgit commit -m new &&\n+\tgit checkout -b test -t other &&\n+\tgit reset --hard master &&\n+\tgit pull\n+'\n+\n+test_expect_success 'git pull non-fast-forward (ff-only)' '\n+\ttest_when_finished \"git checkout master && git branch -D other test\" &&\n+\ttest_config pull.mode ff-only &&\n+\tgit checkout -b other master^ &&\n+\t>new &&\n+\tgit add new &&\n+\tgit commit -m new &&\n+\tgit checkout -b test -t other &&\n+\tgit reset --hard master &&\n+\ttest_must_fail git pull\n+'\n+\n+test_expect_success 'git pull non-fast-forward with merge (ff-only)' '\n+\ttest_when_finished \"git checkout master && git branch -D other test\" &&\n+\ttest_config pull.mode ff-only &&\n+\tgit checkout -b other master^ &&\n+\t>new &&\n+\tgit add new &&\n+\tgit commit -m new &&\n+\tgit checkout -b test -t other &&\n+\tgit reset --hard master &&\n+\tgit pull --no-rebase\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"410759","messageId":"20201125032938.786393-2-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":"20201125032938.786393-1-felipe.contreras@gmail.com","subject":"[PATCH v8 01/10] pull: refactor fast-forward check","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:29Z","receivedAt":"2020-11-25T03:30:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This way we will be able to do the check unconditionally (merge or\nrebase).\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/pull.c | 26 +++++++++++++++-----------\n 1 file changed, 15 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 17aa63cd35..e2de0d4c91 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -906,6 +906,20 @@ static int run_rebase(const struct object_id *curr_head,\n \treturn ret;\n }\n \n+static int get_can_ff(struct object_id *orig_head, struct object_id *orig_merge_head)\n+{\n+\tint ret;\n+\tstruct commit_list *list = NULL;\n+\tstruct commit *merge_head, *head;\n+\n+\thead = lookup_commit_reference(the_repository, orig_head);\n+\tcommit_list_insert(head, &list);\n+\tmerge_head = lookup_commit_reference(the_repository, orig_merge_head);\n+\tret = repo_is_descendant_of(the_repository, merge_head, list);\n+\tfree_commit_list(list);\n+\treturn ret;\n+}\n+\n int cmd_pull(int argc, const char **argv, const char *prefix)\n {\n \tconst char *repo, **refspecs;\n@@ -1016,22 +1030,12 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t    submodule_touches_in_range(the_repository, &rebase_fork_point, &curr_head))\n \t\t\tdie(_(\"cannot rebase with locally recorded submodule modifications\"));\n \t\tif (!autostash) {\n-\t\t\tstruct commit_list *list = NULL;\n-\t\t\tstruct commit *merge_head, *head;\n-\n-\t\t\thead = lookup_commit_reference(the_repository,\n-\t\t\t\t\t\t       &orig_head);\n-\t\t\tcommit_list_insert(head, &list);\n-\t\t\tmerge_head = lookup_commit_reference(the_repository,\n-\t\t\t\t\t\t\t     &merge_heads.oid[0]);\n-\t\t\tif (repo_is_descendant_of(the_repository,\n-\t\t\t\t\t\t  merge_head, list)) {\n+\t\t\tif (get_can_ff(&orig_head, &merge_heads.oid[0])) {\n \t\t\t\t/* we can fast-forward this without invoking rebase */\n \t\t\t\topt_ff = \"--ff-only\";\n \t\t\t\tran_ff = 1;\n \t\t\t\tret = run_merge();\n \t\t\t}\n-\t\t\tfree_commit_list(list);\n \t\t}\n \t\tif (!ran_ff)\n \t\t\tret = run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);\n-- \n2.29.2\n\n"},{"id":"410760","messageId":"20201125032938.786393-11-felipe.contreras@gmail.com","threadId":"54712","inReplyTo":"20201125032938.786393-1-felipe.contreras@gmail.com","subject":"[PATCH v8 10/10] pull: improve default warning","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:29:38Z","receivedAt":"2020-11-25T03:30:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Also, use pull.mode to determine when to show it, and --ff/--no-ff\nshouldn't squelch the warning.\n\nAlso, improve the documentation so \"git pull --help\" actually does\nexplain what's going on.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-pull.txt   | 17 +++++++++++++++\n builtin/pull.c               | 34 +++++++++++++++---------------\n t/t5520-pull.sh              | 14 +++++++++++++\n t/t7601-merge-pull-config.sh | 40 +++++++-----------------------------\n 4 files changed, 55 insertions(+), 50 deletions(-)\n\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex 5c3fb67c01..ad33d2472c 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -38,6 +38,20 @@ as set by linkgit:git-branch[1] `--track`.\n Assume the following history exists and the current branch is\n \"`master`\":\n \n+------------\n+\t  A---B---C master on origin\n+\t /\n+    D---E master\n+------------\n+\n+Then `git pull` will merge in a fast-foward way up to the new master.\n+\n+------------\n+    D---E---A---B---C master, origin/master\n+------------\n+\n+However, a non-fast-foward case looks very different.\n+\n ------------\n \t  A---B---C master on origin\n \t /\n@@ -46,6 +60,9 @@ Assume the following history exists and the current branch is\n \torigin/master in your repository\n ------------\n \n+By default `git pull` will warn about these situations, however, most likely\n+you would want to force a merge, which you can do with `git pull --no-rebase`.\n+\n Then \"`git pull`\" will fetch and replay the changes from the remote\n `master` branch since it diverged from the local `master` (i.e., `E`)\n until its current commit (`C`) on top of `master` and record the\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 3aa7f56142..8e577b6322 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1060,25 +1060,25 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \n \tcan_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);\n \n-\tif (!opt_rebase && !can_ff && opt_verbosity >= 0 && !opt_ff) {\n-\t\twarning(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n-\t\t\t\"discouraged. You can squelch this message by running one of the following\\n\"\n-\t\t\t\"commands sometime before your next pull:\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"  git config pull.mode merge    # (the default strategy)\\n\"\n-\t\t\t\"  git config pull.mode rebase\\n\"\n-\t\t\t\"  git config pull.mode ff-only  # fast-forward only\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n-\t\t\t\"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n-\t\t\t\"or --ff-only on the command line to override the configured default per\\n\"\n-\t\t\t\"invocation.\\n\"));\n+\tif (!can_ff) {\n+\t\tif (mode == PULL_MODE_FF_ONLY)\n+\t\t\tdie(_(\"The pull was not fast-forward, please either merge or rebase.\\n\"\n+\t\t\t\t\"If unsure, run 'git pull --no-rebase'.\"));\n+\t\telse if (!mode && opt_verbosity >= 0) {\n+\t\t\twarning(_(\"The pull was not fast-forward, in the future you will have to choose a merge, or a rebase.\\n\"\n+\t\t\t\t\"To squelch this message and maintain the current behavior, use:\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"  git config --global pull.mode merge\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"To squelch this message and adopt the new behavior now, use:\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"  git config --global push.mode ff-only\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"Falling back to old style for now (merge).\\n\"\n+\t\t\t\t\"Read 'git pull --help' for more information.\"));\n+\t\t}\n \t}\n \n-\tif (mode == PULL_MODE_FF_ONLY && !can_ff)\n-\t\tdie(_(\"The pull was not fast-forward, please either merge or rebase.\\n\"\n-\t\t\t\"If unsure, run 'git pull --no-rebase'.\"));\n-\n \tif (opt_rebase >= REBASE_TRUE) {\n \t\tint ret = 0;\n \t\tif ((recurse_submodules == RECURSE_SUBMODULES_ON ||\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex ba78c16d73..29d44d000e 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -897,4 +897,18 @@ test_expect_success 'git pull non-fast-forward with merge (ff-only)' '\n \tgit pull --no-rebase\n '\n \n+test_expect_success 'git pull non-fast-forward (default)' '\n+\ttest_when_finished \"git checkout master && git branch -D other test\" &&\n+\tgit checkout -b other master^ &&\n+\t>new &&\n+\tgit add new &&\n+\tgit commit -m new &&\n+\tgit checkout -b test -t other &&\n+\tgit reset --hard master &&\n+\tgit pull 2> error &&\n+\tcat error &&\n+\tgrep -q \"The pull was not fast-forward\" error &&\n+\tgrep -q \"in the future you will have to choose\" error\n+'\n+\n test_done\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex 4a36ad30e2..8a93b37d87 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -30,71 +30,45 @@ test_expect_success 'setup' '\n test_expect_success 'pull.rebase not set' '\n \tgit reset --hard c2 &&\n \tgit pull . c1 2>err &&\n-\ttest_i18ngrep \"Pulling without specifying how to reconcile\" err\n+\ttest_i18ngrep \"choose a merge, or a rebase\" err\n '\n \n test_expect_success 'pull.rebase not set (fast-forward)' '\n \tgit reset --hard c0 &&\n \tgit pull . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n+\ttest_i18ngrep ! \"choose a merge, or a rebase\" err\n '\n \n test_expect_success 'pull.mode set' '\n \tgit reset --hard c2 &&\n \ttest_config pull.mode merge &&\n \tgit pull . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n-'\n-\n-test_expect_success 'pull.rebase not set and pull.ff=true' '\n-\tgit reset --hard c2 &&\n-\ttest_config pull.ff true &&\n-\tgit pull . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n-'\n-\n-test_expect_success 'pull.rebase not set and pull.ff=false' '\n-\tgit reset --hard c2 &&\n-\ttest_config pull.ff false &&\n-\tgit pull . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n+\ttest_i18ngrep ! \"choose a merge, or a rebase\" err\n '\n \n test_expect_success 'pull.rebase not set and pull.ff=only' '\n \tgit reset --hard c2 &&\n \ttest_config pull.ff only &&\n \ttest_must_fail git pull . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n+\ttest_i18ngrep ! \"choose a merge, or a rebase\" err\n '\n \n test_expect_success 'pull.rebase not set and --rebase given' '\n \tgit reset --hard c2 &&\n \tgit pull --rebase . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n+\ttest_i18ngrep ! \"choose a merge, or a rebase\" err\n '\n \n test_expect_success 'pull.rebase not set and --no-rebase given' '\n \tgit reset --hard c2 &&\n \tgit pull --no-rebase . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n-'\n-\n-test_expect_success 'pull.rebase not set and --ff given' '\n-\tgit reset --hard c2 &&\n-\tgit pull --ff . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n-'\n-\n-test_expect_success 'pull.rebase not set and --no-ff given' '\n-\tgit reset --hard c2 &&\n-\tgit pull --no-ff . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n+\ttest_i18ngrep ! \"choose a merge, or a rebase\" err\n '\n \n test_expect_success 'pull.rebase not set and --ff-only given' '\n \tgit reset --hard c2 &&\n \ttest_must_fail git pull --ff-only . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n+\ttest_i18ngrep ! \"choose a merge, or a rebase\" err\n '\n \n test_expect_success 'merge c1 with c2' '\n-- \n2.29.2\n\n"},{"id":"410779","messageId":"a3be85cd-a01a-4ed9-bf98-09785e79fb85@iee.email","threadId":"54712","inReplyTo":"20201125032938.786393-9-felipe.contreras@gmail.com","subject":"Re: [PATCH v8 08/10] pull: add pull.mode","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-11-25T12:31:22Z","receivedAt":"2020-11-25T12:31:28Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Accidental repetition of phrasing from branch.<name>.pullmode ?\n\nOn 25/11/2020 03:29, Felipe Contreras wrote:\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> +\tand 'rebase'. See \"branch.<name>.pullmode\" for doing\n> +\tthis in a non branch-specific manner.\n> +\ns/non//\n\nSame problem in 09/10 I think.\nI didn't check the code... Spotted while browsing.\n--\nPhilip\n"},{"id":"411141","messageId":"CAMP44s0VuP+GkrcyNtQnTerZOiMm4f1qSzuizHgsMddHqOvjXg@mail.gmail.com","threadId":"54712","inReplyTo":"a3be85cd-a01a-4ed9-bf98-09785e79fb85@iee.email","subject":"Re: [PATCH v8 08/10] pull: add pull.mode","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-02T05:04:42Z","receivedAt":"2020-12-02T05:05:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 25, 2020 at 6:31 AM Philip Oakley <philipoakley@iee.email> wrote:\n>\n> Accidental repetition of phrasing from branch.<name>.pullmode ?\n>\n> On 25/11/2020 03:29, Felipe Contreras wrote:\n> > +pull.mode::\n> > +     When \"git pull\" is run, this determines if it would either merge or\n> > +     rebase the fetched branch. The possible values are 'merge',\n> > +     and 'rebase'. See \"branch.<name>.pullmode\" for doing\n> > +     this in a non branch-specific manner.\n> > +\n> s/non//\n>\n> Same problem in 09/10 I think.\n> I didn't check the code... Spotted while browsing.\n\nWeird. I could swear I copied the text from pull.rebase. Anyway, now I\ndid: \"see branch.<name>.pullmode\" for setting this on a per-branch\nbasis.\"\n\n-- \nFelipe Contreras\n"}]}