{"thread":{"id":"54711","subject":"[RFC 1/2] pull: warn that pulling will not merge by default in Git 3.0","startedAt":"2020-11-25T02:14:57Z","lastAt":"2020-12-12T01:09:57Z","messageCount":18,"participants":["Alex Henrie","Felipe Contreras","Philip Oakley","Junio C Hamano","Jacob Keller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"410747","messageId":"20201125020931.248427-1-alexhenrie24@gmail.com","threadId":"54711","inReplyTo":null,"subject":"[RFC 1/2] pull: warn that pulling will not merge by default in Git 3.0","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2020-11-25T02:09:30Z","receivedAt":"2020-11-25T02:14:57Z","isPatch":false,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"Signed-off-by: Alex Henrie \n---\n builtin/pull.c | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 17aa63cd35..4d80efe5b7 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -345,13 +345,14 @@ 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\twarning(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n-\t\t\t\"discouraged. You can squelch this message by running one of the following\\n\"\n-\t\t\t\"commands sometime before your next pull:\\n\"\n+\t\twarning(_(\"Starting in Git 3.0, the default behavior of `git pull` will change\\n\"\n+\t\t\t\"when it is not specified how to reconcile divergent branches. You can\\n\"\n+\t\t\t\"squelch this message by running one of the following commands sometime\\n\"\n+\t\t\t\"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 false # merge (the current default)\\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\" git config pull.ff only # fast-forward only (the future default)\\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-- \n2.29.2\n\n"},{"id":"410748","messageId":"20201125020931.248427-2-alexhenrie24@gmail.com","threadId":"54711","inReplyTo":"20201125020931.248427-1-alexhenrie24@gmail.com","subject":"[RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2020-11-25T02:09:31Z","receivedAt":"2020-11-25T02:14:57Z","isPatch":false,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"Signed-off-by: Alex Henrie \n---\n Documentation/merge-options.txt | 14 +++++---\n builtin/pull.c | 41 +++++++++---------------\n t/t5520-pull.sh | 16 ++++-----\n t/t5521-pull-options.sh | 4 +--\n t/t5524-pull-msg.sh | 4 +--\n t/t5553-set-upstream.sh | 20 ++++++------\n t/t5604-clone-reference.sh | 4 +--\n t/t6402-merge-rename.sh | 20 ++++++------\n t/t6409-merge-subtree.sh | 6 ++--\n t/t6417-merge-ours-theirs.sh | 10 +++---\n t/t7600-merge.sh | 2 +-\n t/t7601-merge-pull-config.sh | 57 ---------------------------------\n t/t7603-merge-reduce-heads.sh | 2 +-\n 13 files changed, 70 insertions(+), 130 deletions(-)\n\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex eb0aabd396..1eb79e2939 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -43,10 +43,16 @@ set to `no` at the beginning of them.\n --no-ff::\n --ff-only::\n \tSpecifies how a merge is handled when the merged-in history is\n-\talready a descendant of the current history. `--ff` is the\n-\tdefault unless merging an annotated (and possibly signed) tag\n-\tthat is not stored in its natural place in the `refs/tags/`\n-\thierarchy, in which case `--no-ff` is assumed.\n+\talready a descendant of the current history.\n+\t+\n+\tIn `git pull`, `--ff-only` is the default unless `--rebase` or\n+\t`--no-rebase` is specified, in which case the default is `--ff`.\n+\tHowever, the default changes again to `--no-ff` in the unlikely\n+\tscenario where an annotated (and possibly signed) tag that is not\n+\tstored in its natural place in the `refs/tags/` hierarchy is being\n+\tmerged.\n+\t+\n+\tIn `git merge`, `--ff` is always the default.\n +\n With `--ff`, when possible resolve the merge as a fast-forward (only\n update the branch pointer to match the merged branch; do not create a\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 4d80efe5b7..c703560a77 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -292,17 +292,21 @@ static void set_reflog_message(int argc, const char **argv)\n }\n \n /**\n- * If pull.ff is unset, returns NULL. If pull.ff is \"true\", returns \"--ff\". If\n- * pull.ff is \"false\", returns \"--no-ff\". If pull.ff is \"only\", returns\n- * \"--ff-only\". Otherwise, if pull.ff is set to an invalid value, die with an\n- * error.\n+ * If pull.ff is unset, returns NULL if pull.rebase is set or \"--ff-only\" if\n+ * pull.rebase is not set. If pull.ff is \"true\", returns \"--ff\". If pull.ff is\n+ * \"false\", returns \"--no-ff\". If pull.ff is \"only\", returns \"--ff-only\".\n+ * Otherwise, if pull.ff is set to an invalid value, die with an error.\n */\n static const char *config_get_ff(void)\n {\n \tconst char *value;\n \n-\tif (git_config_get_value(\"pull.ff\", &value))\n-\t\treturn NULL;\n+\tif (git_config_get_value(\"pull.ff\", &value)) {\n+\t\tif (opt_rebase < 0)\n+\t\t\treturn \"--ff-only\";\n+\t\telse\n+\t\t\treturn NULL;\n+\t}\n \n \tswitch (git_parse_maybe_bool(value)) {\n \tcase 0:\n@@ -322,7 +326,7 @@ static const char *config_get_ff(void)\n * value of \"branch.$curr_branch.rebase\", where $curr_branch is the current\n * branch, and if HEAD is detached or the configuration key does not exist,\n * looks for the value of \"pull.rebase\". If both configuration keys do not\n- * exist, returns REBASE_FALSE.\n+ * exist, returns REBASE_INVALID.\n */\n static enum rebase_type config_get_rebase(void)\n {\n@@ -344,23 +348,7 @@ static enum rebase_type config_get_rebase(void)\n \tif (!git_config_get_value(\"pull.rebase\", &value))\n \t\treturn parse_config_rebase(\"pull.rebase\", value, 1);\n \n-\tif (opt_verbosity >= 0 && !opt_ff) {\n-\t\twarning(_(\"Starting in Git 3.0, the default behavior of `git pull` will change\\n\"\n-\t\t\t\"when it is not specified how to reconcile divergent branches. You can\\n\"\n-\t\t\t\"squelch this message by running one of the following commands sometime\\n\"\n-\t\t\t\"before your next pull:\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\" git config pull.rebase false # merge (the current default)\\n\"\n-\t\t\t\" git config pull.rebase true # rebase\\n\"\n-\t\t\t\" git config pull.ff only # fast-forward only (the future default)\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n-\t\t\t\"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n-\t\t\t\"or --ff-only on the command line to override the configured default per\\n\"\n-\t\t\t\"invocation.\\n\"));\n-\t}\n-\n-\treturn REBASE_FALSE;\n+\treturn REBASE_INVALID;\n }\n \n /**\n@@ -931,11 +919,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \n \tparse_repo_refspecs(argc, argv, &repo, &refspecs);\n \n+\tif (opt_rebase < 0)\n+\t\topt_rebase = config_get_rebase();\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+\t\topt_rebase = REBASE_FALSE;\n \n \tif (read_cache_unmerged())\n \t\tdie_resolve_conflict(\"pull\");\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 9fae07cdfa..856eccd706 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -15,7 +15,7 @@ test_pull_autostash () {\n \tgit reset --hard before-rebase &&\n \techo dirty >new_file &&\n \tgit add new_file &&\n-\tgit pull \"$@\" . copy &&\n+\tgit pull --ff \"$@\" . copy &&\n \ttest_cmp_rev HEAD^\"$expect_parent_num\" copy &&\n \techo dirty >expect &&\n \ttest_cmp expect new_file &&\n@@ -27,7 +27,7 @@ test_pull_autostash_fail () {\n \tgit reset --hard before-rebase &&\n \techo dirty >new_file &&\n \tgit add new_file &&\n-\ttest_must_fail git pull \"$@\" . copy 2>err &&\n+\ttest_must_fail git pull --ff \"$@\" . copy 2>err &&\n \ttest_i18ngrep -E \"uncommitted changes.|overwritten by merge:\" err\n }\n \n@@ -210,7 +210,7 @@ test_expect_success 'fail if upstream branch does not exist' '\n \ttest_config branch.test.merge refs/heads/nonexisting &&\n \techo file >expect &&\n \ttest_cmp expect file &&\n-\ttest_must_fail git pull 2>err &&\n+\ttest_must_fail git pull --no-rebase 2>err &&\n \ttest_i18ngrep \"no such ref was fetched\" err &&\n \ttest_cmp expect file\n '\n@@ -223,17 +223,17 @@ test_expect_success 'fail if the index has unresolved entries' '\n \ttest_commit modified2 file &&\n \tgit ls-files -u >unmerged &&\n \ttest_must_be_empty unmerged &&\n-\ttest_must_fail git pull . second &&\n+\ttest_must_fail git pull --no-rebase . second &&\n \tgit ls-files -u >unmerged &&\n \ttest_file_not_empty unmerged &&\n \tcp file expected &&\n-\ttest_must_fail git pull . second 2>err &&\n+\ttest_must_fail git pull --no-rebase . second 2>err &&\n \ttest_i18ngrep \"Pulling is not possible because you have unmerged files.\" err &&\n \ttest_cmp expected file &&\n \tgit add file &&\n \tgit ls-files -u >unmerged &&\n \ttest_must_be_empty unmerged &&\n-\ttest_must_fail git pull . second 2>err &&\n+\ttest_must_fail git pull --no-rebase . second 2>err &&\n \ttest_i18ngrep \"You have not concluded your merge\" err &&\n \ttest_cmp expected file\n '\n@@ -243,7 +243,7 @@ test_expect_success 'fast-forwards working tree if branch head is updated' '\n \ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n \techo file >expect &&\n \ttest_cmp expect file &&\n-\tgit pull . second:third 2>err &&\n+\tgit pull --no-rebase . second:third 2>err &&\n \ttest_i18ngrep \"fetch updated the current branch head\" err &&\n \techo modified >expect &&\n \ttest_cmp expect file &&\n@@ -256,7 +256,7 @@ test_expect_success 'fast-forward fails with conflicting work tree' '\n \techo file >expect &&\n \ttest_cmp expect file &&\n \techo conflict >file &&\n-\ttest_must_fail git pull . second:third 2>err &&\n+\ttest_must_fail git pull --no-rebase . second:third 2>err &&\n \ttest_i18ngrep \"Cannot fast-forward your working tree\" err &&\n \techo conflict >expect &&\n \ttest_cmp expect file &&\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex db1a381cd9..24801857ec 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -175,8 +175,8 @@ test_expect_success 'git pull --allow-unrelated-histories' '\n \t) &&\n \t(\n \t\tcd dst &&\n-\t\ttest_must_fail git pull ../src side &&\n-\t\tgit pull --allow-unrelated-histories ../src side\n+\t\ttest_must_fail git pull --no-rebase ../src side &&\n+\t\tgit pull --no-rebase --allow-unrelated-histories ../src side\n \t)\n '\n \ndiff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\nindex c278adaa5a..b2be3605f5 100755\n--- a/t/t5524-pull-msg.sh\n+++ b/t/t5524-pull-msg.sh\n@@ -28,7 +28,7 @@ test_expect_success setup '\n test_expect_success pull '\n (\n \tcd cloned &&\n-\tgit pull --log &&\n+\tgit pull --no-rebase --log &&\n \tgit log -2 &&\n \tgit cat-file commit HEAD >result &&\n \tgrep Dollar result\n@@ -41,7 +41,7 @@ test_expect_success '--log=1 limits shortlog length' '\n \tgit reset --hard HEAD^ &&\n \ttest \"$(cat afile)\" = original &&\n \ttest \"$(cat bfile)\" = added &&\n-\tgit pull --log=1 &&\n+\tgit pull --no-rebase --log=1 &&\n \tgit log -3 &&\n \tgit cat-file commit HEAD >result &&\n \tgrep Dollar result &&\ndiff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh\nindex 7622981cbf..53b9add9f6 100755\n--- a/t/t5553-set-upstream.sh\n+++ b/t/t5553-set-upstream.sh\n@@ -105,34 +105,34 @@ test_expect_success 'setup commit on master and other pull' '\n \n test_expect_success 'pull --set-upstream upstream master sets branch master but not other' '\n \tclear_config master other &&\n-\tgit pull --set-upstream upstream master &&\n+\tgit pull --no-rebase --set-upstream upstream master &&\n \tcheck_config master upstream refs/heads/master &&\n \tcheck_config_missing other\n '\n \n test_expect_success 'pull --set-upstream master:other2 does not set the branch other2' '\n \tclear_config other2 &&\n-\tgit pull --set-upstream upstream master:other2 &&\n+\tgit pull --no-rebase --set-upstream upstream master:other2 &&\n \tcheck_config_missing other2\n '\n \n test_expect_success 'pull --set-upstream upstream other sets branch master' '\n \tclear_config master other &&\n-\tgit pull --set-upstream upstream other &&\n+\tgit pull --no-rebase --set-upstream upstream other &&\n \tcheck_config master upstream refs/heads/other &&\n \tcheck_config_missing other\n '\n \n test_expect_success 'pull --set-upstream upstream tag does not set the tag' '\n \tclear_config three &&\n-\tgit pull --tags --set-upstream upstream three &&\n+\tgit pull --no-rebase --tags --set-upstream upstream three &&\n \tcheck_config_missing three\n '\n \n test_expect_success 'pull --set-upstream http://nosuchdomain.example.com fails with invalid url' '\n \t# master explicitly not cleared, we check that it is not touched from previous value\n \tclear_config other other2 three &&\n-\ttest_must_fail git pull --set-upstream http://nosuchdomain.example.com &&\n+\ttest_must_fail git pull --no-rebase --set-upstream http://nosuchdomain.example.com &&\n \tcheck_config master upstream refs/heads/other &&\n \tcheck_config_missing other &&\n \tcheck_config_missing other2 &&\n@@ -141,16 +141,16 @@ test_expect_success 'pull --set-upstream http://nosuchdomain.example.com fails w\n \n test_expect_success 'pull --set-upstream upstream HEAD sets branch HEAD' '\n \tclear_config master other &&\n-\tgit pull --set-upstream upstream HEAD &&\n+\tgit pull --no-rebase --set-upstream upstream HEAD &&\n \tcheck_config master upstream HEAD &&\n \tgit checkout other &&\n-\tgit pull --set-upstream upstream HEAD &&\n+\tgit pull --no-rebase --set-upstream upstream HEAD &&\n \tcheck_config other upstream HEAD\n '\n \n test_expect_success 'pull --set-upstream upstream with more than one branch does nothing' '\n \tclear_config master three &&\n-\tgit pull --set-upstream upstream master three &&\n+\tgit pull --no-rebase --set-upstream upstream master three &&\n \tcheck_config_missing master &&\n \tcheck_config_missing three\n '\n@@ -159,7 +159,7 @@ test_expect_success 'pull --set-upstream with valid URL sets upstream to URL' '\n \tclear_config master other other2 &&\n \tgit checkout master &&\n \turl=\"file://$PWD\" &&\n-\tgit pull --set-upstream \"$url\" &&\n+\tgit pull --no-rebase --set-upstream \"$url\" &&\n \tcheck_config master \"$url\" HEAD &&\n \tcheck_config_missing other &&\n \tcheck_config_missing other2\n@@ -169,7 +169,7 @@ test_expect_success 'pull --set-upstream with valid URL and branch sets branch'\n \tclear_config master other other2 &&\n \tgit checkout master &&\n \turl=\"file://$PWD\" &&\n-\tgit pull --set-upstream \"$url\" master &&\n+\tgit pull --no-rebase --set-upstream \"$url\" master &&\n \tcheck_config master \"$url\" refs/heads/master &&\n \tcheck_config_missing other &&\n \tcheck_config_missing other2\ndiff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh\nindex 2f7be23044..b9e11eceec 100755\n--- a/t/t5604-clone-reference.sh\n+++ b/t/t5604-clone-reference.sh\n@@ -84,7 +84,7 @@ test_expect_success 'updating origin' '\n '\n \n test_expect_success 'pulling changes from origin' '\n-\tgit -C C pull origin\n+\tgit -C C pull --no-rebase origin\n '\n \n # the 2 local objects are commit and tree from the merge\n@@ -93,7 +93,7 @@ test_expect_success 'that alternate to origin gets used' '\n '\n \n test_expect_success 'pulling changes from origin' '\n-\tgit -C D pull origin\n+\tgit -C D pull --no-rebase origin\n '\n \n # the 5 local objects are expected; file3 blob, commit in A to add it\ndiff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh\nindex 3f64f62224..ad140a8def 100755\n--- a/t/t6402-merge-rename.sh\n+++ b/t/t6402-merge-rename.sh\n@@ -100,7 +100,7 @@ test_expect_success 'setup' '\n test_expect_success 'pull renaming branch into unrenaming one' \\\n '\n \tgit show-branch &&\n-\ttest_expect_code 1 git pull . white &&\n+\ttest_expect_code 1 git pull --no-rebase . white &&\n \tgit ls-files -s &&\n \tgit ls-files -u B >b.stages &&\n \ttest_line_count = 3 b.stages &&\n@@ -118,7 +118,7 @@ test_expect_success 'pull renaming branch into another renaming one' \\\n \trm -f B &&\n \tgit reset --hard &&\n \tgit checkout red &&\n-\ttest_expect_code 1 git pull . white &&\n+\ttest_expect_code 1 git pull --no-rebase . white &&\n \tgit ls-files -u B >b.stages &&\n \ttest_line_count = 3 b.stages &&\n \tgit ls-files -s N >n.stages &&\n@@ -134,7 +134,7 @@ test_expect_success 'pull unrenaming branch into renaming one' \\\n '\n \tgit reset --hard &&\n \tgit show-branch &&\n-\ttest_expect_code 1 git pull . master &&\n+\ttest_expect_code 1 git pull --no-rebase . master &&\n \tgit ls-files -u B >b.stages &&\n \ttest_line_count = 3 b.stages &&\n \tgit ls-files -s N >n.stages &&\n@@ -150,7 +150,7 @@ test_expect_success 'pull conflicting renames' \\\n '\n \tgit reset --hard &&\n \tgit show-branch &&\n-\ttest_expect_code 1 git pull . blue &&\n+\ttest_expect_code 1 git pull --no-rebase . blue &&\n \tgit ls-files -u A >a.stages &&\n \ttest_line_count = 1 a.stages &&\n \tgit ls-files -u B >b.stages &&\n@@ -170,7 +170,7 @@ test_expect_success 'interference with untracked working tree file' '\n \tgit reset --hard &&\n \tgit show-branch &&\n \techo >A this file should not matter &&\n-\ttest_expect_code 1 git pull . white &&\n+\ttest_expect_code 1 git pull --no-rebase . white &&\n \ttest_path_is_file A\n '\n \n@@ -180,7 +180,7 @@ test_expect_success 'interference with untracked working tree file' '\n \tgit show-branch &&\n \trm -f A &&\n \techo >A this file should not matter &&\n-\ttest_expect_code 1 git pull . red &&\n+\ttest_expect_code 1 git pull --no-rebase . red &&\n \ttest_path_is_file A\n '\n \n@@ -190,7 +190,7 @@ test_expect_success 'interference with untracked working tree file' '\n \tgit checkout -f master &&\n \tgit tag -f anchor &&\n \tgit show-branch &&\n-\tgit pull . yellow &&\n+\tgit pull --no-rebase . yellow &&\n \ttest_path_is_missing M &&\n \tgit reset --hard anchor\n '\n@@ -203,7 +203,7 @@ test_expect_success 'updated working tree file should prevent the merge' '\n \tgit show-branch &&\n \techo >>M one line addition &&\n \tcat M >M.saved &&\n-\ttest_expect_code 128 git pull . yellow &&\n+\ttest_expect_code 128 git pull --no-rebase . yellow &&\n \ttest_cmp M M.saved &&\n \trm -f M.saved\n '\n@@ -217,7 +217,7 @@ test_expect_success 'updated working tree file should prevent the merge' '\n \techo >>M one line addition &&\n \tcat M >M.saved &&\n \tgit update-index M &&\n-\ttest_expect_code 128 git pull . yellow &&\n+\ttest_expect_code 128 git pull --no-rebase . yellow &&\n \ttest_cmp M M.saved &&\n \trm -f M.saved\n '\n@@ -229,7 +229,7 @@ test_expect_success 'interference with untracked working tree file' '\n \tgit tag -f anchor &&\n \tgit show-branch &&\n \techo >M this file should not matter &&\n-\tgit pull . master &&\n+\tgit pull --no-rebase . master &&\n \ttest_path_is_file M &&\n \t! {\n \t\tgit ls-files -s |\ndiff --git a/t/t6409-merge-subtree.sh b/t/t6409-merge-subtree.sh\nindex b8e8b6f642..b79cb0a368 100755\n--- a/t/t6409-merge-subtree.sh\n+++ b/t/t6409-merge-subtree.sh\n@@ -97,7 +97,7 @@ test_expect_success 'merge update' '\n \tgit checkout -b topic_2 &&\n \tgit commit -m \"update git-gui\" &&\n \tcd ../git &&\n-\tgit pull -s subtree gui topic_2 &&\n+\tgit pull --no-rebase -s subtree gui topic_2 &&\n \tgit ls-files -s >actual &&\n \t(\n \t\techo \"100644 $o3 0\tgit-gui/git-gui.sh\" &&\n@@ -126,7 +126,7 @@ test_expect_success 'initial ambiguous subtree' '\n test_expect_success 'merge using explicit' '\n \tcd ../git &&\n \tgit reset --hard topic_2 &&\n-\tgit pull -Xsubtree=git-gui gui topic_2 &&\n+\tgit pull --no-rebase -Xsubtree=git-gui gui topic_2 &&\n \tgit ls-files -s >actual &&\n \t(\n \t\techo \"100644 $o3 0\tgit-gui/git-gui.sh\" &&\n@@ -139,7 +139,7 @@ test_expect_success 'merge using explicit' '\n test_expect_success 'merge2 using explicit' '\n \tcd ../git &&\n \tgit reset --hard topic_2 &&\n-\tgit pull -Xsubtree=git-gui2 gui topic_2 &&\n+\tgit pull --no-rebase -Xsubtree=git-gui2 gui topic_2 &&\n \tgit ls-files -s >actual &&\n \t(\n \t\techo \"100644 $o1 0\tgit-gui/git-gui.sh\" &&\ndiff --git a/t/t6417-merge-ours-theirs.sh b/t/t6417-merge-ours-theirs.sh\nindex 0aebc6c028..ba9b100562 100755\n--- a/t/t6417-merge-ours-theirs.sh\n+++ b/t/t6417-merge-ours-theirs.sh\n@@ -66,11 +66,11 @@ test_expect_success 'binary file with -Xours/-Xtheirs' '\n '\n \n test_expect_success 'pull passes -X to underlying merge' '\n-\tgit reset --hard master && git pull -s recursive -Xours . side &&\n-\tgit reset --hard master && git pull -s recursive -X ours . side &&\n-\tgit reset --hard master && git pull -s recursive -Xtheirs . side &&\n-\tgit reset --hard master && git pull -s recursive -X theirs . side &&\n-\tgit reset --hard master && test_must_fail git pull -s recursive -X bork . side\n+\tgit reset --hard master && git pull --no-rebase -s recursive -Xours . side &&\n+\tgit reset --hard master && git pull --no-rebase -s recursive -X ours . side &&\n+\tgit reset --hard master && git pull --no-rebase -s recursive -Xtheirs . side &&\n+\tgit reset --hard master && git pull --no-rebase -s recursive -X theirs . side &&\n+\tgit reset --hard master && test_must_fail git pull --no-rebase -s recursive -X bork . side\n '\n \n test_expect_success SYMLINKS 'symlink with -Xours/-Xtheirs' '\ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 1c85f75555..3ad47f0925 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -891,7 +891,7 @@ test_expect_success 'merge annotated/signed tag w/o tracking' '\n \t\t# tag from the \"upstream\", this pull defaults to --no-ff\n \t\tcd dst &&\n \t\tgit pull .. c0 &&\n-\t\tgit pull .. anno1 &&\n+\t\tgit pull --no-rebase .. anno1 &&\n \t\tgit rev-parse HEAD^2 >actual &&\n \t\ttest_cmp expect actual\n \t)\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex c5c4ea5fc0..c6c44ec570 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -27,63 +27,6 @@ test_expect_success 'setup' '\n \tgit tag c3\n '\n \n-test_expect_success 'pull.rebase not set' '\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 c0 &&\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-\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-\ttest_config pull.ff only &&\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 --rebase given' '\n-\tgit reset --hard c0 &&\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 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 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 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-\ttest_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n-'\n-\n test_expect_success 'merge c1 with c2' '\n \tgit reset --hard c1 &&\n \ttest -f c0.c &&\ndiff --git a/t/t7603-merge-reduce-heads.sh b/t/t7603-merge-reduce-heads.sh\nindex 98948955ae..27cd94ad6f 100755\n--- a/t/t7603-merge-reduce-heads.sh\n+++ b/t/t7603-merge-reduce-heads.sh\n@@ -68,7 +68,7 @@ test_expect_success 'merge c1 with c2, c3, c4, c5' '\n \n test_expect_success 'pull c2, c3, c4, c5 into c1' '\n \tgit reset --hard c1 &&\n-\tgit pull . c2 c3 c4 c5 &&\n+\tgit pull --no-rebase . c2 c3 c4 c5 &&\n \ttest \"$(git rev-parse c1)\" != \"$(git rev-parse HEAD)\" &&\n \ttest \"$(git rev-parse c1)\" = \"$(git rev-parse HEAD^1)\" &&\n \ttest \"$(git rev-parse c2)\" = \"$(git rev-parse HEAD^2)\" &&\n-- \n2.29.2\n\n"},{"id":"410762","messageId":"CAMP44s18Sd=DWMhFWCSDyGEz74ZKVF2kukr_=Exe1CjnPULCoA@mail.gmail.com","threadId":"54711","inReplyTo":"20201125020931.248427-2-alexhenrie24@gmail.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:45:21Z","receivedAt":"2020-11-25T03:45:57Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Nov 24, 2020 at 8:14 PM Alex Henrie wrote:\n\nBefore making this the default we need a solution *right now* that is\na sane default.\n\n> Signed-off-by: Alex Henrie \n> ---\n> Documentation/merge-options.txt | 14 +++++---\n> builtin/pull.c | 41 +++++++++---------------\n> t/t5520-pull.sh | 16 ++++-----\n> t/t5521-pull-options.sh | 4 +--\n> t/t5524-pull-msg.sh | 4 +--\n> t/t5553-set-upstream.sh | 20 ++++++------\n> t/t5604-clone-reference.sh | 4 +--\n> t/t6402-merge-rename.sh | 20 ++++++------\n> t/t6409-merge-subtree.sh | 6 ++--\n> t/t6417-merge-ours-theirs.sh | 10 +++---\n> t/t7600-merge.sh | 2 +-\n> t/t7601-merge-pull-config.sh | 57 ---------------------------------\n> t/t7603-merge-reduce-heads.sh | 2 +-\n> 13 files changed, 70 insertions(+), 130 deletions(-)\n>\n> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\n> index eb0aabd396..1eb79e2939 100644\n> --- a/Documentation/merge-options.txt\n> +++ b/Documentation/merge-options.txt\n> @@ -43,10 +43,16 @@ set to `no` at the beginning of them.\n> --no-ff::\n> --ff-only::\n> Specifies how a merge is handled when the merged-in history is\n> - already a descendant of the current history. `--ff` is the\n> - default unless merging an annotated (and possibly signed) tag\n> - that is not stored in its natural place in the `refs/tags/`\n> - hierarchy, in which case `--no-ff` is assumed.\n> + already a descendant of the current history.\n> + +\n> + In `git pull`, `--ff-only` is the default unless `--rebase` or\n> + `--no-rebase` is specified, in which case the default is `--ff`.\n> + However, the default changes again to `--no-ff` in the unlikely\n> + scenario where an annotated (and possibly signed) tag that is not\n> + stored in its natural place in the `refs/tags/` hierarchy is being\n> + merged.\n> + +\n> + In `git merge`, `--ff` is always the default.\n> +\n> With `--ff`, when possible resolve the merge as a fast-forward (only\n> update the branch pointer to match the merged branch; do not create a\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 4d80efe5b7..c703560a77 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -292,17 +292,21 @@ static void set_reflog_message(int argc, const char **argv)\n> }\n>\n> /**\n> - * If pull.ff is unset, returns NULL. If pull.ff is \"true\", returns \"--ff\". If\n> - * pull.ff is \"false\", returns \"--no-ff\". If pull.ff is \"only\", returns\n> - * \"--ff-only\". Otherwise, if pull.ff is set to an invalid value, die with an\n> - * error.\n> + * If pull.ff is unset, returns NULL if pull.rebase is set or \"--ff-only\" if\n> + * pull.rebase is not set. If pull.ff is \"true\", returns \"--ff\". If pull.ff is\n> + * \"false\", returns \"--no-ff\". If pull.ff is \"only\", returns \"--ff-only\".\n> + * Otherwise, if pull.ff is set to an invalid value, die with an error.\n> */\n> static const char *config_get_ff(void)\n> {\n> const char *value;\n>\n> - if (git_config_get_value(\"pull.ff\", &value))\n> - return NULL;\n> + if (git_config_get_value(\"pull.ff\", &value)) {\n> + if (opt_rebase < 0)\n> + return \"--ff-only\";\n> + else\n> + return NULL;\n> + }\n>\n> switch (git_parse_maybe_bool(value)) {\n> case 0:\n> @@ -322,7 +326,7 @@ static const char *config_get_ff(void)\n> * value of \"branch.$curr_branch.rebase\", where $curr_branch is the current\n> * branch, and if HEAD is detached or the configuration key does not exist,\n> * looks for the value of \"pull.rebase\". If both configuration keys do not\n> - * exist, returns REBASE_FALSE.\n> + * exist, returns REBASE_INVALID.\n> */\n> static enum rebase_type config_get_rebase(void)\n> {\n> @@ -344,23 +348,7 @@ static enum rebase_type config_get_rebase(void)\n> if (!git_config_get_value(\"pull.rebase\", &value))\n> return parse_config_rebase(\"pull.rebase\", value, 1);\n>\n> - if (opt_verbosity >= 0 && !opt_ff) {\n> - warning(_(\"Starting in Git 3.0, the default behavior of `git pull` will change\\n\"\n> - \"when it is not specified how to reconcile divergent branches. You can\\n\"\n> - \"squelch this message by running one of the following commands sometime\\n\"\n> - \"before your next pull:\\n\"\n> - \"\\n\"\n> - \" git config pull.rebase false # merge (the current default)\\n\"\n> - \" git config pull.rebase true # rebase\\n\"\n> - \" git config pull.ff only # fast-forward only (the future default)\\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> - return REBASE_FALSE;\n> + return REBASE_INVALID;\n> }\n>\n> /**\n> @@ -931,11 +919,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n>\n> parse_repo_refspecs(argc, argv, &repo, &refspecs);\n>\n> + if (opt_rebase < 0)\n> + opt_rebase = config_get_rebase();\n> +\n> if (!opt_ff)\n> opt_ff = xstrdup_or_null(config_get_ff());\n>\n> if (opt_rebase < 0)\n> - opt_rebase = config_get_rebase();\n> + opt_rebase = REBASE_FALSE;\n>\n> if (read_cache_unmerged())\n> die_resolve_conflict(\"pull\");\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 9fae07cdfa..856eccd706 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -15,7 +15,7 @@ test_pull_autostash () {\n> git reset --hard before-rebase &&\n> echo dirty >new_file &&\n> git add new_file &&\n> - git pull \"$@\" . copy &&\n> + git pull --ff \"$@\" . copy &&\n> test_cmp_rev HEAD^\"$expect_parent_num\" copy &&\n> echo dirty >expect &&\n> test_cmp expect new_file &&\n> @@ -27,7 +27,7 @@ test_pull_autostash_fail () {\n> git reset --hard before-rebase &&\n> echo dirty >new_file &&\n> git add new_file &&\n> - test_must_fail git pull \"$@\" . copy 2>err &&\n> + test_must_fail git pull --ff \"$@\" . copy 2>err &&\n> test_i18ngrep -E \"uncommitted changes.|overwritten by merge:\" err\n> }\n>\n> @@ -210,7 +210,7 @@ test_expect_success 'fail if upstream branch does not exist' '\n> test_config branch.test.merge refs/heads/nonexisting &&\n> echo file >expect &&\n> test_cmp expect file &&\n> - test_must_fail git pull 2>err &&\n> + test_must_fail git pull --no-rebase 2>err &&\n> test_i18ngrep \"no such ref was fetched\" err &&\n> test_cmp expect file\n> '\n> @@ -223,17 +223,17 @@ test_expect_success 'fail if the index has unresolved entries' '\n> test_commit modified2 file &&\n> git ls-files -u >unmerged &&\n> test_must_be_empty unmerged &&\n> - test_must_fail git pull . second &&\n> + test_must_fail git pull --no-rebase . second &&\n> git ls-files -u >unmerged &&\n> test_file_not_empty unmerged &&\n> cp file expected &&\n> - test_must_fail git pull . second 2>err &&\n> + test_must_fail git pull --no-rebase . second 2>err &&\n> test_i18ngrep \"Pulling is not possible because you have unmerged files.\" err &&\n> test_cmp expected file &&\n> git add file &&\n> git ls-files -u >unmerged &&\n> test_must_be_empty unmerged &&\n> - test_must_fail git pull . second 2>err &&\n> + test_must_fail git pull --no-rebase . second 2>err &&\n> test_i18ngrep \"You have not concluded your merge\" err &&\n> test_cmp expected file\n> '\n> @@ -243,7 +243,7 @@ test_expect_success 'fast-forwards working tree if branch head is updated' '\n> test_when_finished \"git checkout -f copy && git branch -D third\" &&\n> echo file >expect &&\n> test_cmp expect file &&\n> - git pull . second:third 2>err &&\n> + git pull --no-rebase . second:third 2>err &&\n> test_i18ngrep \"fetch updated the current branch head\" err &&\n> echo modified >expect &&\n> test_cmp expect file &&\n> @@ -256,7 +256,7 @@ test_expect_success 'fast-forward fails with conflicting work tree' '\n> echo file >expect &&\n> test_cmp expect file &&\n> echo conflict >file &&\n> - test_must_fail git pull . second:third 2>err &&\n> + test_must_fail git pull --no-rebase . second:third 2>err &&\n> test_i18ngrep \"Cannot fast-forward your working tree\" err &&\n> echo conflict >expect &&\n> test_cmp expect file &&\n> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\n> index db1a381cd9..24801857ec 100755\n> --- a/t/t5521-pull-options.sh\n> +++ b/t/t5521-pull-options.sh\n> @@ -175,8 +175,8 @@ test_expect_success 'git pull --allow-unrelated-histories' '\n> ) &&\n> (\n> cd dst &&\n> - test_must_fail git pull ../src side &&\n> - git pull --allow-unrelated-histories ../src side\n> + test_must_fail git pull --no-rebase ../src side &&\n> + git pull --no-rebase --allow-unrelated-histories ../src side\n> )\n> '\n>\n> diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\n> index c278adaa5a..b2be3605f5 100755\n> --- a/t/t5524-pull-msg.sh\n> +++ b/t/t5524-pull-msg.sh\n> @@ -28,7 +28,7 @@ test_expect_success setup '\n> test_expect_success pull '\n> (\n> cd cloned &&\n> - git pull --log &&\n> + git pull --no-rebase --log &&\n> git log -2 &&\n> git cat-file commit HEAD >result &&\n> grep Dollar result\n> @@ -41,7 +41,7 @@ test_expect_success '--log=1 limits shortlog length' '\n> git reset --hard HEAD^ &&\n> test \"$(cat afile)\" = original &&\n> test \"$(cat bfile)\" = added &&\n> - git pull --log=1 &&\n> + git pull --no-rebase --log=1 &&\n> git log -3 &&\n> git cat-file commit HEAD >result &&\n> grep Dollar result &&\n> diff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh\n> index 7622981cbf..53b9add9f6 100755\n> --- a/t/t5553-set-upstream.sh\n> +++ b/t/t5553-set-upstream.sh\n> @@ -105,34 +105,34 @@ test_expect_success 'setup commit on master and other pull' '\n>\n> test_expect_success 'pull --set-upstream upstream master sets branch master but not other' '\n> clear_config master other &&\n> - git pull --set-upstream upstream master &&\n> + git pull --no-rebase --set-upstream upstream master &&\n> check_config master upstream refs/heads/master &&\n> check_config_missing other\n> '\n>\n> test_expect_success 'pull --set-upstream master:other2 does not set the branch other2' '\n> clear_config other2 &&\n> - git pull --set-upstream upstream master:other2 &&\n> + git pull --no-rebase --set-upstream upstream master:other2 &&\n> check_config_missing other2\n> '\n>\n> test_expect_success 'pull --set-upstream upstream other sets branch master' '\n> clear_config master other &&\n> - git pull --set-upstream upstream other &&\n> + git pull --no-rebase --set-upstream upstream other &&\n> check_config master upstream refs/heads/other &&\n> check_config_missing other\n> '\n>\n> test_expect_success 'pull --set-upstream upstream tag does not set the tag' '\n> clear_config three &&\n> - git pull --tags --set-upstream upstream three &&\n> + git pull --no-rebase --tags --set-upstream upstream three &&\n> check_config_missing three\n> '\n>\n> test_expect_success 'pull --set-upstream http://nosuchdomain.example.com fails with invalid url' '\n> # master explicitly not cleared, we check that it is not touched from previous value\n> clear_config other other2 three &&\n> - test_must_fail git pull --set-upstream http://nosuchdomain.example.com &&\n> + test_must_fail git pull --no-rebase --set-upstream http://nosuchdomain.example.com &&\n> check_config master upstream refs/heads/other &&\n> check_config_missing other &&\n> check_config_missing other2 &&\n> @@ -141,16 +141,16 @@ test_expect_success 'pull --set-upstream http://nosuchdomain.example.com fails w\n>\n> test_expect_success 'pull --set-upstream upstream HEAD sets branch HEAD' '\n> clear_config master other &&\n> - git pull --set-upstream upstream HEAD &&\n> + git pull --no-rebase --set-upstream upstream HEAD &&\n> check_config master upstream HEAD &&\n> git checkout other &&\n> - git pull --set-upstream upstream HEAD &&\n> + git pull --no-rebase --set-upstream upstream HEAD &&\n> check_config other upstream HEAD\n> '\n>\n> test_expect_success 'pull --set-upstream upstream with more than one branch does nothing' '\n> clear_config master three &&\n> - git pull --set-upstream upstream master three &&\n> + git pull --no-rebase --set-upstream upstream master three &&\n> check_config_missing master &&\n> check_config_missing three\n> '\n> @@ -159,7 +159,7 @@ test_expect_success 'pull --set-upstream with valid URL sets upstream to URL' '\n> clear_config master other other2 &&\n> git checkout master &&\n> url=\"file://$PWD\" &&\n> - git pull --set-upstream \"$url\" &&\n> + git pull --no-rebase --set-upstream \"$url\" &&\n> check_config master \"$url\" HEAD &&\n> check_config_missing other &&\n> check_config_missing other2\n> @@ -169,7 +169,7 @@ test_expect_success 'pull --set-upstream with valid URL and branch sets branch'\n> clear_config master other other2 &&\n> git checkout master &&\n> url=\"file://$PWD\" &&\n> - git pull --set-upstream \"$url\" master &&\n> + git pull --no-rebase --set-upstream \"$url\" master &&\n> check_config master \"$url\" refs/heads/master &&\n> check_config_missing other &&\n> check_config_missing other2\n> diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh\n> index 2f7be23044..b9e11eceec 100755\n> --- a/t/t5604-clone-reference.sh\n> +++ b/t/t5604-clone-reference.sh\n> @@ -84,7 +84,7 @@ test_expect_success 'updating origin' '\n> '\n>\n> test_expect_success 'pulling changes from origin' '\n> - git -C C pull origin\n> + git -C C pull --no-rebase origin\n> '\n>\n> # the 2 local objects are commit and tree from the merge\n> @@ -93,7 +93,7 @@ test_expect_success 'that alternate to origin gets used' '\n> '\n>\n> test_expect_success 'pulling changes from origin' '\n> - git -C D pull origin\n> + git -C D pull --no-rebase origin\n> '\n>\n> # the 5 local objects are expected; file3 blob, commit in A to add it\n> diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh\n> index 3f64f62224..ad140a8def 100755\n> --- a/t/t6402-merge-rename.sh\n> +++ b/t/t6402-merge-rename.sh\n> @@ -100,7 +100,7 @@ test_expect_success 'setup' '\n> test_expect_success 'pull renaming branch into unrenaming one' \\\n> '\n> git show-branch &&\n> - test_expect_code 1 git pull . white &&\n> + test_expect_code 1 git pull --no-rebase . white &&\n> git ls-files -s &&\n> git ls-files -u B >b.stages &&\n> test_line_count = 3 b.stages &&\n> @@ -118,7 +118,7 @@ test_expect_success 'pull renaming branch into another renaming one' \\\n> rm -f B &&\n> git reset --hard &&\n> git checkout red &&\n> - test_expect_code 1 git pull . white &&\n> + test_expect_code 1 git pull --no-rebase . white &&\n> git ls-files -u B >b.stages &&\n> test_line_count = 3 b.stages &&\n> git ls-files -s N >n.stages &&\n> @@ -134,7 +134,7 @@ test_expect_success 'pull unrenaming branch into renaming one' \\\n> '\n> git reset --hard &&\n> git show-branch &&\n> - test_expect_code 1 git pull . master &&\n> + test_expect_code 1 git pull --no-rebase . master &&\n> git ls-files -u B >b.stages &&\n> test_line_count = 3 b.stages &&\n> git ls-files -s N >n.stages &&\n> @@ -150,7 +150,7 @@ test_expect_success 'pull conflicting renames' \\\n> '\n> git reset --hard &&\n> git show-branch &&\n> - test_expect_code 1 git pull . blue &&\n> + test_expect_code 1 git pull --no-rebase . blue &&\n> git ls-files -u A >a.stages &&\n> test_line_count = 1 a.stages &&\n> git ls-files -u B >b.stages &&\n> @@ -170,7 +170,7 @@ test_expect_success 'interference with untracked working tree file' '\n> git reset --hard &&\n> git show-branch &&\n> echo >A this file should not matter &&\n> - test_expect_code 1 git pull . white &&\n> + test_expect_code 1 git pull --no-rebase . white &&\n> test_path_is_file A\n> '\n>\n> @@ -180,7 +180,7 @@ test_expect_success 'interference with untracked working tree file' '\n> git show-branch &&\n> rm -f A &&\n> echo >A this file should not matter &&\n> - test_expect_code 1 git pull . red &&\n> + test_expect_code 1 git pull --no-rebase . red &&\n> test_path_is_file A\n> '\n>\n> @@ -190,7 +190,7 @@ test_expect_success 'interference with untracked working tree file' '\n> git checkout -f master &&\n> git tag -f anchor &&\n> git show-branch &&\n> - git pull . yellow &&\n> + git pull --no-rebase . yellow &&\n> test_path_is_missing M &&\n> git reset --hard anchor\n> '\n> @@ -203,7 +203,7 @@ test_expect_success 'updated working tree file should prevent the merge' '\n> git show-branch &&\n> echo >>M one line addition &&\n> cat M >M.saved &&\n> - test_expect_code 128 git pull . yellow &&\n> + test_expect_code 128 git pull --no-rebase . yellow &&\n> test_cmp M M.saved &&\n> rm -f M.saved\n> '\n> @@ -217,7 +217,7 @@ test_expect_success 'updated working tree file should prevent the merge' '\n> echo >>M one line addition &&\n> cat M >M.saved &&\n> git update-index M &&\n> - test_expect_code 128 git pull . yellow &&\n> + test_expect_code 128 git pull --no-rebase . yellow &&\n> test_cmp M M.saved &&\n> rm -f M.saved\n> '\n> @@ -229,7 +229,7 @@ test_expect_success 'interference with untracked working tree file' '\n> git tag -f anchor &&\n> git show-branch &&\n> echo >M this file should not matter &&\n> - git pull . master &&\n> + git pull --no-rebase . master &&\n> test_path_is_file M &&\n> ! {\n> git ls-files -s |\n> diff --git a/t/t6409-merge-subtree.sh b/t/t6409-merge-subtree.sh\n> index b8e8b6f642..b79cb0a368 100755\n> --- a/t/t6409-merge-subtree.sh\n> +++ b/t/t6409-merge-subtree.sh\n> @@ -97,7 +97,7 @@ test_expect_success 'merge update' '\n> git checkout -b topic_2 &&\n> git commit -m \"update git-gui\" &&\n> cd ../git &&\n> - git pull -s subtree gui topic_2 &&\n> + git pull --no-rebase -s subtree gui topic_2 &&\n> git ls-files -s >actual &&\n> (\n> echo \"100644 $o3 0 git-gui/git-gui.sh\" &&\n> @@ -126,7 +126,7 @@ test_expect_success 'initial ambiguous subtree' '\n> test_expect_success 'merge using explicit' '\n> cd ../git &&\n> git reset --hard topic_2 &&\n> - git pull -Xsubtree=git-gui gui topic_2 &&\n> + git pull --no-rebase -Xsubtree=git-gui gui topic_2 &&\n> git ls-files -s >actual &&\n> (\n> echo \"100644 $o3 0 git-gui/git-gui.sh\" &&\n> @@ -139,7 +139,7 @@ test_expect_success 'merge using explicit' '\n> test_expect_success 'merge2 using explicit' '\n> cd ../git &&\n> git reset --hard topic_2 &&\n> - git pull -Xsubtree=git-gui2 gui topic_2 &&\n> + git pull --no-rebase -Xsubtree=git-gui2 gui topic_2 &&\n> git ls-files -s >actual &&\n> (\n> echo \"100644 $o1 0 git-gui/git-gui.sh\" &&\n> diff --git a/t/t6417-merge-ours-theirs.sh b/t/t6417-merge-ours-theirs.sh\n> index 0aebc6c028..ba9b100562 100755\n> --- a/t/t6417-merge-ours-theirs.sh\n> +++ b/t/t6417-merge-ours-theirs.sh\n> @@ -66,11 +66,11 @@ test_expect_success 'binary file with -Xours/-Xtheirs' '\n> '\n>\n> test_expect_success 'pull passes -X to underlying merge' '\n> - git reset --hard master && git pull -s recursive -Xours . side &&\n> - git reset --hard master && git pull -s recursive -X ours . side &&\n> - git reset --hard master && git pull -s recursive -Xtheirs . side &&\n> - git reset --hard master && git pull -s recursive -X theirs . side &&\n> - git reset --hard master && test_must_fail git pull -s recursive -X bork . side\n> + git reset --hard master && git pull --no-rebase -s recursive -Xours . side &&\n> + git reset --hard master && git pull --no-rebase -s recursive -X ours . side &&\n> + git reset --hard master && git pull --no-rebase -s recursive -Xtheirs . side &&\n> + git reset --hard master && git pull --no-rebase -s recursive -X theirs . side &&\n> + git reset --hard master && test_must_fail git pull --no-rebase -s recursive -X bork . side\n> '\n>\n> test_expect_success SYMLINKS 'symlink with -Xours/-Xtheirs' '\n> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\n> index 1c85f75555..3ad47f0925 100755\n> --- a/t/t7600-merge.sh\n> +++ b/t/t7600-merge.sh\n> @@ -891,7 +891,7 @@ test_expect_success 'merge annotated/signed tag w/o tracking' '\n> # tag from the \"upstream\", this pull defaults to --no-ff\n> cd dst &&\n> git pull .. c0 &&\n> - git pull .. anno1 &&\n> + git pull --no-rebase .. anno1 &&\n> git rev-parse HEAD^2 >actual &&\n> test_cmp expect actual\n> )\n> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\n> index c5c4ea5fc0..c6c44ec570 100755\n> --- a/t/t7601-merge-pull-config.sh\n> +++ b/t/t7601-merge-pull-config.sh\n> @@ -27,63 +27,6 @@ test_expect_success 'setup' '\n> git tag c3\n> '\n>\n> -test_expect_success 'pull.rebase not set' '\n> - git reset --hard c0 &&\n> - git pull . c1 2>err &&\n> - test_i18ngrep \"Pulling without specifying how to reconcile\" err\n> -'\n> -\n> -test_expect_success 'pull.rebase not set and pull.ff=true' '\n> - git reset --hard c0 &&\n> - test_config pull.ff true &&\n> - git pull . c1 2>err &&\n> - test_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n> -'\n> -\n> -test_expect_success 'pull.rebase not set and pull.ff=false' '\n> - git reset --hard c0 &&\n> - test_config pull.ff false &&\n> - git pull . c1 2>err &&\n> - test_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n> -'\n> -\n> -test_expect_success 'pull.rebase not set and pull.ff=only' '\n> - git reset --hard c0 &&\n> - test_config pull.ff only &&\n> - git pull . c1 2>err &&\n> - test_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n> -'\n> -\n> -test_expect_success 'pull.rebase not set and --rebase given' '\n> - git reset --hard c0 &&\n> - git pull --rebase . c1 2>err &&\n> - test_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n> -'\n> -\n> -test_expect_success 'pull.rebase not set and --no-rebase given' '\n> - git reset --hard c0 &&\n> - git pull --no-rebase . c1 2>err &&\n> - test_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n> -'\n> -\n> -test_expect_success 'pull.rebase not set and --ff given' '\n> - git reset --hard c0 &&\n> - git pull --ff . c1 2>err &&\n> - test_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n> -'\n> -\n> -test_expect_success 'pull.rebase not set and --no-ff given' '\n> - git reset --hard c0 &&\n> - git pull --no-ff . c1 2>err &&\n> - test_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n> -'\n> -\n> -test_expect_success 'pull.rebase not set and --ff-only given' '\n> - git reset --hard c0 &&\n> - git pull --ff-only . c1 2>err &&\n> - test_i18ngrep ! \"Pulling without specifying how to reconcile\" err\n> -'\n> -\n> test_expect_success 'merge c1 with c2' '\n> git reset --hard c1 &&\n> test -f c0.c &&\n> diff --git a/t/t7603-merge-reduce-heads.sh b/t/t7603-merge-reduce-heads.sh\n> index 98948955ae..27cd94ad6f 100755\n> --- a/t/t7603-merge-reduce-heads.sh\n> +++ b/t/t7603-merge-reduce-heads.sh\n> @@ -68,7 +68,7 @@ test_expect_success 'merge c1 with c2, c3, c4, c5' '\n>\n> test_expect_success 'pull c2, c3, c4, c5 into c1' '\n> git reset --hard c1 &&\n> - git pull . c2 c3 c4 c5 &&\n> + git pull --no-rebase . c2 c3 c4 c5 &&\n> test \"$(git rev-parse c1)\" != \"$(git rev-parse HEAD)\" &&\n> test \"$(git rev-parse c1)\" = \"$(git rev-parse HEAD^1)\" &&\n> test \"$(git rev-parse c2)\" = \"$(git rev-parse HEAD^2)\" &&\n> --\n> 2.29.2\n>\n\n\n-- \nFelipe Contreras\n"},{"id":"410763","messageId":"CAMP44s3-fJ=mrYjGTW3AUACBmEhyrdD0dRktdaKHK9b1MTbk2A@mail.gmail.com","threadId":"54711","inReplyTo":"CAMP44s18Sd=DWMhFWCSDyGEz74ZKVF2kukr_=Exe1CjnPULCoA@mail.gmail.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-25T03:47:55Z","receivedAt":"2020-11-25T03:48:09Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Nov 24, 2020 at 9:45 PM Felipe Contreras\n wrote:\n> On Tue, Nov 24, 2020 at 8:14 PM Alex Henrie wrote:\n>\n> Before making this the default we need a solution *right now* that is\n> a sane default.\n\nThis mail was sent by mistake. I was going to say:\n\nWe need a warning like:\n\n The pull was not fast-forward, please either merge or rebase.\n\nWhen the default (pull.ff=only) is set.\n\nIt is the current status that is more urgent to fix.\n\n-- \nFelipe Contreras\n"},{"id":"410781","messageId":"67e55d85-63a4-9a47-e633-52b2ece23758@iee.email","threadId":"54711","inReplyTo":"CAMP44s3-fJ=mrYjGTW3AUACBmEhyrdD0dRktdaKHK9b1MTbk2A@mail.gmail.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-11-25T13:25:11Z","receivedAt":"2020-11-25T13:25:15Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 25/11/2020 03:47, Felipe Contreras wrote:\n> On Tue, Nov 24, 2020 at 9:45 PM Felipe Contreras\n> wrote:\n>> On Tue, Nov 24, 2020 at 8:14 PM Alex Henrie wrote:\n>>\n>> Before making this the default we need a solution *right now* that is\n>> a sane default.\n> This mail was sent by mistake. I was going to say:\n>\n> We need a warning like:\n>\n> The pull was not fast-forward, please either merge or rebase.\n>\n> When the default (pull.ff=only) is set.\n>\n> It is the current status that is more urgent to fix.\n>\n\nMaybe this could also, as an interim measure, be a doc change right in\nthe first paragraph of the `git pull --help` description section to warn\nthat its current default may not be suitable for most users, and to see\nthe `--ff-only` option (and variants) and its matching config variable.\n\ne.g. \"In its default mode, git pull uses the --ff option and is\nshorthand for git fetch followed by git merge FETCH_HEAD. The --ff-only\noption may be more suitable for modern usage. It can be set using `git\nconfig pull.ff only`.\"\n (then once v3.0 arrives the discussion can be flipped)\n\nIt's worth making sure that the manuals are easy to read.\n\nPhilip\n"},{"id":"411140","messageId":"CAMP44s3Y6SxvxEH8snUi6LKTOmnuNOt7x1XSn2p5_zPQb0Ez2w@mail.gmail.com","threadId":"54711","inReplyTo":"67e55d85-63a4-9a47-e633-52b2ece23758@iee.email","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-02T04:43:48Z","receivedAt":"2020-12-02T04:44:58Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 25, 2020 at 7:25 AM Philip Oakley wrote:\n>\n> On 25/11/2020 03:47, Felipe Contreras wrote:\n> > On Tue, Nov 24, 2020 at 9:45 PM Felipe Contreras\n> > wrote:\n> >> On Tue, Nov 24, 2020 at 8:14 PM Alex Henrie wrote:\n> >>\n> >> Before making this the default we need a solution *right now* that is\n> >> a sane default.\n> > This mail was sent by mistake. I was going to say:\n> >\n> > We need a warning like:\n> >\n> > The pull was not fast-forward, please either merge or rebase.\n> >\n> > When the default (pull.ff=only) is set.\n> >\n> > It is the current status that is more urgent to fix.\n> >\n>\n> Maybe this could also, as an interim measure, be a doc change right in\n> the first paragraph of the `git pull --help` description section to warn\n> that its current default may not be suitable for most users, and to see\n> the `--ff-only` option (and variants) and its matching config variable.\n>\n> e.g. \"In its default mode, git pull uses the --ff option and is\n> shorthand for git fetch followed by git merge FETCH_HEAD. The --ff-only\n> option may be more suitable for modern usage. It can be set using `git\n> config pull.ff only`.\"\n> (then once v3.0 arrives the discussion can be flipped)\n>\n> It's worth making sure that the manuals are easy to read.\n\nThis is what my patch attempted to do:\n\nhttps://lore.kernel.org/git/20201125032938.786393-11-felipe.contreras@gmail.com/\n\nBefore warning that a non-fast-forward pull failed, it's probably\nsensible to explain what a fast-forward is.\n\n-- \nFelipe Contreras\n"},{"id":"411198","messageId":"xmqqmtyviq9e.fsf@gitster.c.googlers.com","threadId":"54711","inReplyTo":"20201125020931.248427-2-alexhenrie24@gmail.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-03T02:21:01Z","receivedAt":"2020-12-03T02:22:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Henrie writes:\n\n> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\n> index eb0aabd396..1eb79e2939 100644\n> --- a/Documentation/merge-options.txt\n> +++ b/Documentation/merge-options.txt\n> @@ -43,10 +43,16 @@ set to `no` at the beginning of them.\n> --no-ff::\n> --ff-only::\n> \tSpecifies how a merge is handled when the merged-in history is\n> -\talready a descendant of the current history. `--ff` is the\n> -\tdefault unless merging an annotated (and possibly signed) tag\n> -\tthat is not stored in its natural place in the `refs/tags/`\n> -\thierarchy, in which case `--no-ff` is assumed.\n> +\talready a descendant of the current history.\n> +\t+\n> +\tIn `git pull`, `--ff-only` is the default unless `--rebase` or\n> +\t`--no-rebase` is specified, in which case the default is `--ff`.\n> +\tHowever, the default changes again to `--no-ff` in the unlikely\n> +\tscenario where an annotated (and possibly signed) tag that is not\n> +\tstored in its natural place in the `refs/tags/` hierarchy is being\n> +\tmerged.\n> +\t+\n> +\tIn `git merge`, `--ff` is always the default.\n\nAs we can see one line below, I think these new paragraphs must be\ndedented.\n\n> +\n> With `--ff`, when possible resolve the merge as a fast-forward (only\n> update the branch pointer to match the merged branch; do not create a\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 4d80efe5b7..c703560a77 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -292,17 +292,21 @@ static void set_reflog_message(int argc, const char **argv)\n> }\n> \n> /**\n> - * If pull.ff is unset, returns NULL. If pull.ff is \"true\", returns \"--ff\". If\n> - * pull.ff is \"false\", returns \"--no-ff\". If pull.ff is \"only\", returns\n> - * \"--ff-only\". Otherwise, if pull.ff is set to an invalid value, die with an\n> - * error.\n> + * If pull.ff is unset, returns NULL if pull.rebase is set or \"--ff-only\" if\n> + * pull.rebase is not set. If pull.ff is \"true\", returns \"--ff\". If pull.ff is\n> + * \"false\", returns \"--no-ff\". If pull.ff is \"only\", returns \"--ff-only\".\n> + * Otherwise, if pull.ff is set to an invalid value, die with an error.\n> */\n> static const char *config_get_ff(void)\n> {\n> \tconst char *value;\n> \n> -\tif (git_config_get_value(\"pull.ff\", &value))\n> -\t\treturn NULL;\n> +\tif (git_config_get_value(\"pull.ff\", &value)) {\n> +\t\tif (opt_rebase < 0)\n> +\t\t\treturn \"--ff-only\";\n> +\t\telse\n> +\t\t\treturn NULL;\n> +\t}\n\nSo, the idea is that we currently return NULL when pull.ff is set,\nbut now we also check \"pull.rebase\" etc. and give \"--ff-only\" when\nwe did not choose --[no-]rebase and lack the configuration. So the\ndefault (i.e. when pull.ff and pull.rebase are not set) would be as\nif the user said\n\n\tgit pull --ff-only --no-rebase\n\nI am not seeing what problem this tries to solve, exactly, though.\n\nI would have expected that for those who did not choose between\nmerge and rebase (either with the pull.rebase configuration or from\nthe command line --[no-]rebase option) the command would behave as\nif --ff-only is given, regardless of how pull.ff configuration is\nset. That way, those who are unconfigured will just follow along\nwhat happens at the upstream, until they have their own development,\nat which point \"--ff-only\" can no longer be satisfied and their\n\"pull\" would fail until they choose how to consolidate their work\nwith the upstream.\n\nAnd once they choose between merge and rebase, we no longer have to\nforce \"--ff-only\" and things will start working as specified by the\nuser. In other words, what I would have expected in the previous\nparagraph is a minimum and natural extension of the current \"you\nmust pick between merge and rebase or you'll get an advice message\nuntil you do so\"---we do not warn for those who haven't chosen as\nlong as they are only riding along with the upstream without having\ntheir own development.\n\nHaving said that, back to what you did in your patch (read: the\nfollowing comments are not about achieving what I would have\nexpected; it is about what this patch from you seems to be trying to\ndo).\n\nThe above code in config_get_ff() obviously depends on the order in\nthe caller to make this call and determine opt_rebase's value and I\nsee you tweaked the caller to call config_get_rebase() before it\nmakes a call to this helper.\n\nIt feels a bit brittle now because of this subtle dependency.\n\nPerhaps it is a good idea to introduce another helper that\ncmd_pull() calls, and make that helper call these two helpers that\ndetermine the value of opt_rebase and opt_ff, to hide the\ndependency.\n\n> @@ -322,7 +326,7 @@ static const char *config_get_ff(void)\n> * value of \"branch.$curr_branch.rebase\", where $curr_branch is the current\n> * branch, and if HEAD is detached or the configuration key does not exist,\n> * looks for the value of \"pull.rebase\". If both configuration keys do not\n> - * exist, returns REBASE_FALSE.\n> + * exist, returns REBASE_INVALID.\n> */\n> static enum rebase_type config_get_rebase(void)\n> {\n> @@ -344,23 +348,7 @@ static enum rebase_type config_get_rebase(void)\n> \tif (!git_config_get_value(\"pull.rebase\", &value))\n> \t\treturn parse_config_rebase(\"pull.rebase\", value, 1);\n> \n> -\tif (opt_verbosity >= 0 && !opt_ff) {\n> -\t\twarning(_(\"Starting in Git 3.0, the default behavior of `git pull` will change\\n\"\n> -\t\t\t\"when it is not specified how to reconcile divergent branches. You can\\n\"\n> -\t\t\t\"squelch this message by running one of the following commands sometime\\n\"\n> -\t\t\t\"before your next pull:\\n\"\n> -\t\t\t\"\\n\"\n> -\t\t\t\" git config pull.rebase false # merge (the current default)\\n\"\n> -\t\t\t\" git config pull.rebase true # rebase\\n\"\n> -\t\t\t\" git config pull.ff only # fast-forward only (the future default)\\n\"\n> -\t\t\t\"\\n\"\n> -\t\t\t\"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n> -\t\t\t\"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n> -\t\t\t\"or --ff-only on the command line to override the configured default per\\n\"\n> -\t\t\t\"invocation.\\n\"));\n> -\t}\n> -\n> -\treturn REBASE_FALSE;\n> +\treturn REBASE_INVALID;\n> }\n\nHmph, and who reacts to this REBASE_INVALID value? Shouldn't there\nbe something that catches this INVALID value and stop/complain?\n\n> \n> /**\n> @@ -931,11 +919,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n> \n> \tparse_repo_refspecs(argc, argv, &repo, &refspecs);\n> \n> +\tif (opt_rebase < 0)\n> +\t\topt_rebase = config_get_rebase();\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> +\t\topt_rebase = REBASE_FALSE;\n> \n> \tif (read_cache_unmerged())\n> \t\tdie_resolve_conflict(\"pull\");\n\nAnother thing. Does \"git pull --rebase --ff-only\" behave sensibly?\n\"rebase our history on top of theirs only if we are truly ahead\" is\na bit strange way to say \"we only ride along without having our own\ndevelopment, and once we discover we have our own stuff, stop us\".\nIIRC, \"git pull --rebase\" ignored the \"--ff-only\" request and\nrebased our own stuff on top of their history anyway, which we would\nwant to fix.\n"},{"id":"411214","messageId":"CAMP44s1Hwun+P=j5BBbVUT-ACS4hJCyRCJT-=6WvwK913fXq7g@mail.gmail.com","threadId":"54711","inReplyTo":"xmqqmtyviq9e.fsf@gitster.c.googlers.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-03T09:07:59Z","receivedAt":"2020-12-03T09:09:15Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Dec 2, 2020 at 8:21 PM Junio C Hamano wrote:\n\n> So, the idea is that we currently return NULL when pull.ff is set,\n> but now we also check \"pull.rebase\" etc. and give \"--ff-only\" when\n> we did not choose --[no-]rebase and lack the configuration. So the\n> default (i.e. when pull.ff and pull.rebase are not set) would be as\n> if the user said\n>\n> git pull --ff-only --no-rebase\n>\n> I am not seeing what problem this tries to solve, exactly, though.\n>\n> I would have expected that for those who did not choose between\n> merge and rebase (either with the pull.rebase configuration or from\n> the command line --[no-]rebase option) the command would behave as\n> if --ff-only is given, regardless of how pull.ff configuration is\n> set.\n\nThat's right, I would have expected the same as well, but right now\nthe pull warning is disabled when you have a pull.ff configuration (or\n--[no-]ff[-only]), as it was done in commit: 54200cef86 (pull: don't\nwarn if pull.ff has been set).\n\nIn my mind doing \"git pull\" is the equivalent of doing an implicit\n\"git pull --ff\", so I don't see why setting \"pull.ff=true\" should\nchange the warning. The warning should not be about lack of\nconfiguration (we want git to eventually work correctly without\nconfiguration), it should be about something unexpected about to\nhappen (i.e. a non-fast-forward merge).\n\nSo quite likely Alex extended the logic of the current warning to the\ndefault mode, which I don't think is right.\n\nWe want the logic of the warning to change: to be displayed a) only\nwhen there's a non-fast-forward, b) when the user has not specified or\nconfigured either a merge or a rebase.\n\nOnly after a while of this warning on the wild should the default mode\nbe changed.\n\nBut we need to fix the logic of the warning first.\n\n> > @@ -344,23 +348,7 @@ static enum rebase_type config_get_rebase(void)\n> > if (!git_config_get_value(\"pull.rebase\", &value))\n> > return parse_config_rebase(\"pull.rebase\", value, 1);\n\n...\n\n> > - return REBASE_FALSE;\n> > + return REBASE_INVALID;\n> > }\n>\n> Hmph, and who reacts to this REBASE_INVALID value? Shouldn't there\n> be something that catches this INVALID value and stop/complain?\n\nLater on there's a \"if (opt_rebase < 0) opt_rebase = REBASE_FALSE\", so\nthis is only temporarily set as REBASE_INVALID for config_get_ff().\n\nI don't find the semantics of this approach appealing, which is why I\nintroduced REBASE_DEFAULT in my patch series, so that the meaning of\nREBASE_INVALID is not muddled.\n\n> Another thing. Does \"git pull --rebase --ff-only\" behave sensibly?\n> \"rebase our history on top of theirs only if we are truly ahead\" is\n> a bit strange way to say \"we only ride along without having our own\n> development, and once we discover we have our own stuff, stop us\".\n> IIRC, \"git pull --rebase\" ignored the \"--ff-only\" request and\n> rebased our own stuff on top of their history anyway, which we would\n> want to fix.\n\nWe would want this new mode to consistently fail with a good\nuser-friendly message, better than \"fatal: Not possible to\nfast-forward, aborting.\".\n\nBut this mode is the opposite of \"git pull --rebase\", which is why it\ndoesn't make sense to do \"git pull --rebase --ff-only\" (which is\nignored right now).\n\nIn other words: --rebase should disable the --ff-only mode.\n\nAlso, we would want --no-rebase to disable the default --ff-only mode.\nThat would require changing the semantics of --ff-only, so that \"git\npull --no-rebase --ff-only\" doesn't make sense (as --ff-only is\noverridden by --no-rebase).\n\nIf we do this, then the only time where --ff-only would make sense is\nin \"git pull --ff-only\" (no --rebase or --no-rebase), and if we change\nthe semantics this way, then there's no need for my suggested\npull.mode (although it still might be desired).\n\nMoreover, I see no tests that check for this new behavior.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"411261","messageId":"xmqq7dpyix1d.fsf@gitster.c.googlers.com","threadId":"54711","inReplyTo":"CAMP44s1Hwun+P=j5BBbVUT-ACS4hJCyRCJT-=6WvwK913fXq7g@mail.gmail.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-03T18:06:54Z","receivedAt":"2020-12-03T18:07:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> In other words: --rebase should disable the --ff-only mode.\n>\n> Also, we would want --no-rebase to disable the default --ff-only mode.\n\nYes, and for that a configured pull.rebase counts the same as a\ncommand line option. As you agreed earlier in the part omitted from\nthe quote with my \"I would have expected...\", we want to say:\n\n There are two ways to consolidate your own work with the history\n of the other side, that gives a result in vastly different\n shapes that suit for different workflows, and \"git pull\" will\n not choose between them for you. Unless you say which one\n between rebase and merge you want to use, it will work only for\n fast-forward updating from the other side and nothing else.\n\nand the \"default to --ff-only when the user does not say\" is a means\nto do so. When the end-user gives an explicit preference between\nthe merge/rebase, none of the above would apply.\n\n> That would require changing the semantics of --ff-only, so that \"git\n> pull --no-rebase --ff-only\" doesn't make sense (as --ff-only is\n> overridden by --no-rebase).\n\nI do not think such a conclusion follows from \"we do not want to use\nthe 'by default force the --ff-only' when the user chooses between\nmerge and rebase\". Specifically, I do not agree with \"as --ff-only\nis overridden\" in your statement.\n\nWhen the end user says \"git pull --no-rebase --ff-only\", it means to\nme:\n\n I choose not to use rebase---my preference is to merge in the\n other history into mine, and I want to reject any non-fast\n update in this invocation.\n\nand I find it quite sensible, especially in a modern world where you\npractically must set pull.rebase to one way or the other. A\ndeveloper X, an individual contributor to the project who uses\nrebase most of the time, may use \"git pull --no-rebase\" from\nsomebody else's repository (i.e. not X's \"upstream\") when a help is\noffered by another developer Y who forked from X to advance X's\nwork. If the upstream project does not want to see merge commits in\na side branch (i.e. the history X offers to the project), X may ask\nY to make sure Y builds on top of the whole of X's work, and adding\n\"--ff-only\" to the command line would be a way to make sure the\nresult is fast-forward.\n\n> If we do this, then the only time where --ff-only would make sense is\n> in \"git pull --ff-only\" (no --rebase or --no-rebase), and if we change\n> the semantics this way, then there's no need for my suggested\n> pull.mode (although it still might be desired).\n>\n> Moreover, I see no tests that check for this new behavior.\n\nA proposal to change behaviour needs to come with tests to protect\nnew behaviour before we can merge, but we should be more lenient on\na patch with RFC label on it ;-).\n\nApparently the patch had me say \"I am not seeing what problem this\ntries to solve, exactly\", and a test may have helped to demonstrate\nthe intention of the change better, even in its RFC state.\n\n"},{"id":"411271","messageId":"xmqqmtyuhemi.fsf@gitster.c.googlers.com","threadId":"54711","inReplyTo":"xmqq7dpyix1d.fsf@gitster.c.googlers.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-03T19:29:57Z","receivedAt":"2020-12-03T19:30:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> That would require changing the semantics of --ff-only, so that \"git\n>> pull --no-rebase --ff-only\" doesn't make sense (as --ff-only is\n>> overridden by --no-rebase).\n>\n> I do not think such a conclusion follows from \"we do not want to use\n> the 'by default force the --ff-only' when the user chooses between\n> merge and rebase\". Specifically, I do not agree with \"as --ff-only\n> is overridden\" in your statement.\n\nAh, sorry, I mis-read your three lines above.\n\nThere are currently two ways \"git pull\" consolidates your work with\nthe other history. By default, you are \"pulling\" work from your\ncontributors (and that is what \"pull request\" means---contributors\nask you to pull, and you take their work at your discretion) and the\nonly way that makes sense is to merge their history into yours. The\nother is you are updating your branch by rebasing your work on top\nof what happend in their history.\n\nAnd if we introduce a third-way, i.e. \"we do not handle the case\nwhere you have your own development at all, this is only to maintain\npristine copy from your upstream\", and repurpose \"--ff-only\" for\nthat purpose, yes, what you said above does make sense. At that\npoint, there is no reason to disagree with \"as --ff-only is\noverridden\" part of your statement---in your new world, \"--ff-only\"\nis redesigned to act that way.\n\nIn retrospect, \"git pull --rebase\" was a UI mistake. What the other\nside means is totally different in the operation from what the other\nside is in \"git pull\". The former is for you to catch up with your\nupstream and the latter is for you, who _is_ the upstream to others,\nto take others work in as their upstream. If we instead introduced\na separate command, say \"git update\", that is \"fetch followed by\nrebase\" (just like \"git pull\" is \"fetch followed by merge\"), to\nrebase your work on top of updated upstream, there wouldn't be a\nneed for us to be having this discussion.\n\nIt probably is water under the bridge at this point. Perhaps if\nsomebody builds a time-machine for me, I'll go back 13 years and\ngive my younger self this wisdom ;-)\n\n\n\n\n\n\n\n"},{"id":"411287","messageId":"CAMP44s0K48xJy0gD6jLoiDr9GYDmD3MVmWYVbsNyXyBnPcvj1Q@mail.gmail.com","threadId":"54711","inReplyTo":"xmqqmtyuhemi.fsf@gitster.c.googlers.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-03T23:05:28Z","receivedAt":"2020-12-03T23:06:23Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Dec 3, 2020 at 1:30 PM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> >> That would require changing the semantics of --ff-only, so that \"git\n> >> pull --no-rebase --ff-only\" doesn't make sense (as --ff-only is\n> >> overridden by --no-rebase).\n> >\n> > I do not think such a conclusion follows from \"we do not want to use\n> > the 'by default force the --ff-only' when the user chooses between\n> > merge and rebase\". Specifically, I do not agree with \"as --ff-only\n> > is overridden\" in your statement.\n>\n> Ah, sorry, I mis-read your three lines above.\n>\n> There are currently two ways \"git pull\" consolidates your work with\n> the other history. By default, you are \"pulling\" work from your\n> contributors (and that is what \"pull request\" means---contributors\n> ask you to pull, and you take their work at your discretion) and the\n> only way that makes sense is to merge their history into yours.\n\nAnd this is the reason why I warn about using one's worldview too\nmuch. The *vast* majority of users do not use \"git pull\" this way;\nthere are more contributors than maintainers.\n\nWhat they want is to \"merge your history into theirs\", not the other\nway around. So the only way they can do that correctly is by doing\n\"git fetch\" + \"git merge\". Which is why so many people say to avoid\n\"git pull\" altogether.\n\nSo, a newcomer that doesn't know much about git and does a \"git pull\",\nis pretty much guaranteed to do something unintended. If he/she is a\nuser, the merge will have the parents the other way around, but even\nas a maintainer, the project might not like merges, and he/she will\nintroduce an unwanted merge, or worse; an evil merge.\n\n> The other is you are updating your branch by rebasing your work on\n> top of what happend in their history.\n\nAgain, what the user might want is the opposite. If the user is a\nmaintainer, these two:\n\n git pull --merge github john\n git pull --rebase github john\n\nShould be about their history to yours (or on top of yours).\n\n> And if we introduce a third-way, i.e. \"we do not handle the case\n> where you have your own development at all, this is only to maintain\n> pristine copy from your upstream\", and repurpose \"--ff-only\" for\n> that purpose, yes, what you said above does make sense. At that\n> point, there is no reason to disagree with \"as --ff-only is\n> overridden\" part of your statement---in your new world, \"--ff-only\"\n> is redesigned to act that way.\n\nThat's right. Otherwise \"git pull --no-rebase\" will fail; you will\nhave to specify --ff (or --no-ff) for it to work. And that doesn't\nmake sense to me.\n\nSpecifying --no-rebase should override the default --ff-only mode (or\npull.mode=ff-only).\n\n> In retrospect, \"git pull --rebase\" was a UI mistake. What the other\n> side means is totally different in the operation from what the other\n> side is in \"git pull\". The former is for you to catch up with your\n> upstream and the latter is for you, who _is_ the upstream to others,\n> to take others work in as their upstream. If we instead introduced\n> a separate command, say \"git update\", that is \"fetch followed by\n> rebase\" (just like \"git pull\" is \"fetch followed by merge\"), to\n> rebase your work on top of updated upstream, there wouldn't be a\n> need for us to be having this discussion.\n>\n> It probably is water under the bridge at this point. Perhaps if\n> somebody builds a time-machine for me, I'll go back 13 years and\n> give my younger self this wisdom ;-)\n\nYou don't have to go back 13 years ago, you can go back 6 years ago\nwhen I wrote all the patches for git update [1], explained the summary\nof the problem [2], and others urged git developers to pay more\nattention to the patch [3].\n\nBut as you say; water under the bridge.\n\nToday there are 3 things to do:\n\n1. Improve the annoying warning\n2. Consider changing the semantics of --ff-only, or implement pull.mode=ff-only\n3. Consider a new \"git update\" command\n\nSince my new (2020) patches for pull.mode (solve 1 and 2) have not\nbeen reviewed, I'm thinking there's too much inertia and perhaps it's\ntime to cash in the chips and concentrate only on 1.\n\nCheers.\n\n[1] https://github.com/felipec/git/commit/d38f1641fc33535aa3c92cf6d3a30334324d3488\n[2] https://lore.kernel.org/git/5366db742d494_18f9e4b308aa@nysa.notmuch/\n[3] https://lore.kernel.org/git/CAGK7Mr4uucBN=17ph5pBjrz7yP60By1sERU9oBL+c2-gsMDmrw@mail.gmail.com/\n\n-- \nFelipe Contreras\n"},{"id":"411289","messageId":"CA+P7+xq1QNzKbnZAwNn4bf6b4g5KS1ySB0dvgp7n1jyk13Vrog@mail.gmail.com","threadId":"54711","inReplyTo":"CAMP44s0K48xJy0gD6jLoiDr9GYDmD3MVmWYVbsNyXyBnPcvj1Q@mail.gmail.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2020-12-04T00:53:07Z","receivedAt":"2020-12-04T00:54:22Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Dec 3, 2020 at 3:08 PM Felipe Contreras\n wrote:\n>\n> On Thu, Dec 3, 2020 at 1:30 PM Junio C Hamano wrote:\n> >\n> > Junio C Hamano writes:\n> >\n> > >> That would require changing the semantics of --ff-only, so that \"git\n> > >> pull --no-rebase --ff-only\" doesn't make sense (as --ff-only is\n> > >> overridden by --no-rebase).\n> > >\n> > > I do not think such a conclusion follows from \"we do not want to use\n> > > the 'by default force the --ff-only' when the user chooses between\n> > > merge and rebase\". Specifically, I do not agree with \"as --ff-only\n> > > is overridden\" in your statement.\n> >\n> > Ah, sorry, I mis-read your three lines above.\n> >\n> > There are currently two ways \"git pull\" consolidates your work with\n> > the other history. By default, you are \"pulling\" work from your\n> > contributors (and that is what \"pull request\" means---contributors\n> > ask you to pull, and you take their work at your discretion) and the\n> > only way that makes sense is to merge their history into yours.\n>\n> And this is the reason why I warn about using one's worldview too\n> much. The *vast* majority of users do not use \"git pull\" this way;\n> there are more contributors than maintainers.\n>\n> What they want is to \"merge your history into theirs\", not the other\n> way around. So the only way they can do that correctly is by doing\n> \"git fetch\" + \"git merge\". Which is why so many people say to avoid\n> \"git pull\" altogether.\n>\n> So, a newcomer that doesn't know much about git and does a \"git pull\",\n> is pretty much guaranteed to do something unintended. If he/she is a\n> user, the merge will have the parents the other way around, but even\n> as a maintainer, the project might not like merges, and he/she will\n> introduce an unwanted merge, or worse; an evil merge.\n>\n\nAs someone who spends a significant amount of energy at work educating\nnewcomers to git, this is a significant problem.\n\n> > The other is you are updating your branch by rebasing your work on\n> > top of what happend in their history.\n>\n> Again, what the user might want is the opposite. If the user is a\n> maintainer, these two:\n>\n> git pull --merge github john\n> git pull --rebase github john\n>\n> Should be about their history to yours (or on top of yours).\n>\n> > And if we introduce a third-way, i.e. \"we do not handle the case\n> > where you have your own development at all, this is only to maintain\n> > pristine copy from your upstream\", and repurpose \"--ff-only\" for\n> > that purpose, yes, what you said above does make sense. At that\n> > point, there is no reason to disagree with \"as --ff-only is\n> > overridden\" part of your statement---in your new world, \"--ff-only\"\n> > is redesigned to act that way.\n>\n> That's right. Otherwise \"git pull --no-rebase\" will fail; you will\n> have to specify --ff (or --no-ff) for it to work. And that doesn't\n> make sense to me.\n>\n> Specifying --no-rebase should override the default --ff-only mode (or\n> pull.mode=ff-only).\n>\n> > In retrospect, \"git pull --rebase\" was a UI mistake. What the other\n> > side means is totally different in the operation from what the other\n> > side is in \"git pull\". The former is for you to catch up with your\n> > upstream and the latter is for you, who _is_ the upstream to others,\n> > to take others work in as their upstream. If we instead introduced\n> > a separate command, say \"git update\", that is \"fetch followed by\n> > rebase\" (just like \"git pull\" is \"fetch followed by merge\"), to\n> > rebase your work on top of updated upstream, there wouldn't be a\n> > need for us to be having this discussion.\n> >\n> > It probably is water under the bridge at this point. Perhaps if\n> > somebody builds a time-machine for me, I'll go back 13 years and\n> > give my younger self this wisdom ;-)\n>\n> You don't have to go back 13 years ago, you can go back 6 years ago\n> when I wrote all the patches for git update [1], explained the summary\n> of the problem [2], and others urged git developers to pay more\n> attention to the patch [3].\n>\n> But as you say; water under the bridge.\n>\n\n\n> Today there are 3 things to do:\n>\n> 1. Improve the annoying warning\n> 2. Consider changing the semantics of --ff-only, or implement pull.mode=ff-only\n> 3. Consider a new \"git update\" command\n>\n> Since my new (2020) patches for pull.mode (solve 1 and 2) have not\n> been reviewed, I'm thinking there's too much inertia and perhaps it's\n> time to cash in the chips and concentrate only on 1.\n>\n\nI think that solving (1) is relatively simple and worth doing. I think\nsolving (2) is trickier but worth doing in the medium time frame.\n\nDoing (3) may have value.. We did it for git checkout already\nrecently, by adding git switch and git restore-files.. But it's a\nbigger undertaking.\n\nI personally like changing the default of pull to be \"I will only fast\nforward, and you need to tell me what to do if history isn't fast\nforward\". This does cause some strain as it forces users to now have\nto understand and pick which thing they want, (rebase or merge) when\ncombining history.. But I think that's better than users not being\nasked to pick and just having merge be the default unless they go out\nof their way to set the rebase by default option.\n\n> Cheers.\n>\n> [1] https://github.com/felipec/git/commit/d38f1641fc33535aa3c92cf6d3a30334324d3488\n> [2] https://lore.kernel.org/git/5366db742d494_18f9e4b308aa@nysa.notmuch/\n> [3] https://lore.kernel.org/git/CAGK7Mr4uucBN=17ph5pBjrz7yP60By1sERU9oBL+c2-gsMDmrw@mail.gmail.com/\n>\n> --\n> Felipe Contreras\n"},{"id":"411291","messageId":"xmqqpn3qfhps.fsf@gitster.c.googlers.com","threadId":"54711","inReplyTo":"xmqqmtyuhemi.fsf@gitster.c.googlers.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-04T02:06:07Z","receivedAt":"2020-12-04T02:06:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>>> That would require changing the semantics of --ff-only, so that \"git\n>>> pull --no-rebase --ff-only\" doesn't make sense (as --ff-only is\n>>> overridden by --no-rebase).\n>>\n>> I do not think such a conclusion follows from \"we do not want to use\n>> the 'by default force the --ff-only' when the user chooses between\n>> merge and rebase\". Specifically, I do not agree with \"as --ff-only\n>> is overridden\" in your statement.\n>\n> Ah, sorry, I mis-read your three lines above.\n> ...\n> And if we introduce a third-way, i.e. \"we do not handle the case\n> where you have your own development at all, this is only to maintain\n> pristine copy from your upstream\", and repurpose \"--ff-only\" for\n> that purpose, yes, what you said above does make sense. At that\n> point, there is no reason to disagree with \"as --ff-only is\n> overridden\" part of your statement---in your new world, \"--ff-only\"\n> is redesigned to act that way.\n\nJust to avoid misunderstanding, I only meant to say that the first\nthree lines I quoted is internally consistent (unlike the message I\nwas responding to, in which I said \"I disagree---the conclusion does\nnot follow\"). It no way means I think such a re-definition of what\n\"--ff-only\" means is a great design.\n\nWhat we want to see can be done without such backward incompatible\nchanges, e.g. declaring the combination of \"--ff-only\" and\n\"--[no-]rebase\" incompatible and/or the last one wins, I would say,\nand I suspect Alex's RFC was an attempt to make the first step in\nthat direction.\n\nWhat is most missing in the current system is a fix for the way in\nwhich \"--rebase\" interacts with \"--ff-only\". Immediately after\nfetching, if our current branch is not a subset of what we fetched\nfrom the other side, the command should die. Otherwise, it should\njust rebase what we have (which is nothing) on top of the tip of the\nhistory of the other side (which is to fast-forward our tip and the\nworking tree to their tip). \n\nAnother thing we would want to change is to make the \"you must\nchoose between rebase and merge\" trigger a lot more lazily. If our\nside does not have our own development and their history is a\ndescendent of what we have, the user shouldn't have to choose. Only\nwhen the history they have does not fast-forward, we have to abort\ngiving the \"you must choose between the two\" warning message.\n\nWhen the user tells us to do rebase or merge, nothing (except that\n\"--ff-only\" should prevent the rebase from going forward) should\nhave to be changed.\n"},{"id":"411317","messageId":"CAMP44s02N9LJxi_eme8+nqEQameKtpNJtTXWDT2q3_EPV09gag@mail.gmail.com","threadId":"54711","inReplyTo":"xmqqpn3qfhps.fsf@gitster.c.googlers.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-04T06:37:28Z","receivedAt":"2020-12-04T06:38:22Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Dec 3, 2020 at 8:06 PM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > Junio C Hamano writes:\n> >\n> >>> That would require changing the semantics of --ff-only, so that \"git\n> >>> pull --no-rebase --ff-only\" doesn't make sense (as --ff-only is\n> >>> overridden by --no-rebase).\n> >>\n> >> I do not think such a conclusion follows from \"we do not want to use\n> >> the 'by default force the --ff-only' when the user chooses between\n> >> merge and rebase\". Specifically, I do not agree with \"as --ff-only\n> >> is overridden\" in your statement.\n> >\n> > Ah, sorry, I mis-read your three lines above.\n> > ...\n> > And if we introduce a third-way, i.e. \"we do not handle the case\n> > where you have your own development at all, this is only to maintain\n> > pristine copy from your upstream\", and repurpose \"--ff-only\" for\n> > that purpose, yes, what you said above does make sense. At that\n> > point, there is no reason to disagree with \"as --ff-only is\n> > overridden\" part of your statement---in your new world, \"--ff-only\"\n> > is redesigned to act that way.\n>\n> Just to avoid misunderstanding, I only meant to say that the first\n> three lines I quoted is internally consistent (unlike the message I\n> was responding to, in which I said \"I disagree---the conclusion does\n> not follow\"). It no way means I think such a re-definition of what\n> \"--ff-only\" means is a great design.\n\nYes, that's what I parsed.\n\n> What we want to see can be done without such backward incompatible\n> changes, e.g. declaring the combination of \"--ff-only\" and\n> \"--[no-]rebase\" incompatible and/or the last one wins, I would say,\n> and I suspect Alex's RFC was an attempt to make the first step in\n> that direction.\n\nIt's debatable whether or not that is \"backwards incompatible\".\n\nCurrently \"--no-rebase --ff-only\" fails if the merge is\nnon-fast-forward. With the proposed change of semantics it would work.\nThat's a change.\n\n> What is most missing in the current system is a fix for the way in\n> which \"--rebase\" interacts with \"--ff-only\". Immediately after\n> fetching, if our current branch is not a subset of what we fetched\n> from the other side, the command should die. Otherwise, it should\n> just rebase what we have (which is nothing) on top of the tip of the\n> history of the other side (which is to fast-forward our tip and the\n> working tree to their tip).\n\nKeep in mind the whole point of the changes: to make --ff-only the\ndefault. If you make \"git pull --rebase --ff-only\" fail if\nnot-fast-forward, then \"git pull --rebase\" will fail if you make\n--ff-only the default.\n\nI don't know if I'm not doing a good job of stating that --rebase\nshould override (and ignore) --ff-only, that's the way it should\ninteract.\n\nHere it goes again. In short, this is the target behavior:\n\n* git pull --ff-only # fail unless fast-forward\n* git pull --merge --ff-only # ignore --ff-only\n* git pull --rebase --ff-only # ignore --ff-only\n\n> Another thing we would want to change is to make the \"you must\n> choose between rebase and merge\" trigger a lot more lazily. If our\n> side does not have our own development and their history is a\n> descendent of what we have, the user shouldn't have to choose. Only\n> when the history they have does not fast-forward, we have to abort\n> giving the \"you must choose between the two\" warning message.\n\nYes, I already sent patches for that [1].\n\n> When the user tells us to do rebase or merge, nothing (except that\n> \"--ff-only\" should prevent the rebase from going forward) should\n> have to be changed.\n\nIf --ff-only prevents the rebase and the merge from going forward,\nthen it cannot be the default.\n\nMaybe my last patch series can make things clearer [2].\n\nCheers.\n\n[1] https://lore.kernel.org/git/20201204061623.1170745-11-felipe.contreras@gmail.com/\n[2] https://lore.kernel.org/git/20201204061623.1170745-1-felipe.contreras@gmail.com/T/\n\n-- \nFelipe Contreras\n"},{"id":"411360","messageId":"xmqqh7p1fjml.fsf@gitster.c.googlers.com","threadId":"54711","inReplyTo":"CAMP44s02N9LJxi_eme8+nqEQameKtpNJtTXWDT2q3_EPV09gag@mail.gmail.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-04T19:37:06Z","receivedAt":"2020-12-04T19:38:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> What we want to see can be done without such backward incompatible\n>> changes, e.g. declaring the combination of \"--ff-only\" and\n>> \"--[no-]rebase\" incompatible and/or the last one wins, I would say,\n>> and I suspect Alex's RFC was an attempt to make the first step in\n>> that direction.\n>\n> It's debatable whether or not that is \"backwards incompatible\".\n>\n> Currently \"--no-rebase --ff-only\" fails if the merge is\n> non-fast-forward. With the proposed change of semantics it would work.\n> That's a change.\n\nBut with such a change, \"--ff-only --no-rebase\" would work by\nignoring the \"I want to reject non-ff situation\" request from the\nuser, no?\n\n> Keep in mind the whole point of the changes: to make --ff-only the\n> default.\n\nSorry, I know you keep repeating that \"keep in mind\", but I do not\nquite see why anybody needs to keep that in mind. Has a concensus\nthat the repurposed --ff-only should be the default been\nestablished?\n\n\n"},{"id":"411406","messageId":"CAMP44s1Na1cf8=VdoejKr7JesmS6H3Gf=tkHLq4Zit0nxZXs4A@mail.gmail.com","threadId":"54711","inReplyTo":"xmqqh7p1fjml.fsf@gitster.c.googlers.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-04T21:11:49Z","receivedAt":"2020-12-04T21:12:43Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Dec 4, 2020 at 1:37 PM Junio C Hamano wrote:\n>\n> Felipe Contreras writes:\n>\n> >> What we want to see can be done without such backward incompatible\n> >> changes, e.g. declaring the combination of \"--ff-only\" and\n> >> \"--[no-]rebase\" incompatible and/or the last one wins, I would say,\n> >> and I suspect Alex's RFC was an attempt to make the first step in\n> >> that direction.\n> >\n> > It's debatable whether or not that is \"backwards incompatible\".\n> >\n> > Currently \"--no-rebase --ff-only\" fails if the merge is\n> > non-fast-forward. With the proposed change of semantics it would work.\n> > That's a change.\n>\n> But with such a change, \"--ff-only --no-rebase\" would work by\n> ignoring the \"I want to reject non-ff situation\" request from the\n> user, no?\n\nYes. And that's a change.\n\nThat's why the \"pull.mode=ff-only\" solution is cleaner; it doesn't\nneed --ff-only to change its semantics.\n\n> > Keep in mind the whole point of the changes: to make --ff-only the\n> > default.\n>\n> Sorry, I know you keep repeating that \"keep in mind\", but I do not\n> quite see why anybody needs to keep that in mind. Has a concensus\n> that the repurposed --ff-only should be the default been\n> established?\n\nThat's the whole point of this patch (pull: default pull.ff to \"only\"\nwhen pull.rebase is not set either).\n\nIf not --ff-only, then \"pull.mode=ff-only\".\n\nAre you saying making the default be fast-forward merges only is not\nthe eventual end-goal?\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"412095","messageId":"CAMMLpeQA7VW_C4yw_8n6j_SCoGr8k4VUOUaEp98UxUAMR6-MVw@mail.gmail.com","threadId":"54711","inReplyTo":"xmqqmtyviq9e.fsf@gitster.c.googlers.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2020-12-11T20:38:46Z","receivedAt":"2020-12-11T21:36:10Z","isPatch":false,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Wed, Dec 2, 2020 at 7:21 PM Junio C Hamano wrote:\n>\n> So, the idea is that we currently return NULL when pull.ff is set,\n> but now we also check \"pull.rebase\" etc. and give \"--ff-only\" when\n> we did not choose --[no-]rebase and lack the configuration. So the\n> default (i.e. when pull.ff and pull.rebase are not set) would be as\n> if the user said\n>\n> git pull --ff-only --no-rebase\n>\n> I am not seeing what problem this tries to solve, exactly, though.\n>\n> I would have expected that for those who did not choose between\n> merge and rebase (either with the pull.rebase configuration or from\n> the command line --[no-]rebase option) the command would behave as\n> if --ff-only is given, regardless of how pull.ff configuration is\n> set. That way, those who are unconfigured will just follow along\n> what happens at the upstream, until they have their own development,\n> at which point \"--ff-only\" can no longer be satisfied and their\n> \"pull\" would fail until they choose how to consolidate their work\n> with the upstream.\n\nMy goal was to make `git pull` without arguments and without any\nconfiguration set (the most common case) reject non-fast-forwards, and\nnot add any new config variables or substantially change the semantics\nof the existing ones. In my opinion it's fine that setting pull.ff\nwould change the behavior of `git pull` without arguments; I don't\nthink that that would be bad behavior.\n\nI am glad to see so much discussion surrounding the behavior of and\ndocumentation for `git pull`, but unfortunately at this point I'm\nhaving trouble following the discussion as a whole. I'm hoping that it\nsettles down some so that it becomes more clear what I can do to help.\n\n-Alex\n"},{"id":"412109","messageId":"CAMP44s299UcdutpkSZsKFiwxUfj7n4SXTHw7A8rAWPS6iDJacg@mail.gmail.com","threadId":"54711","inReplyTo":"CAMMLpeQA7VW_C4yw_8n6j_SCoGr8k4VUOUaEp98UxUAMR6-MVw@mail.gmail.com","subject":"Re: [RFC 2/2] pull: default pull.ff to \"only\" when pull.rebase is not set either","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-12T01:08:31Z","receivedAt":"2020-12-12T01:09:57Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Dec 11, 2020 at 2:38 PM Alex Henrie wrote:\n>\n> On Wed, Dec 2, 2020 at 7:21 PM Junio C Hamano wrote:\n> >\n> > So, the idea is that we currently return NULL when pull.ff is set,\n> > but now we also check \"pull.rebase\" etc. and give \"--ff-only\" when\n> > we did not choose --[no-]rebase and lack the configuration. So the\n> > default (i.e. when pull.ff and pull.rebase are not set) would be as\n> > if the user said\n> >\n> > git pull --ff-only --no-rebase\n> >\n> > I am not seeing what problem this tries to solve, exactly, though.\n> >\n> > I would have expected that for those who did not choose between\n> > merge and rebase (either with the pull.rebase configuration or from\n> > the command line --[no-]rebase option) the command would behave as\n> > if --ff-only is given, regardless of how pull.ff configuration is\n> > set. That way, those who are unconfigured will just follow along\n> > what happens at the upstream, until they have their own development,\n> > at which point \"--ff-only\" can no longer be satisfied and their\n> > \"pull\" would fail until they choose how to consolidate their work\n> > with the upstream.\n>\n> My goal was to make `git pull` without arguments and without any\n> configuration set (the most common case) reject non-fast-forwards,\n\nThis is what we all want, in my opinion.\n\n> and not add any new config variables\n\nBut we need first to give users the ability to test this new mode with\na configuration variable.\n\nIt's how we get there that we disagree.\n\n> In my opinion it's fine that setting pull.ff\n> would change the behavior of `git pull` without arguments; I don't\n> think that that would be bad behavior.\n\nNo, it's not bad behavior. But we need to warn our users that this\nchange is coming first, and how to try it out.\n\nThat's what I think.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}