{"thread":{"id":"54788","subject":"[PATCH v4 01/19] doc: pull: explain what is a fast-forward","startedAt":"2020-12-08T00:27:34Z","lastAt":"2020-12-08T00:58:26Z","messageCount":21,"participants":["Felipe Contreras"],"isPatch":true,"patchVersion":4,"patchTotal":19},"messages":[{"id":"411684","messageId":"20201208002648.1370414-2-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 01/19] doc: pull: explain what is a fast-forward","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:30Z","receivedAt":"2020-12-08T00:27:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We want users to know what is a fast-forward in order to understand the\ndefault warning.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/git-pull.txt | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex 5c3fb67c01..e1605a81b3 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-forward way up to the new master.\n+\n+------------\n+ D---E---A---B---C master, origin/master\n+------------\n+\n+However, a non-fast-forward case looks very different.\n+\n ------------\n \t A---B---C master on origin\n \t /\n-- \n2.29.2\n\n"},{"id":"411685","messageId":"20201208002648.1370414-3-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 02/19] pull: improve default warning","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:31Z","receivedAt":"2020-12-08T00:27:36Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We want to:\n\n1. Be clear about what \"specifying\" means; merge, or rebase.\n2. Mention a direct shortcut for people that just want to get on with\n their lives: git pull --no-rebase.\n3. Have a quick reference for users to understand what this\n \"fast-forward\" business means.\n\nThis patch does all three.\n\nThanks to the previous patch now \"git pull --help\" explains what a\nfast-forward is, and a further patch changes --no-rebase to --merge so\nit's actually user friendly.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/pull.c | 24 ++++++++++++------------\n 1 file changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex aa56ebcdd0..d6e707f8f9 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -345,18 +345,18 @@ static enum rebase_type config_get_rebase(void)\n \t\treturn parse_config_rebase(\"pull.rebase\", value, 1);\n \n \tif (opt_verbosity >= 0 && !opt_ff) {\n-\t\tadvise(_(\"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\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n+\t\t\t\"you need to specify if you want a merge, or a rebase.\\n\"\n+\t\t\t\"You can squelch this message by running one of the following commands:\\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.\\n\"\n+\t\t\t\"If unsure, run \\\"git pull --no-rebase\\\".\\n\"\n+\t\t\t\"Read \\\"git pull --help\\\" for more information.\"));\n \t}\n \n \treturn REBASE_FALSE;\n-- \n2.29.2\n\n"},{"id":"411686","messageId":"20201208002648.1370414-4-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 03/19] pull: refactor fast-forward check","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:32Z","receivedAt":"2020-12-08T00:27:37Z","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 \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 d6e707f8f9..e2964a0b5f 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -924,6 +924,20 @@ static int run_rebase(const struct object_id *newbase,\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@@ -1040,22 +1054,12 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t submodule_touches_in_range(the_repository, &upstream, &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(&newbase, &upstream);\n-- \n2.29.2\n\n"},{"id":"411687","messageId":"20201208002648.1370414-5-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 04/19] pull: cleanup autostash check","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:33Z","receivedAt":"2020-12-08T00:27:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Currently \"git pull --rebase\" takes a shortcut in the case a\nfast-forward merge is possible; run_merge() is called with --ff-only.\n\nHowever, \"git merge\" didn't have an --autostash option, so, when \"git\npull --rebase --autostash\" was called *and* the fast-forward merge\nshortcut was taken, then the pull failed.\n\nThis was fixed in commit f15e7cf5cc (pull: ff --rebase --autostash\nworks in dirty repo, 2017-06-01) by simply skipping the fast-forward\nmerge shortcut.\n\nLater on \"git merge\" learned the --autostash option [a03b55530a\n(merge: teach --autostash option, 2020-04-07)], and so did \"git pull\"\n[d9f15d37f1 (pull: pass --autostash to merge, 2020-04-07)].\n\nTherefore it's not necessary to skip the fast-forward merge shortcut\nanymore when called with --rebase --autostash.\n\nLet's always take the fast-forward merge shortcut by essentially\nreverting f15e7cf5cc.\n\nReviewed-by: Elijah Newren \nSigned-off-by: Felipe Contreras \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 e2964a0b5f..d44ca2ffde 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -944,7 +944,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@@ -977,8 +976,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@@ -1053,13 +1052,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, &upstream, &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(&newbase, &upstream);\n-- \n2.29.2\n\n"},{"id":"411688","messageId":"20201208002648.1370414-1-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":null,"subject":"[PATCH v4 00/19] pull: default ff-only mode","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:29Z","receivedAt":"2020-12-08T00:27:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The old thread \"Pull is Mostly Evil\" [1] came to haunt us again.\n\nThis is my latest attempt to try to fix it.\n\nThis patch series is divided in 3 parts:\n\n== Part I ==\n\n1. Improve the current documentation\n2. Improve the current default warning\n3. Minimize the instances where we display the default warning\n4. Add a --merge option\n5. Fix the behavior of the warning with --ff\n\n== Part II ==\n\n1. Introduce pull.mode\n2. Introduce pull.mode=ff-only\n3. Advice on future changes, and suggest changing pull.mode\n\n== Part III ==\n\n1. Change the default mode to pull.mode=ff-only\n\nv3 of the series only did part I, and the interdiff is only of that part.\n\nSince then the change in semantics of --ff-only is dropped, because\nthat solution didn't pan out, and now I'm sending the one I think\nit will: \"pull.mode=ff-only\".\n\nPlus a fixed typo, and fixed a merge conflict with the latest master\n(not in the interdiff).\n\nIf this is a bit overwhelming, you can check the branches in my gitlab[2].\n\n * fc/pull/improvements (part 1)\n * fc/pull/ff-only (part 2)\n * fc/pull/ff-only-switch (part 3)\n\n[1] https://lore.kernel.org/git/5363BB9F.40102@xiplink.com/\n[2] https://gitlab.com/felipec/git/\n\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex c220da143a..21b50aff77 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -44,13 +44,13 @@ Assume the following history exists and the current branch is\n D---E master\n ------------\n \n-Then `git pull` will merge in a fast-foward way up to the new master.\n+Then `git pull` will merge in a fast-forward 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+However, a non-fast-forward case looks very different.\n \n ------------\n \t A---B---C master on origin\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 0735c77f42..97a7657473 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -112,24 +112,6 @@ static int opt_show_forced_updates = -1;\n static char *set_upstream;\n static struct strvec opt_fetch = STRVEC_INIT;\n \n-static int parse_opt_ff_only(const struct option *opt, const char *arg, int unset)\n-{\n-\tchar **value = opt->value;\n-\topt_rebase = REBASE_DEFAULT;\n-\tfree(*value);\n-\t*value = xstrdup_or_null(\"--ff-only\");\n-\treturn 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-\tfree(opt_ff);\n-\topt_ff = NULL;\n-\t*value = REBASE_FALSE;\n-\treturn 0;\n-}\n-\n static struct option pull_options[] = {\n \t/* Shared options */\n \tOPT__VERBOSITY(&opt_verbosity),\n@@ -147,9 +129,8 @@ static struct option pull_options[] = {\n \t\t\"(false|true|merges|preserve|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_SET_INT('m', \"merge\", &opt_rebase,\n+\t\tN_(\"incorporate changes by merging\"), REBASE_FALSE),\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@@ -178,9 +159,9 @@ static struct option pull_options[] = {\n \tOPT_PASSTHRU(0, \"ff\", &opt_ff, NULL,\n \t\tN_(\"allow fast-forward\"),\n \t\tPARSE_OPT_NOARG),\n-\tOPT_CALLBACK_F(0, \"ff-only\", &opt_ff, NULL,\n+\tOPT_PASSTHRU(0, \"ff-only\", &opt_ff, NULL,\n \t\tN_(\"abort if fast-forward is not possible\"),\n-\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_ff_only),\n+\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG),\n \tOPT_PASSTHRU(0, \"verify-signatures\", &opt_verify_signatures, NULL,\n \t\tN_(\"verify that the named commit has a valid GPG signature\"),\n \t\tPARSE_OPT_NOARG),\n@@ -1027,25 +1008,19 @@ 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 (!can_ff && !opt_rebase) {\n-\t\tif (opt_ff && !strcmp(opt_ff, \"--ff-only\"))\n-\t\t\tdie(_(\"The pull was not fast-forward, please either merge or rebase.\"));\n-\n-\t\tif (opt_verbosity >= 0 && (!opt_ff || !strcmp(opt_ff, \"--ff\"))) {\n-\t\t\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n-\t\t\t\t\"you need to specify if you want a merge, a rebase, or a fast-forward.\\n\"\n-\t\t\t\t\"You can squelch this message by running one of the following commands:\\n\"\n-\t\t\t\t\"\\n\"\n-\t\t\t\t\" git config pull.rebase false # merge (the default strategy)\\n\"\n-\t\t\t\t\" git config pull.rebase true # rebase\\n\"\n-\t\t\t\t\" git config pull.ff only # fast-forward only\\n\"\n-\t\t\t\t\"\\n\"\n-\t\t\t\t\"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n-\t\t\t\t\"preference for all repositories.\\n\"\n-\t\t\t\t\"If unsure, run \\\"git pull --merge\\\".\\n\"\n-\t\t\t\t\"Read \\\"git pull --help\\\" for more information.\"\n-\t\t\t\t));\n-\t\t}\n+\tif (!opt_rebase && !can_ff && opt_verbosity >= 0 && (!opt_ff || !strcmp(opt_ff, \"--ff\"))) {\n+\t\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n+\t\t\t\"you need to specify if you want a merge, or a rebase.\\n\"\n+\t\t\t\"You can squelch this message by running one of the following commands:\\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.\\n\"\n+\t\t\t\"If unsure, run \\\"git pull --merge\\\".\\n\"\n+\t\t\t\"Read \\\"git pull --help\\\" for more information.\"));\n \t}\n \n \tif (opt_rebase >= REBASE_TRUE) {\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 0cdac4010b..9fae07cdfa 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -819,89 +819,4 @@ test_expect_success 'git pull --rebase against local branch' '\n \ttest_cmp expect file2\n '\n \n-setup_other () {\n-\ttest_when_finished \"git checkout master && git branch -D other test\" &&\n-\tgit checkout -b other $1 &&\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-}\n-\n-setup_ff () {\n-\tsetup_other master\n-}\n-\n-setup_non_ff () {\n-\tsetup_other master^\n-}\n-\n-test_expect_success 'fast-forward (ff-only)' '\n-\ttest_config pull.ff only &&\n-\tsetup_ff &&\n-\tgit pull\n-'\n-\n-test_expect_success 'non-fast-forward (ff-only)' '\n-\ttest_config pull.ff only &&\n-\tsetup_non_ff &&\n-\ttest_must_fail git pull\n-'\n-\n-test_expect_success 'non-fast-forward with merge (ff-only)' '\n-\ttest_config pull.ff only &&\n-\tsetup_non_ff &&\n-\tgit pull --merge\n-'\n-\n-test_expect_success 'non-fast-forward with rebase (ff-only)' '\n-\ttest_config pull.ff only &&\n-\tsetup_non_ff &&\n-\tgit pull --rebase\n-'\n-\n-test_expect_success 'non-fast-forward error message (ff-only)' '\n-\ttest_config pull.ff only &&\n-\tsetup_non_ff &&\n-\ttest_must_fail git pull 2> error &&\n-\tcat error &&\n-\tgrep -q \"The pull was not fast-forward\" error\n-'\n-\n-test_expect_success '--merge overrides --ff-only' '\n-\tsetup_non_ff &&\n-\tgit pull --ff-only --merge\n-'\n-\n-test_expect_success '--rebase overrides --ff-only' '\n-\tsetup_non_ff &&\n-\tgit pull --ff-only --rebase\n-'\n-\n-test_expect_success '--ff-only overrides --merge' '\n-\tsetup_non_ff &&\n-\ttest_must_fail git pull --merge --ff-only\n-'\n-\n-test_expect_success '--ff-only overrides pull.rebase=false' '\n-\ttest_config pull.rebase false &&\n-\tsetup_non_ff &&\n-\ttest_must_fail git pull --ff-only\n-'\n-\n-test_expect_success 'pull.rebase=true overrides pull.ff=only' '\n-\ttest_config pull.ff only &&\n-\ttest_config pull.rebase true &&\n-\tsetup_non_ff &&\n-\tgit pull\n-'\n-\n-test_expect_success 'pull.rebase=false overrides pull.ff=only' '\n-\ttest_config pull.ff only &&\n-\ttest_config pull.rebase false &&\n-\tsetup_non_ff &&\n-\ttest_must_fail git pull\n-'\n-\n test_done\n\n\nFelipe Contreras (19):\n doc: pull: explain what is a fast-forward\n pull: improve default warning\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 pull: trivial whitespace style fix\n pull: introduce --merge option\n pull: show warning with --ff\n rebase: add REBASE_DEFAULT\n pull: move configurations fetches\n test: merge-pull-config: trivial cleanup\n test: pull-options: revert unnecessary changes\n pull: trivial memory fix\n pull: add pull.mode\n pull: add pull.mode=ff-only\n pull: advice of future changes\n future: pull: enable ff-only mode by default\n\n Documentation/config/branch.txt | 6 ++\n Documentation/config/pull.txt | 6 ++\n Documentation/git-pull.txt | 24 ++++-\n builtin/pull.c | 157 +++++++++++++++++++++-----------\n builtin/remote.c | 22 ++++-\n rebase.c | 12 +++\n rebase.h | 13 ++-\n t/t5520-pull.sh | 87 ++++++++++++++++++\n t/t5521-pull-options.sh | 22 ++---\n t/t7601-merge-pull-config.sh | 60 ------------\n 10 files changed, 282 insertions(+), 127 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"411689","messageId":"20201208002648.1370414-7-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 06/19] pull: move default warning","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:35Z","receivedAt":"2020-12-08T00:28:14Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Eventually we want to be able to display the warning only when\nfast-forward merges are not possible.\n\nIn order to do so we need to move the default warning up to the point\nwhere we can check if we can fast-forward or not.\n\nAdditionally, config_get_rebase() was probably never its true home.\n\nThis requires a temporary variable to check if we are in the\n\"default mode\" (no --rebase or --no-rebase specified). But this is only\ntemporary; another patch in the series gets rid of that.\n\nSigned-off-by: Felipe Contreras \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 6aeca4aeae..3b84ebf100 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\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n-\t\t\t\"you need to specify if you want a merge, or a rebase.\\n\"\n-\t\t\t\"You can squelch this message by running one of the following commands:\\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.\\n\"\n-\t\t\t\"If unsure, run \\\"git pull --no-rebase\\\".\\n\"\n-\t\t\t\"Read \\\"git pull --help\\\" for more information.\"));\n-\t}\n+\tdefault_mode = 1;\n \n \treturn REBASE_FALSE;\n }\n@@ -944,6 +933,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@@ -1039,6 +1029,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\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n+\t\t\t\"you need to specify if you want a merge, or a rebase.\\n\"\n+\t\t\t\"You can squelch this message by running one of the following commands:\\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.\\n\"\n+\t\t\t\"If unsure, run \\\"git pull --no-rebase\\\".\\n\"\n+\t\t\t\"Read \\\"git pull --help\\\" for more information.\"));\n+\t}\n+\n \tif (opt_rebase) {\n \t\tint ret = 0;\n \n@@ -1052,7 +1059,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t submodule_touches_in_range(the_repository, &upstream, &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":"411690","messageId":"20201208002648.1370414-6-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 05/19] pull: trivial cleanup","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:34Z","receivedAt":"2020-12-08T00:28:15Z","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 \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 d44ca2ffde..6aeca4aeae 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1041,7 +1041,6 @@ 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 \n \t\tstruct object_id newbase;\n \t\tstruct object_id upstream;\n@@ -1052,14 +1051,14 @@ 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, &upstream, &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(&newbase, &upstream);\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":"411691","messageId":"20201208002648.1370414-8-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 07/19] pull: display default warning only when non-ff","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:36Z","receivedAt":"2020-12-08T00:28:16Z","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\nThis requires the tests to pick another base, so the merge is not\nfast-forward. And in the cases where --ff-only is specified add\ntest_must_fail (since now they are non-fast-forward).\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/git-pull.txt | 3 +++\n builtin/pull.c | 2 +-\n t/t7601-merge-pull-config.sh | 28 +++++++++++++++++-----------\n 3 files changed, 21 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex e1605a81b3..2fb184ab5f 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -60,6 +60,9 @@ However, a non-fast-forward case looks very different.\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 3b84ebf100..ab410d675f 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1031,7 +1031,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\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n \t\t\t\"you need to specify if you want a merge, or a rebase.\\n\"\n \t\t\t\"You can squelch this message by running one of the following commands:\\n\"\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex 6774e9d86f..6b4adab8b1 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -28,7 +28,7 @@ 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 -c color.advice=always pull . c1 2>err &&\n \ttest_decode_color decoded &&\n \ttest_i18ngrep \"hint: \" decoded &&\n@@ -36,54 +36,60 @@ test_expect_success 'pull.rebase not set' '\n \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":"411692","messageId":"20201208002648.1370414-9-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 08/19] pull: trivial whitespace style fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:37Z","receivedAt":"2020-12-08T00:28:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Two spaces unaligned to anything is not part of the coding-style. A\nsingle tab is.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/pull.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex ab410d675f..4c91dd291b 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -128,9 +128,9 @@ static struct option pull_options[] = {\n \t/* Options passed to git-merge or git-rebase */\n \tOPT_GROUP(N_(\"Options related to merging\")),\n \tOPT_CALLBACK_F('r', \"rebase\", &opt_rebase,\n-\t \"(false|true|merges|preserve|interactive)\",\n-\t N_(\"incorporate changes by rebasing rather than merging\"),\n-\t PARSE_OPT_OPTARG, parse_opt_rebase),\n+\t\t\"(false|true|merges|preserve|interactive)\",\n+\t\tN_(\"incorporate changes by rebasing rather than merging\"),\n+\t\tPARSE_OPT_OPTARG, parse_opt_rebase),\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-- \n2.29.2\n\n"},{"id":"411693","messageId":"20201208002648.1370414-10-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 09/19] pull: introduce --merge option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:38Z","receivedAt":"2020-12-08T00:28:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Previously --no-rebase (which still works for backwards compatibility).\n\nNow we can update the default warning, and the git-pull(1) man page to\nuse --merge instead of the non-intuitive --no-rebase.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/git-pull.txt | 9 ++++++---\n builtin/pull.c | 4 +++-\n t/t7601-merge-pull-config.sh | 4 ++--\n 3 files changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex 2fb184ab5f..21b50aff77 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -61,7 +61,7 @@ However, a non-fast-forward case looks very different.\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+you would want to force a merge, which you can do with `git pull --merge`.\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@@ -148,8 +148,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-\tOverride earlier --rebase.\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 4c91dd291b..3874434421 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -131,6 +131,8 @@ static struct option pull_options[] = {\n \t\t\"(false|true|merges|preserve|interactive)\",\n \t\tN_(\"incorporate changes by rebasing rather than merging\"),\n \t\tPARSE_OPT_OPTARG, parse_opt_rebase),\n+\tOPT_SET_INT('m', \"merge\", &opt_rebase,\n+\t\tN_(\"incorporate changes by merging\"), REBASE_FALSE),\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@@ -1042,7 +1044,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\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.\\n\"\n-\t\t\t\"If unsure, run \\\"git pull --no-rebase\\\".\\n\"\n+\t\t\t\"If unsure, run \\\"git pull --merge\\\".\\n\"\n \t\t\t\"Read \\\"git pull --help\\\" for more information.\"));\n \t}\n \ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex 6b4adab8b1..1de64e6cc5 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -69,9 +69,9 @@ test_expect_success 'pull.rebase not set and --rebase given' '\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+test_expect_success 'pull.rebase not set and --merge given' '\n \tgit reset --hard c2 &&\n-\tgit pull --no-rebase . c1 2>err &&\n+\tgit pull --merge . c1 2>err &&\n \ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n '\n \n-- \n2.29.2\n\n"},{"id":"411694","messageId":"20201208002648.1370414-11-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 10/19] pull: show warning with --ff","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:39Z","receivedAt":"2020-12-08T00:28:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It's unclear why --ff should remove the warning, since:\n\n git pull --ff\n\nIs implicitly the same as:\n\n git pull\n\nUnless of course pull.ff is specified otherwise.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/pull.c | 2 +-\n t/t7601-merge-pull-config.sh | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 3874434421..f5bdae680f 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1033,7 +1033,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 && !can_ff && opt_verbosity >= 0 && !opt_ff) {\n+\tif (default_mode && !can_ff && opt_verbosity >= 0 && (!opt_ff || !strcmp(opt_ff, \"--ff\"))) {\n \t\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n \t\t\t\"you need to specify if you want a merge, or a rebase.\\n\"\n \t\t\t\"You can squelch this message by running one of the following commands:\\n\"\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex 1de64e6cc5..d709799f8b 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -46,7 +46,7 @@ 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+\ttest_i18ngrep \"Pulling without specifying how to reconcile\" err\n '\n \n test_expect_success 'pull.rebase not set and pull.ff=false' '\n@@ -78,7 +78,7 @@ test_expect_success 'pull.rebase not set and --merge given' '\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+\ttest_i18ngrep \"Pulling without specifying how to reconcile\" err\n '\n \n test_expect_success 'pull.rebase not set and --no-ff given' '\n-- \n2.29.2\n\n"},{"id":"411695","messageId":"20201208002648.1370414-12-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 11/19] rebase: add REBASE_DEFAULT","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:40Z","receivedAt":"2020-12-08T00:28:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"By introducing a default we can distinguish when the user has forced an\noption.\n\nTherefore there's no need for an extra \"default_mode\" variable (it's the\nsame as opt_rebase == REBASE_DEFAULT), not is there any need to\ninitialize opt_rebase to an invalid value.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/pull.c | 24 ++++++++++--------------\n rebase.h | 3 ++-\n 2 files changed, 12 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex f5bdae680f..350df6f3c5 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@@ -76,7 +74,7 @@ static char *opt_progress;\n 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 rebase_type opt_rebase;\n static char *opt_diffstat;\n static char *opt_log;\n static char *opt_signoff;\n@@ -348,9 +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-\tdefault_mode = 1;\n-\n-\treturn REBASE_FALSE;\n+\treturn REBASE_DEFAULT;\n }\n \n /**\n@@ -445,7 +441,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@@ -458,7 +454,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@@ -473,7 +469,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n \t\t\tremote_name = _(\"\");\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@@ -956,7 +952,7 @@ 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+\tif (!opt_rebase)\n \t\topt_rebase = config_get_rebase();\n \n \tif (read_cache_unmerged())\n@@ -968,7 +964,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@@ -1028,12 +1024,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 || !strcmp(opt_ff, \"--ff\"))) {\n+\tif (!opt_rebase && !can_ff && opt_verbosity >= 0 && (!opt_ff || !strcmp(opt_ff, \"--ff\"))) {\n \t\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n \t\t\t\"you need to specify if you want a merge, or a rebase.\\n\"\n \t\t\t\"You can squelch this message by running one of the following commands:\\n\"\n@@ -1048,7 +1044,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\t\"Read \\\"git pull --help\\\" for more information.\"));\n \t}\n \n-\tif (opt_rebase) {\n+\tif (opt_rebase >= REBASE_TRUE) {\n \t\tint ret = 0;\n \n \t\tstruct object_id newbase;\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":"411696","messageId":"20201208002648.1370414-13-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 12/19] pull: move configurations fetches","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:41Z","receivedAt":"2020-12-08T00:28:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Now that we have FETCH_DEFAULT we can fetch the configuration before\nparsing the argument options.\n\nThe options will override the configuration, and if they don't;\nopt_rebase will remain being FETCH_DEFAULT.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/pull.c | 9 +++------\n 1 file changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 350df6f3c5..addb454e63 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -933,6 +933,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tstruct object_id rebase_fork_point;\n \tint can_ff;\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@@ -949,12 +952,6 @@ 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-\n-\tif (!opt_rebase)\n-\t\topt_rebase = config_get_rebase();\n-\n \tif (read_cache_unmerged())\n \t\tdie_resolve_conflict(\"pull\");\n \n-- \n2.29.2\n\n"},{"id":"411697","messageId":"20201208002648.1370414-15-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 14/19] test: pull-options: revert unnecessary changes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:43Z","receivedAt":"2020-12-08T00:28:27Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Commit d18c950a69 (pull: warn if the user didn't say whether to\nrebase or to merge, 2020-03-09) changed a number of tests in t5521\nand added some new tests in t7601, but it was not explained why the\nchanges in t5521 were made.\n\nThe reason seems to be to silence the warnings while running the tests,\nbut we want to see the warnings if they happen.\n\nCc: Alex Henrie \nSigned-off-by: Felipe Contreras \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":"411698","messageId":"20201208002648.1370414-16-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 15/19] pull: trivial memory fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:44Z","receivedAt":"2020-12-08T00:28:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The opt_ff variable is supposed to have an allocated string (strdup), we\ncan't just overwrite it with a const char *.\n\nFunctionally it doesn't matter, since after this point opt_ff is never\nfreed, only accessed, but still...\n\nIt's better to be consistent.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/pull.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex addb454e63..118fbdeb62 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1056,7 +1056,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \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\tfree(opt_ff);\n+\t\t\topt_ff = xstrdup_or_null(\"--ff-only\");\n \t\t\tret = run_merge();\n \t\t} else {\n \t\t\tret = run_rebase(&newbase, &upstream);\n-- \n2.29.2\n\n"},{"id":"411699","messageId":"20201208002648.1370414-14-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 13/19] test: merge-pull-config: trivial cleanup","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:42Z","receivedAt":"2020-12-08T00:28:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Commit e01ae2a4a7 introduced an extra space.\n\nSigned-off-by: Felipe Contreras \n---\n t/t7601-merge-pull-config.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex d709799f8b..8a6aae564a 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -33,7 +33,6 @@ test_expect_success 'pull.rebase not set' '\n \ttest_decode_color decoded &&\n \ttest_i18ngrep \"hint: \" decoded &&\n \ttest_i18ngrep \"Pulling without specifying how to reconcile\" decoded\n-\n '\n \n test_expect_success 'pull.rebase not set (fast-forward)' '\n-- \n2.29.2\n\n"},{"id":"411700","messageId":"20201208002648.1370414-18-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 17/19] pull: add pull.mode=ff-only","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:46Z","receivedAt":"2020-12-08T00:28:57Z","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; pushing them. This is one of the reasons many experienced users\nprefer to avoid 'git pull', and recommend newcomers to avoid it as well.\n\nTo escape 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 \n---\n Documentation/config/branch.txt | 4 +--\n Documentation/config/pull.txt | 4 +--\n builtin/pull.c | 5 +++-\n builtin/remote.c | 3 +++\n rebase.c | 2 ++\n rebase.h | 3 ++-\n t/t5520-pull.sh | 45 +++++++++++++++++++++++++++++++++\n 7 files changed, 60 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt\nindex b57bf2c308..39f60cd8f7 100644\n--- a/Documentation/config/branch.txt\n+++ b/Documentation/config/branch.txt\n@@ -98,8 +98,8 @@ for details).\n \n branch..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', and\n-\t'rebase'. See \"pull.mode\" for doing this in a non\n+\trebase the fetched branch. The possible values are 'merge',\n+\t'rebase', and 'ff-only'. See \"pull.mode\" for doing this in a non\n \tbranch-specific manner.\n \n branch..description::\ndiff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt\nindex f4385cde33..40778d9120 100644\n--- a/Documentation/config/pull.txt\n+++ b/Documentation/config/pull.txt\n@@ -32,8 +32,8 @@ 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..pullmode\" for setting this on a\n-\tper-branch basis.\n+\t'rebase', and 'ff-only'. See \"branch..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\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 7756c8aea1..5a67667b79 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1088,6 +1088,9 @@ 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 (mode == PULL_MODE_FF_ONLY && !can_ff)\n+\t\tdie(_(\"The pull was not fast-forward, please either merge or rebase.\\n\"));\n+\n \tif (!opt_rebase && !can_ff && opt_verbosity >= 0 && (!opt_ff || !strcmp(opt_ff, \"--ff\"))) {\n \t\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n \t\t\t\"you need to specify if you want a merge, or a rebase.\\n\"\n@@ -1095,7 +1098,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.\\n\"\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..b5b007b175 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -861,4 +861,49 @@ test_expect_success 'git pull --rebase against local branch' '\n \ttest_cmp expect file2\n '\n \n+setup_other () {\n+\ttest_when_finished \"git checkout master && git branch -D other test\" &&\n+\tgit checkout -b other $1 &&\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+}\n+\n+setup_ff () {\n+\tsetup_other master\n+}\n+\n+setup_non_ff () {\n+\tsetup_other master^\n+}\n+\n+test_expect_success 'fast-forward (pull.mode=ff-only)' '\n+\tsetup_ff &&\n+\tgit -c pull.mode=ff-only pull\n+'\n+\n+test_expect_success 'non-fast-forward (pull.mode=ff-only)' '\n+\tsetup_non_ff &&\n+\ttest_must_fail git -c pull.mode=ff-only pull\n+'\n+\n+test_expect_success 'non-fast-forward with merge (pull.mode=ff-only)' '\n+\tsetup_non_ff &&\n+\tgit -c pull.mode=ff-only pull --merge\n+'\n+\n+test_expect_success 'non-fast-forward with rebase (pull.mode=ff-only)' '\n+\tsetup_non_ff &&\n+\tgit -c pull.mode=ff-only pull --rebase\n+'\n+\n+test_expect_success 'non-fast-forward error message (pull.mode=ff-only)' '\n+\tsetup_non_ff &&\n+\ttest_must_fail git -c pull.mode=ff-only pull 2> error &&\n+\tcat error &&\n+\ttest_i18ngrep \"The pull was not fast-forward\" error\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"411701","messageId":"20201208002648.1370414-20-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 19/19] future: pull: enable ff-only mode by default","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:48Z","receivedAt":"2020-12-08T00:28:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The user has been warned that this change was coming and should have\nalready configured his/her preference.\n\nIt's time to flip the switch and make ff-only the default.\n\nThere's no need for the annoying warning anymore.\n\nTODO: Do we want to keep the \"pull.mode=ff-only\" tests and essentially run\nthem twice?\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/git-pull.txt | 4 +-\n builtin/pull.c | 19 +---------\n t/t5520-pull.sh | 28 +++++---------\n t/t7601-merge-pull-config.sh | 72 ------------------------------------\n 4 files changed, 13 insertions(+), 110 deletions(-)\n\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex 21b50aff77..672371e989 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -60,8 +60,8 @@ However, a non-fast-forward case looks very different.\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 --merge`.\n+By default `git pull` will fail on these situations, you will have to\n+specify whether you want `--merge` or `--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`)\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 734a2c04b8..7ddc11a4bc 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -392,7 +392,7 @@ static enum pull_mode_type config_get_pull_mode(void)\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+\treturn PULL_MODE_FF_ONLY;\n }\n \n /**\n@@ -1091,23 +1091,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\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 \n-\tif (!opt_rebase && !can_ff && opt_verbosity >= 0 && (!opt_ff || !strcmp(opt_ff, \"--ff\"))) {\n-\t\tadvise(_(\"The pull was not fast-forward, in the future you will have to choose a merge, or a rebase.\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"To quell this message you have two main options:\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"1. Adopt the new behavior:\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\" git config --global pull.mode ff-only\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"2. Maintain the current behavior:\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\" git config --global pull.mode merge\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"For now we will fall back to the traditional behavior (merge).\\n\"\n-\t\t\t\"Read \\\"git pull --help\\\" for more information.\"));\n-\t}\n-\n \tif (opt_rebase >= REBASE_TRUE) {\n \t\tint ret = 0;\n \ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex d4718dbc02..84685786da 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -879,39 +879,31 @@ setup_non_ff () {\n \tsetup_other master^\n }\n \n-test_expect_success 'fast-forward (pull.mode=ff-only)' '\n+test_expect_success 'fast-forward (default)' '\n \tsetup_ff &&\n-\tgit -c pull.mode=ff-only pull\n+\tgit pull\n '\n \n-test_expect_success 'non-fast-forward (pull.mode=ff-only)' '\n+test_expect_success 'non-fast-forward (default)' '\n \tsetup_non_ff &&\n-\ttest_must_fail git -c pull.mode=ff-only pull\n+\ttest_must_fail git pull\n '\n \n-test_expect_success 'non-fast-forward with merge (pull.mode=ff-only)' '\n+test_expect_success 'non-fast-forward with merge (default)' '\n \tsetup_non_ff &&\n-\tgit -c pull.mode=ff-only pull --merge\n+\tgit pull --merge\n '\n \n-test_expect_success 'non-fast-forward with rebase (pull.mode=ff-only)' '\n+test_expect_success 'non-fast-forward with rebase (default)' '\n \tsetup_non_ff &&\n-\tgit -c pull.mode=ff-only pull --rebase\n+\tgit pull --rebase\n '\n \n-test_expect_success 'non-fast-forward error message (pull.mode=ff-only)' '\n+test_expect_success 'non-fast-forward error message (default)' '\n \tsetup_non_ff &&\n-\ttest_must_fail git -c pull.mode=ff-only pull 2> error &&\n+\ttest_must_fail git pull 2> error &&\n \tcat error &&\n \ttest_i18ngrep \"The pull was not fast-forward\" error\n '\n \n-test_expect_success 'non-fast-forward warning (default)' '\n-\tsetup_non_ff &&\n-\tgit pull 2> error &&\n-\tcat error &&\n-\ttest_i18ngrep \"The pull was not fast-forward\" error &&\n-\ttest_i18ngrep \"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 149fc2a009..c6c44ec570 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -27,78 +27,6 @@ test_expect_success 'setup' '\n \tgit tag c3\n '\n \n-test_expect_success 'pull.rebase not set' '\n-\tgit reset --hard c2 &&\n-\tgit -c color.advice=always pull . c1 2>err &&\n-\ttest_decode_color decoded &&\n-\ttest_i18ngrep \"hint: \" decoded &&\n-\ttest_i18ngrep \"in the future you will have to choose\" decoded\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 ! \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" 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 \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" err\n-'\n-\n-test_expect_success 'pull.rebase not set and --merge given' '\n-\tgit reset --hard c2 &&\n-\tgit pull --merge . c1 2>err &&\n-\ttest_i18ngrep ! \"in the future you will have to choose\" 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 \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" err\n-'\n-\n test_expect_success 'merge c1 with c2' '\n \tgit reset --hard c1 &&\n \ttest -f c0.c &&\n-- \n2.29.2\n\n"},{"id":"411702","messageId":"20201208002648.1370414-17-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 16/19] pull: add pull.mode","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:45Z","receivedAt":"2020-12-08T00:28:57Z","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..pullmode'.\n\nFor now we simply override the rebase mode.\n\nAnd when any --rebase or --merge options are specified the mode is\nupdated accordingly.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/config/branch.txt | 6 +++\n Documentation/config/pull.txt | 6 +++\n builtin/pull.c | 70 +++++++++++++++++++++++++++++++--\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, 164 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt\nindex cc5f3249fc..b57bf2c308 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..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', and\n+\t'rebase'. See \"pull.mode\" for doing this in a non\n+\tbranch-specific manner.\n+\n branch..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..f4385cde33 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..pullmode\" for setting this on a\n+\tper-branch basis.\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 118fbdeb62..7756c8aea1 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 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 [] [ [...]]\"),\n \tNULL\n@@ -129,8 +151,9 @@ static struct option pull_options[] = {\n \t\t\"(false|true|merges|preserve|interactive)\",\n \t\tN_(\"incorporate changes by rebasing rather than merging\"),\n \t\tPARSE_OPT_OPTARG, parse_opt_rebase),\n-\tOPT_SET_INT('m', \"merge\", &opt_rebase,\n-\t\tN_(\"incorporate changes by merging\"), REBASE_FALSE),\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@@ -349,6 +372,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@@ -935,6 +981,22 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \n \topt_ff = xstrdup_or_null(config_get_ff());\n \topt_rebase = config_get_rebase();\n+\tmode = config_get_pull_mode();\n+\n+\tif (!opt_rebase && 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 = REBASE_TRUE;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tbreak;\n+\t\t}\n+\t} else if (opt_rebase > 0) {\n+\t\tmode = opt_rebase >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;\n+\t}\n \n \tif (!getenv(\"GIT_REFLOG_ACTION\"))\n \t\tset_reflog_message(argc, argv);\n@@ -1031,8 +1093,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\t\"you need to specify if you want a merge, or a rebase.\\n\"\n \t\t\t\"You can squelch this message by running one of the following commands:\\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 8a6aae564a..25ca239c17 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -41,6 +41,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":"411703","messageId":"20201208002648.1370414-19-felipe.contreras@gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"[PATCH v4 18/19] pull: advice of future changes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:26:47Z","receivedAt":"2020-12-08T00:29:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Now that we have \"pull.mode=ff-only\", we can make it the default any\ntime we want to.\n\nFor now, simply explain the upcoming changes in the default warning, and\nmention how to turn on the future default mode.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/pull.c | 20 +++++++++++---------\n t/t5520-pull.sh | 8 ++++++++\n t/t7601-merge-pull-config.sh | 22 +++++++++++-----------\n 3 files changed, 30 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 5a67667b79..734a2c04b8 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1092,17 +1092,19 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"The pull was not fast-forward, please either merge or rebase.\\n\"));\n \n \tif (!opt_rebase && !can_ff && opt_verbosity >= 0 && (!opt_ff || !strcmp(opt_ff, \"--ff\"))) {\n-\t\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n-\t\t\t\"you need to specify if you want a merge, or a rebase.\\n\"\n-\t\t\t\"You can squelch this message by running one of the following commands:\\n\"\n+\t\tadvise(_(\"The pull was not fast-forward, in the future you will have to choose a merge, or a rebase.\\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\"To quell this message you have two main options:\\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.\\n\"\n-\t\t\t\"If unsure, run \\\"git pull --merge\\\".\\n\"\n+\t\t\t\"1. Adopt the new behavior:\\n\"\n+\t\t\t\"\\n\"\n+\t\t\t\" git config --global pull.mode ff-only\\n\"\n+\t\t\t\"\\n\"\n+\t\t\t\"2. Maintain the current behavior:\\n\"\n+\t\t\t\"\\n\"\n+\t\t\t\" git config --global pull.mode merge\\n\"\n+\t\t\t\"\\n\"\n+\t\t\t\"For now we will fall back to the traditional behavior (merge).\\n\"\n \t\t\t\"Read \\\"git pull --help\\\" for more information.\"));\n \t}\n \ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex b5b007b175..d4718dbc02 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -906,4 +906,12 @@ test_expect_success 'non-fast-forward error message (pull.mode=ff-only)' '\n \ttest_i18ngrep \"The pull was not fast-forward\" error\n '\n \n+test_expect_success 'non-fast-forward warning (default)' '\n+\tsetup_non_ff &&\n+\tgit pull 2> error &&\n+\tcat error &&\n+\ttest_i18ngrep \"The pull was not fast-forward\" error &&\n+\ttest_i18ngrep \"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 25ca239c17..149fc2a009 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -32,71 +32,71 @@ test_expect_success 'pull.rebase not set' '\n \tgit -c color.advice=always pull . c1 2>err &&\n \ttest_decode_color decoded &&\n \ttest_i18ngrep \"hint: \" decoded &&\n-\ttest_i18ngrep \"Pulling without specifying how to reconcile\" decoded\n+\ttest_i18ngrep \"in the future you will have to choose\" decoded\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 ! \"in the future you will have to choose\" 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+\ttest_i18ngrep ! \"in the future you will have to choose\" 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+\ttest_i18ngrep \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" err\n '\n \n test_expect_success 'pull.rebase not set and --merge given' '\n \tgit reset --hard c2 &&\n \tgit pull --merge . c1 2>err &&\n-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n+\ttest_i18ngrep ! \"in the future you will have to choose\" 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+\ttest_i18ngrep \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" 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 ! \"in the future you will have to choose\" err\n '\n \n test_expect_success 'merge c1 with c2' '\n-- \n2.29.2\n\n"},{"id":"411705","messageId":"CAMP44s2hUCd9qc83LReGyjy8N+u++eK6VjwGhDhrX0f0SbKmig@mail.gmail.com","threadId":"54788","inReplyTo":"20201208002648.1370414-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v4 00/19] pull: default ff-only mode","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:57:32Z","receivedAt":"2020-12-08T00:58:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nJunio, this is intended for you, I will describe step by step how the\nwarning evolves.\n\nStep 0: This is what we have today:\n\n Pulling without specifying how to reconcile divergent branches is\n discouraged. You can squelch this message by running one of the following\n commands sometime before your next pull:\n\n git config pull.rebase false # merge (the default strategy)\n git config pull.rebase true # rebase\n git config pull.ff only # fast-forward only\n\n You can replace \"git config\" with \"git config --global\" to set a default\n preference for all repositories. You can also pass --rebase, --no-rebase,\n or --ff-only on the command line to override the configured default per\n invocation.\n\n> Felipe Contreras (19):\n> doc: pull: explain what is a fast-forward\n> pull: improve default warning\n\nStep 1: This is low-hanging fruit that can be fixed today without any\nchange in behavior:\n\n Pulling without specifying how to reconcile divergent branches is discouraged;\n you need to specify if you want a merge, or a rebase.\n You can squelch this message by running one of the following commands:\n\n git config pull.rebase false # merge (the default strategy)\n git config pull.rebase true # rebase\n git config pull.ff only # fast-forward only\n\n You can replace \"git config\" with \"git config --global\" to set a default\n preference for all repositories.\n If unsure, run \"git pull --no-rebase\".\n Read \"git pull --help\" for more information.\n\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\nAt this point we can update the warning to mention that we are inside\na non-fast-forward case. But it's not necessary.\n\n> pull: trivial whitespace style fix\n> pull: introduce --merge option\n\ns/--no-rebase/--merge/\n\n> pull: show warning with --ff\n> rebase: add REBASE_DEFAULT\n> pull: move configurations fetches\n> test: merge-pull-config: trivial cleanup\n> test: pull-options: revert unnecessary changes\n> pull: trivial memory fix\n\nThis is the end of part I. At this point the default mode is still\n\"merge\", and the only behavior change is that the warning is printed\nonly on the non-fast-forward case.\n\n========================================================================\n\n> pull: add pull.mode\n\nStep 2: pull.mode={merge,rebase} are specified\n\n Pulling without specifying how to reconcile divergent branches is discouraged;\n you need to specify if you want a merge, or a rebase.\n You can squelch this message by running one of the following commands:\n\n git config pull.mode merge # (the default strategy)\n git config pull.mode rebase\n git config pull.ff only # fast-forward only\n\n You can replace \"git config\" with \"git config --global\" to set a default\n preference for all repositories.\n If unsure, run \"git pull --merge\".\n Read \"git pull --help\" for more information.\n\n> pull: add pull.mode=ff-only\n\nStep 3: Now pull.mode=ff-only\n\n Pulling without specifying how to reconcile divergent branches is discouraged;\n you need to specify if you want a merge, or a rebase.\n You can squelch this message by running one of the following commands:\n\n git config pull.mode merge # (the default strategy)\n git config pull.mode rebase\n git config pull.mode ff-only # fast-forward only\n\n You can replace \"git config\" with \"git config --global\" to set a default\n preference for all repositories.\n If unsure, run \"git pull --merge\".\n Read \"git pull --help\" for more information.\n\nHowever, now in addition to the warning, there's an error message:\n\n The pull was not fast-forward, please either merge or rebase.\n\nThis error message is *only* triggered when the user has manually\nconfigured \"pull.mode=ff-only\". And it is an error. The program dies.\n\nAnd it's not meant to be temporary; it's permanent behavior.\n\n> pull: advice of future changes\n\nStep 4: Now that pull.mode=ff-only is in place, we can aim for it\nbeing the default, and we can tell our users that it will be the\ndefault in the future.\n\n The pull was not fast-forward, in the future you will have to choose\na merge, or a rebase.\n\n To quell this message you have two main options:\n\n 1. Adopt the new behavior:\n\n git config --global pull.mode ff-only\n\n 2. Maintain the current behavior:\n\n git config --global pull.mode merge\n\n For now we will fall back to the traditional behavior (merge).\n Read \"git pull --help\" for more information.\n\nThis is the end of part II. At this point the default is still \"merge\".\n\nUnlike part I, in part II we have committed to pull.mode=ff-only to be\nthe new default, and we are already telling users that they can use\nthis new mode to test the new behavior.\n\nWe should probably stay a couple of releases at this point, with this\nwarning, and the new behavior already configurable.\n\nElijah: notice how there's no mention of `git pull --merge`, because\nin my opinion now we are telling users this is a temporary *warning*,\nand the way to get rid of it properly should be very clear.\n\n========================================================================\n\n> future: pull: enable ff-only mode by default\n\nThe last patch finally enables the ff-only mode by default, and the\nwarning is removed forever.\n\nThe only thing that remains now is the fatal error:\n\n The pull was not fast-forward, please either merge or rebase.\n\nThis fatal error is avoided by pull.mode=merge, pull.mode=rebase,\npull.rebase=true, pull.rebase=false, pull.rebase=$other, --merge, or\n--rebase.\n\nIt *only* happens when the user does a vanilla \"git pull\", it's a\nnon-fast-forward update, the user has configured pull.mode=ff-only or\nhas not configured any of the above.\n\nIs it more clear what is my proposal?\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}