{"thread":{"id":"56092","subject":"[PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","startedAt":"2021-07-11T01:28:15Z","lastAt":"2021-07-14T17:31:26Z","messageCount":28,"participants":["Alex Henrie","Felipe Contreras","Phillip Wood","Junio C Hamano","Elijah Newren","Son Luong Ngoc"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"429665","messageId":"20210711012604.947321-1-alexhenrie24@gmail.com","threadId":"56092","inReplyTo":null,"subject":"[PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2021-07-11T01:26:04Z","receivedAt":"2021-07-11T01:28:15Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"The warning about pulling without specifying how to reconcile divergent\nbranches says that after setting pull.rebase to true, --ff-only can\nstill be passed on the command line to require a fast-forward. Make that\nactually work.\n\nSigned-off-by: Alex Henrie \n---\n advice.c | 5 +++++\n advice.h | 1 +\n builtin/merge.c | 2 +-\n builtin/pull.c | 11 ++++++++---\n t/t7601-merge-pull-config.sh | 24 ++++++++++++++++++++++++\n 5 files changed, 39 insertions(+), 4 deletions(-)\n\ndiff --git a/advice.c b/advice.c\nindex 0b9c89c48a..337e8f342b 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -286,6 +286,11 @@ void NORETURN die_conclude_merge(void)\n \tdie(_(\"Exiting because of unfinished merge.\"));\n }\n \n+void NORETURN die_ff_impossible(void)\n+{\n+\tdie(_(\"Not possible to fast-forward, aborting.\"));\n+}\n+\n void advise_on_updating_sparse_paths(struct string_list *pathspec_list)\n {\n \tstruct string_list_item *item;\ndiff --git a/advice.h b/advice.h\nindex bd26c385d0..1624043838 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -95,6 +95,7 @@ void advise_if_enabled(enum advice_type type, const char *advice, ...);\n int error_resolve_conflict(const char *me);\n void NORETURN die_resolve_conflict(const char *me);\n void NORETURN die_conclude_merge(void);\n+void NORETURN die_ff_impossible(void);\n void advise_on_updating_sparse_paths(struct string_list *pathspec_list);\n void detach_advice(const char *new_name);\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex a8a843b1f5..aa920ac524 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1620,7 +1620,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (fast_forward == FF_ONLY)\n-\t\tdie(_(\"Not possible to fast-forward, aborting.\"));\n+\t\tdie_ff_impossible();\n \n \tif (autostash)\n \t\tcreate_autostash(the_repository,\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 3e13f81084..d979660482 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1046,9 +1046,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \n \tcan_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);\n \n-\tif (rebase_unspecified && !opt_ff && !can_ff) {\n-\t\tif (opt_verbosity >= 0)\n-\t\t\tshow_advice_pull_non_ff();\n+\tif (!can_ff) {\n+\t\tif (opt_ff) {\n+\t\t\tif (!strcmp(opt_ff, \"--ff-only\"))\n+\t\t\t\tdie_ff_impossible();\n+\t\t} else {\n+\t\t\tif (rebase_unspecified && opt_verbosity >= 0)\n+\t\t\t\tshow_advice_pull_non_ff();\n+\t\t}\n \t}\n \n \tif (opt_rebase) {\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex 52e8ccc933..b5a09a60f9 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -183,6 +183,30 @@ test_expect_success 'pull prevents non-fast-forward with \"only\" in pull.ff' '\n \ttest_must_fail git pull . c3\n '\n \n+test_expect_success 'pull prevents non-fast-forward with pull.ff=only and pull.rebase=true' '\n+\tgit reset --hard c1 &&\n+\ttest_config pull.ff only &&\n+\ttest_config pull.rebase true &&\n+\ttest_must_fail git pull . c3\n+'\n+\n+test_expect_success 'pull prevents non-fast-forward with pull.ff=only and pull.rebase=false' '\n+\tgit reset --hard c1 &&\n+\ttest_config pull.ff only &&\n+\ttest_config pull.rebase false &&\n+\ttest_must_fail git pull . c3\n+'\n+\n+test_expect_success 'pull prevents non-fast-forward with --rebase --ff-only' '\n+\tgit reset --hard c1 &&\n+\ttest_must_fail git pull --rebase --ff-only . c3\n+'\n+\n+test_expect_success 'pull prevents non-fast-forward with --no-rebase --ff-only' '\n+\tgit reset --hard c1 &&\n+\ttest_must_fail git pull --no-rebase --ff-only . c3\n+'\n+\n test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '\n \tgit reset --hard c1 &&\n \tgit config pull.twohead ours &&\n-- \n2.32.0.171.gfa4a44ff46\n\n"},{"id":"429710","messageId":"60eb259e1a75d_974bf20895@natae.notmuch","threadId":"56092","inReplyTo":"20210711012604.947321-1-alexhenrie24@gmail.com","subject":"RE: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-11T17:08:46Z","receivedAt":"2021-07-11T17:08:50Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Alex Henrie wrote:\n> The warning about pulling without specifying how to reconcile divergent\n> branches says that after setting pull.rebase to true, --ff-only can\n> still be passed on the command line to require a fast-forward. Make that\n> actually work.\n\nI don't know where that is being said, but it's wrong: --ff-only is\nmeant for merge only.\n\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -1046,9 +1046,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n> \n> \tcan_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);\n> \n> -\tif (rebase_unspecified && !opt_ff && !can_ff) {\n> -\t\tif (opt_verbosity >= 0)\n> -\t\t\tshow_advice_pull_non_ff();\n> +\tif (!can_ff) {\n> +\t\tif (opt_ff) {\n> +\t\t\tif (!strcmp(opt_ff, \"--ff-only\"))\n> +\t\t\t\tdie_ff_impossible();\n\nAs I've mentioned multiple times already, this is wrong.\n\nThe advice clearly says:\n\n You can also pass --rebase, --no-rebase, or --ff-only on the command\n line to override the configured default per invocation.\n\nWith your patch now this is even less true:\n\n git -c pull.ff=only pull --rebase\n\n> +\t\t} else {\n> +\t\t\tif (rebase_unspecified && opt_verbosity >= 0)\n> +\t\t\t\tshow_advice_pull_non_ff();\n> +\t\t}\n> \t}\n> \n> \tif (opt_rebase) {\n> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\n> index 52e8ccc933..b5a09a60f9 100755\n> --- a/t/t7601-merge-pull-config.sh\n> +++ b/t/t7601-merge-pull-config.sh\n> @@ -183,6 +183,30 @@ test_expect_success 'pull prevents non-fast-forward with \"only\" in pull.ff' '\n> \ttest_must_fail git pull . c3\n> '\n\nCan you add this test [1] so I don't have to explain the same thing over\nand over?\n\n test_expect_success 'pull allows non-fast-forward with \"only\" in pull.ff if --rebase' '\n git reset --hard c1 &&\n test_config pull.ff only &&\n git pull --rebase . c3\n '\n\nCheers.\n\n> +test_expect_success 'pull prevents non-fast-forward with pull.ff=only and pull.rebase=true' '\n> +\tgit reset --hard c1 &&\n> +\ttest_config pull.ff only &&\n> +\ttest_config pull.rebase true &&\n> +\ttest_must_fail git pull . c3\n> +'\n> +\n> +test_expect_success 'pull prevents non-fast-forward with pull.ff=only and pull.rebase=false' '\n> +\tgit reset --hard c1 &&\n> +\ttest_config pull.ff only &&\n> +\ttest_config pull.rebase false &&\n> +\ttest_must_fail git pull . c3\n> +'\n> +\n> +test_expect_success 'pull prevents non-fast-forward with --rebase --ff-only' '\n> +\tgit reset --hard c1 &&\n> +\ttest_must_fail git pull --rebase --ff-only . c3\n> +'\n> +\n> +test_expect_success 'pull prevents non-fast-forward with --no-rebase --ff-only' '\n> +\tgit reset --hard c1 &&\n> +\ttest_must_fail git pull --no-rebase --ff-only . c3\n> +'\n> +\n> test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '\n> \tgit reset --hard c1 &&\n> \tgit config pull.twohead ours &&\n> -- \n\n[1] https://lore.kernel.org/git/20210711170703.651081-1-felipe.contreras@gmail.com/\n\n-- \nFelipe Contreras\n"},{"id":"429711","messageId":"CAMMLpeRwpxEe5qJumwXa5hyjeUBrTRDU3-1OSHDNc=UiNqN48w@mail.gmail.com","threadId":"56092","inReplyTo":"60eb259e1a75d_974bf20895@natae.notmuch","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2021-07-11T20:00:19Z","receivedAt":"2021-07-11T20:00:34Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Sun, Jul 11, 2021 at 11:08 AM Felipe Contreras\n wrote:\n>\n> Alex Henrie wrote:\n> > The warning about pulling without specifying how to reconcile divergent\n> > branches says that after setting pull.rebase to true, --ff-only can\n> > still be passed on the command line to require a fast-forward. Make that\n> > actually work.\n>\n> I don't know where that is being said, but it's wrong: --ff-only is\n> meant for merge only.\n>\n> > --- a/builtin/pull.c\n> > +++ b/builtin/pull.c\n> > @@ -1046,9 +1046,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n> >\n> > can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);\n> >\n> > - if (rebase_unspecified && !opt_ff && !can_ff) {\n> > - if (opt_verbosity >= 0)\n> > - show_advice_pull_non_ff();\n> > + if (!can_ff) {\n> > + if (opt_ff) {\n> > + if (!strcmp(opt_ff, \"--ff-only\"))\n> > + die_ff_impossible();\n>\n> As I've mentioned multiple times already, this is wrong.\n>\n> The advice clearly says:\n>\n> You can also pass --rebase, --no-rebase, or --ff-only on the command\n> line to override the configured default per invocation.\n>\n> With your patch now this is even less true:\n>\n> git -c pull.ff=only pull --rebase\n\nI think it's an improvement over the current situation. --no-rebase\ndoes not override pull.ff=only, so it makes sense that --rebase does\nnot override pull.ff=only either. Besides, it's generally better to\nabort instead of rewriting history if it's not perfectly clear that\nthe user meant to rewrite the history.\n\n-Alex\n"},{"id":"429712","messageId":"60eb6588c6ef2_a2044208b0@natae.notmuch","threadId":"56092","inReplyTo":"CAMMLpeRwpxEe5qJumwXa5hyjeUBrTRDU3-1OSHDNc=UiNqN48w@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-11T21:41:28Z","receivedAt":"2021-07-11T21:41:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Alex Henrie wrote:\n> On Sun, Jul 11, 2021 at 11:08 AM Felipe Contreras\n> wrote:\n> >\n> > Alex Henrie wrote:\n> > > The warning about pulling without specifying how to reconcile divergent\n> > > branches says that after setting pull.rebase to true, --ff-only can\n> > > still be passed on the command line to require a fast-forward. Make that\n> > > actually work.\n> >\n> > I don't know where that is being said, but it's wrong: --ff-only is\n> > meant for merge only.\n> >\n> > > --- a/builtin/pull.c\n> > > +++ b/builtin/pull.c\n> > > @@ -1046,9 +1046,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n> > >\n> > > can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);\n> > >\n> > > - if (rebase_unspecified && !opt_ff && !can_ff) {\n> > > - if (opt_verbosity >= 0)\n> > > - show_advice_pull_non_ff();\n> > > + if (!can_ff) {\n> > > + if (opt_ff) {\n> > > + if (!strcmp(opt_ff, \"--ff-only\"))\n> > > + die_ff_impossible();\n> >\n> > As I've mentioned multiple times already, this is wrong.\n> >\n> > The advice clearly says:\n> >\n> > You can also pass --rebase, --no-rebase, or --ff-only on the command\n> > line to override the configured default per invocation.\n> >\n> > With your patch now this is even less true:\n> >\n> > git -c pull.ff=only pull --rebase\n> \n> I think it's an improvement over the current situation. --no-rebase\n> does not override pull.ff=only, so it makes sense that --rebase does\n> not override pull.ff=only either.\n\nI disagree, but that's not the point, the point is that now the advice\nmessage is wrong since --rebase doesn't override pull.ff=only.\n\nAdditionally the documentation is inaccurate too because at no point\ndoes pull.ff mention anything about rebase:\n\npull.ff::\n\tBy default, Git does not create an extra merge commit when merging\n\ta commit that is a descendant of the current commit. Instead, the\n\ttip of the current branch is fast-forwarded. When set to `false`,\n\tthis variable tells Git to create an extra merge commit in such\n\ta case (equivalent to giving the `--no-ff` option from the command\n\tline). When set to `only`, only such fast-forward merges are\n\tallowed (equivalent to giving the `--ff-only` option from the\n\tcommand line). This setting overrides `merge.ff` when pulling.\n\n-- \nFelipe Contreras\n"},{"id":"429719","messageId":"00e246b1-c712-e6a5-5c27-89127d796098@gmail.com","threadId":"56092","inReplyTo":"20210711012604.947321-1-alexhenrie24@gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-07-12T10:21:04Z","receivedAt":"2021-07-12T10:21:10Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alex\n\n\nOn 11/07/2021 02:26, Alex Henrie wrote:\n> The warning about pulling without specifying how to reconcile divergent\n> branches says that after setting pull.rebase to true, --ff-only can\n> still be passed on the command line to require a fast-forward. Make that\n> actually work.\n\nThanks for revising this patch, I like this approach much better. I do \nhowever have some concerns about the interaction of pull.ff with the \nrebase config and command line options. I'd naively expect the following \nbehavior (where rebase can fast-forward if possible)\n\n pull.ff pull.rebase commandline action\n only not false rebase\n only not false --no-rebase fast-forward only\n * not false --ff-only fast-forward only\n only not false --ff merge --ff\n only not false --no-ff merge --no-ff\n only false fast-forward only\n only false --rebase rebase\n only false --ff merge --ff\n only false --no-ff merge --no-ff\n\nI don't think enforcing fast-forward only for rebases makes sense unless \nit is given on the command line. If the user gives `--rebase` \n`--ff-only` on the command line then we should either error out or take \nthe last one in which case `pull --rebase --ff-only` would fast-forward \nonly but `pull --ff-only --rebase` would rebase. We should also decide \nwhat to do when the user has pull.ff set to something other than only \nand also has pull.rebase to something other than false set - I'd guess \nwe'd want to rebase unless there is a merge option on the command line \nbut I haven't thought about those cases.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Alex Henrie \n> ---\n> advice.c | 5 +++++\n> advice.h | 1 +\n> builtin/merge.c | 2 +-\n> builtin/pull.c | 11 ++++++++---\n> t/t7601-merge-pull-config.sh | 24 ++++++++++++++++++++++++\n> 5 files changed, 39 insertions(+), 4 deletions(-)\n> \n> diff --git a/advice.c b/advice.c\n> index 0b9c89c48a..337e8f342b 100644\n> --- a/advice.c\n> +++ b/advice.c\n> @@ -286,6 +286,11 @@ void NORETURN die_conclude_merge(void)\n> \tdie(_(\"Exiting because of unfinished merge.\"));\n> }\n> \n> +void NORETURN die_ff_impossible(void)\n> +{\n> +\tdie(_(\"Not possible to fast-forward, aborting.\"));\n> +}\n> +\n> void advise_on_updating_sparse_paths(struct string_list *pathspec_list)\n> {\n> \tstruct string_list_item *item;\n> diff --git a/advice.h b/advice.h\n> index bd26c385d0..1624043838 100644\n> --- a/advice.h\n> +++ b/advice.h\n> @@ -95,6 +95,7 @@ void advise_if_enabled(enum advice_type type, const char *advice, ...);\n> int error_resolve_conflict(const char *me);\n> void NORETURN die_resolve_conflict(const char *me);\n> void NORETURN die_conclude_merge(void);\n> +void NORETURN die_ff_impossible(void);\n> void advise_on_updating_sparse_paths(struct string_list *pathspec_list);\n> void detach_advice(const char *new_name);\n> \n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index a8a843b1f5..aa920ac524 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -1620,7 +1620,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n> \t}\n> \n> \tif (fast_forward == FF_ONLY)\n> -\t\tdie(_(\"Not possible to fast-forward, aborting.\"));\n> +\t\tdie_ff_impossible();\n> \n> \tif (autostash)\n> \t\tcreate_autostash(the_repository,\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 3e13f81084..d979660482 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -1046,9 +1046,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n> \n> \tcan_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);\n> \n> -\tif (rebase_unspecified && !opt_ff && !can_ff) {\n> -\t\tif (opt_verbosity >= 0)\n> -\t\t\tshow_advice_pull_non_ff();\n> +\tif (!can_ff) {\n> +\t\tif (opt_ff) {\n> +\t\t\tif (!strcmp(opt_ff, \"--ff-only\"))\n> +\t\t\t\tdie_ff_impossible();\n> +\t\t} else {\n> +\t\t\tif (rebase_unspecified && opt_verbosity >= 0)\n> +\t\t\t\tshow_advice_pull_non_ff();\n> +\t\t}\n> \t}\n> \n> \tif (opt_rebase) {\n> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\n> index 52e8ccc933..b5a09a60f9 100755\n> --- a/t/t7601-merge-pull-config.sh\n> +++ b/t/t7601-merge-pull-config.sh\n> @@ -183,6 +183,30 @@ test_expect_success 'pull prevents non-fast-forward with \"only\" in pull.ff' '\n> \ttest_must_fail git pull . c3\n> '\n> \n> +test_expect_success 'pull prevents non-fast-forward with pull.ff=only and pull.rebase=true' '\n> +\tgit reset --hard c1 &&\n> +\ttest_config pull.ff only &&\n> +\ttest_config pull.rebase true &&\n> +\ttest_must_fail git pull . c3\n> +'\n> +\n> +test_expect_success 'pull prevents non-fast-forward with pull.ff=only and pull.rebase=false' '\n> +\tgit reset --hard c1 &&\n> +\ttest_config pull.ff only &&\n> +\ttest_config pull.rebase false &&\n> +\ttest_must_fail git pull . c3\n> +'\n> +\n> +test_expect_success 'pull prevents non-fast-forward with --rebase --ff-only' '\n> +\tgit reset --hard c1 &&\n> +\ttest_must_fail git pull --rebase --ff-only . c3\n> +'\n> +\n> +test_expect_success 'pull prevents non-fast-forward with --no-rebase --ff-only' '\n> +\tgit reset --hard c1 &&\n> +\ttest_must_fail git pull --no-rebase --ff-only . c3\n> +'\n> +\n> test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '\n> \tgit reset --hard c1 &&\n> \tgit config pull.twohead ours &&\n> \n\n"},{"id":"429758","messageId":"60ec682262994_a452520837@natae.notmuch","threadId":"56092","inReplyTo":"00e246b1-c712-e6a5-5c27-89127d796098@gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-12T16:04:50Z","receivedAt":"2021-07-12T16:04:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Phillip Wood wrote:\n> On 11/07/2021 02:26, Alex Henrie wrote:\n> > The warning about pulling without specifying how to reconcile divergent\n> > branches says that after setting pull.rebase to true, --ff-only can\n> > still be passed on the command line to require a fast-forward. Make that\n> > actually work.\n> \n> Thanks for revising this patch, I like this approach much better. I do \n> however have some concerns about the interaction of pull.ff with the \n> rebase config and command line options. I'd naively expect the following \n> behavior (where rebase can fast-forward if possible)\n> \n> pull.ff pull.rebase commandline action\n> only not false rebase\n\nAgreed. (pull.ff applies only for --merge)\n\n> only not false --no-rebase fast-forward only\n\nAgreed. (--no-rebase is --merge, and pull.ff applies)\n\n> * not false --ff-only fast-forward only\n\nDisagree. (--ff-only is for --merge)\n\nWe would need to change the documentation and the advice warning for\nthis to be correct.\n\n> only not false --ff merge --ff\n\nDisagree.\n\nThis is a rebase, --ff should be ignored.\n\nJunio already proposed --ff and other options to imply a merge [1], but\nI already explained why that is problematic [2].\n\n> only not false --no-ff merge --no-ff\n\nDisagree. (ditto)\n\n> only false fast-forward only\n> only false --rebase rebase\n> only false --ff merge --ff\n> only false --no-ff merge --no-ff\n\nAgreed.\n\n> I don't think enforcing fast-forward only for rebases makes sense unless \n> it is given on the command line.\n\nBut why? This is inconsistent.\n\nEverywhere else in git the configuration is another way of specifying\nthe command line. This would be the first instance where it would not be\nthe case.\n\n> If the user gives `--rebase` `--ff-only` on the command line then we\n> should either error out or take the last one in which case `pull\n> --rebase --ff-only` would fast-forward only but `pull --ff-only\n> --rebase` would rebase.\n\nFollowing the same logic `pull --ff-only --merge` would ignore the\nprevious --ff-only, wouldn't it?\n\n\nThis is a pretty significant semantic change and nowhere in this patch\nit's explained who this is supposed to help, or what is the motivtion\nbehind it.\n\n[1] https://lore.kernel.org/git/xmqqmtyf8hfm.fsf@gitster.c.googlers.com/\n[2] https://lore.kernel.org/git/5fd8aa6a52e81_190cd7208c8@natae.notmuch/\n\n-- \nFelipe Contreras\n"},{"id":"429763","messageId":"CAMMLpeQYwPDjc8WVFmwVismJPBYAifQS96+R8hDj+pch_db+Qg@mail.gmail.com","threadId":"56092","inReplyTo":"00e246b1-c712-e6a5-5c27-89127d796098@gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2021-07-12T16:29:02Z","receivedAt":"2021-07-12T16:29:17Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Mon, Jul 12, 2021 at 4:21 AM Phillip Wood wrote:\n>\n> On 11/07/2021 02:26, Alex Henrie wrote:\n> > The warning about pulling without specifying how to reconcile divergent\n> > branches says that after setting pull.rebase to true, --ff-only can\n> > still be passed on the command line to require a fast-forward. Make that\n> > actually work.\n>\n> Thanks for revising this patch, I like this approach much better. I do\n> however have some concerns about the interaction of pull.ff with the\n> rebase config and command line options. I'd naively expect the following\n> behavior (where rebase can fast-forward if possible)\n>\n> pull.ff pull.rebase commandline action\n> only not false rebase\n> only not false --no-rebase fast-forward only\n> * not false --ff-only fast-forward only\n> only not false --ff merge --ff\n> only not false --no-ff merge --no-ff\n> only false fast-forward only\n> only false --rebase rebase\n> only false --ff merge --ff\n> only false --no-ff merge --no-ff\n>\n> I don't think enforcing fast-forward only for rebases makes sense unless\n> it is given on the command line. If the user gives `--rebase`\n> `--ff-only` on the command line then we should either error out or take\n> the last one in which case `pull --rebase --ff-only` would fast-forward\n> only but `pull --ff-only --rebase` would rebase. We should also decide\n> what to do when the user has pull.ff set to something other than only\n> and also has pull.rebase to something other than false set - I'd guess\n> we'd want to rebase unless there is a merge option on the command line\n> but I haven't thought about those cases.\n\nI was thinking of --rebase and --ff-only as orthogonal variables.\nNevertheless, we could make --rebase imply --ff, which would be pretty\neasy to explain in the documentation for the command-line options.\nThat way, even though pull.rebase=true with pull.ff=only would enforce\nfast-forward-only, the user could easily override it with `git pull\n-r`. Would you accept that compromise?\n\n-Alex\n"},{"id":"429777","messageId":"xmqqpmvn5ukj.fsf@gitster.g","threadId":"56092","inReplyTo":"00e246b1-c712-e6a5-5c27-89127d796098@gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-12T17:08:12Z","receivedAt":"2021-07-12T17:08:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Thanks for revising this patch, I like this approach much better. I do\n> however have some concerns about the interaction of pull.ff with the \n> rebase config and command line options. I'd naively expect the\n> following behavior (where rebase can fast-forward if possible)\n>\n> pull.ff pull.rebase commandline action\n> only not false rebase\n> only not false --no-rebase fast-forward only\n> * not false --ff-only fast-forward only\n> only not false --ff merge --ff\n> only not false --no-ff merge --no-ff\n> only false fast-forward only\n> only false --rebase rebase\n> only false --ff merge --ff\n> only false --no-ff merge --no-ff\n\nDo you mean by \"not false\" something other than \"true\"? Are you\ntrying to capture what should happen when these configuration\noptions are unspecified as well (and your \"not false\" is \"either set\nto true or unspecified\")? I ask because the first row does not make\nany sense to me. It seems to say\n\n \"If pull.ff is set to 'only', pull.rebase is not set to 'false',\n and the command line does not say anything, we will rebase\".\n\nI do agree with you that the command line options\n\n --ff-only\n --ff (aka \"allow ff\")\n --no-ff\n\nshould override pull.ff and\n\n --rebase\n --no-rebase (aka \"merge\")\n\nshould override pull.rebase configuration settings and also override\npull.ff set to 'only' (i.e. the user explicitly wants intregration\nof the two histories and at that point \"I usually just follow along\"\nwhich is \"pull.ff=only\" no longer applies).\n"},{"id":"429781","messageId":"60ec7c3636327_a45252081d@natae.notmuch","threadId":"56092","inReplyTo":"xmqqpmvn5ukj.fsf@gitster.g","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-12T17:30:30Z","receivedAt":"2021-07-12T17:30:35Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n> > Thanks for revising this patch, I like this approach much better. I do\n> > however have some concerns about the interaction of pull.ff with the \n> > rebase config and command line options. I'd naively expect the\n> > following behavior (where rebase can fast-forward if possible)\n> >\n> > pull.ff pull.rebase commandline action\n> > only not false rebase\n> > only not false --no-rebase fast-forward only\n> > * not false --ff-only fast-forward only\n> > only not false --ff merge --ff\n> > only not false --no-ff merge --no-ff\n> > only false fast-forward only\n> > only false --rebase rebase\n> > only false --ff merge --ff\n> > only false --no-ff merge --no-ff\n> \n> Do you mean by \"not false\" something other than \"true\"? Are you\n> trying to capture what should happen when these configuration\n> options are unspecified as well (and your \"not false\" is \"either set\n> to true or unspecified\")? I ask because the first row does not make\n> any sense to me. It seems to say\n> \n> \"If pull.ff is set to 'only', pull.rebase is not set to 'false',\n> and the command line does not say anything, we will rebase\".\n\nNo, pull.rebase can have values other than true, like \"merges\".\n\n git -c pull.ff=only -c pull.rebase=merges pull\n\nThis should rebase because pull.ff=only is meant only for --merge.\n\n> I do agree with you that the command line options\n> \n> --ff-only\n> --ff (aka \"allow ff\")\n> --no-ff\n> \n> should override pull.ff and\n\nYes.\n\n> --rebase\n> --no-rebase (aka \"merge\")\n> \n> should override pull.rebase configuration settings\n\nYes.\n\n> and also override pull.ff set to 'only'\n\nNo.\n\npull.ff=only is specifically set for when the user wants to do a merge,\n*not* a rebase.\n\n git -c pull.ff=only pull # fast-forward merge\n git -c pull.ff=only pull --rebase # rebase\n git -c pull.ff=only pull --no-rebase # fast-forward merge\n\nWhether the merge is implied becuse it's the default, or the user has\nexplicitely specified it with --no-rebase (should be --merge) does not\nmatter.\n\n> (i.e. the user explicitly\n> wants intregration of the two histories and at that point \"I usually\n> just follow along\" which is \"pull.ff=only\" no longer applies).\n\nNowhere in the description of pull.ff does it say that --no-rebase turns\noff pull.ff=only.\n\npull.ff::\n\tBy default, Git does not create an extra merge commit when merging\n\ta commit that is a descendant of the current commit. Instead, the\n\ttip of the current branch is fast-forwarded. When set to `false`,\n\tthis variable tells Git to create an extra merge commit in such\n\ta case (equivalent to giving the `--no-ff` option from the command\n\tline). When set to `only`, only such fast-forward merges are\n\tallowed (equivalent to giving the `--ff-only` option from the\n\tcommand line). This setting overrides `merge.ff` when pulling.\n\nThat would be a pretty convoluted semantic change.\n\nAnd BTW, why would these four produce different results?\n\n git -c pull.ff=only -c pull.rebase=false pull\n git -c pull.ff=only pull --no-rebase\n git -c pull.rebase=false pull --ff-only\n git pull --ff-only --no-rebase\n\n-- \nFelipe Contreras\n"},{"id":"429783","messageId":"60ec7f276909f_a4525208c7@natae.notmuch","threadId":"56092","inReplyTo":"CAMMLpeQYwPDjc8WVFmwVismJPBYAifQS96+R8hDj+pch_db+Qg@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-12T17:43:03Z","receivedAt":"2021-07-12T17:43:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Alex Henrie wrote:\n> On Mon, Jul 12, 2021 at 4:21 AM Phillip Wood wrote:\n> >\n> > On 11/07/2021 02:26, Alex Henrie wrote:\n> > > The warning about pulling without specifying how to reconcile divergent\n> > > branches says that after setting pull.rebase to true, --ff-only can\n> > > still be passed on the command line to require a fast-forward. Make that\n> > > actually work.\n> >\n> > Thanks for revising this patch, I like this approach much better. I do\n> > however have some concerns about the interaction of pull.ff with the\n> > rebase config and command line options. I'd naively expect the following\n> > behavior (where rebase can fast-forward if possible)\n> >\n> > pull.ff pull.rebase commandline action\n> > only not false rebase\n> > only not false --no-rebase fast-forward only\n> > * not false --ff-only fast-forward only\n> > only not false --ff merge --ff\n> > only not false --no-ff merge --no-ff\n> > only false fast-forward only\n> > only false --rebase rebase\n> > only false --ff merge --ff\n> > only false --no-ff merge --no-ff\n> >\n> > I don't think enforcing fast-forward only for rebases makes sense unless\n> > it is given on the command line. If the user gives `--rebase`\n> > `--ff-only` on the command line then we should either error out or take\n> > the last one in which case `pull --rebase --ff-only` would fast-forward\n> > only but `pull --ff-only --rebase` would rebase. We should also decide\n> > what to do when the user has pull.ff set to something other than only\n> > and also has pull.rebase to something other than false set - I'd guess\n> > we'd want to rebase unless there is a merge option on the command line\n> > but I haven't thought about those cases.\n> \n> I was thinking of --rebase and --ff-only as orthogonal variables.\n> Nevertheless, we could make --rebase imply --ff, which would be pretty\n> easy to explain in the documentation for the command-line options.\n> That way, even though pull.rebase=true with pull.ff=only would enforce\n> fast-forward-only, the user could easily override it with `git pull\n> -r`. Would you accept that compromise?\n\nWhat happens if the user has configured `pull.ff=no`?\n\n-- \nFelipe Contreras\n"},{"id":"429785","messageId":"CABPp-BERS0iiiVhSsSs6dkqzBVTQgwJUjjKaZQEzRDGRUdObcQ@mail.gmail.com","threadId":"56092","inReplyTo":"xmqqpmvn5ukj.fsf@gitster.g","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-07-12T17:50:57Z","receivedAt":"2021-07-12T17:51:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Jul 12, 2021 at 10:08 AM Junio C Hamano wrote:\n>\n> Phillip Wood writes:\n>\n> > Thanks for revising this patch, I like this approach much better. I do\n> > however have some concerns about the interaction of pull.ff with the\n> > rebase config and command line options. I'd naively expect the\n> > following behavior (where rebase can fast-forward if possible)\n> >\n> > pull.ff pull.rebase commandline action\n> > only not false rebase\n> > only not false --no-rebase fast-forward only\n> > * not false --ff-only fast-forward only\n> > only not false --ff merge --ff\n> > only not false --no-ff merge --no-ff\n> > only false fast-forward only\n> > only false --rebase rebase\n> > only false --ff merge --ff\n> > only false --no-ff merge --no-ff\n>\n> Do you mean by \"not false\" something other than \"true\"? Are you\n> trying to capture what should happen when these configuration\n> options are unspecified as well (and your \"not false\" is \"either set\n> to true or unspecified\")? I ask because the first row does not make\n> any sense to me. It seems to say\n>\n> \"If pull.ff is set to 'only', pull.rebase is not set to 'false',\n> and the command line does not say anything, we will rebase\".\n\nI think Phillip is trying to answer what to do when pull.ff and\npull.rebase conflict. If I read his \"not false\" means \"is set to\nsomething other than false\", then I agree with his table, but I think\nhe missed covering some cases.\n\nI think his table says that pull.rebase=false cannot conflict with\npull.ff settings, but any other value for pull.rebase can. That makes\nsense to me.\n\nI'd similarly say that pull.ff=true cannot conflict with any\npull.rebase settings...but that both pull.ff=only AND pull.ff=false\nconflict with pull.rebase={true,merges}.\n\nMy opinion would be:\n * conflicting command line flags results in the last one winning.\n * --no-rebase makes pull.ff determine the action.\n * --ff makes pull.rebase determine the action.\n * any other command line flag (-r|--rebase|--no-ff|--ff-only)\noverrides both pull.ff and pull.rebase\n * If no command line option is given, and pull.ff and pull.rebase\nconflict, then error out.\n\nI believe my recommendation above is consistent with every entry in\nPhillip's table except the first line (where I suggest erroring out\ninstead).\n"},{"id":"429788","messageId":"e63d65f4-048e-464b-439c-4f8f48e72216@gmail.com","threadId":"56092","inReplyTo":"xmqqpmvn5ukj.fsf@gitster.g","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-07-12T17:54:34Z","receivedAt":"2021-07-12T17:54:38Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 12/07/2021 18:08, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> Thanks for revising this patch, I like this approach much better. I do\n>> however have some concerns about the interaction of pull.ff with the\n>> rebase config and command line options. I'd naively expect the\n>> following behavior (where rebase can fast-forward if possible)\n>>\n>> pull.ff pull.rebase commandline action\n>> only not false rebase\n>> only not false --no-rebase fast-forward only\n>> * not false --ff-only fast-forward only\n>> only not false --ff merge --ff\n>> only not false --no-ff merge --no-ff\n>> only false fast-forward only\n>> only false --rebase rebase\n>> only false --ff merge --ff\n>> only false --no-ff merge --no-ff\n> \n> Do you mean by \"not false\" something other than \"true\"?\n\nI mean pull.rebase is set to true/interactive/merges/preserve\n\n> Are you\n> trying to capture what should happen when these configuration\n> options are unspecified as well (and your \"not false\" is \"either set\n> to true or unspecified\")? I ask because the first row does not make\n> any sense to me. It seems to say\n> \n> \"If pull.ff is set to 'only', pull.rebase is not set to 'false',\n> and the command line does not say anything, we will rebase\".\n\nYes because if pull.rebase is not false then the user wants to rebase.\n\n> I do agree with you that the command line options\n> \n> --ff-only\n> --ff (aka \"allow ff\")\n> --no-ff\n> \n> should override pull.ff and\n> \n> --rebase\n> --no-rebase (aka \"merge\")\n> \n> should override pull.rebase configuration settings and also override\n> pull.ff set to 'only' (i.e. the user explicitly wants intregration\n> of the two histories and at that point \"I usually just follow along\"\n> which is \"pull.ff=only\" no longer applies).\n\nI'd assumed --no-rebase just meant \"merge respecting pull.ff instead of \nrebasing\" rather than \"merge ignoring pull.ff instead of rebasing\"\n\nBest Wishes\n\nPhillip\n"},{"id":"429809","messageId":"60ec87d03f7c5_a231f20826@natae.notmuch","threadId":"56092","inReplyTo":"CABPp-BERS0iiiVhSsSs6dkqzBVTQgwJUjjKaZQEzRDGRUdObcQ@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-12T18:20:00Z","receivedAt":"2021-07-12T18:20:14Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Elijah Newren wrote:\n> On Mon, Jul 12, 2021 at 10:08 AM Junio C Hamano wrote:\n> >\n> > Phillip Wood writes:\n> >\n> > > Thanks for revising this patch, I like this approach much better. I do\n> > > however have some concerns about the interaction of pull.ff with the\n> > > rebase config and command line options. I'd naively expect the\n> > > following behavior (where rebase can fast-forward if possible)\n> > >\n> > > pull.ff pull.rebase commandline action\n> > > only not false rebase\n> > > only not false --no-rebase fast-forward only\n> > > * not false --ff-only fast-forward only\n> > > only not false --ff merge --ff\n> > > only not false --no-ff merge --no-ff\n> > > only false fast-forward only\n> > > only false --rebase rebase\n> > > only false --ff merge --ff\n> > > only false --no-ff merge --no-ff\n> >\n> > Do you mean by \"not false\" something other than \"true\"? Are you\n> > trying to capture what should happen when these configuration\n> > options are unspecified as well (and your \"not false\" is \"either set\n> > to true or unspecified\")? I ask because the first row does not make\n> > any sense to me. It seems to say\n> >\n> > \"If pull.ff is set to 'only', pull.rebase is not set to 'false',\n> > and the command line does not say anything, we will rebase\".\n> \n> I think Phillip is trying to answer what to do when pull.ff and\n> pull.rebase conflict. If I read his \"not false\" means \"is set to\n> something other than false\", then I agree with his table, but I think\n> he missed covering some cases.\n> \n> I think his table says that pull.rebase=false cannot conflict with\n> pull.ff settings, but any other value for pull.rebase can. That makes\n> sense to me.\n> \n> I'd similarly say that pull.ff=true cannot conflict with any\n> pull.rebase settings...but that both pull.ff=only AND pull.ff=false\n> conflict with pull.rebase={true,merges}.\n> \n> My opinion would be:\n> * conflicting command line flags results in the last one winning.\n> * --no-rebase makes pull.ff determine the action.\n> * --ff makes pull.rebase determine the action.\n> * any other command line flag (-r|--rebase|--no-ff|--ff-only)\n> overrides both pull.ff and pull.rebase\n> * If no command line option is given, and pull.ff and pull.rebase\n> conflict, then error out.\n> \n> I believe my recommendation above is consistent with every entry in\n> Phillip's table except the first line (where I suggest erroring out\n> instead).\n\nNo:\n\n git -c pull.ff=only -c pull.rebase=true pull --ff\n\nIn Phillip's table that does a merge, in your rules that's a rebase.\n\n\nMoreover, since when does git do something different depending if the\naction was configured or specified in the command line? They are\nsupposed to be two ways of doing th same thing:\n\n git -c pull.ff=only -c pull.rebase=true pull\n git -c pull.ff=only pull --rebase\n\nWhy would those do different things?\n\nThe documentation is pretty clear:\n\n See `pull.rebase`, `branch..rebase` and `branch.autoSetupRebase` in\n linkgit:git-config[1] if you want to make `git pull` always use\n `--rebase` instead of merging.\n\nIf you start adding exceptions on top of exceptions the documentation\nwill become a mess.\n\n-- \nFelipe Contreras\n"},{"id":"429810","messageId":"CAMMLpeRX3iMwT9NJ+ULHgAhS3A=nAybgDYFHomkY3sif-H+F4g@mail.gmail.com","threadId":"56092","inReplyTo":"CABPp-BERS0iiiVhSsSs6dkqzBVTQgwJUjjKaZQEzRDGRUdObcQ@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2021-07-12T18:20:01Z","receivedAt":"2021-07-12T18:20:21Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Mon, Jul 12, 2021 at 11:51 AM Elijah Newren wrote:\n>\n> On Mon, Jul 12, 2021 at 10:08 AM Junio C Hamano wrote:\n> >\n> > Phillip Wood writes:\n> >\n> > > Thanks for revising this patch, I like this approach much better. I do\n> > > however have some concerns about the interaction of pull.ff with the\n> > > rebase config and command line options. I'd naively expect the\n> > > following behavior (where rebase can fast-forward if possible)\n> > >\n> > > pull.ff pull.rebase commandline action\n> > > only not false rebase\n> > > only not false --no-rebase fast-forward only\n> > > * not false --ff-only fast-forward only\n> > > only not false --ff merge --ff\n> > > only not false --no-ff merge --no-ff\n> > > only false fast-forward only\n> > > only false --rebase rebase\n> > > only false --ff merge --ff\n> > > only false --no-ff merge --no-ff\n> >\n> > Do you mean by \"not false\" something other than \"true\"? Are you\n> > trying to capture what should happen when these configuration\n> > options are unspecified as well (and your \"not false\" is \"either set\n> > to true or unspecified\")? I ask because the first row does not make\n> > any sense to me. It seems to say\n> >\n> > \"If pull.ff is set to 'only', pull.rebase is not set to 'false',\n> > and the command line does not say anything, we will rebase\".\n>\n> I think Phillip is trying to answer what to do when pull.ff and\n> pull.rebase conflict. If I read his \"not false\" means \"is set to\n> something other than false\", then I agree with his table, but I think\n> he missed covering some cases.\n>\n> I think his table says that pull.rebase=false cannot conflict with\n> pull.ff settings, but any other value for pull.rebase can. That makes\n> sense to me.\n>\n> I'd similarly say that pull.ff=true cannot conflict with any\n> pull.rebase settings...but that both pull.ff=only AND pull.ff=false\n> conflict with pull.rebase={true,merges}.\n>\n> My opinion would be:\n> * conflicting command line flags results in the last one winning.\n> * --no-rebase makes pull.ff determine the action.\n> * --ff makes pull.rebase determine the action.\n> * any other command line flag (-r|--rebase|--no-ff|--ff-only)\n> overrides both pull.ff and pull.rebase\n> * If no command line option is given, and pull.ff and pull.rebase\n> conflict, then error out.\n>\n> I believe my recommendation above is consistent with every entry in\n> Phillip's table except the first line (where I suggest erroring out\n> instead).\n\nI'm not sure that --no-ff should imply --no-rebase because `git\nrebase` actually has a --no-ff option to rewrite commits even when\nfast-forwarding is possible. And it's not really necessary to make\n--ff-only imply --no-rebase because we're going to make `git pull`\nhandle --ff-only itself without invoking `git merge`. However, the\nrest of this proposal could be implemented in a straightforward manner\nby making --rebase on the command line imply --ff, and I think that\nwould be a fine solution.\n\n-Alex\n"},{"id":"429811","messageId":"CAMMLpeSBgURtX+MKbABKdmFuuoTA-Dw3h8uONwNeiP5aqcnfpA@mail.gmail.com","threadId":"56092","inReplyTo":"CAMMLpeRX3iMwT9NJ+ULHgAhS3A=nAybgDYFHomkY3sif-H+F4g@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2021-07-12T18:24:56Z","receivedAt":"2021-07-12T18:25:11Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Mon, Jul 12, 2021 at 12:20 PM Alex Henrie wrote:\n>\n> And it's not really necessary to make\n> --ff-only imply --no-rebase because we're going to make `git pull`\n> handle --ff-only itself without invoking `git merge`.\n\nSorry, I misspoke. I was thinking of the case where fast-forwarding is\nimpossible. If fast-forwarding is possible, --ff-only already\neffectively implies --no-rebase, and we might want to make that\nexplicit in the documentation.\n\n-Alex\n"},{"id":"429821","messageId":"xmqq8s2b489p.fsf@gitster.g","threadId":"56092","inReplyTo":"CAMMLpeSBgURtX+MKbABKdmFuuoTA-Dw3h8uONwNeiP5aqcnfpA@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-12T19:55:14Z","receivedAt":"2021-07-12T19:55:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Henrie writes:\n\n> Sorry, I misspoke. I was thinking of the case where fast-forwarding is\n> impossible.\n\nWhen we cannot fast-forward (i.e. we have our own development that\nis not in the tip of their history),\n\n --ff-only would cause the operation fail\n --ff would become no-op (as it merely allows fast-forwarding)\n --no-ff would become no-op (as it merely forbids fast-forwarding)\n\nand the latter two case, we'd either merge or rebase (with possibly\nspecified mode like --preserve-merges). I thought the current\ndocumentation is already fairly clear on this point?\n\n> If fast-forwarding is possible, --ff-only already effectively\n> implies --no-rebase, and we might want to make that explicit in\n> the documentation.\n\nWhen we fast-forward (i.e. their history is descendant from ours,\nand the user did not give --no-ff), it does not matter if it is done\nusing the merge backend, the rebase backend, or by the \"git pull\"\nwrapper. The end user does not care. The end result is that the tip\nof the branch now points at the tip of the history we pulled from\nthe other side and that is all what matters.\n\nSo, from that point of view, I do not think we want to say rebase or\nmerge or anything else for such a case in the documentation.\n\nTHanks.\n"},{"id":"429826","messageId":"60eca3e6a0337_a659b20858@natae.notmuch","threadId":"56092","inReplyTo":"xmqq8s2b489p.fsf@gitster.g","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-12T20:19:50Z","receivedAt":"2021-07-12T20:19:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Alex Henrie writes:\n> \n> > Sorry, I misspoke. I was thinking of the case where fast-forwarding is\n> > impossible.\n> \n> When we cannot fast-forward (i.e. we have our own development that\n> is not in the tip of their history),\n> \n> --ff-only would cause the operation fail\n> --ff would become no-op (as it merely allows fast-forwarding)\n> --no-ff would become no-op (as it merely forbids fast-forwarding)\n> \n> and the latter two case, we'd either merge or rebase (with possibly\n> specified mode like --preserve-merges). I thought the current\n> documentation is already fairly clear on this point?\n\nCorrect. But the documentation says absolutly nothing about\n`--ff-only --rebase`.\n\n> > If fast-forwarding is possible, --ff-only already effectively\n> > implies --no-rebase, and we might want to make that explicit in\n> > the documentation.\n> \n> When we fast-forward (i.e. their history is descendant from ours,\n> and the user did not give --no-ff), it does not matter if it is done\n> using the merge backend, the rebase backend, or by the \"git pull\"\n> wrapper. The end user does not care. The end result is that the tip\n> of the branch now points at the tip of the history we pulled from\n> the other side and that is all what matters.\n> \n> So, from that point of view, I do not think we want to say rebase or\n> merge or anything else for such a case in the documentation.\n\nCorrect again. But currently the only way a fast-forward is possible is\nby doing `git merge --ff-only`, which is a merge done in a fast-forward\nway.\n\nIf we add my proposed `git fast-forward` [1], then the documentation\ncould be updated to say that --ff-only does a `git fast-forward`, and\nthus there's no need for `git merge` (or `git rebase`).\n\n[1] https://lore.kernel.org/git/20210705123209.1808663-4-felipe.contreras@gmail.com/\n\n-- \nFelipe Contreras\n"},{"id":"429830","messageId":"CABPp-BGGi_b3PeFZ4-uErLS2vad-mX5gcuO+=nfgQreRMSCYZw@mail.gmail.com","threadId":"56092","inReplyTo":"CAMMLpeRX3iMwT9NJ+ULHgAhS3A=nAybgDYFHomkY3sif-H+F4g@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-07-12T20:37:45Z","receivedAt":"2021-07-12T20:37:57Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Jul 12, 2021 at 11:20 AM Alex Henrie wrote:\n>\n> On Mon, Jul 12, 2021 at 11:51 AM Elijah Newren wrote:\n> >\n> > On Mon, Jul 12, 2021 at 10:08 AM Junio C Hamano wrote:\n> > >\n> > > Phillip Wood writes:\n> > >\n> > > > Thanks for revising this patch, I like this approach much better. I do\n> > > > however have some concerns about the interaction of pull.ff with the\n> > > > rebase config and command line options. I'd naively expect the\n> > > > following behavior (where rebase can fast-forward if possible)\n> > > >\n> > > > pull.ff pull.rebase commandline action\n> > > > only not false rebase\n> > > > only not false --no-rebase fast-forward only\n> > > > * not false --ff-only fast-forward only\n> > > > only not false --ff merge --ff\n> > > > only not false --no-ff merge --no-ff\n> > > > only false fast-forward only\n> > > > only false --rebase rebase\n> > > > only false --ff merge --ff\n> > > > only false --no-ff merge --no-ff\n> > >\n> > > Do you mean by \"not false\" something other than \"true\"? Are you\n> > > trying to capture what should happen when these configuration\n> > > options are unspecified as well (and your \"not false\" is \"either set\n> > > to true or unspecified\")? I ask because the first row does not make\n> > > any sense to me. It seems to say\n> > >\n> > > \"If pull.ff is set to 'only', pull.rebase is not set to 'false',\n> > > and the command line does not say anything, we will rebase\".\n> >\n> > I think Phillip is trying to answer what to do when pull.ff and\n> > pull.rebase conflict. If I read his \"not false\" means \"is set to\n> > something other than false\", then I agree with his table, but I think\n> > he missed covering some cases.\n> >\n> > I think his table says that pull.rebase=false cannot conflict with\n> > pull.ff settings, but any other value for pull.rebase can. That makes\n> > sense to me.\n> >\n> > I'd similarly say that pull.ff=true cannot conflict with any\n> > pull.rebase settings...but that both pull.ff=only AND pull.ff=false\n> > conflict with pull.rebase={true,merges}.\n> >\n> > My opinion would be:\n> > * conflicting command line flags results in the last one winning.\n> > * --no-rebase makes pull.ff determine the action.\n> > * --ff makes pull.rebase determine the action.\n> > * any other command line flag (-r|--rebase|--no-ff|--ff-only)\n> > overrides both pull.ff and pull.rebase\n> > * If no command line option is given, and pull.ff and pull.rebase\n> > conflict, then error out.\n> >\n> > I believe my recommendation above is consistent with every entry in\n> > Phillip's table except the first line (where I suggest erroring out\n> > instead).\n>\n> I'm not sure that --no-ff should imply --no-rebase because `git\n> rebase` actually has a --no-ff option to rewrite commits even when\n> fast-forwarding is possible. And it's not really necessary to make\n> --ff-only imply --no-rebase because we're going to make `git pull`\n> handle --ff-only itself without invoking `git merge`. However, the\n> rest of this proposal could be implemented in a straightforward manner\n> by making --rebase on the command line imply --ff, and I think that\n> would be a fine solution.\n\ngit rebase has a --no-ff but it doesn't do anything like what git\npull's --no-ff has long been documented to do. git pull's --no-ff\nvery clearly states that a merge commit will be created, and thus is\ntied to merging instead of rebasing.\n\nAlso, I don't think pull needs to provide a union of all merge and\nrebase options; so there's no need to add a way to invoke a 'rebase\n--no-ff' from pull. (Folks can run individual fetch & merge or rebase\nsteps, after all.) I think we should concentrate on making sure that\nwe provide reasonable behavior when conflicting options/command-lines\nfrom the already provided set are specified.\n"},{"id":"429833","messageId":"CABPp-BH+LPbfdgixvSEGpLxwCHHSK99PFmE3q04jPZjLaqr5Xg@mail.gmail.com","threadId":"56092","inReplyTo":"xmqq8s2b489p.fsf@gitster.g","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-07-12T20:51:18Z","receivedAt":"2021-07-12T20:51:32Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Jul 12, 2021 at 12:55 PM Junio C Hamano wrote:\n>\n> Alex Henrie writes:\n>\n> > Sorry, I misspoke. I was thinking of the case where fast-forwarding is\n> > impossible.\n>\n> When we cannot fast-forward (i.e. we have our own development that\n> is not in the tip of their history),\n>\n> --ff-only would cause the operation fail\n> --ff would become no-op (as it merely allows fast-forwarding)\n> --no-ff would become no-op (as it merely forbids fast-forwarding)\n>\n> and the latter two case, we'd either merge or rebase (with possibly\n> specified mode like --preserve-merges). I thought the current\n> documentation is already fairly clear on this point?\n\ngit pull's --no-ff is documented to \"create a merge commit in all\ncases\", and thus as worded, seems incompatible with rebasing to me.\nTreating --no-ff as a no-op when we cannot fast-forward (i.e. allowing\nrebasing to happen) could be seen as a backwards incompatible change\nat this point.\n\nHaving --ff be compatible with rebasing works because the end result\nwill be the same as described in the existing documentation.\n\n> > If fast-forwarding is possible, --ff-only already effectively\n> > implies --no-rebase, and we might want to make that explicit in\n> > the documentation.\n>\n> When we fast-forward (i.e. their history is descendant from ours,\n> and the user did not give --no-ff), it does not matter if it is done\n> using the merge backend, the rebase backend, or by the \"git pull\"\n> wrapper. The end user does not care. The end result is that the tip\n> of the branch now points at the tip of the history we pulled from\n> the other side and that is all what matters.\n>\n> So, from that point of view, I do not think we want to say rebase or\n> merge or anything else for such a case in the documentation.\n\nAll three of --ff, --no-ff, and --ff-only come from\nDocumentation/merge-options.txt and are shared between git-merge and\ngit-pull. The description of each of those items mentions \"merges\" or\n\"merging\" at least once in every sentence.\n"},{"id":"429837","messageId":"60ecaedae42a8_a68ed20825@natae.notmuch","threadId":"56092","inReplyTo":"CABPp-BGGi_b3PeFZ4-uErLS2vad-mX5gcuO+=nfgQreRMSCYZw@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-12T21:06:34Z","receivedAt":"2021-07-12T21:06:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Elijah Newren wrote:\n\n> I think we should concentrate on making sure that we provide\n> reasonable behavior when conflicting options/command-lines from the\n> already provided set are specified.\n\nThe only way to do that in a way that a) does not significantly alter\nexisting semantics, b) can be clearly documented, and c) can be\nimplemented without a plethora of exceptions, is to create a new\nconfiguration orthogonal to all the existing ones:\n\n pull.mode=fast-forward\n\n-- \nFelipe Contreras\n"},{"id":"429850","messageId":"xmqqbl772l4m.fsf@gitster.g","threadId":"56092","inReplyTo":"CABPp-BH+LPbfdgixvSEGpLxwCHHSK99PFmE3q04jPZjLaqr5Xg@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-12T23:00:25Z","receivedAt":"2021-07-12T23:00:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> When we cannot fast-forward (i.e. we have our own development that\n>> is not in the tip of their history),\n>>\n>> --ff-only would cause the operation fail\n>> --ff would become no-op (as it merely allows fast-forwarding)\n>> --no-ff would become no-op (as it merely forbids fast-forwarding)\n>>\n>> and the latter two case, we'd either merge or rebase (with possibly\n>> specified mode like --preserve-merges). I thought the current\n>> documentation is already fairly clear on this point?\n>\n> git pull's --no-ff is documented to \"create a merge commit in all\n> cases\", and thus as worded, seems incompatible with rebasing to me.\n\nIt smells like a \"too literally to be useful\" interpretation of a\npice of documentation that has no relevance to \"pull --rebase\" to\nme, though. It comes from merge-options.txt and would not be\nrelevant to \"git pull --rebase\" to begin with.\n"},{"id":"429851","messageId":"60eccaa4c70d_a71772087f@natae.notmuch","threadId":"56092","inReplyTo":"xmqqbl772l4m.fsf@gitster.g","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-12T23:05:08Z","receivedAt":"2021-07-12T23:05:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Elijah Newren writes:\n> \n> >> When we cannot fast-forward (i.e. we have our own development that\n> >> is not in the tip of their history),\n> >>\n> >> --ff-only would cause the operation fail\n> >> --ff would become no-op (as it merely allows fast-forwarding)\n> >> --no-ff would become no-op (as it merely forbids fast-forwarding)\n> >>\n> >> and the latter two case, we'd either merge or rebase (with possibly\n> >> specified mode like --preserve-merges). I thought the current\n> >> documentation is already fairly clear on this point?\n> >\n> > git pull's --no-ff is documented to \"create a merge commit in all\n> > cases\", and thus as worded, seems incompatible with rebasing to me.\n> \n> It smells like a \"too literally to be useful\" interpretation of a\n> pice of documentation that has no relevance to \"pull --rebase\" to\n> me, though. It comes from merge-options.txt and would not be\n> relevant to \"git pull --rebase\" to begin with.\n\nBut Elijah's statement is correct.\n\n`git pull --no-ff --rebase` is undocumented. Period.\n\n-- \nFelipe Contreras\n"},{"id":"429852","messageId":"CABPp-BHV4bkyKswbg0gBH8uJG7knH4fBC=fitRJK4_LCP50Tog@mail.gmail.com","threadId":"56092","inReplyTo":"xmqqbl772l4m.fsf@gitster.g","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-07-12T23:24:12Z","receivedAt":"2021-07-12T23:24:27Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Jul 12, 2021 at 4:00 PM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> >> When we cannot fast-forward (i.e. we have our own development that\n> >> is not in the tip of their history),\n> >>\n> >> --ff-only would cause the operation fail\n> >> --ff would become no-op (as it merely allows fast-forwarding)\n> >> --no-ff would become no-op (as it merely forbids fast-forwarding)\n> >>\n> >> and the latter two case, we'd either merge or rebase (with possibly\n> >> specified mode like --preserve-merges). I thought the current\n> >> documentation is already fairly clear on this point?\n> >\n> > git pull's --no-ff is documented to \"create a merge commit in all\n> > cases\", and thus as worded, seems incompatible with rebasing to me.\n>\n> It smells like a \"too literally to be useful\" interpretation of a\n> pice of documentation that has no relevance to \"pull --rebase\" to\n> me, though. It comes from merge-options.txt and would not be\n> relevant to \"git pull --rebase\" to begin with.\n\n\"git pull --rebase\" isn't a very interesting case -- it's missing\n--no-ff, and has an explicit flag declaring user intent making both\npull.ff and pull.rebase irrelevant. It doesn't really help us handle\nthe tougher cases at all.\n\nLet me back up and see if I can explain a bit better.\n\n1. In general, we allow command line options to countermand either\nconfiguration options or other conflicting command line options, e.g.\n--no-gpg-sign can countermand --gpg-sign=$KEY or commit.gpgSign\nsetting.\n\n2. Starting with simple conflicts, git-pull has two sets of them of\nthe above form:\n * --rebase, --no-rebase, pull.rebase\n * --no-ff, --ff-only, --ff, pull.ff\n\n3. git-pull *also* has conflicting options across these types.\n * --rebase[={true,merges,preserve,interactive}] and --ff-only\n * possibly also --no-ff with any of those rebase flags\n * the underlying pull.rebase and pull.ff which are meant to mirror\nthese flags thus also can conflict\n\n4. The way we handle conflicting options in git is typically:\n * The last command line option countermands any earlier ones on the\ncommand line\n * Command line options countermand anything in config\n * We do not assume any ordering in config, so if two configuration\noptions conflict, it's either an error or we document which one\noverrides the other (e.g. diff.renames=false overrides\ndiff.renameLimit)\n\nSo my proposal is just to do 4 above, noting that:\n * pull.rebase set to anything other than \"false\" conflicts with\npull.ff set to anything other than \"true\" (and similarly for the\nequivalent command line options)\n * If both pull.rebase and pull.ff are set (in conflicting fashion)\nand no countermanding command line flags are set, I recommend throwing\nan error; it's not clear that having one override the other would\nmatch user intent.\n"},{"id":"430001","messageId":"CAL3xRKdOyVWvcLXK7zoXtFPiHBjgL24zi5hhg+3yjowwSUPgmg@mail.gmail.com","threadId":"56092","inReplyTo":"20210711012604.947321-1-alexhenrie24@gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Son Luong Ngoc","fromEmail":"sluongng@gmail.com","sentAt":"2021-07-14T08:37:28Z","receivedAt":"2021-07-14T08:37:43Z","isPatch":true,"sender":{"key":"sluongng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26684313?v=4"},"body":"Hi folks,\n\nI am out of the loop in this thread but I have been seeing strange behaviors\nwith pull.rebase=true in the 'next' branch and also in the 'master'\nbranch in recent days.\n\n > git version\n git version 2.32.0.432.gabb21c7263\n > git config -l | grep pull\n pull.rebase=true\n pull.ff=false\n\nBut a git pull would still run fast-forward.\nSome of our users (including myself) rely on disabling fast-forward to emit the\nper-file change log summary after each git-pull\n\n Updating 245f278cb729..5e8d960db7b3\n Fast-forward\n some/file/dir.ext | 44 ++++++++++++++++++++++++++++++++++++++++++++\n another/file/dir.ext | 6 +++---\n 2 files changed, 47 insertions(+), 3 deletions(-)\n\nIn a big, fast moving monorepo, this summary is a lot of noise and\nswitching to pull.rebase=true\nused to be the way to turn it off. If the change is intended for next\nversion release, is there a\nworkaround for this?\n\nCheers,\nSon Luong\n"},{"id":"430059","messageId":"60eeff69293fb_10e52087a@natae.notmuch","threadId":"56092","inReplyTo":"CAL3xRKdOyVWvcLXK7zoXtFPiHBjgL24zi5hhg+3yjowwSUPgmg@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-14T15:14:49Z","receivedAt":"2021-07-14T15:14:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hello,\n\nSon Luong Ngoc wrote:\n> I am out of the loop in this thread but I have been seeing strange behaviors\n> with pull.rebase=true in the 'next' branch and also in the 'master'\n> branch in recent days.\n> \n> > git version\n> git version 2.32.0.432.gabb21c7263\n> > git config -l | grep pull\n> pull.rebase=true\n> pull.ff=false\n> \n> But a git pull would still run fast-forward.\n> Some of our users (including myself) rely on disabling fast-forward to emit the\n> per-file change log summary after each git-pull\n> \n> Updating 245f278cb729..5e8d960db7b3\n> Fast-forward\n> some/file/dir.ext | 44 ++++++++++++++++++++++++++++++++++++++++++++\n> another/file/dir.ext | 6 +++---\n> 2 files changed, 47 insertions(+), 3 deletions(-)\n> \n> In a big, fast moving monorepo, this summary is a lot of noise and\n> switching to pull.rebase=true\n> used to be the way to turn it off.\n\nThis is probably due to 340062243a (pull: cleanup autostash check,\n2021-06-17).\n\nI bet you have `rebase.autostash=true` configured as well.\n\nIt seems to me you were relying on a bug.\n\n> If the change is intended for next\n> version release, is there a\n> workaround for this?\n\ngit pull -n ?\n\n-- \nFelipe Contreras\n"},{"id":"430060","messageId":"CABPp-BEqnTuDgC0Bb+feFj=vBB48wdb60thO+Pq3N0jL74jH8Q@mail.gmail.com","threadId":"56092","inReplyTo":"CAL3xRKdOyVWvcLXK7zoXtFPiHBjgL24zi5hhg+3yjowwSUPgmg@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-07-14T15:22:58Z","receivedAt":"2021-07-14T15:23:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Jul 14, 2021 at 1:37 AM Son Luong Ngoc wrote:\n>\n> Hi folks,\n>\n> I am out of the loop in this thread but I have been seeing strange behaviors\n> with pull.rebase=true in the 'next' branch and also in the 'master'\n> branch in recent days.\n\nI'm not surprised it happens with recent versions, but I'd expect this\nto have happened with older versions too. Is this not reproducible\nwith git-2.32.0 or older git versions?\n\n> > git version\n> git version 2.32.0.432.gabb21c7263\n> > git config -l | grep pull\n> pull.rebase=true\n> pull.ff=false\n\nSo, you have conflicting configuration options set. pull.ff=false\nmaps to --no-ff which is documented to create a merge.\npull.rebase=true maps to --rebase which says to run a rebase.\n\nYou probably want to drop one of these.\n\n> But a git pull would still run fast-forward.\n> Some of our users (including myself) rely on disabling fast-forward to emit the\n> per-file change log summary after each git-pull\n>\n> Updating 245f278cb729..5e8d960db7b3\n> Fast-forward\n> some/file/dir.ext | 44 ++++++++++++++++++++++++++++++++++++++++++++\n> another/file/dir.ext | 6 +++---\n> 2 files changed, 47 insertions(+), 3 deletions(-)\n>\n> In a big, fast moving monorepo, this summary is a lot of noise and\n> switching to pull.rebase=true\n> used to be the way to turn it off. If the change is intended for next\n> version release, is there a\n> workaround for this?\n\nThanks for the report. This particular commit has not yet been picked\nup, not even in seen. But it's a good example of how conflicting\nconfiguration really ought to result in an error rather than randomly\npicking one to trump, and suggests why we should complete the patch.\n\nHowever, since I'm commenting on this and the stat information appears\nto be important to you, note that there are also merge.stat and\nrebase.stat configuration variables for controlling whether those are\nshown at the end of merge and rebase operations.\n\nHope that helps,\nElijah\n"},{"id":"430081","messageId":"xmqq8s28yfsj.fsf@gitster.g","threadId":"56092","inReplyTo":"CABPp-BEqnTuDgC0Bb+feFj=vBB48wdb60thO+Pq3N0jL74jH8Q@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-14T17:19:08Z","receivedAt":"2021-07-14T17:19:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> However, since I'm commenting on this and the stat information appears\n> to be important to you, note that there are also merge.stat and\n> rebase.stat configuration variables for controlling whether those are\n> shown at the end of merge and rebase operations.\n\nTrue. It is troubling that people try to avoid fast-forward (which\nis a very cheap operation) and rebase their work on top of the other\nside (which could unnecessarily smudge their object store with\npointless replaying of the commits) only because they want to avoid\nthe informational messages *and* there is an easy way to squelch\nthese messages. There is some gap in documentation?\n\nThanks.\n"},{"id":"430090","messageId":"60ef1f6a83b61_9460a208cc@natae.notmuch","threadId":"56092","inReplyTo":"CABPp-BEqnTuDgC0Bb+feFj=vBB48wdb60thO+Pq3N0jL74jH8Q@mail.gmail.com","subject":"Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-14T17:31:22Z","receivedAt":"2021-07-14T17:31:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Elijah Newren wrote:\n> On Wed, Jul 14, 2021 at 1:37 AM Son Luong Ngoc wrote:\n> > I am out of the loop in this thread but I have been seeing strange behaviors\n> > with pull.rebase=true in the 'next' branch and also in the 'master'\n> > branch in recent days.\n> \n> I'm not surprised it happens with recent versions, but I'd expect this\n> to have happened with older versions too. Is this not reproducible\n> with git-2.32.0 or older git versions?\n\nI already provided an accurate target [1].\n\n> > > git version\n> > git version 2.32.0.432.gabb21c7263\n> > > git config -l | grep pull\n> > pull.rebase=true\n> > pull.ff=false\n> \n> So, you have conflicting configuration options set. pull.ff=false\n> maps to --no-ff which is documented to create a merge.\n> pull.rebase=true maps to --rebase which says to run a rebase.\n> \n> You probably want to drop one of these.\n\n`pull.ff` will be honored by `git pull --merge`.\n\n[1] https://lore.kernel.org/git/60eeff69293fb_10e52087a@natae.notmuch/\n\n-- \nFelipe Contreras\n"}]}