{"thread":{"id":"59355","subject":"[PATCH 0/2] advice: add diverging advice","startedAt":"2023-03-08T02:48:49Z","lastAt":"2023-03-09T23:45:00Z","messageCount":7,"participants":["Felipe Contreras","Taylor Blau","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"473165","messageId":"20230308024834.1562386-1-felipe.contreras@gmail.com","threadId":"59355","inReplyTo":null,"subject":"[PATCH 0/2] advice: add diverging advice","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-08T02:48:32Z","receivedAt":"2023-03-08T02:48:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Not all our users are experts in git who understand their configurations\nperfectly, some might be stuck in a simple error:\n\n Not possible to fast-forward, aborting.\n\nThat's not very friendly.\n\nLet's add a bit more advice in case the user doesn't know what's going\non.\n\nFelipe Contreras (2):\n advice: add diverging advice for novices\n advice: make diverging advice configurable\n\n Documentation/config/advice.txt | 2 ++\n advice.c | 9 +++++++++\n advice.h | 1 +\n 3 files changed, 12 insertions(+)\n\n-- \n2.39.2\n\n"},{"id":"473166","messageId":"20230308024834.1562386-2-felipe.contreras@gmail.com","threadId":"59355","inReplyTo":"20230308024834.1562386-1-felipe.contreras@gmail.com","subject":"[PATCH 1/2] advice: add diverging advice for novices","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-08T02:48:33Z","receivedAt":"2023-03-08T02:48:56Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The user might not necessarily know why ff only was configured, maybe an\nadmin did it, or the installer (Git for Windows), or perhaps they just\nfollowed some online advice.\n\nThis can happen not only on pull.ff=only, but merge.ff=only too.\n\nEven worse if the user has configured pull.rebase=false and\nmerge.ff=only, because in those cases a diverging merge will constantly\nkeep failing. There's no trivial way to get out of this other than\n`git merge --no-ff`.\n\nLet's not assume our users are experts in git who completely understand\nall their configurations.\n\nSigned-off-by: Felipe Contreras \n---\n advice.c | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/advice.c b/advice.c\nindex fd18968943..c3fb631f93 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -217,6 +217,13 @@ void NORETURN die_conclude_merge(void)\n \n void NORETURN die_ff_impossible(void)\n {\n+\tadvise(_(\"Diverging branches can't be fast-forwarded, you need to either:\\n\"\n+\t\t\"\\n\"\n+\t\t\"\\tgit merge --no-ff\\n\"\n+\t\t\"\\n\"\n+\t\t\"or:\\n\"\n+\t\t\"\\n\"\n+\t\t\"\\tgit rebase\\n\"));\n \tdie(_(\"Not possible to fast-forward, aborting.\"));\n }\n \n-- \n2.39.2\n\n"},{"id":"473167","messageId":"20230308024834.1562386-3-felipe.contreras@gmail.com","threadId":"59355","inReplyTo":"20230308024834.1562386-1-felipe.contreras@gmail.com","subject":"[PATCH 2/2] advice: make diverging advice configurable","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-08T02:48:34Z","receivedAt":"2023-03-08T02:48:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n Documentation/config/advice.txt | 2 ++\n advice.c | 4 +++-\n advice.h | 1 +\n 3 files changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex a00d0100a8..c96b5b2e5d 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -136,4 +136,6 @@ advice.*::\n \t\tAdvice shown when either linkgit:git-add[1] or linkgit:git-rm[1]\n \t\tis asked to update index entries outside the current sparse\n \t\tcheckout.\n+\tdiverging::\n+\t\tAdvice shown when a fast-forward is not possible.\n --\ndiff --git a/advice.c b/advice.c\nindex c3fb631f93..fbc59f9c6d 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -44,6 +44,7 @@ static struct {\n \t[ADVICE_COMMIT_BEFORE_MERGE]\t\t\t= { \"commitBeforeMerge\", 1 },\n \t[ADVICE_DETACHED_HEAD]\t\t\t\t= { \"detachedHead\", 1 },\n \t[ADVICE_SUGGEST_DETACHING_HEAD]\t\t\t= { \"suggestDetachingHead\", 1 },\n+\t[ADVICE_DIVERGING]\t\t\t\t= { \"diverging\", 1 },\n \t[ADVICE_FETCH_SHOW_FORCED_UPDATES]\t\t= { \"fetchShowForcedUpdates\", 1 },\n \t[ADVICE_GRAFT_FILE_DEPRECATED]\t\t\t= { \"graftFileDeprecated\", 1 },\n \t[ADVICE_IGNORED_HOOK]\t\t\t\t= { \"ignoredHook\", 1 },\n@@ -217,7 +218,8 @@ void NORETURN die_conclude_merge(void)\n \n void NORETURN die_ff_impossible(void)\n {\n-\tadvise(_(\"Diverging branches can't be fast-forwarded, you need to either:\\n\"\n+\tadvise_if_enabled(ADVICE_DIVERGING,\n+\t\t_(\"Diverging branches can't be fast-forwarded, you need to either:\\n\"\n \t\t\"\\n\"\n \t\t\"\\tgit merge --no-ff\\n\"\n \t\t\"\\n\"\ndiff --git a/advice.h b/advice.h\nindex 07e0f76833..41b5bc127c 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -21,6 +21,7 @@ struct string_list;\n \tADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME,\n \tADVICE_COMMIT_BEFORE_MERGE,\n \tADVICE_DETACHED_HEAD,\n+\tADVICE_DIVERGING,\n \tADVICE_SUGGEST_DETACHING_HEAD,\n \tADVICE_FETCH_SHOW_FORCED_UPDATES,\n \tADVICE_GRAFT_FILE_DEPRECATED,\n-- \n2.39.2\n\n"},{"id":"473199","messageId":"ZAix68A9e6RHz69y@nand.local","threadId":"59355","inReplyTo":"20230308024834.1562386-1-felipe.contreras@gmail.com","subject":"Re: [PATCH 0/2] advice: add diverging advice","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-03-08T16:03:55Z","receivedAt":"2023-03-08T16:05:49Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Felipe,\n\nOn Tue, Mar 07, 2023 at 08:48:32PM -0600, Felipe Contreras wrote:\n> Not all our users are experts in git who understand their configurations\n> perfectly, some might be stuck in a simple error:\n>\n> Not possible to fast-forward, aborting.\n>\n> That's not very friendly.\n>\n> Let's add a bit more advice in case the user doesn't know what's going\n> on.\n\nThanks for improving this case. I think the new advice() is reasonable\nand will be helpful for users who might otherwise get stuck here.\n\nI don't think that splitting it into two separate patches was strictly\nnecessary. If I were queuing, I'd probably squash the two together, but\nI leave that one to Junio to figure out ;-).\n\nThanks,\nTaylor\n"},{"id":"473203","messageId":"xmqqjzzrupqo.fsf@gitster.g","threadId":"59355","inReplyTo":"ZAix68A9e6RHz69y@nand.local","subject":"Re: [PATCH 0/2] advice: add diverging advice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-08T17:17:35Z","receivedAt":"2023-03-08T17:20:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> I don't think that splitting it into two separate patches was strictly\n> necessary. If I were queuing, I'd probably squash the two together, ...\n\nOne advantage of sending a topic like this as two patches is that,\nif the review discussion leads to a consensus that the new help\nmessage should be given unconditionally to everybody, only [1/2] can\nbe queued while dropping [2/2]. But the point of advise() is to\nserve as a training wheel that can be disabled by users who no\nlonger need it, so need for such a \"flexibility\" may not appear in\ntopics that add new advise() calls all that often, I would imagine.\n\nI am willing to do the squashing on the receiving end. The effect\nof two patches combined together is a good improvement, and the\nproposed log message for the first one covers why we want to do both\nof these two.\n\nThanks, both, for writing and reviewing.\n\n"},{"id":"473204","messageId":"xmqqcz5jupqh.fsf@gitster.g","threadId":"59355","inReplyTo":"20230308024834.1562386-2-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/2] advice: add diverging advice for novices","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-08T17:17:42Z","receivedAt":"2023-03-08T17:20:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> The user might not necessarily know why ff only was configured, maybe an\n> admin did it, or the installer (Git for Windows), or perhaps they just\n> followed some online advice.\n>\n> This can happen not only on pull.ff=only, but merge.ff=only too.\n>\n> Even worse if the user has configured pull.rebase=false and\n> merge.ff=only, because in those cases a diverging merge will constantly\n> keep failing. There's no trivial way to get out of this other than\n> `git merge --no-ff`.\n\nA good description. Without this explained, the instruction to run\n\"git merge\" with \"--no-ff\" in the text would have been puzzling to\nreaders. At least I was initially puzzled as I read the patch text\nbefore the proposed log message.\n\n> void NORETURN die_ff_impossible(void)\n> {\n> +\tadvise(_(\"Diverging branches can't be fast-forwarded, you need to either:\\n\"\n> +\t\t\"\\n\"\n> +\t\t\"\\tgit merge --no-ff\\n\"\n> +\t\t\"\\n\"\n> +\t\t\"or:\\n\"\n> +\t\t\"\\n\"\n> +\t\t\"\\tgit rebase\\n\"));\n> \tdie(_(\"Not possible to fast-forward, aborting.\"));\n> }\n"},{"id":"473330","messageId":"xmqqv8j97ami.fsf@gitster.g","threadId":"59355","inReplyTo":"ZAix68A9e6RHz69y@nand.local","subject":"Re: [PATCH 0/2] advice: add diverging advice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-09T23:44:53Z","receivedAt":"2023-03-09T23:45:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> On Tue, Mar 07, 2023 at 08:48:32PM -0600, Felipe Contreras wrote:\n>> Not all our users are experts in git who understand their configurations\n>> perfectly, some might be stuck in a simple error:\n>>\n>> Not possible to fast-forward, aborting.\n>>\n>> That's not very friendly.\n>>\n>> Let's add a bit more advice in case the user doesn't know what's going\n>> on.\n>\n> Thanks for improving this case. I think the new advice() is reasonable\n> and will be helpful for users who might otherwise get stuck here.\n\nWe may want to further tweak the advise() message depending on the\ncaller [*], but that can come on top after these two patches settle.\n\n[Footnote]\n\n* When a failed \"git pull\" gave the new advise() message, the user\n may not be advanced enough to adjust the advice to run \"git merge\n --no-ff\" to the given situation, for example.\n\n\n"}]}