{"thread":{"id":"54771","subject":"[PATCH v3 01/16] doc: pull: explain what is a fast-forward","startedAt":"2020-12-05T19:54:17Z","lastAt":"2020-12-08T20:59:01Z","messageCount":33,"participants":["Felipe Contreras","Junio C Hamano","Elijah Newren"],"isPatch":true,"patchVersion":3,"patchTotal":16},"messages":[{"id":"411513","messageId":"20201205195313.1557473-2-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 01/16] doc: pull: explain what is a fast-forward","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:52:58Z","receivedAt":"2020-12-05T19:54:17Z","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..dc812139f4 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-- \n2.29.2\n\n"},{"id":"411514","messageId":"20201205195313.1557473-4-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 03/16] pull: refactor fast-forward check","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:00Z","receivedAt":"2020-12-05T19:54:17Z","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 d71344fe28..7a5b343fe2 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -907,6 +907,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@@ -1017,22 +1031,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":"411515","messageId":"20201205195313.1557473-6-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 05/16] pull: trivial cleanup","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:02Z","receivedAt":"2020-12-05T19:54:17Z","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 d2f80e8615..feadded1b6 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1024,19 +1024,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":"411516","messageId":"20201205195313.1557473-5-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 04/16] pull: cleanup autostash check","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:01Z","receivedAt":"2020-12-05T19:54:17Z","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 7a5b343fe2..d2f80e8615 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -927,7 +927,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@@ -960,8 +959,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@@ -1030,13 +1029,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":"411517","messageId":"20201205195313.1557473-3-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 02/16] pull: improve default warning","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:52:59Z","receivedAt":"2020-12-05T19:54:17Z","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, rebase, or\n fast-forward.\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 | 25 +++++++++++++------------\n 1 file changed, 13 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 1034372f8b..d71344fe28 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -345,18 +345,19 @@ 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, a rebase, or a fast-forward.\\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\t\t));\n \t}\n \n \treturn REBASE_FALSE;\n-- \n2.29.2\n\n"},{"id":"411518","messageId":"20201205195313.1557473-1-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":null,"subject":"[PATCH v3 00/16] pull: default warning improvements","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:52:57Z","receivedAt":"2020-12-05T19:54:17Z","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 back again.\n\nThe main solution--reject non-fast-forward merges by default--seems to have lost traction (again).\n\nThere are multiple approaches floating around, but no clear path forward.\n\nThis patch series attempts to fix as much as possible of the current situation without committing to\nany particular solution.\n\nIt does:\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. Improve the error message with --ff-only\n6. Fix the behavior of the warning with --ff\n\nAnd tentatively (and this should be the only controversial change):\n\n7. Change the semantics of --ff-only\n\nIt does not:\n\n* Introduce the suggested pull.mode\n* Change the current default (still --ff)\n\nIt is not a complete solution, but should improve the current situation.\n\nSince v2:\n\n1. The warning is updated to say \"a merge, a rebase, or a fast-forward\"\n2. A suggestion 'If unsure, run \"git pull --merge\".' is removed\n3. A new REBASE_DEFAULT is introduced\n4. Tests are revamped using helpers\n5. The logic of --merge and --ff-only is updated so one can override the other\n6. A bunch of commit messages are updated\n7. A memory inconsistency is fixed\n\n[1] https://lore.kernel.org/git/5363BB9F.40102@xiplink.com/\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex e389ffcdc3..0735c77f42 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@@ -114,6 +112,24 @@ 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@@ -131,8 +147,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\"), 0),\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@@ -161,9 +178,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_PASSTHRU(0, \"ff-only\", &opt_ff, NULL,\n+\tOPT_CALLBACK_F(0, \"ff-only\", &opt_ff, NULL,\n \t\tN_(\"abort if fast-forward is not possible\"),\n-\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG),\n+\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_ff_only),\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@@ -348,9 +365,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 +460,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 +473,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 +488,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@@ -919,6 +934,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@@ -935,12 +953,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 < 0)\n-\t\topt_rebase = config_get_rebase();\n-\n \tif (read_cache_unmerged())\n \t\tdie_resolve_conflict(\"pull\");\n \n@@ -950,7 +962,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@@ -1010,19 +1022,18 @@ 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 (!can_ff && default_mode) {\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\t\t\t\"If unsure, run \\\"git pull --merge\\\".\"));\n-\t\t}\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\\n\"\n-\t\t\t\t\"discouraged; you need to specify if you want a merge, or a rebase.\\n\"\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@@ -1037,11 +1048,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\t/* Disable --ff-only when --merge is specified */\n-\tif (!can_ff && !default_mode && !opt_rebase && opt_ff && !strcmp(opt_ff, \"--ff-only\"))\n-\t\topt_ff = NULL;\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) &&\n@@ -1050,7 +1057,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(&curr_head, merge_heads.oid, &rebase_fork_point);\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,\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex eec6224fb0..0cdac4010b 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -819,66 +819,89 @@ 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+setup_other () {\n \ttest_when_finished \"git checkout master && git branch -D other test\" &&\n-\ttest_config pull.ff only &&\n-\tgit checkout -b other master &&\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+\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 'git pull non-fast-forward (ff-only)' '\n-\ttest_when_finished \"git checkout master && git branch -D other test\" &&\n+test_expect_success 'non-fast-forward (ff-only)' '\n \ttest_config pull.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+\tsetup_non_ff &&\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+test_expect_success 'non-fast-forward with merge (ff-only)' '\n \ttest_config pull.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+\tsetup_non_ff &&\n+\tgit pull --merge\n '\n \n-test_expect_success 'git pull non-fast-forward with rebase (ff-only)' '\n-\ttest_when_finished \"git checkout master && git branch -D other test\" &&\n+test_expect_success 'non-fast-forward with rebase (ff-only)' '\n \ttest_config pull.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+\tsetup_non_ff &&\n \tgit pull --rebase\n '\n \n-test_expect_success 'git pull non-fast-forward error message' '\n-\ttest_when_finished \"git checkout master && git branch -D other test\" &&\n+test_expect_success 'non-fast-forward error message (ff-only)' '\n \ttest_config pull.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+\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 (16):\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 pull: add proper error with --ff-only\n test: merge-pull-config: trivial cleanup\n test: pull-options: revert unnecessary changes\n pull: trivial memory fix\n\n Documentation/git-pull.txt | 24 +++++-\n builtin/pull.c | 140 +++++++++++++++++++++--------------\n rebase.h | 3 +-\n t/t5520-pull.sh | 85 +++++++++++++++++++++\n t/t5521-pull-options.sh | 22 +++---\n t/t7601-merge-pull-config.sh | 35 +++++----\n 6 files changed, 224 insertions(+), 85 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"411519","messageId":"20201205195313.1557473-17-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 16/16] pull: trivial memory fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:13Z","receivedAt":"2020-12-05T19:54:27Z","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 54c58618e9..0735c77f42 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1057,7 +1057,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(&curr_head, merge_heads.oid, &rebase_fork_point);\n-- \n2.29.2\n\n"},{"id":"411520","messageId":"20201205195313.1557473-15-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 14/16] test: merge-pull-config: trivial cleanup","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:11Z","receivedAt":"2020-12-05T19:54:27Z","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":"411521","messageId":"20201205195313.1557473-7-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 06/16] pull: move default warning","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:03Z","receivedAt":"2020-12-05T19:54:42Z","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 \"default\nmode\" (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 | 39 +++++++++++++++++++++++----------------\n 1 file changed, 23 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex feadded1b6..3fd9bd1bd2 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,21 +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, a rebase, or a fast-forward.\\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\t\t));\n-\t}\n+\tdefault_mode = 1;\n \n \treturn REBASE_FALSE;\n }\n@@ -927,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@@ -1022,6 +1011,24 @@ 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, a rebase, or a fast-forward.\\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\t\t));\n+\t}\n+\n \tif (opt_rebase) {\n \t\tint ret = 0;\n \t\tif ((recurse_submodules == RECURSE_SUBMODULES_ON ||\n@@ -1029,7 +1036,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":"411522","messageId":"20201205195313.1557473-9-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 08/16] pull: trivial whitespace style fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:05Z","receivedAt":"2020-12-05T19:54:43Z","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 300b17b962..cecbacc549 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":"411523","messageId":"20201205195313.1557473-8-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 07/16] pull: display default warning only when non-ff","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:04Z","receivedAt":"2020-12-05T19:54:43Z","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 dc812139f4..ad33d2472c 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -60,6 +60,9 @@ However, a non-fast-foward 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 3fd9bd1bd2..300b17b962 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\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, a rebase, or a fast-forward.\\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":"411524","messageId":"20201205195313.1557473-10-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 09/16] pull: introduce --merge option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:06Z","receivedAt":"2020-12-05T19:54:44Z","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 compatbility).\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 ad33d2472c..c220da143a 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -61,7 +61,7 @@ However, a non-fast-foward 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 cecbacc549..da91d78a22 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@@ -1024,7 +1026,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\t\t));\n \t}\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":"411525","messageId":"20201205195313.1557473-14-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 13/16] pull: add proper error with --ff-only","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:10Z","receivedAt":"2020-12-05T19:54:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The current error is not user-friendly:\n\n fatal: not possible to fast-forward, aborting.\n\nWe want something that actually explains what is going on:\n\n The pull was not fast-forward, please either merge or rebase.\n\nThe user can get rid of the warning by doing either --merge or\n--rebase.\n\nExcept: doing \"git pull --merge\" is not actually enough; we would return\nto the previous behavior: \"fatal: not possible to fast-forward,\naborting\". In order to do the right thing we will have to change the\nsemantics of --ff-only.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/pull.c | 60 +++++++++++++++++++++++-----------\n t/t5520-pull.sh | 85 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 127 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex e0157d013f..54c58618e9 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -112,6 +112,24 @@ 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@@ -129,8 +147,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@@ -159,9 +178,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_PASSTHRU(0, \"ff-only\", &opt_ff, NULL,\n+\tOPT_CALLBACK_F(0, \"ff-only\", &opt_ff, NULL,\n \t\tN_(\"abort if fast-forward is not possible\"),\n-\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG),\n+\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_ff_only),\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@@ -1008,20 +1027,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 || !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, a rebase, or a fast-forward.\\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\t\t));\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 \t}\n \n \tif (opt_rebase >= REBASE_TRUE) {\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 9fae07cdfa..0cdac4010b 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -819,4 +819,89 @@ 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-- \n2.29.2\n\n"},{"id":"411526","messageId":"20201205195313.1557473-13-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 12/16] pull: move configurations fetches","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:09Z","receivedAt":"2020-12-05T19:54:47Z","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 347ac89eee..e0157d013f 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -915,6 +915,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@@ -931,12 +934,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":"411527","messageId":"20201205195313.1557473-16-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 15/16] test: pull-options: revert unnecessary changes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:12Z","receivedAt":"2020-12-05T19:54:51Z","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":"411528","messageId":"20201205195313.1557473-11-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 10/16] pull: show warning with --ff","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:07Z","receivedAt":"2020-12-05T19:54:52Z","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 da91d78a22..2bd6ee9d19 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1015,7 +1015,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, a rebase, or a fast-forward.\\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":"411529","messageId":"20201205195313.1557473-12-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"[PATCH v3 11/16] rebase: add REBASE_DEFAULT","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T19:53:08Z","receivedAt":"2020-12-05T19:54:53Z","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 2bd6ee9d19..347ac89eee 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@@ -938,7 +934,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@@ -950,7 +946,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@@ -1010,12 +1006,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, a rebase, or a fast-forward.\\n\"\n \t\t\t\"You can squelch this message by running one of the following commands:\\n\"\n@@ -1031,7 +1027,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\t));\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":"411531","messageId":"CAMP44s3xqjoJm5AL6dLcS6R-RFGGOdQ39W+ZY3_PWL+WMeCxjw@mail.gmail.com","threadId":"54771","inReplyTo":"20201205195313.1557473-14-felipe.contreras@gmail.com","subject":"Re: [PATCH v3 13/16] pull: add proper error with --ff-only","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T20:15:35Z","receivedAt":"2020-12-05T20:16:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Dec 5, 2020 at 1:53 PM Felipe Contreras\n wrote:\n>\n> The current error is not user-friendly:\n>\n> fatal: not possible to fast-forward, aborting.\n>\n> We want something that actually explains what is going on:\n>\n> The pull was not fast-forward, please either merge or rebase.\n>\n> The user can get rid of the warning by doing either --merge or\n> --rebase.\n>\n> Except: doing \"git pull --merge\" is not actually enough; we would return\n> to the previous behavior: \"fatal: not possible to fast-forward,\n> aborting\". In order to do the right thing we will have to change the\n> semantics of --ff-only.\n\nThis commit is wrong, the next one somehow got squashed into it.\n\nI'm sending the two patches separately.\n\n-- \nFelipe Contreras\n"},{"id":"411532","messageId":"20201205201933.1560133-1-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"CAMP44s3xqjoJm5AL6dLcS6R-RFGGOdQ39W+ZY3_PWL+WMeCxjw@mail.gmail.com","subject":"[PATCH 1/2] pull: add proper error with --ff-only","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T20:19:32Z","receivedAt":"2020-12-05T20:20:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The current error is not user-friendly:\n\n fatal: not possible to fast-forward, aborting.\n\nWe want something that actually explains what is going on:\n\n The pull was not fast-forward, please either merge or rebase.\n\nThe user can get rid of the warning by doing either --merge or\n--rebase.\n\nExcept: doing \"git pull --merge\" is not actually enough; we would return\nto the previous behavior: \"fatal: not possible to fast-forward,\naborting\". In order to do the right thing we will have to change the\nsemantics of --ff-only.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/pull.c | 33 ++++++++++++++++++--------------\n t/t5520-pull.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 69 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex e0157d013f..44ec6e7216 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1008,20 +1008,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 || !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, a rebase, or a fast-forward.\\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\t\t));\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 \t}\n \n \tif (opt_rebase >= REBASE_TRUE) {\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 9fae07cdfa..067780e658 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -819,4 +819,54 @@ 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_failure '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_done\n-- \n2.29.2\n\n"},{"id":"411533","messageId":"20201205201933.1560133-2-felipe.contreras@gmail.com","threadId":"54771","inReplyTo":"20201205201933.1560133-1-felipe.contreras@gmail.com","subject":"[PATCH 2/2] tentative: pull: change the semantics of --ff-only","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-05T20:19:33Z","receivedAt":"2020-12-05T20:20:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We want the last thing specified to override everything previously.\n\nFor example --merge should override a previous \"pull.ff = only\"\nconfiguration.\n\nAnd --ff-only should override a previous \"pull.rebase = true\"\nconfiguration.\n\nCurrently \"git pull --ff-only --merge\" fails with:\n\n fatal: not possible to fast-forward, aborting.\n\nBut that's not what we want; we want --merge to override --ff-only and\ndo a --no-ff merge.\n\nThis is a backwards-incompatible change that achieves that.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/pull.c | 27 +++++++++++++++++++++++----\n t/t5520-pull.sh | 37 ++++++++++++++++++++++++++++++++++++-\n 2 files changed, 59 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 44ec6e7216..54c58618e9 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -112,6 +112,24 @@ 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@@ -129,8 +147,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@@ -159,9 +178,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_PASSTHRU(0, \"ff-only\", &opt_ff, NULL,\n+\tOPT_CALLBACK_F(0, \"ff-only\", &opt_ff, NULL,\n \t\tN_(\"abort if fast-forward is not possible\"),\n-\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG),\n+\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_ff_only),\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),\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 067780e658..0cdac4010b 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -849,7 +849,7 @@ test_expect_success 'non-fast-forward (ff-only)' '\n \ttest_must_fail git pull\n '\n \n-test_expect_failure 'non-fast-forward with merge (ff-only)' '\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@@ -869,4 +869,39 @@ test_expect_success 'non-fast-forward error message (ff-only)' '\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-- \n2.29.2\n\n"},{"id":"411644","messageId":"xmqq8sa99wh3.fsf@gitster.c.googlers.com","threadId":"54771","inReplyTo":"20201205195313.1557473-2-felipe.contreras@gmail.com","subject":"Re: [PATCH v3 01/16] doc: pull: explain what is a fast-forward","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-07T20:45:12Z","receivedAt":"2020-12-07T20:46:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> We want users to know what is a fast-forward in order to understand the\n> default warning.\n\nThe intention is very good, but ...\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... I find the phrase \"in a fast-forward way\" a bit awkward.\nPerhaps use the 'fast-forward' as a verb, i.e.\n\n\tThen `git pull` notices that what is being merged is a\n\tdescendant of our current branch, and fast-forwards our\n\t'master' branch to the commit.\n\nor something like that? It should be in line with the spirit in\nwhich glossary defines fast-forward, I would think.\n\n> +\n> +------------\n> + D---E---A---B---C master, origin/master\n> +------------\n> +\n> +However, a non-fast-foward case looks very different.\n\ns/foward/forward/ (the same typo exists above);\n\n> ------------\n> \t A---B---C master on origin\n> \t /\n"},{"id":"411645","messageId":"xmqq4kkx9vzx.fsf@gitster.c.googlers.com","threadId":"54771","inReplyTo":"20201205195313.1557473-3-felipe.contreras@gmail.com","subject":"Re: [PATCH v3 02/16] pull: improve default warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-07T20:55:30Z","receivedAt":"2020-12-07T20:56:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> We want to:\n>\n> 1. Be clear about what \"specifying\" means; merge, rebase, or\n> fast-forward.\n> 2. Mention a direct shortcut for people that just want to get on with\n> their lives: git pull --no-rebase.\n> 3. Have a quick reference for users to understand what this\n> \"fast-forward\" business means.\n>\n> This patch does all three.\n>\n> Thanks to the previous patch now \"git pull --help\" explains what a\n> fast-forward is, and a further patch changes --no-rebase to --merge so\n> it's actually user friendly.\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/pull.c | 25 +++++++++++++------------\n> 1 file changed, 13 insertions(+), 12 deletions(-)\n>\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 1034372f8b..d71344fe28 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -345,18 +345,19 @@ 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\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, a rebase, or a fast-forward.\\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\t\t));\n> \t}\n\nIt is an improvement to say what they can do from the command line\nin order to get out of the current situation, before giving them a\nconfiguration advice.\n\nBut the exact instruction for the command line in the original seems\nto have been lost during the rewrite, which I think makes the \"what\nto do now\" advice less valuable than it could be.\n\nI personally think it is backwards to update this message before\nfixing the condition under which it triggers (I think by now\neverybody involved in the thread understands that we do not want to\ngive this advise that does not stop to behave as-is---it should be\nmade not to trigger if we know the history would fast-forward, and\nwhen it triggers it should stop the operation). It may appear OK as\nlong as we get the right end-state, but because this message must be\nrewritten when the triggering condition changes (i.e. when we stop\ngiving this message when the history fast-forwards, there is no\npoint in offering the fast-forward-only as a viable option), it\nseems to me a needless detour.\n\n"},{"id":"411646","messageId":"xmqqwnxt8hev.fsf@gitster.c.googlers.com","threadId":"54771","inReplyTo":"20201205195313.1557473-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v3 00/16] pull: default warning improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-07T20:55:52Z","receivedAt":"2020-12-07T20:57:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> It is not a complete solution, but should improve the current situation.\n>\n> Since v2:\n>\n> 1. The warning is updated to say \"a merge, a rebase, or a fast-forward\"\n> 2. A suggestion 'If unsure, run \"git pull --merge\".' is removed\n> 3. A new REBASE_DEFAULT is introduced\n> 4. Tests are revamped using helpers\n> 5. The logic of --merge and --ff-only is updated so one can override the other\n> 6. A bunch of commit messages are updated\n> 7. A memory inconsistency is fixed\n\nOn what base has this series been built? Applying to 'master' seems\nto stop at the \"refactor fast-forward check\" step.\n"},{"id":"411651","messageId":"CAMP44s1ZDXzGfEqpTeiG=aGAYK40ebnBLQKAbA7KGtcePGARfw@mail.gmail.com","threadId":"54771","inReplyTo":"xmqq8sa99wh3.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 01/16] doc: pull: explain what is a fast-forward","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-07T22:22:40Z","receivedAt":"2020-12-07T22:23:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Dec 7, 2020 at 2:45 PM Junio C Hamano wrote:\n>\n> Felipe Contreras writes:\n>\n> > We want users to know what is a fast-forward in order to understand the\n> > default warning.\n>\n> The intention is very good, but ...\n>\n> > +------------\n> > + A---B---C master on origin\n> > + /\n> > + D---E master\n> > +------------\n> > +\n> > +Then `git pull` will merge in a fast-foward way up to the new master.\n>\n> ... I find the phrase \"in a fast-forward way\" a bit awkward.\n> Perhaps use the 'fast-forward' as a verb, i.e.\n>\n> Then `git pull` notices that what is being merged is a\n> descendant of our current branch, and fast-forwards our\n> 'master' branch to the commit.\n>\n> or something like that? It should be in line with the spirit in\n> which glossary defines fast-forward, I would think.\n\nThe glossary defines a fast-forward as:\n\n A fast-forward is a special type of `merge`\n\nSo, if you consider \"merge\" a noun, then a fast-forward is an\nadjective. If you consider it a verb, then it's an adverb. But it's\nnot a verb.\n\nIf it was a verb, then we should have `git fast-forward`, which may\nnot be a terrible idea, but right now a fast-forward is a modifier.\n\nAt least that's what I have in my mind, and the glossary seems to agree.\n\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> s/foward/forward/ (the same typo exists above);\n\nAll right.\n\n-- \nFelipe Contreras\n"},{"id":"411652","messageId":"CAMP44s1aYqzCVvELH8zULaTkOdgLSSAQ0LE8WfgQKLPfU2MHfg@mail.gmail.com","threadId":"54771","inReplyTo":"xmqq4kkx9vzx.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 02/16] pull: improve default warning","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-07T22:29:34Z","receivedAt":"2020-12-07T22:30:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Dec 7, 2020 at 2:55 PM Junio C Hamano wrote:\n>\n> Felipe Contreras writes:\n>\n> > We want to:\n> >\n> > 1. Be clear about what \"specifying\" means; merge, rebase, or\n> > fast-forward.\n> > 2. Mention a direct shortcut for people that just want to get on with\n> > their lives: git pull --no-rebase.\n> > 3. Have a quick reference for users to understand what this\n> > \"fast-forward\" business means.\n> >\n> > This patch does all three.\n> >\n> > Thanks to the previous patch now \"git pull --help\" explains what a\n> > fast-forward is, and a further patch changes --no-rebase to --merge so\n> > it's actually user friendly.\n> >\n> > Signed-off-by: Felipe Contreras \n> > ---\n> > builtin/pull.c | 25 +++++++++++++------------\n> > 1 file changed, 13 insertions(+), 12 deletions(-)\n> >\n> > diff --git a/builtin/pull.c b/builtin/pull.c\n> > index 1034372f8b..d71344fe28 100644\n> > --- a/builtin/pull.c\n> > +++ b/builtin/pull.c\n> > @@ -345,18 +345,19 @@ static enum rebase_type config_get_rebase(void)\n> > return parse_config_rebase(\"pull.rebase\", value, 1);\n> >\n> > if (opt_verbosity >= 0 && !opt_ff) {\n> > - advise(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n> > - \"discouraged. You can squelch this message by running one of the following\\n\"\n> > - \"commands sometime before your next pull:\\n\"\n> > - \"\\n\"\n> > - \" git config pull.rebase false # merge (the default strategy)\\n\"\n> > - \" git config pull.rebase true # rebase\\n\"\n> > - \" git config pull.ff only # fast-forward only\\n\"\n> > - \"\\n\"\n> > - \"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n> > - \"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n> > - \"or --ff-only on the command line to override the configured default per\\n\"\n> > - \"invocation.\\n\"));\n>\n>\n> > + advise(_(\"Pulling without specifying how to reconcile divergent branches is discouraged;\\n\"\n> > + \"you need to specify if you want a merge, a rebase, or a fast-forward.\\n\"\n> > + \"You can squelch this message by running one of the following commands:\\n\"\n> > + \"\\n\"\n> > + \" git config pull.rebase false # merge (the default strategy)\\n\"\n> > + \" git config pull.rebase true # rebase\\n\"\n> > + \" git config pull.ff only # fast-forward only\\n\"\n> > + \"\\n\"\n> > + \"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n> > + \"preference for all repositories.\\n\"\n> > + \"If unsure, run \\\"git pull --no-rebase\\\".\\n\"\n> > + \"Read \\\"git pull --help\\\" for more information.\"\n> > + ));\n> > }\n>\n> It is an improvement to say what they can do from the command line\n> in order to get out of the current situation, before giving them a\n> configuration advice.\n>\n> But the exact instruction for the command line in the original seems\n> to have been lost during the rewrite, which I think makes the \"what\n> to do now\" advice less valuable than it could be.\n\nIn my opinion we should not dump the entire contents of the manpage in\na warning. The warning should be as brief as possible while giving\nthese:\n\n1. A description of what's going on\n2. A suggested way to move forward\n3. A quick way to opt-out\n4. A way to find more information\n\nThat's it.\n\nOf course when it comes to succinctness other developers tend to\ndisagree with me, but that's my opinion.\n\n> I personally think it is backwards to update this message before\n> fixing the condition under which it triggers (I think by now\n> everybody involved in the thread understands that we do not want to\n> give this advise that does not stop to behave as-is---it should be\n> made not to trigger if we know the history would fast-forward, and\n> when it triggers it should stop the operation). It may appear OK as\n> long as we get the right end-state, but because this message must be\n> rewritten when the triggering condition changes (i.e. when we stop\n> giving this message when the history fast-forwards, there is no\n> point in offering the fast-forward-only as a viable option), it\n> seems to me a needless detour.\n\nExcept it's not necessary to rewrite it further, not in step 1.\n\nMaybe it will be clearer when I send all the patches.\n\n-- \nFelipe Contreras\n"},{"id":"411653","messageId":"CAMP44s2sDPT8mQqSuUZ7HaN8EBXOdKBkU5t4-1k6EZfmRa0Mow@mail.gmail.com","threadId":"54771","inReplyTo":"xmqqwnxt8hev.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 00/16] pull: default warning improvements","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-07T22:33:21Z","receivedAt":"2020-12-07T22:34:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Dec 7, 2020 at 2:55 PM Junio C Hamano wrote:\n>\n> Felipe Contreras writes:\n>\n> > It is not a complete solution, but should improve the current situation.\n> >\n> > Since v2:\n> >\n> > 1. The warning is updated to say \"a merge, a rebase, or a fast-forward\"\n> > 2. A suggestion 'If unsure, run \"git pull --merge\".' is removed\n> > 3. A new REBASE_DEFAULT is introduced\n> > 4. Tests are revamped using helpers\n> > 5. The logic of --merge and --ff-only is updated so one can override the other\n> > 6. A bunch of commit messages are updated\n> > 7. A memory inconsistency is fixed\n>\n> On what base has this series been built? Applying to 'master' seems\n> to stop at the \"refactor fast-forward check\" step.\n\nNinth batch.\n\nThe 'pb/pull-rebase-recurse-submodules' merge is the one causing the\nconflicts (I think).\n\n-- \nFelipe Contreras\n"},{"id":"411654","messageId":"CABPp-BEkKMuZHWJ8mrFUFm3okDDsMHeZZNxtGzgq-RLLddMxdw@mail.gmail.com","threadId":"54771","inReplyTo":"CAMP44s1ZDXzGfEqpTeiG=aGAYK40ebnBLQKAbA7KGtcePGARfw@mail.gmail.com","subject":"Re: [PATCH v3 01/16] doc: pull: explain what is a fast-forward","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-12-07T22:40:37Z","receivedAt":"2020-12-07T22:41:45Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Mon, Dec 7, 2020 at 2:22 PM Felipe Contreras\n wrote:\n>\n> On Mon, Dec 7, 2020 at 2:45 PM Junio C Hamano wrote:\n> >\n> > Felipe Contreras writes:\n> >\n> > > We want users to know what is a fast-forward in order to understand the\n> > > default warning.\n> >\n> > The intention is very good, but ...\n> >\n> > > +------------\n> > > + A---B---C master on origin\n> > > + /\n> > > + D---E master\n> > > +------------\n> > > +\n> > > +Then `git pull` will merge in a fast-foward way up to the new master.\n> >\n> > ... I find the phrase \"in a fast-forward way\" a bit awkward.\n> > Perhaps use the 'fast-forward' as a verb, i.e.\n> >\n> > Then `git pull` notices that what is being merged is a\n> > descendant of our current branch, and fast-forwards our\n> > 'master' branch to the commit.\n> >\n> > or something like that? It should be in line with the spirit in\n> > which glossary defines fast-forward, I would think.\n>\n> The glossary defines a fast-forward as:\n>\n> A fast-forward is a special type of `merge`\n>\n> So, if you consider \"merge\" a noun, then a fast-forward is an\n> adjective. If you consider it a verb, then it's an adverb. But it's\n> not a verb.\n\nA square is a special type of a rectangle, but that doesn't make\n\"square\" an adjective; both square and rectangle are nouns.\n\n> If it was a verb, then we should have `git fast-forward`, which may\n> not be a terrible idea, but right now a fast-forward is a modifier.\n>\n> At least that's what I have in my mind, and the glossary seems to agree.\n\nIf you read the release notes and even various messages printed by\ngit, \"fast-forwards\", \"fast-forwarded\", \"fast-forwarding\", and \"to\nfast-forward\" all appear multiple times. And yes, \"fast-forward\" also\nappears multiple times as a noun in addition to the various uses as a\nverb. So, I'd say the glossary just isn't comprehensive because in\nthis case we have a word that serves as both a noun and a verb.\n\n\nGoing back to the text Junio highlighted, I agree with him that the\nphrase looks really awkward, and much prefer his suggestion\n(regardless of whether it aligns with the current glossary).\n\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> > s/foward/forward/ (the same typo exists above);\n>\n> All right.\n>\n> --\n> Felipe Contreras\n"},{"id":"411655","messageId":"xmqqblf58b99.fsf@gitster.c.googlers.com","threadId":"54771","inReplyTo":"CABPp-BEkKMuZHWJ8mrFUFm3okDDsMHeZZNxtGzgq-RLLddMxdw@mail.gmail.com","subject":"Re: [PATCH v3 01/16] doc: pull: explain what is a fast-forward","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-07T23:08:50Z","receivedAt":"2020-12-07T23:09:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> > ... I find the phrase \"in a fast-forward way\" a bit awkward.\n>> > Perhaps use the 'fast-forward' as a verb, i.e.\n>> >\n>> > Then `git pull` notices that what is being merged is a\n>> > descendant of our current branch, and fast-forwards our\n>> > 'master' branch to the commit.\n>> >\n>> > or something like that? It should be in line with the spirit in\n>> > which glossary defines fast-forward, I would think.\n>> ...\n> If you read the release notes and even various messages printed by\n> git, \"fast-forwards\", \"fast-forwarded\", \"fast-forwarding\", and \"to\n> fast-forward\" all appear multiple times. And yes, \"fast-forward\" also\n> appears multiple times as a noun in addition to the various uses as a\n> verb. So, I'd say the glossary just isn't comprehensive because in\n> this case we have a word that serves as both a noun and a verb.\n\nAh, sorry, I didn't mean noun-vs-verb when I mentioned the glossary.\n\nI thought that the idea that the word can be used as a verb, after\ndiscussing advise() messages that tells the users that they can\n\"merge, rebase or fast-forward\", was given and not something anybody\nneeds to be explained about.\n\nThe other half of what I suggested was to explain what situation is\nfast-forwardable, i.e. \"notices ... is a descendant of\", and I made\nsure that the explanation was in line with the grossary. Without it\nexplained in-place in the text, readers who need to be told what a\nfast-forward is needs to go to and come back from the glossary while\nreading this page, which was what I tried to improve while we are\ntrying to find a better phrasing.\n"},{"id":"411682","messageId":"CAMP44s0g7JLTZZs=O3gUcEiPt--p3gXEPHL0GsBqoqweKA=9Rw@mail.gmail.com","threadId":"54771","inReplyTo":"CABPp-BEkKMuZHWJ8mrFUFm3okDDsMHeZZNxtGzgq-RLLddMxdw@mail.gmail.com","subject":"Re: [PATCH v3 01/16] doc: pull: explain what is a fast-forward","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:17:37Z","receivedAt":"2020-12-08T00:18:46Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Dec 7, 2020 at 4:40 PM Elijah Newren wrote:\n> On Mon, Dec 7, 2020 at 2:22 PM Felipe Contreras\n> wrote:\n\n> > The glossary defines a fast-forward as:\n> >\n> > A fast-forward is a special type of `merge`\n> >\n> > So, if you consider \"merge\" a noun, then a fast-forward is an\n> > adjective. If you consider it a verb, then it's an adverb. But it's\n> > not a verb.\n>\n> A square is a special type of a rectangle, but that doesn't make\n> \"square\" an adjective; both square and rectangle are nouns.\n\nWords have multiple definitions. The word \"square\" is both a noun, and\nan adjective [1]. It's perfectly fine to say \"square corner\".\n\nJust like it's perfectly fine to say \"fast-forward merge\", or \"quick sort\".\n\nAnd that's how many people use it:\n\nhttps://git-scm.com/docs/git-merge#_fast_forward_merge\nhttps://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html\nhttps://www.atlassian.com/git/tutorials/using-branches/git-merge#:~:text=Fast%20Forward%20Merge\n\nPlus there's many instances in the documentation:\n\n* non-fast-forward update\n* fast-forward merges\n* fast-forward check\n* \"fast-forward-ness\"\n* fast-forward situation\n* fast-forward push\n* non-fast-forward pushes\n* non-fast-forward reference\n* fast-forward cases\n* \"fast-forward\" merge\n\n> > If it was a verb, then we should have `git fast-forward`, which may\n> > not be a terrible idea, but right now a fast-forward is a modifier.\n> >\n> > At least that's what I have in my mind, and the glossary seems to agree.\n>\n> If you read the release notes and even various messages printed by\n> git, \"fast-forwards\", \"fast-forwarded\", \"fast-forwarding\", and \"to\n> fast-forward\" all appear multiple times. And yes, \"fast-forward\" also\n> appears multiple times as a noun in addition to the various uses as a\n> verb. So, I'd say the glossary just isn't comprehensive because in\n> this case we have a word that serves as both a noun and a verb.\n\nIt can be a noun, a verb, an adjective, and an adverb. But the\nquestion is not what it can be, but what it actually is. I'm just\ntelling you my rationale:\n\n1. noun: it doesn't make sense because you don't create, pick, show,\nor push a \"fast-forward\"\n2. verb: there's no idiom to tell git \"do fast-forward\"\n3. adjective: there are merge nouns (commits), but no instances of\nfast-forward merge commits, like say octopus merge commits\n4. adverb: you can tell git \"do merge\", and \"do fast-forward merge\"\n\nSo, in my opinion a fast-forward today can only logically be an adverb.\n\nLike a bubble sort is a special type of sort, and theoretically you\ncan say \"do a bubble\", but it's just weird. My mind is left hanging: a\nbubble $what? Likewise, when people say \"do me a solid\", I'm\nannoyed... A solid $what?! They mean \"a solid favor\". People do it, so\nit's part of language, but it doesn't stop it from being weird in my\nopinion.\n\nAn adverb typically answers the question \"in what way?\". Do me a\nfavor... In what way? In a solid way. Do a sort... In what way? In a\nquick way. Do a move... In what way? In a bold way. Do a merge... In\nwhat way? In a fast-forward way.\n\n> Going back to the text Junio highlighted, I agree with him that the\n> phrase looks really awkward, and much prefer his suggestion\n> (regardless of whether it aligns with the current glossary).\n\nNormally I don't show credentials, but in this case I think it might\nbe relevant. I've read multiple linguists, like Noam Chomsky, and\nSteven Pinker. I follow many others and read their articles. I also\nread The Pinker's Sense of Style: The Thinking Person's Guide to\nWriting in the 21st Century [2], which I can't recommend enough for\npeople writing technical documents or any other classic style. I have\nan arguably successful blog with more than 200 articles and more than\n1 million views, which is regularly linked from other blogs, and\ntechnical resources. I constantly get thanked both in person, and\nonline for what I write. And at some point I was asked by a publisher\nto write a book about Git (which I didn't feel prepared for at that\ntime).\n\nSo, clearly at least some people value the way I write.\n\nI'm not trying to be stubborn here, I just honestly put effort into\nthe art of writing, and I do care deeply about language.\n\nOf course I might be wrong in this particular instance, but if I am,\nit's not because of lack of effort.\n\nI think fast-forward is mainly an adverb, but even if it isn't the\nmain usage; it's still clearly an usage.\n\nCheers.\n\n[1] https://www.merriam-webster.com/dictionary/square\n[2] https://www.amazon.com/Sense-Style-Thinking-Persons-Writing/dp/0143127799\n\n-- \nFelipe Contreras\n"},{"id":"411683","messageId":"CAMP44s3jmCBaZKM1rEfTA7=HR-gr3TfHK2tH+wACsHwYYWT7iA@mail.gmail.com","threadId":"54771","inReplyTo":"xmqqblf58b99.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 01/16] doc: pull: explain what is a fast-forward","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T00:20:17Z","receivedAt":"2020-12-08T00:21:32Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Dec 7, 2020 at 5:08 PM Junio C Hamano wrote:\n\n> I thought that the idea that the word can be used as a verb, after\n> discussing advise() messages that tells the users that they can\n> \"merge, rebase or fast-forward\", was given and not something anybody\n> needs to be explained about.\n\nI removed that in the latest version, precisely because you can't tell\ngit pull to resolve a diverging branch situation by doing a\n\"fast-forward\". It's only merge or rebase.\n\n-- \nFelipe Contreras\n"},{"id":"411706","messageId":"CABPp-BESMs1tuVoLFMy-BahSChFz7oANqTaeJShFa_zDbEnvBA@mail.gmail.com","threadId":"54771","inReplyTo":"CAMP44s0g7JLTZZs=O3gUcEiPt--p3gXEPHL0GsBqoqweKA=9Rw@mail.gmail.com","subject":"Re: [PATCH v3 01/16] doc: pull: explain what is a fast-forward","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-12-08T01:23:00Z","receivedAt":"2020-12-08T01:23:54Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Dec 7, 2020 at 4:17 PM Felipe Contreras\n wrote:\n>\n> On Mon, Dec 7, 2020 at 4:40 PM Elijah Newren wrote:\n> > On Mon, Dec 7, 2020 at 2:22 PM Felipe Contreras\n> > wrote:\n>\n> > > The glossary defines a fast-forward as:\n> > >\n> > > A fast-forward is a special type of `merge`\n> > >\n> > > So, if you consider \"merge\" a noun, then a fast-forward is an\n> > > adjective. If you consider it a verb, then it's an adverb. But it's\n> > > not a verb.\n> >\n> > A square is a special type of a rectangle, but that doesn't make\n> > \"square\" an adjective; both square and rectangle are nouns.\n>\n> Words have multiple definitions. The word \"square\" is both a noun, and\n> an adjective [1]. It's perfectly fine to say \"square corner\".\n>\n> Just like it's perfectly fine to say \"fast-forward merge\", or \"quick sort\".\n>\n> And that's how many people use it:\n>\n> https://git-scm.com/docs/git-merge#_fast_forward_merge\n> https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html\n> https://www.atlassian.com/git/tutorials/using-branches/git-merge#:~:text=Fast%20Forward%20Merge\n>\n> Plus there's many instances in the documentation:\n>\n> * non-fast-forward update\n> * fast-forward merges\n> * fast-forward check\n> * \"fast-forward-ness\"\n> * fast-forward situation\n> * fast-forward push\n> * non-fast-forward pushes\n> * non-fast-forward reference\n> * fast-forward cases\n> * \"fast-forward\" merge\n\nYeah, and the number of \"fast-forward merge\" instances suggest I'm\nlosing the battle on \"fast-forward\" not being a merge but a different\nthing. So maybe I'm losing multiple battles here. :-)\n\n> > > If it was a verb, then we should have `git fast-forward`, which may\n> > > not be a terrible idea, but right now a fast-forward is a modifier.\n> > >\n> > > At least that's what I have in my mind, and the glossary seems to agree.\n> >\n> > If you read the release notes and even various messages printed by\n> > git, \"fast-forwards\", \"fast-forwarded\", \"fast-forwarding\", and \"to\n> > fast-forward\" all appear multiple times. And yes, \"fast-forward\" also\n> > appears multiple times as a noun in addition to the various uses as a\n> > verb. So, I'd say the glossary just isn't comprehensive because in\n> > this case we have a word that serves as both a noun and a verb.\n>\n> It can be a noun, a verb, an adjective, and an adverb. But the\n> question is not what it can be, but what it actually is. I'm just\n> telling you my rationale:\n>\n> 1. noun: it doesn't make sense because you don't create, pick, show,\n> or push a \"fast-forward\"\n> 2. verb: there's no idiom to tell git \"do fast-forward\"\n> 3. adjective: there are merge nouns (commits), but no instances of\n> fast-forward merge commits, like say octopus merge commits\n> 4. adverb: you can tell git \"do merge\", and \"do fast-forward merge\"\n>\n> So, in my opinion a fast-forward today can only logically be an adverb.\n>\n> Like a bubble sort is a special type of sort, and theoretically you\n> can say \"do a bubble\", but it's just weird. My mind is left hanging: a\n> bubble $what? Likewise, when people say \"do me a solid\", I'm\n> annoyed... A solid $what?! They mean \"a solid favor\". People do it, so\n> it's part of language, but it doesn't stop it from being weird in my\n> opinion.\n>\n> An adverb typically answers the question \"in what way?\". Do me a\n> favor... In what way? In a solid way. Do a sort... In what way? In a\n> quick way. Do a move... In what way? In a bold way. Do a merge... In\n> what way? In a fast-forward way.\n>\n> > Going back to the text Junio highlighted, I agree with him that the\n> > phrase looks really awkward, and much prefer his suggestion\n> > (regardless of whether it aligns with the current glossary).\n>\n> Normally I don't show credentials, but in this case I think it might\n> be relevant. I've read multiple linguists, like Noam Chomsky, and\n> Steven Pinker. I follow many others and read their articles. I also\n> read The Pinker's Sense of Style: The Thinking Person's Guide to\n> Writing in the 21st Century [2], which I can't recommend enough for\n> people writing technical documents or any other classic style. I have\n> an arguably successful blog with more than 200 articles and more than\n> 1 million views, which is regularly linked from other blogs, and\n> technical resources. I constantly get thanked both in person, and\n> online for what I write. And at some point I was asked by a publisher\n> to write a book about Git (which I didn't feel prepared for at that\n> time).\n>\n> So, clearly at least some people value the way I write.\n>\n> I'm not trying to be stubborn here, I just honestly put effort into\n> the art of writing, and I do care deeply about language.\n>\n> Of course I might be wrong in this particular instance, but if I am,\n> it's not because of lack of effort.\n>\n> I think fast-forward is mainly an adverb, but even if it isn't the\n> main usage; it's still clearly an usage.\n>\n> Cheers.\n>\n> [1] https://www.merriam-webster.com/dictionary/square\n> [2] https://www.amazon.com/Sense-Style-Thinking-Persons-Writing/dp/0143127799\n\nYou have very compelling arguments that fast-forward often serves as\nan adverb (and if I'd thought a little closer, I would have remembered\nthat I use \"fast-forward update\" myself). You have me convinced.\n\nHowever, I am somewhat less convinced that \"fast-forward\" doesn't also\nserve as a noun or a verb. Perhaps you are trying to argue how it\n*should* be used rather than how it *is* used, in which case I don't\nhave any counter-arguments for you (I'm less well linguistically\ntrained). But if you look at how it is used, the number of times \"a\nfast-forward update\" is shortened to \"a fast-forward\" in the\ndocumentation suggests to me it's often a noun, and the number of\ntimes that variants such as \"fast-forwards\", \"fast-forwarded\",\n\"fast-forwarding\", and \"to fast-forward\" appear in both the docs and\noutput messages means that it's also frequently used as a verb as\nwell. The fact that Junio expressed surprise upthread (\"I thought\nthat the idea that the word can be used as a verb...was given and not\nsomething anybody needs to be explained about\") also suggests that\nusage of fast-forward as a verb is common. Anyway, I think trying to\ntreat \"fast-forward\" as solely an adverb results in awkward phrases\nlike \"in a fast-forward way\" instead of just using the much simpler\nverb form.\n\nAlso, re-reading my earlier email, it looks like it could easily come\nacross as curt. My apologies if it did read that way.\n"},{"id":"411718","messageId":"CAMP44s2JMNML2a8ckdrorgMH6x+NduvWsvRn-p6O=XbYBofNgg@mail.gmail.com","threadId":"54771","inReplyTo":"CABPp-BESMs1tuVoLFMy-BahSChFz7oANqTaeJShFa_zDbEnvBA@mail.gmail.com","subject":"Re: [PATCH v3 01/16] doc: pull: explain what is a fast-forward","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T14:37:30Z","receivedAt":"2020-12-08T14:38:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Dec 7, 2020 at 7:23 PM Elijah Newren wrote:\n\n> Yeah, and the number of \"fast-forward merge\" instances suggest I'm\n> losing the battle on \"fast-forward\" not being a merge but a different\n> thing. So maybe I'm losing multiple battles here. :-)\n\nIn theory it could be both. I just don't see how.\n\n> You have very compelling arguments that fast-forward often serves as\n> an adverb (and if I'd thought a little closer, I would have remembered\n> that I use \"fast-forward update\" myself). You have me convinced.\n\nGreat!\n\n> However, I am somewhat less convinced that \"fast-forward\" doesn't also\n> serve as a noun or a verb.\n\nI'm not saying it doesn't serve as a noun or a verb. It is certainly\nused in that way *in addition* to being an adverb. I'm just saying in\nmy opinion it's primarily an adverb.\n\n> Perhaps you are trying to argue how it\n> *should* be used rather than how it *is* used, in which case I don't\n> have any counter-arguments for you (I'm less well linguistically\n> trained).\n\nNot quite. I agree it is used both as a noun and a verb. And I\nwouldn't attempt to mandate how words should be used (I'm against\nprescriptiveness).\n\nI'm just arguing from the point of view of the mental model. There is\nno such thing as a fast-forward object.\n\nTake for example the word \"calibration\". It is a noun, but you can't\npoint to any calibration thing. It comes from the verb calibrating,\nand such conversions are called nominalizations.\n\nI'm currently re-reading The Sense of Style, and it's interesting that\nin Chapter 2 Steven Pinker mentions precisely these nouns, which he\ncalls \"zombie nouns\". They certainly do exist, and people use them,\nbut they suck the lifeblood out of prose. Take for example\n\"comprehension checks were used as exclusion criteria\" (zombie nouns),\ncompared to \"we excluded people who failed to understand the\ninstructions\" (live verbs).\n\nThis article from writing expert Helen Sword does a great job of\nexplaining them:\n\nhttps://opinionator.blogs.nytimes.com/2012/07/23/zombie-nouns/\n\nYes, \"a merge done in a fast-forward way\", is a \"fast-forward merge\",\nwhich can be nouned as \"a fast-forward\". It's just not very alive.\n\n> The fact that Junio expressed surprise upthread (\"I thought\n> that the idea that the word can be used as a verb...was given and not\n> something anybody needs to be explained about\") also suggests that\n> usage of fast-forward as a verb is common. Anyway, I think trying to\n> treat \"fast-forward\" as solely an adverb results in awkward phrases\n> like \"in a fast-forward way\" instead of just using the much simpler\n> verb form.\n\nYes, it is common also. I'm just saying unless there's a \"git\nfast-forward\" command you can't really tell git \"do a fast-forward\",\nwhat you tell git is: \"do a merge in a fast-forward way\". It's a valid\nway of thinking, just not the way I think.\n\nBTW, I find it interesting that there are many instances of\n\"fast-forward update\" in the documentation, and back then I did create\na \"git update\" tool (essentially a copy of \"git pull\"), so this\nsuggests there's a void that such a tool certainly would fill.\n\nWith such a tool we would have \"fast-forward merge\", \"fast-forward\npull\", and \"fast-forward update\", which indicates that adverb is the\nmost natural notion.\n\n> Also, re-reading my earlier email, it looks like it could easily come\n> across as curt. My apologies if it did read that way.\n\nNo worries. I didn't take it as such. Same from my side; I'm just\nstating my opinion.\n\nCheers.\n\n[1] https://github.com/felipec/git/commit/d38f1641fc33535aa3c92cf6d3a30334324d3488\n\n-- \nFelipe Contreras\n"},{"id":"411743","messageId":"CAMP44s0jERkEoJgCvV3SkehviW0HhLZHCw0jfKUBn1kd=YBtEQ@mail.gmail.com","threadId":"54771","inReplyTo":"CAMP44s2JMNML2a8ckdrorgMH6x+NduvWsvRn-p6O=XbYBofNgg@mail.gmail.com","subject":"Re: [PATCH v3 01/16] doc: pull: explain what is a fast-forward","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-08T18:55:15Z","receivedAt":"2020-12-08T20:59:01Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Dec 8, 2020 at 8:37 AM Felipe Contreras\n wrote:\n\n> Take for example the word \"calibration\". It is a noun, but you can't\n> point to any calibration thing. It comes from the verb calibrating,\n> and such conversions are called nominalizations.\n>\n> I'm currently re-reading The Sense of Style, and it's interesting that\n> in Chapter 2 Steven Pinker mentions precisely these nouns, which he\n> calls \"zombie nouns\". They certainly do exist, and people use them,\n> but they suck the lifeblood out of prose. Take for example\n> \"comprehension checks were used as exclusion criteria\" (zombie nouns),\n> compared to \"we excluded people who failed to understand the\n> instructions\" (live verbs).\n\nActually I found this video, which is more digestible and enjoyable:\n\nZombie Nouns and the Passive Voice in Writing\nhttps://www.youtube.com/watch?v=sS-Txm3R3v8\n\n-- \nFelipe Contreras\n"}]}