{"thread":{"id":"57926","subject":"[PATCH 0/3] Die preserve ggg","startedAt":"2022-05-26T09:21:12Z","lastAt":"2022-06-11T19:22:43Z","messageCount":35,"participants":["Philip Oakley via GitGitGadget","Ævar Arnfjörð Bjarmason","Philip Oakley","René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"456156","messageId":"pull.1242.git.1653556865.gitgitgadget@gmail.com","threadId":"57926","inReplyTo":null,"subject":"[PATCH 0/3] Die preserve ggg","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-26T09:21:02Z","receivedAt":"2022-05-26T09:21:12Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"This short series is a follow up to GitGitGadget \"Update the die()\npreserve-merges messages to help some users (PR #1155)\" [1].\n\nThe first patch is a tidy up of the --preserve option to highlight that it\nis now Deleted, rather than Deprecated.\n\nIn response to Avar's comments that the former error message merely\n'tantilised without telling' the user what to do, it became obvious that the\nunderling problem was that the user was unable to git rebase --abort which\nwas also fatal, when a preserve-rebase was in progress.\n\nThus the main update is to allow the rebase --abort command, even when a\n--preserve is in progress, to proceed. The --abort code was unchanged by the\nremoval of the preserve option, as the resetting and clean up of internal\nstate is common to the other rebase options.\n\nThe user facing fatal message now simply advises to abort, or downgrade to a\nversion that has preserve-merges to complete the rebase.\n\nThe final patch highlights that some IDEs still allow the setting of the\npreserve-merges option as a pull config setup.\n\nPhilip Oakly\n\n[1] GitLore ref pull.1155.git.1645526016.gitgitgadget@gmail.com\nhttps://lore.kernel.org/git/pull.1155.git.1645526016.gitgitgadget@gmail.com/\n\nPhilip Oakley (3):\n rebase.c: state preserve-merges has been removed\n rebase: help users when dying with `preserve-merges`\n rebase: note `preserve` merges may be a pull config option\n\n builtin/rebase.c | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\n\nbase-commit: c4f0e309ae745751d08727f24e8ff55e56355755\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1242%2FPhilipOakley%2Fdie_preserve_ggg-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1242/PhilipOakley/die_preserve_ggg-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1242\n-- \ngitgitgadget\n"},{"id":"456157","messageId":"0a4c81d8cafdc048fa89c24fcfa4e2715a17d176.1653556865.git.gitgitgadget@gmail.com","threadId":"57926","inReplyTo":"pull.1242.git.1653556865.gitgitgadget@gmail.com","subject":"[PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-26T09:21:03Z","receivedAt":"2022-05-26T09:21:15Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: Philip Oakley \n\nSince feebd2d256 (rebase: hide --preserve-merges option, 2019-10-18)\nthis option is now removed as stated in the subsequent release notes.\n\nFix the option tip.\n\nSigned-off-by: Philip Oakley \n---\n builtin/rebase.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 7ab50cda2ad..6ce7e98a6f1 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1110,7 +1110,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG,\n \t\t\tparse_opt_interactive),\n \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n-\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n+\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n \t\t\t\t \"ignoring them\"),\n \t\t\t 1, PARSE_OPT_HIDDEN),\n \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n-- \ngitgitgadget\n\n"},{"id":"456158","messageId":"d0fb54105940f19809eeb5d5e156bf3889d16b0c.1653556865.git.gitgitgadget@gmail.com","threadId":"57926","inReplyTo":"pull.1242.git.1653556865.gitgitgadget@gmail.com","subject":"[PATCH 2/3] rebase: help users when dying with `preserve-merges`","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-26T09:21:04Z","receivedAt":"2022-05-26T09:21:33Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: Philip Oakley \n\nGit will die if a \"rebase --preserve-merges\" is in progress.\nUsers cannot --quit, --abort or --continue the rebase.\n\nMake the `rebase --abort` option available to allow users to remove\ntraces of any preserve-merges rebase, even if they had upgraded\nduring a rebase.\n\nOne trigger was an unexpectedly difficult to resolve conflict, as\nreported on the `git-users` group.\n(https://groups.google.com/g/git-for-windows/c/3jMWbBlXXHM)\n\nTell the user the options to resolve the problem manually.\n\nSigned-off-by: Philip Oakley \n---\n builtin/rebase.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 6ce7e98a6f1..aada25a8870 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1182,8 +1182,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t} else if (is_directory(merge_dir())) {\n \t\tstrbuf_reset(&buf);\n \t\tstrbuf_addf(&buf, \"%s/rewritten\", merge_dir());\n-\t\tif (is_directory(buf.buf)) {\n-\t\t\tdie(\"`rebase -p` is no longer supported\");\n+\t\tif (is_directory(buf.buf) && !(action == ACTION_ABORT)) {\n+\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n+\t\t\t\"Use `git rebase --abort` to terminate current rebase.\\n\"\n+\t\t\t\"Or downgrade to v2.33, or earlier, to complete the rebase.\\n\");\n \t\t} else {\n \t\t\tstrbuf_reset(&buf);\n \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n-- \ngitgitgadget\n\n"},{"id":"456159","messageId":"ece3eecdc4de44cdec1b6efa9079930721db85ad.1653556865.git.gitgitgadget@gmail.com","threadId":"57926","inReplyTo":"pull.1242.git.1653556865.gitgitgadget@gmail.com","subject":"[PATCH 3/3] rebase: note `preserve` merges may be a pull config option","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-26T09:21:05Z","receivedAt":"2022-05-26T09:21:38Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: Philip Oakley \n\nThe `--preserve-merges` option was removed by v2.35.0. However\nusers may not be aware that it is also a Pull option, and it is\nstill offered by major IDE vendors such as Visual Studio.\n\nExtend the `--preserve-merges` die message to also direct users to\nthe use of the `preserve` option in the `pull` config.\n\nSigned-off-by: Philip Oakley \n---\n builtin/rebase.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex aada25a8870..6fc0aaebbb8 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1205,7 +1205,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\t builtin_rebase_usage, 0);\n \n \tif (preserve_merges_selected)\n-\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\"));\n+\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\\n\"\n+\t\t\t\"Your `pull` configuration, may also invoke this option.\"));\n \n \tif (action != ACTION_NONE && total_argc != 2) {\n \t\tusage_with_options(builtin_rebase_usage,\n-- \ngitgitgadget\n"},{"id":"456160","messageId":"220526.86bkvk7hoo.gmgdl@evledraar.gmail.com","threadId":"57926","inReplyTo":"0a4c81d8cafdc048fa89c24fcfa4e2715a17d176.1653556865.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-26T09:40:42Z","receivedAt":"2022-05-26T09:43:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n\n> From: Philip Oakley \n>\n> Since feebd2d256 (rebase: hide --preserve-merges option, 2019-10-18)\n> this option is now removed as stated in the subsequent release notes.\n>\n> Fix the option tip.\n>\n> Signed-off-by: Philip Oakley \n> ---\n> builtin/rebase.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 7ab50cda2ad..6ce7e98a6f1 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1110,7 +1110,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG,\n> \t\t\tparse_opt_interactive),\n> \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n> -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n> +\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n> \t\t\t\t \"ignoring them\"),\n> \t\t\t 1, PARSE_OPT_HIDDEN),\n> \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n\nI have some local patches for this more generally, but for\nPARSE_OPT_HIDDEN options we never do anything with the \"argh\" field,\ni.e. it's only used for showing the \"git -h\" output, and if it's\nhidden it won't be there.\n\nSo there's no point in changing this string, nor to have translators\nfocus on it, it'll never be used.\n\nThis series shouldn't fix the general issue (which parse-options.c\nshould really be BUG()-ing about, after fixing the existing\noccurances. But For this one we could just set this to have a string of\n\"\" or something, only the string you're changing in 3/3 will be seen by\nanyone.\n"},{"id":"456161","messageId":"220526.867d687hd5.gmgdl@evledraar.gmail.com","threadId":"57926","inReplyTo":"d0fb54105940f19809eeb5d5e156bf3889d16b0c.1653556865.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] rebase: help users when dying with `preserve-merges`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-26T09:43:54Z","receivedAt":"2022-05-26T09:50:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n\n> From: Philip Oakley \n>\n> Git will die if a \"rebase --preserve-merges\" is in progress.\n> Users cannot --quit, --abort or --continue the rebase.\n>\n> Make the `rebase --abort` option available to allow users to remove\n> traces of any preserve-merges rebase, even if they had upgraded\n> during a rebase.\n>\n> One trigger was an unexpectedly difficult to resolve conflict, as\n> reported on the `git-users` group.\n> (https://groups.google.com/g/git-for-windows/c/3jMWbBlXXHM)\n>\n> Tell the user the options to resolve the problem manually.\n>\n> Signed-off-by: Philip Oakley \n> ---\n> builtin/rebase.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 6ce7e98a6f1..aada25a8870 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1182,8 +1182,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \t} else if (is_directory(merge_dir())) {\n> \t\tstrbuf_reset(&buf);\n> \t\tstrbuf_addf(&buf, \"%s/rewritten\", merge_dir());\n> -\t\tif (is_directory(buf.buf)) {\n> -\t\t\tdie(\"`rebase -p` is no longer supported\");\n> +\t\tif (is_directory(buf.buf) && !(action == ACTION_ABORT)) {\n> +\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n> +\t\t\t\"Use `git rebase --abort` to terminate current rebase.\\n\"\n> +\t\t\t\"Or downgrade to v2.33, or earlier, to complete the rebase.\\n\");\n> \t\t} else {\n> \t\t\tstrbuf_reset(&buf);\n> \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n\nExisting issue: No _(), shouldn't we add it?\n\nI wonder if we should use die_message() + advise() in these cases,\ni.e. stick to why we died in die_message() and have the advise() make\nsuggestions, as e4921d877ab (tracking branches: add advice to ambiguous\nrefspec error, 2022-04-01) does.\n\nBut then again adding new advice is currently a bit of an excercise in\nboilerplate, and this seems fine for a transitory option.\n\nI think you don't need to add a trailing \\n though...\n\n"},{"id":"456162","messageId":"220526.8635gw7has.gmgdl@evledraar.gmail.com","threadId":"57926","inReplyTo":"ece3eecdc4de44cdec1b6efa9079930721db85ad.1653556865.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] rebase: note `preserve` merges may be a pull config option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-26T09:50:40Z","receivedAt":"2022-05-26T09:51:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n\n> From: Philip Oakley \n>\n> The `--preserve-merges` option was removed by v2.35.0. However\n> users may not be aware that it is also a Pull option, and it is\n> still offered by major IDE vendors such as Visual Studio.\n>\n> Extend the `--preserve-merges` die message to also direct users to\n> the use of the `preserve` option in the `pull` config.\n>\n> Signed-off-by: Philip Oakley \n> ---\n> builtin/rebase.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index aada25a8870..6fc0aaebbb8 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1205,7 +1205,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \t\t\t builtin_rebase_usage, 0);\n> \n> \tif (preserve_merges_selected)\n> -\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\"));\n> +\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\\n\"\n> +\t\t\t\"Your `pull` configuration, may also invoke this option.\"));\n> \n> \tif (action != ACTION_NONE && total_argc != 2) {\n> \t\tusage_with_options(builtin_rebase_usage,\n\nDitto 2/3 about maybe die_message() + advise(). In this case that has\nthe slight advantace of allowing us to keep the existing translated\nstring as-is.\n\nBut also, is *our* pull configuration causing us to end up here? I\nvaguely recall that being discussed (probably in answer to a question of\nmine) in the earlier round, or is this the IDE picking it up & invoking\nus like this?\n\n"},{"id":"456163","messageId":"220526.86y1yo62jl.gmgdl@evledraar.gmail.com","threadId":"57926","inReplyTo":"pull.1242.git.1653556865.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Die preserve ggg","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-26T09:54:59Z","receivedAt":"2022-05-26T09:55:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n\n> This short series is a follow up to GitGitGadget \"Update the die()\n> preserve-merges messages to help some users (PR #1155)\" [1].\n>\n> The first patch is a tidy up of the --preserve option to highlight that it\n> is now Deleted, rather than Deprecated.\n>\n> In response to Avar's comments that the former error message merely\n> 'tantilised without telling' the user what to do, it became obvious that the\n> underling problem was that the user was unable to git rebase --abort which\n> was also fatal, when a preserve-rebase was in progress.\n\nThanks a lot for following up on this, this all looks OK to me. I had\nsome minor comments about maybe tweaking this & that, but as far as I'm\nconcerned this could go in as-is, depending on whether you think it\nneeds a re-roll in response to my comments + others.\n"},{"id":"456168","messageId":"7367a18a-2e41-22e2-d8c5-2ccff71c58a0@iee.email","threadId":"57926","inReplyTo":"220526.86bkvk7hoo.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-26T11:40:16Z","receivedAt":"2022-05-26T11:40:23Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 26/05/2022 10:40, Ævar Arnfjörð Bjarmason wrote:\n> On Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n>\n>> From: Philip Oakley \n>>\n>> Since feebd2d256 (rebase: hide --preserve-merges option, 2019-10-18)\n>> this option is now removed as stated in the subsequent release notes.\n>>\n>> Fix the option tip.\n>>\n>> Signed-off-by: Philip Oakley \n>> ---\n>> builtin/rebase.c | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>> index 7ab50cda2ad..6ce7e98a6f1 100644\n>> --- a/builtin/rebase.c\n>> +++ b/builtin/rebase.c\n>> @@ -1110,7 +1110,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>> \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG,\n>> \t\t\tparse_opt_interactive),\n>> \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n>> -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n>> +\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n>> \t\t\t\t \"ignoring them\"),\n>> \t\t\t 1, PARSE_OPT_HIDDEN),\n>> \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n> I have some local patches for this more generally, but for\n> PARSE_OPT_HIDDEN options we never do anything with the \"argh\" field,\n> i.e. it's only used for showing the \"git -h\" output, and if it's\n> hidden it won't be there.\n>\n> So there's no point in changing this string, nor to have translators\n> focus on it, it'll never be used.\n\nI still think it's useful for those that read the code, as it reminds\nfolks about what it is/was, and why it's hidden, hence the usefulness of\nthe change, from my perspective. I'm flexible either way, but didn't\nfeel the 'DEPRECATED' was correct information.\n>\n> This series shouldn't fix the general issue (which parse-options.c\n> should really be BUG()-ing about, after fixing the existing\n> occurances. But For this one we could just set this to have a string of\n> \"\" or something, only the string you're changing in 3/3 will be seen by\n> anyone.\n\n"},{"id":"456169","messageId":"00229772-f075-0b0c-7810-7debf6b971bc@iee.email","threadId":"57926","inReplyTo":"220526.867d687hd5.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/3] rebase: help users when dying with `preserve-merges`","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-26T11:44:49Z","receivedAt":"2022-05-26T11:46:08Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 26/05/2022 10:43, Ævar Arnfjörð Bjarmason wrote:\n> On Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n>\n>> From: Philip Oakley \n>>\n>> Git will die if a \"rebase --preserve-merges\" is in progress.\n>> Users cannot --quit, --abort or --continue the rebase.\n>>\n>> Make the `rebase --abort` option available to allow users to remove\n>> traces of any preserve-merges rebase, even if they had upgraded\n>> during a rebase.\n>>\n>> One trigger was an unexpectedly difficult to resolve conflict, as\n>> reported on the `git-users` group.\n>> (https://groups.google.com/g/git-for-windows/c/3jMWbBlXXHM)\n>>\n>> Tell the user the options to resolve the problem manually.\n>>\n>> Signed-off-by: Philip Oakley \n>> ---\n>> builtin/rebase.c | 6 ++++--\n>> 1 file changed, 4 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>> index 6ce7e98a6f1..aada25a8870 100644\n>> --- a/builtin/rebase.c\n>> +++ b/builtin/rebase.c\n>> @@ -1182,8 +1182,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>> \t} else if (is_directory(merge_dir())) {\n>> \t\tstrbuf_reset(&buf);\n>> \t\tstrbuf_addf(&buf, \"%s/rewritten\", merge_dir());\n>> -\t\tif (is_directory(buf.buf)) {\n>> -\t\t\tdie(\"`rebase -p` is no longer supported\");\n>> +\t\tif (is_directory(buf.buf) && !(action == ACTION_ABORT)) {\n>> +\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n>> +\t\t\t\"Use `git rebase --abort` to terminate current rebase.\\n\"\n>> +\t\t\t\"Or downgrade to v2.33, or earlier, to complete the rebase.\\n\");\n>> \t\t} else {\n>> \t\t\tstrbuf_reset(&buf);\n>> \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n> Existing issue: No _(), shouldn't we add it?\nThis `strbuf_addf` is forming a path for internal use. It just happens\nto look like legible English ;-)\n>\n> I wonder if we should use die_message() + advise() in these cases,\n> i.e. stick to why we died in die_message() and have the advise() make\n> suggestions, as e4921d877ab (tracking branches: add advice to ambiguous\n> refspec error, 2022-04-01) does.\n\nAh, maybe it's my message.. that needs translating.\n>\n> But then again adding new advice is currently a bit of an excercise in\n> boilerplate, and this seems fine for a transitory option.\nI can go with that ;-)\n>\n> I think you don't need to add a trailing \\n though...\nOops, just a little extra line spacing for emphasis maybe ?\n\n"},{"id":"456170","messageId":"1e7dd39e-0491-e561-be1f-7666fcf62bc6@iee.email","threadId":"57926","inReplyTo":"220526.8635gw7has.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 3/3] rebase: note `preserve` merges may be a pull config option","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-26T12:01:04Z","receivedAt":"2022-05-26T12:01:10Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"hi,\nOn 26/05/2022 10:50, Ævar Arnfjörð Bjarmason wrote:\n> On Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n>\n>> From: Philip Oakley \n>>\n>> The `--preserve-merges` option was removed by v2.35.0. However\n>> users may not be aware that it is also a Pull option, and it is\n>> still offered by major IDE vendors such as Visual Studio.\n>>\n>> Extend the `--preserve-merges` die message to also direct users to\n>> the use of the `preserve` option in the `pull` config.\n>>\n>> Signed-off-by: Philip Oakley \n>> ---\n>> builtin/rebase.c | 3 ++-\n>> 1 file changed, 2 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>> index aada25a8870..6fc0aaebbb8 100644\n>> --- a/builtin/rebase.c\n>> +++ b/builtin/rebase.c\n>> @@ -1205,7 +1205,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>> \t\t\t builtin_rebase_usage, 0);\n>> \n>> \tif (preserve_merges_selected)\n>> -\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\"));\n>> +\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\\n\"\n>> +\t\t\t\"Your `pull` configuration, may also invoke this option.\"));\n>> \n>> \tif (action != ACTION_NONE && total_argc != 2) {\n>> \t\tusage_with_options(builtin_rebase_usage,\n> Ditto 2/3 about maybe die_message() + advise(). \nI'm not that enamoured about hiding die message details behind an advice\noption. In this case it not meant to be a regular reminder type thing,\nrather a one-off fix-it-forever sort of `advice'. At least that my\nreasoning.\n\n> In this case that has\n> the slight advantace of allowing us to keep the existing translated\n> string as-is.\n>\n> But also, is *our* pull configuration causing us to end up here?\nYes, but. The extra message is about fixing all places that the user may\nhave setup a config for using preserve-merges, not just here. The fact\nthat IDEs offer a menu for adding that setting makes it easy for users\nto get into this.\nI'd agree that pull already has detection for this, but I was looking to\navoid the 'fool me once, fool me twice' scenarios.\n\nIt could be dropped if thought over zealous.\n> I\n> vaguely recall that being discussed (probably in answer to a question of\n> mine) in the earlier round, or is this the IDE picking it up & invoking\n> us like this?\n>\n\n"},{"id":"456172","messageId":"b794f028-97be-beb8-b815-4d9b8aa8b643@iee.email","threadId":"57926","inReplyTo":"220526.86y1yo62jl.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/3] Die preserve ggg","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-26T12:57:39Z","receivedAt":"2022-05-26T12:57:44Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 26/05/2022 10:54, Ævar Arnfjörð Bjarmason wrote:\n> On Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n>\n>> This short series is a follow up to GitGitGadget \"Update the die()\n>> preserve-merges messages to help some users (PR #1155)\" [1].\n>>\n>> The first patch is a tidy up of the --preserve option to highlight that it\n>> is now Deleted, rather than Deprecated.\n>>\n>> In response to Avar's comments that the former error message merely\n>> 'tantilised without telling' the user what to do, it became obvious that the\n>> underling problem was that the user was unable to git rebase --abort which\n>> was also fatal, when a preserve-rebase was in progress.\n> Thanks a lot for following up on this, this all looks OK to me. I had\n> some minor comments about maybe tweaking this & that, but as far as I'm\n> concerned this could go in as-is, depending on whether you think it\n> needs a re-roll in response to my comments + others.\nThanks, I am a bit busy with family issues, so I'd rather use as-is,\nunless other feel that the tweaks will be worth it.\nI'm effectively off-line for the next 5-6 days anyway.\n--\nPhilip\n"},{"id":"456173","messageId":"19baf95d-67d4-d7ed-72a6-96d098171d3a@web.de","threadId":"57926","inReplyTo":"220526.86bkvk7hoo.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2022-05-26T13:02:29Z","receivedAt":"2022-05-26T13:02:38Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 26.05.22 um 11:40 schrieb Ævar Arnfjörð Bjarmason:\n>\n> On Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n>\n>> From: Philip Oakley \n>>\n>> Since feebd2d256 (rebase: hide --preserve-merges option, 2019-10-18)\n>> this option is now removed as stated in the subsequent release notes.\n>>\n>> Fix the option tip.\n>>\n>> Signed-off-by: Philip Oakley \n>> ---\n>> builtin/rebase.c | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>> index 7ab50cda2ad..6ce7e98a6f1 100644\n>> --- a/builtin/rebase.c\n>> +++ b/builtin/rebase.c\n>> @@ -1110,7 +1110,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>> \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG,\n>> \t\t\tparse_opt_interactive),\n>> \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n>> -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n>> +\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n>> \t\t\t\t \"ignoring them\"),\n>> \t\t\t 1, PARSE_OPT_HIDDEN),\n>> \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n>\n> I have some local patches for this more generally, but for\n> PARSE_OPT_HIDDEN options we never do anything with the \"argh\" field,\n> i.e. it's only used for showing the \"git -h\" output, and if it's\n> hidden it won't be there.\n\nHidden options are shown if you use --help-all instead of -h.\n\nOPT_SET_INT_F always sets the struct option member \"argh\" to NULL. The\nstring changed above is the \"help\" member, not \"argh\".\n\n> So there's no point in changing this string, nor to have translators\n> focus on it, it'll never be used.\n>\n> This series shouldn't fix the general issue (which parse-options.c\n> should really be BUG()-ing about, after fixing the existing\n> occurances. But For this one we could just set this to have a string of\n> \"\" or something, only the string you're changing in 3/3 will be seen by\n> anyone.\n\nWhat is the general issue?\n\nAnyway, the new help text explaining what the option once did is a bit\nconfusing. It would be better to focus on what it's doing now (nothing)\nand/or why we still have it (for backward compatibility), I think.\n\nRené\n"},{"id":"456229","messageId":"xmqq5ylsxccw.fsf@gitster.g","threadId":"57926","inReplyTo":"19baf95d-67d4-d7ed-72a6-96d098171d3a@web.de","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-26T20:33:51Z","receivedAt":"2022-05-26T20:33:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n>>> \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n>>> -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n>>> +\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n>>> \t\t\t\t \"ignoring them\"),\n>>> \t\t\t 1, PARSE_OPT_HIDDEN),\n>>> \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n>\n> Hidden options are shown if you use --help-all instead of -h.\n>\n> OPT_SET_INT_F always sets the struct option member \"argh\" to NULL. The\n> string changed above is the \"help\" member, not \"argh\".\n\nGood points. I do think it is OK to say REMOVED in case --help-all\nasks us to show everything, even though I wonder if we can leave it\nthere until we remove the \"support\" of noticing the user asking for\na now-removed feature.\n\n>> So there's no point in changing this string, nor to have translators\n>> focus on it, it'll never be used.\n>>\n>> This series shouldn't fix the general issue (which parse-options.c\n>> should really be BUG()-ing about, after fixing the existing\n>> occurances. But For this one we could just set this to have a string of\n>> \"\" or something, only the string you're changing in 3/3 will be seen by\n>> anyone.\n>\n> What is the general issue?\n\nI am afraid to ask, after having learned to be worried about those\nlarge rearchitecting projects Ævar talks about X-<.\n\n> Anyway, the new help text explaining what the option once did is a bit\n> confusing. It would be better to focus on what it's doing now (nothing)\n> and/or why we still have it (for backward compatibility), I think.\n\nDo you mean that we should say \"this option used to do such and such\nbut it is now a no-op\" after \"(REMOVED)\" label, instead of the above\n\"this option does such and such\"? I think \"(REMOVED)\" is a strong\nenough hint that lets us get away without saying \"used to\" and \"but\nit is now a no-op\", so I can accept both.\n\nOr do you mean we should say \"(REMOVED) for backward compatibility,\ndoes nothing but errors out\"? I would be less in faviour, then.\nThose who are curious enough to ask --help-all would find it more\nhelpful if we said what it used to do. Otherwise they wouldn't be\nasking --help-all in the first place, no?\n\n\n"},{"id":"456230","messageId":"xmqq1qwgxbys.fsf@gitster.g","threadId":"57926","inReplyTo":"00229772-f075-0b0c-7810-7debf6b971bc@iee.email","subject":"Re: [PATCH 2/3] rebase: help users when dying with `preserve-merges`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-26T20:42:19Z","receivedAt":"2022-05-26T20:42:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n>>> Make the `rebase --abort` option available to allow users to remove\n>>> traces of any preserve-merges rebase, even if they had upgraded\n>>> during a rebase.\n\nThis patch does not make it \"available\", though. \n\n\tSuggest using `--abort` to get out of the situation after a\n\tfailed preserve-rebase and remove traces of ...\n\nperhaps?\n\nI do think the suggestion is worth doing if a user ever gets into\nthe situation, but how likely does it happen? A user has to start\n\"rebase -p\" with older Git, wait until Git gets updated to a future\nversion of Git that includes this change, and then say \"rebase -p\n--continue\"?\n\n>>> \t} else if (is_directory(merge_dir())) {\n>>> \t\tstrbuf_reset(&buf);\n>>> \t\tstrbuf_addf(&buf, \"%s/rewritten\", merge_dir());\n>>> -\t\tif (is_directory(buf.buf)) {\n>>> -\t\t\tdie(\"`rebase -p` is no longer supported\");\n>>> +\t\tif (is_directory(buf.buf) && !(action == ACTION_ABORT)) {\n>>> +\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n>>> +\t\t\t\"Use `git rebase --abort` to terminate current rebase.\\n\"\n>>> +\t\t\t\"Or downgrade to v2.33, or earlier, to complete the rebase.\\n\");\n>>> \t\t} else {\n>>> \t\t\tstrbuf_reset(&buf);\n>>> \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n>> Existing issue: No _(), shouldn't we add it?\n> This `strbuf_addf` is forming a path for internal use. It just happens\n> to look like legible English ;-)\n\nI do not think Ævar meant \"%s/interactive\"; the enhanced message\nabove that you inherited from the original \"no longer supported\"\nthat was not marked for translation.\n\n>> I wonder if we should use die_message() + advise() in these cases,\n>> i.e. stick to why we died in die_message() and have the advise() make\n>> suggestions, as e4921d877ab (tracking branches: add advice to ambiguous\n>> refspec error, 2022-04-01) does.\n>\n> Ah, maybe it's my message.. that needs translating.\n\nYup.\n\nThis whole '-p' business will go away in a few releases down, so a\nlonger message give to the existing die() should be sufficient and\nthere is no need for the choice between \"yes, I am still weaning\nmyself off of rebase -p and want to keep seeing the advice\" and\n\"thanks, I saw the message often enough, you no longer need to tell\nme how to get out\", I would think.\n\nThanks.\n"},{"id":"456231","messageId":"xmqqv8tsvws7.fsf@gitster.g","threadId":"57926","inReplyTo":"ece3eecdc4de44cdec1b6efa9079930721db85ad.1653556865.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] rebase: note `preserve` merges may be a pull config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-26T20:55:36Z","receivedAt":"2022-05-26T20:55:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley via GitGitGadget\" writes:\n\n> From: Philip Oakley \n>\n> The `--preserve-merges` option was removed by v2.35.0. However\n\nAre you sure about that?\n\n52f1e821 (pull: remove support for `--rebase=preserve`, 2021-09-07)\nthat is in v2.34.0 and above dropped pull.rebase=preserve from the\nDocumentation/config/pull.txt (and others). My local collection\nof various Git versions agrees with me. \"git help config\" from\n2.34.0 does not list preserve as a valid choice, but 2.33.0 does.\n\n> users may not be aware that it is also a Pull option, and it is\n> still offered by major IDE vendors such as Visual Studio.\n>\n> Extend the `--preserve-merges` die message to also direct users to\n> the use of the `preserve` option in the `pull` config.\n>\n> Signed-off-by: Philip Oakley \n> ---\n> builtin/rebase.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index aada25a8870..6fc0aaebbb8 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1205,7 +1205,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \t\t\t builtin_rebase_usage, 0);\n> \n> \tif (preserve_merges_selected)\n> -\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\"));\n> +\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\\n\"\n> +\t\t\t\"Your `pull` configuration, may also invoke this option.\"));\n\nWhat is a `pull` configuration? Our configuration variable names\nall have at least one dot in it. I think it is better to be\nexplicit to clarify what exactly we are suggesting to fix.\n\n\"Your `pull.rebase` configuration may be set to 'preserve', which is\nno longer supported; use 'merges' instead\", or somesuch?\n\nThanks.\n"},{"id":"456232","messageId":"32e5088b-35a1-4e8c-098e-18c465a0a0bb@web.de","threadId":"57926","inReplyTo":"xmqq5ylsxccw.fsf@gitster.g","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2022-05-26T21:27:01Z","receivedAt":"2022-05-26T21:27:28Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 26.05.22 um 22:33 schrieb Junio C Hamano:\n> René Scharfe writes:\n>\n>>>> \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n>>>> -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n>>>> +\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n>>>> \t\t\t\t \"ignoring them\"),\n>>>> \t\t\t 1, PARSE_OPT_HIDDEN),\n>>>> \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n\n>> Anyway, the new help text explaining what the option once did is a bit\n>> confusing. It would be better to focus on what it's doing now (nothing)\n>> and/or why we still have it (for backward compatibility), I think.\n>\n> Do you mean that we should say \"this option used to do such and such\n> but it is now a no-op\" after \"(REMOVED)\" label, instead of the above\n> \"this option does such and such\"? I think \"(REMOVED)\" is a strong\n> enough hint that lets us get away without saying \"used to\" and \"but\n> it is now a no-op\", so I can accept both.\n>\n> Or do you mean we should say \"(REMOVED) for backward compatibility,\n> does nothing but errors out\"? I would be less in faviour, then.\n> Those who are curious enough to ask --help-all would find it more\n> helpful if we said what it used to do. Otherwise they wouldn't be\n> asking --help-all in the first place, no?\n\nWhen I see an option labeled \"REMOVED\" then I get confused because a\nthing that says it no longer exists is obviously lying -- a removed\noption would simply not be listed. Here the feature is gone and its\noption remains, but only reports an educational message now.\n\nPerhaps a better option help text would be something like \"no longer\nsupported, consider using --rebase-merges instead\"?\n\nRené\n"},{"id":"456266","messageId":"xmqqmtf3x4hk.fsf@gitster.g","threadId":"57926","inReplyTo":"32e5088b-35a1-4e8c-098e-18c465a0a0bb@web.de","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-26T23:23:51Z","receivedAt":"2022-05-26T23:24:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 26.05.22 um 22:33 schrieb Junio C Hamano:\n>> René Scharfe writes:\n>>\n>>>>> \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n>>>>> -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n>>>>> +\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n>>>>> \t\t\t\t \"ignoring them\"),\n>>>>> \t\t\t 1, PARSE_OPT_HIDDEN),\n>>>>> \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n> ...\n> Perhaps a better option help text would be something like \"no longer\n> supported, consider using --rebase-merges instead\"?\n\nYeah, that would read very well.\n\nThanks.\n"},{"id":"456279","messageId":"08a87733-cb93-5590-3e96-614be2e64dce@iee.email","threadId":"57926","inReplyTo":"xmqqv8tsvws7.fsf@gitster.g","subject":"Re: [PATCH 3/3] rebase: note `preserve` merges may be a pull config option","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-27T12:08:47Z","receivedAt":"2022-05-27T12:30:48Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 26/05/2022 21:55, Junio C Hamano wrote:\n> \"Philip Oakley via GitGitGadget\" writes:\n>\n>> From: Philip Oakley \n>>\n>> The `--preserve-merges` option was removed by v2.35.0. However\n> Are you sure about that?\nNot any more. I think that was because of the version the user reported...\nIt's clearly wrong, as the down grade is to 2.33.0\n\n> 52f1e821 (pull: remove support for `--rebase=preserve`, 2021-09-07)\n> that is in v2.34.0 and above dropped pull.rebase=preserve from the\n> Documentation/config/pull.txt (and others). My local collection\n> of various Git versions agrees with me. \"git help config\" from\n> 2.34.0 does not list preserve as a valid choice, but 2.33.0 does.\n>\n>> users may not be aware that it is also a Pull option, and it is\n>> still offered by major IDE vendors such as Visual Studio.\n>>\n>> Extend the `--preserve-merges` die message to also direct users to\n>> the use of the `preserve` option in the `pull` config.\n>>\n>> Signed-off-by: Philip Oakley \n>> ---\n>> builtin/rebase.c | 3 ++-\n>> 1 file changed, 2 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>> index aada25a8870..6fc0aaebbb8 100644\n>> --- a/builtin/rebase.c\n>> +++ b/builtin/rebase.c\n>> @@ -1205,7 +1205,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>> \t\t\t builtin_rebase_usage, 0);\n>> \n>> \tif (preserve_merges_selected)\n>> -\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\"));\n>> +\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\\n\"\n>> +\t\t\t\"Your `pull` configuration, may also invoke this option.\"));\n> What is a `pull` configuration?\nI was thinking of 'those that relate to the pull command';-)\n\n> Our configuration variable names\n> all have at least one dot in it. I think it is better to be\n> explicit to clarify what exactly we are suggesting to fix.\n>\n> \"Your `pull.rebase` configuration may be set to 'preserve', which is\n> no longer supported; use 'merges' instead\", or somesuch?\n\nThat's a lot better. I'll borrow that..\nP.\n"},{"id":"456282","messageId":"dba0be5c-507c-6827-c752-297c4db1b95f@iee.email","threadId":"57926","inReplyTo":"19baf95d-67d4-d7ed-72a6-96d098171d3a@web.de","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-27T12:12:20Z","receivedAt":"2022-05-27T12:32:36Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi René,\n\nOn 26/05/2022 14:02, René Scharfe wrote:\n> Am 26.05.22 um 11:40 schrieb Ævar Arnfjörð Bjarmason:\n>> On Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n>>\n>>> From: Philip Oakley \n>>>\n>>> Since feebd2d256 (rebase: hide --preserve-merges option, 2019-10-18)\n>>> this option is now removed as stated in the subsequent release notes.\n>>>\n>>> Fix the option tip.\n>>>\n>>> Signed-off-by: Philip Oakley \n>>> ---\n>>> builtin/rebase.c | 2 +-\n>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>\n>>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>>> index 7ab50cda2ad..6ce7e98a6f1 100644\n>>> --- a/builtin/rebase.c\n>>> +++ b/builtin/rebase.c\n>>> @@ -1110,7 +1110,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>>> \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG,\n>>> \t\t\tparse_opt_interactive),\n>>> \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n>>> -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n>>> +\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n>>> \t\t\t\t \"ignoring them\"),\n>>> \t\t\t 1, PARSE_OPT_HIDDEN),\n>>> \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n>> I have some local patches for this more generally, but for\n>> PARSE_OPT_HIDDEN options we never do anything with the \"argh\" field,\n>> i.e. it's only used for showing the \"git -h\" output, and if it's\n>> hidden it won't be there.\n> Hidden options are shown if you use --help-all instead of -h.\n>\n> OPT_SET_INT_F always sets the struct option member \"argh\" to NULL. The\n> string changed above is the \"help\" member, not \"argh\".\nI should probably also add the verb \"was\" to indicate its historic use ;-)\n>> So there's no point in changing this string, nor to have translators\n>> focus on it, it'll never be used.\n>>\n>> This series shouldn't fix the general issue (which parse-options.c\n>> should really be BUG()-ing about, after fixing the existing\n>> occurances. But For this one we could just set this to have a string of\n>> \"\" or something, only the string you're changing in 3/3 will be seen by\n>> anyone.\n> What is the general issue?\n>\n> Anyway, the new help text explaining what the option once did is a bit\n> confusing. It would be better to focus on what it's doing now (nothing)\n> and/or why we still have it (for backward compatibility), I think.\n>\n> René\nP.\n"},{"id":"456283","messageId":"9455d0af-87f2-f331-a440-3d3feb743610@iee.email","threadId":"57926","inReplyTo":"xmqq5ylsxccw.fsf@gitster.g","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-27T12:17:50Z","receivedAt":"2022-05-27T12:37:22Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 26/05/2022 21:33, Junio C Hamano wrote:\n> René Scharfe writes:\n>\n>>>> \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n>>>> -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n>>>> +\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n>>>> \t\t\t\t \"ignoring them\"),\n>>>> \t\t\t 1, PARSE_OPT_HIDDEN),\n>>>> \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n>> Hidden options are shown if you use --help-all instead of -h.\n>>\n>> OPT_SET_INT_F always sets the struct option member \"argh\" to NULL. The\n>> string changed above is the \"help\" member, not \"argh\".\n> Good points. I do think it is OK to say REMOVED in case --help-all\n> asks us to show everything, even though I wonder if we can leave it\n> there until we remove the \"support\" of noticing the user asking for\n> a now-removed feature.\n\nI'll add \"was ..\" to clarify its historic use.\nI expect that it'll be there for many years to catch late upgrading \nusers, as well as those that help others stuck in this trap using a \nmodern portable Git (esp. Windows).\n>>> So there's no point in changing this string, nor to have translators\n>>> focus on it, it'll never be used.\n>>>\n>>>\nThe translation change would need to be a separate patch, no? That would \nmake it easy to drop if not wanted.\nP.\n"},{"id":"456284","messageId":"4ff0622a-9b64-4200-e996-2d1875a52ec8@iee.email","threadId":"57926","inReplyTo":"32e5088b-35a1-4e8c-098e-18c465a0a0bb@web.de","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-27T12:35:03Z","receivedAt":"2022-05-27T12:42:57Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi René\n\nOn 26/05/2022 22:27, René Scharfe wrote:\n> Am 26.05.22 um 22:33 schrieb Junio C Hamano:\n>> René Scharfe writes:\n>>\n>>>>> \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n>>>>> -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n>>>>> +\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n>>>>> \t\t\t\t \"ignoring them\"),\n>>>>> \t\t\t 1, PARSE_OPT_HIDDEN),\n>>>>> \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n>>> Anyway, the new help text explaining what the option once did is a bit\n>>> confusing. It would be better to focus on what it's doing now (nothing)\n>>> and/or why we still have it (for backward compatibility), I think.\n>> Do you mean that we should say \"this option used to do such and such\n>> but it is now a no-op\" after \"(REMOVED)\" label, instead of the above\n>> \"this option does such and such\"? I think \"(REMOVED)\" is a strong\n>> enough hint that lets us get away without saying \"used to\" and \"but\n>> it is now a no-op\", so I can accept both.\n>>\n>> Or do you mean we should say \"(REMOVED) for backward compatibility,\n>> does nothing but errors out\"? I would be less in faviour, then.\n>> Those who are curious enough to ask --help-all would find it more\n>> helpful if we said what it used to do. Otherwise they wouldn't be\n>> asking --help-all in the first place, no?\n> When I see an option labeled \"REMOVED\" then I get confused because a\n> thing that says it no longer exists is obviously lying\n\nThat's a misunderstanding between the response to the command line \noption, and the described operation of the former sub-command/option.\n> -- a removed\n> option would simply not be listed. Here the feature is gone and its\n> option remains, but only reports an educational message now.\n\nThe needed user response is more that educational. In this case (for the \nSeries) they are in a Catch-22 situation, stuck in a no-man's land \nbetween a preserve merges that has been started, and a Git that won't \nproceed. Currently (prior to the series) Git will even refuse to abort..\n>\n> Perhaps a better option help text would be something like \"no longer\n> supported, consider using --rebase-merges instead\"?\nWe'll still need to say _what_ is no longer supported, to ensure the \nuser has context. I'd agree with the suggestion aspect (Junio had \ncommented similarly).\n\nI suspect this problem could be a long, slow burner. We so rarely remove \ncapabilities like this, so it's tricky second guessing how users will \nreact, or when they discover the problem.\n\nP.\n"},{"id":"456285","messageId":"220527.86o7zj2ldi.gmgdl@evledraar.gmail.com","threadId":"57926","inReplyTo":"19baf95d-67d4-d7ed-72a6-96d098171d3a@web.de","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-27T12:34:20Z","receivedAt":"2022-05-27T12:47:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, May 26 2022, René Scharfe wrote:\n\n> Am 26.05.22 um 11:40 schrieb Ævar Arnfjörð Bjarmason:\n>>\n>> On Thu, May 26 2022, Philip Oakley via GitGitGadget wrote:\n>>\n>>> From: Philip Oakley \n>>>\n>>> Since feebd2d256 (rebase: hide --preserve-merges option, 2019-10-18)\n>>> this option is now removed as stated in the subsequent release notes.\n>>>\n>>> Fix the option tip.\n>>>\n>>> Signed-off-by: Philip Oakley \n>>> ---\n>>> builtin/rebase.c | 2 +-\n>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>\n>>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>>> index 7ab50cda2ad..6ce7e98a6f1 100644\n>>> --- a/builtin/rebase.c\n>>> +++ b/builtin/rebase.c\n>>> @@ -1110,7 +1110,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>>> \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG,\n>>> \t\t\tparse_opt_interactive),\n>>> \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n>>> -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n>>> +\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n>>> \t\t\t\t \"ignoring them\"),\n>>> \t\t\t 1, PARSE_OPT_HIDDEN),\n>>> \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n>>\n>> I have some local patches for this more generally, but for\n>> PARSE_OPT_HIDDEN options we never do anything with the \"argh\" field,\n>> i.e. it's only used for showing the \"git -h\" output, and if it's\n>> hidden it won't be there.\n>\n> Hidden options are shown if you use --help-all instead of -h.\n>\n> OPT_SET_INT_F always sets the struct option member \"argh\" to NULL. The\n> string changed above is the \"help\" member, not \"argh\".\n>\n>> So there's no point in changing this string, nor to have translators\n>> focus on it, it'll never be used.\n>>\n>> This series shouldn't fix the general issue (which parse-options.c\n>> should really be BUG()-ing about, after fixing the existing\n>> occurances. But For this one we could just set this to have a string of\n>> \"\" or something, only the string you're changing in 3/3 will be seen by\n>> anyone.\n>\n> What is the general issue?\n\nYou're right. I'd missed that case with --help-all, and remembered\nbriefly testing where we used the string before in some WIP code.\n\nLooking at it again I think I tried NULL-ing a few and running the tests\nwith SANITIZE=address or something, which in this case it looks like\nwe'd 100% pass with. I.e. we have zero test coverage for that subsequent\nNULL dereference.\n\nSorry about the noise. I'll add some tests for this case in some\nparse-options.c sanity checking tests/patches I'm planning to submit at\nsome point.\n"},{"id":"456286","messageId":"c7667b0b-d18c-e2e4-0a9e-45367ee8ac0e@iee.email","threadId":"57926","inReplyTo":"xmqq1qwgxbys.fsf@gitster.g","subject":"Re: [PATCH 2/3] rebase: help users when dying with `preserve-merges`","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-27T12:58:27Z","receivedAt":"2022-05-27T12:58:33Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"\n\nOn 26/05/2022 21:42, Junio C Hamano wrote:\n> Philip Oakley writes:\n>\n>>>> Make the `rebase --abort` option available to allow users to remove\n>>>> traces of any preserve-merges rebase, even if they had upgraded\n>>>> during a rebase.\n> This patch does not make it \"available\", though.\n\nYes it does. Sorry if the terminology or explanation was poor (here we \nare looking at the commit message, not the user facing message?).\n\nCurrently, if the user has an in-progress rebase with preserve-merges, \nand now using the latest Git, they will reach the fatal die(), even if \nthey try any of the git status suggestions of --abort, --continue, etc.  \nEssentially, it's a 'you shouldn't be here', lets stop right now, go \nstraight to jail condition. We do want to permit the `rebase --abort` \ncommand option.\n\nI can swap around the && condition so that it's clearer that we check \nthe user isn't requesting an --abort before checking the internal \ndirectory and then dying.\n> \tSuggest using `--abort` to get out of the situation after a\n> \tfailed preserve-rebase and remove traces of ...\n>\n> perhaps?\n>\n> I do think the suggestion is worth doing if a user ever gets into\n> the situation, but how likely does it happen? A user has to start\n> \"rebase -p\" with older Git,\n\n.. hit a conflict, seeks help. Helper bring a personal portable Git with \nlatest version - Oops.\n\nOr Helper, says \"Oh, your version is old, upgrade, and that'll fix it\", \nagain Oops.\n\n> wait until Git gets updated to a future\n> version of Git that includes this change, and then say \"rebase -p\n> --continue\"?\nYou don't need the -p there ;-)\n\nFor this change, the \"git rebase --continue\" will still die() with the \nfatal: message. We do not have a way to continue. However..\n\nAfter this change, the \"git rebase --abort\" will properly clear and \nclean the repo/status so that the user can then choose what to do.\n\n>\n>>>> \t} else if (is_directory(merge_dir())) {\n>>>> \t\tstrbuf_reset(&buf);\n>>>> \t\tstrbuf_addf(&buf, \"%s/rewritten\", merge_dir());\n>>>> -\t\tif (is_directory(buf.buf)) {\n>>>> -\t\t\tdie(\"`rebase -p` is no longer supported\");\n>>>> +\t\tif (is_directory(buf.buf) && !(action == ACTION_ABORT)) {\n>>>> +\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n>>>> +\t\t\t\"Use `git rebase --abort` to terminate current rebase.\\n\"\n>>>> +\t\t\t\"Or downgrade to v2.33, or earlier, to complete the rebase.\\n\");\n>>>> \t\t} else {\n>>>> \t\t\tstrbuf_reset(&buf);\n>>>> \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n>>> Existing issue: No _(), shouldn't we add it?\n>> This `strbuf_addf` is forming a path for internal use. It just happens\n>> to look like legible English ;-)\n> I do not think Ævar meant \"%s/interactive\"; the enhanced message\n> above that you inherited from the original \"no longer supported\"\n> that was not marked for translation.\nOk.\n>\n>>> I wonder if we should use die_message() + advise() in these cases,\n>>> i.e. stick to why we died in die_message() and have the advise() make\n>>> suggestions, as e4921d877ab (tracking branches: add advice to ambiguous\n>>> refspec error, 2022-04-01) does.\n>> Ah, maybe it's my message.. that needs translating.\n> Yup.\nOk, I'd add a separate patch for that.\n\n> This whole '-p' business will go away in a few releases down, so a\n> longer message give to the existing die() should be sufficient and\n> there is no need for the choice between \"yes, I am still weaning\n> myself off of rebase -p and want to keep seeing the advice\" and\n> \"thanks, I saw the message often enough, you no longer need to tell\n> me how to get out\", I would think.\nI think it will take a long while for all the users, tools providers and \ndistros to get beyond 2.33, so while each user may be weaned quickly, \nthe generic problem is likely to continue to linger.\n\n\nI hope to re-roll later next week. In general it's mainly tweaks and \nfinesse.\n\nPhilip\n\n\n"},{"id":"456297","messageId":"xmqq4k1b6ktb.fsf@gitster.g","threadId":"57926","inReplyTo":"9455d0af-87f2-f331-a440-3d3feb743610@iee.email","subject":"Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-27T15:45:36Z","receivedAt":"2022-05-27T15:45:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> On 26/05/2022 21:33, Junio C Hamano wrote:\n>>>> So there's no point in changing this string, nor to have translators\n>>>> focus on it, it'll never be used.\n>>>>\n>>>>\n> The translation change would need to be a separate patch, no? That\n> would make it easy to drop if not wanted.\n\nI think you are responding to what Ævar said, but the string this\npatch is modifying is already inside N_(), and modifying a string\nthat is already marked for translation in any way (other than\nremoving the N_() or _() around it) incurs the cost to translate the\nupdated string already, with or without any separate patch.\n\nIf we are adding a new die() call that uses a new message, we should\nmark the message for translation from the beginning. The messages\nproduced by die/warning/error are meant to be read by human users,\nso unless there is some very strong reason not to, they should be\nmarked for translation.\n\n\n\n\n"},{"id":"456299","messageId":"xmqqzgj355uc.fsf@gitster.g","threadId":"57926","inReplyTo":"c7667b0b-d18c-e2e4-0a9e-45367ee8ac0e@iee.email","subject":"Re: [PATCH 2/3] rebase: help users when dying with `preserve-merges`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-27T15:54:19Z","receivedAt":"2022-05-27T15:54:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> On 26/05/2022 21:42, Junio C Hamano wrote:\n>> Philip Oakley writes:\n>>\n>>>>> Make the `rebase --abort` option available to allow users to remove\n>>>>> traces of any preserve-merges rebase, even if they had upgraded\n>>>>> during a rebase.\n>> This patch does not make it \"available\", though.\n>\n> Yes it does. Sorry if the terminology or explanation was poor (here we\n> are looking at the commit message, not the user facing message?).\n\nSorry, you're right. I misread the new \"&&\" condition in the patch.\n\n> I hope to re-roll later next week. In general it's mainly tweaks and\n> finesse.\n\nThanks.\n"},{"id":"456669","messageId":"pull.1242.v2.git.1654341469.gitgitgadget@gmail.com","threadId":"57926","inReplyTo":"pull.1242.git.1653556865.gitgitgadget@gmail.com","subject":"[PATCH v2 0/4] Die preserve ggg","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-04T11:17:45Z","receivedAt":"2022-06-04T11:17:59Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"This [2] short series is a follow up to GitGitGadget \"Update the die()\npreserve-merges messages to help some users (PR #1155)\" [1].\n\nSince v1: Additional patch to translate the user facing die message. Bring\nthe --abort preclusion to the start of the if && condition for clarity.\nClarify that the pull.rebase config is complementary to this particular\n'die'. Updates to the commit messages.\n\nv0: The first patch is a tidy up of the --preserve option to highlight that\nit is now Deleted, rather than Deprecated.\n\nIn response to Avar's comments that the former error message merely\n'tantilised without telling' the user what to do, it became obvious that the\nunderling problem was that the user was unable to git rebase --abort which\nwas also fatal, when a preserve-rebase was in progress.\n\nThus the main update is to allow the rebase --abort command, even when a\n--preserve is in progress, to proceed. The --abort code was unchanged by the\nremoval of the preserve option, as the resetting and clean up of internal\nstate is common to the other rebase options.\n\nThe user facing fatal message now simply advises to abort, or downgrade to a\nversion that has preserve-merges to complete the rebase.\n\nThe final patch highlights that some IDEs still allow the setting of the\npreserve-merges option as a pull config setup.\n\nPhilip Oakly\n\n[1] GitLore ref pull.1155.git.1645526016.gitgitgadget@gmail.com\nhttps://lore.kernel.org/git/pull.1155.git.1645526016.gitgitgadget@gmail.com/\n\n[2]\nhttps://lore.kernel.org/git/pull.1242.git.1653556865.gitgitgadget@gmail.com/t/#u\n\nPhilip Oakley (4):\n rebase.c: state preserve-merges has been removed\n rebase: help users when dying with `preserve-merges`\n rebase: note `preserve` merges may be a pull config option\n rebase: translate a die(preserve-merges) message\n\n builtin/rebase.c | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\n\nbase-commit: c4f0e309ae745751d08727f24e8ff55e56355755\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1242%2FPhilipOakley%2Fdie_preserve_ggg-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1242/PhilipOakley/die_preserve_ggg-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1242\n\nRange-diff vs v1:\n\n 1: 0a4c81d8caf ! 1: d60ec67cb06 rebase.c: state preserve-merges has been removed\n @@ Commit message\n Since feebd2d256 (rebase: hide --preserve-merges option, 2019-10-18)\n this option is now removed as stated in the subsequent release notes.\n \n - Fix the option tip.\n + Fix and reflow the option tip.\n \n Signed-off-by: Philip Oakley \n \n @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix\n \t\t\tparse_opt_interactive),\n \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n -\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n -+\t\t\t N_(\"(REMOVED) try to recreate merges instead of \"\n - \t\t\t\t \"ignoring them\"),\n +-\t\t\t\t \"ignoring them\"),\n ++\t\t\t N_(\"(REMOVED) was: try to recreate merges \"\n ++\t\t\t\t \"instead of ignoring them\"),\n \t\t\t 1, PARSE_OPT_HIDDEN),\n \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n + \t\tOPT_CALLBACK_F(0, \"empty\", &options, \"{drop,keep,ask}\",\n 2: d0fb5410594 ! 2: 47f27187529 rebase: help users when dying with `preserve-merges`\n @@ Metadata\n ## Commit message ##\n rebase: help users when dying with `preserve-merges`\n \n - Git will die if a \"rebase --preserve-merges\" is in progress.\n - Users cannot --quit, --abort or --continue the rebase.\n + Git would die if a \"rebase --preserve-merges\" was in progress.\n + Users could neither --quit, --abort, nor --continue the rebase.\n \n Make the `rebase --abort` option available to allow users to remove\n traces of any preserve-merges rebase, even if they had upgraded\n during a rebase.\n \n - One trigger was an unexpectedly difficult to resolve conflict, as\n + One trigger case was an unexpectedly difficult to resolve conflict, as\n reported on the `git-users` group.\n (https://groups.google.com/g/git-for-windows/c/3jMWbBlXXHM)\n \n - Tell the user the options to resolve the problem manually.\n + Other potential use-cases include git-experts using the portable\n + 'Git on a stick' to help users with an older git version.\n \n Signed-off-by: Philip Oakley \n \n @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix\n \t\tstrbuf_addf(&buf, \"%s/rewritten\", merge_dir());\n -\t\tif (is_directory(buf.buf)) {\n -\t\t\tdie(\"`rebase -p` is no longer supported\");\n -+\t\tif (is_directory(buf.buf) && !(action == ACTION_ABORT)) {\n ++\t\tif (!(action == ACTION_ABORT) && is_directory(buf.buf)) {\n +\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n +\t\t\t\"Use `git rebase --abort` to terminate current rebase.\\n\"\n -+\t\t\t\"Or downgrade to v2.33, or earlier, to complete the rebase.\\n\");\n ++\t\t\t\"Or downgrade to v2.33, or earlier, to complete the rebase.\");\n \t\t} else {\n \t\t\tstrbuf_reset(&buf);\n \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n 3: ece3eecdc4d ! 3: fe000f06207 rebase: note `preserve` merges may be a pull config option\n @@ Metadata\n ## Commit message ##\n rebase: note `preserve` merges may be a pull config option\n \n - The `--preserve-merges` option was removed by v2.35.0. However\n - users may not be aware that it is also a Pull option, and it is\n - still offered by major IDE vendors such as Visual Studio.\n + The `--preserve-merges` option was removed by v2.34.0. However\n + users may not be aware that it is also a Pull configuration option,\n + which is still offered by major IDE vendors such as Visual Studio.\n \n Extend the `--preserve-merges` die message to also direct users to\n - the use of the `preserve` option in the `pull` config.\n + the possible use of the `preserve` option in the `pull.rebase` config.\n + This is an additional 'belt and braces' information statement.\n \n Signed-off-by: Philip Oakley \n \n @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix\n \tif (preserve_merges_selected)\n -\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\"));\n +\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\\n\"\n -+\t\t\t\"Your `pull` configuration, may also invoke this option.\"));\n ++\t\t\t\"Note: Your `pull.rebase` configuration may also be set to 'preserve',\\n\"\n ++\t\t\t\"which is no longer supported; use 'merges' instead\"));\n \n \tif (action != ACTION_NONE && total_argc != 2) {\n \t\tusage_with_options(builtin_rebase_usage,\n -: ----------- > 4: ae02c6d5a6e rebase: translate a die(preserve-merges) message\n\n-- \ngitgitgadget\n"},{"id":"456670","messageId":"fe000f062078e544361c87c319830cd36aabbc91.1654341469.git.gitgitgadget@gmail.com","threadId":"57926","inReplyTo":"pull.1242.v2.git.1654341469.gitgitgadget@gmail.com","subject":"[PATCH v2 3/4] rebase: note `preserve` merges may be a pull config option","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-04T11:17:48Z","receivedAt":"2022-06-04T11:18:12Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: Philip Oakley \n\nThe `--preserve-merges` option was removed by v2.34.0. However\nusers may not be aware that it is also a Pull configuration option,\nwhich is still offered by major IDE vendors such as Visual Studio.\n\nExtend the `--preserve-merges` die message to also direct users to\nthe possible use of the `preserve` option in the `pull.rebase` config.\nThis is an additional 'belt and braces' information statement.\n\nSigned-off-by: Philip Oakley \n---\n builtin/rebase.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 17cc776b4b1..5f8921551e1 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1205,7 +1205,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\t builtin_rebase_usage, 0);\n \n \tif (preserve_merges_selected)\n-\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\"));\n+\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\\n\"\n+\t\t\t\"Note: Your `pull.rebase` configuration may also be set to 'preserve',\\n\"\n+\t\t\t\"which is no longer supported; use 'merges' instead\"));\n \n \tif (action != ACTION_NONE && total_argc != 2) {\n \t\tusage_with_options(builtin_rebase_usage,\n-- \ngitgitgadget\n\n"},{"id":"456671","messageId":"47f271875291d24666c5a3cec895421ab646f1f3.1654341469.git.gitgitgadget@gmail.com","threadId":"57926","inReplyTo":"pull.1242.v2.git.1654341469.gitgitgadget@gmail.com","subject":"[PATCH v2 2/4] rebase: help users when dying with `preserve-merges`","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-04T11:17:47Z","receivedAt":"2022-06-04T11:18:12Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: Philip Oakley \n\nGit would die if a \"rebase --preserve-merges\" was in progress.\nUsers could neither --quit, --abort, nor --continue the rebase.\n\nMake the `rebase --abort` option available to allow users to remove\ntraces of any preserve-merges rebase, even if they had upgraded\nduring a rebase.\n\nOne trigger case was an unexpectedly difficult to resolve conflict, as\nreported on the `git-users` group.\n(https://groups.google.com/g/git-for-windows/c/3jMWbBlXXHM)\n\nOther potential use-cases include git-experts using the portable\n'Git on a stick' to help users with an older git version.\n\nSigned-off-by: Philip Oakley \n---\n builtin/rebase.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex bad95d98adf..17cc776b4b1 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1182,8 +1182,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t} else if (is_directory(merge_dir())) {\n \t\tstrbuf_reset(&buf);\n \t\tstrbuf_addf(&buf, \"%s/rewritten\", merge_dir());\n-\t\tif (is_directory(buf.buf)) {\n-\t\t\tdie(\"`rebase -p` is no longer supported\");\n+\t\tif (!(action == ACTION_ABORT) && is_directory(buf.buf)) {\n+\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n+\t\t\t\"Use `git rebase --abort` to terminate current rebase.\\n\"\n+\t\t\t\"Or downgrade to v2.33, or earlier, to complete the rebase.\");\n \t\t} else {\n \t\t\tstrbuf_reset(&buf);\n \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n-- \ngitgitgadget\n\n"},{"id":"456672","messageId":"d60ec67cb0673a9a9ed16c62f69e0a5235966ae2.1654341469.git.gitgitgadget@gmail.com","threadId":"57926","inReplyTo":"pull.1242.v2.git.1654341469.gitgitgadget@gmail.com","subject":"[PATCH v2 1/4] rebase.c: state preserve-merges has been removed","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-04T11:17:46Z","receivedAt":"2022-06-04T11:18:12Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: Philip Oakley \n\nSince feebd2d256 (rebase: hide --preserve-merges option, 2019-10-18)\nthis option is now removed as stated in the subsequent release notes.\n\nFix and reflow the option tip.\n\nSigned-off-by: Philip Oakley \n---\n builtin/rebase.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 7ab50cda2ad..bad95d98adf 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1110,8 +1110,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG,\n \t\t\tparse_opt_interactive),\n \t\tOPT_SET_INT_F('p', \"preserve-merges\", &preserve_merges_selected,\n-\t\t\t N_(\"(DEPRECATED) try to recreate merges instead of \"\n-\t\t\t\t \"ignoring them\"),\n+\t\t\t N_(\"(REMOVED) was: try to recreate merges \"\n+\t\t\t\t \"instead of ignoring them\"),\n \t\t\t 1, PARSE_OPT_HIDDEN),\n \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n \t\tOPT_CALLBACK_F(0, \"empty\", &options, \"{drop,keep,ask}\",\n-- \ngitgitgadget\n\n"},{"id":"456673","messageId":"ae02c6d5a6e9ca3da8bdf2188da933a6b8ec33ed.1654341469.git.gitgitgadget@gmail.com","threadId":"57926","inReplyTo":"pull.1242.v2.git.1654341469.gitgitgadget@gmail.com","subject":"[PATCH v2 4/4] rebase: translate a die(preserve-merges) message","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-04T11:17:49Z","receivedAt":"2022-06-04T11:18:12Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: Philip Oakley \n\nThis is a user facing message for a situation seen in the wild.\n\nTranslate it.\n\nSigned-off-by: Philip Oakley \n---\n builtin/rebase.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 5f8921551e1..640b6046a5a 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1183,9 +1183,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_reset(&buf);\n \t\tstrbuf_addf(&buf, \"%s/rewritten\", merge_dir());\n \t\tif (!(action == ACTION_ABORT) && is_directory(buf.buf)) {\n-\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n+\t\t\tdie(_(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n \t\t\t\"Use `git rebase --abort` to terminate current rebase.\\n\"\n-\t\t\t\"Or downgrade to v2.33, or earlier, to complete the rebase.\");\n+\t\t\t\"Or downgrade to v2.33, or earlier, to complete the rebase.\"));\n \t\t} else {\n \t\t\tstrbuf_reset(&buf);\n \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n-- \ngitgitgadget\n"},{"id":"456738","messageId":"xmqq1qw18yk2.fsf@gitster.g","threadId":"57926","inReplyTo":"fe000f062078e544361c87c319830cd36aabbc91.1654341469.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 3/4] rebase: note `preserve` merges may be a pull config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-06T17:57:33Z","receivedAt":"2022-06-06T17:57:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley via GitGitGadget\" writes:\n\n> From: Philip Oakley \n>\n> The `--preserve-merges` option was removed by v2.34.0. However\n> users may not be aware that it is also a Pull configuration option,\n> which is still offered by major IDE vendors such as Visual Studio.\n>\n> Extend the `--preserve-merges` die message to also direct users to\n> the possible use of the `preserve` option in the `pull.rebase` config.\n> This is an additional 'belt and braces' information statement.\n>\n> Signed-off-by: Philip Oakley \n> ---\n> builtin/rebase.c | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 17cc776b4b1..5f8921551e1 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1205,7 +1205,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \t\t\t builtin_rebase_usage, 0);\n> \n> \tif (preserve_merges_selected)\n> -\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\"));\n> +\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\\n\"\n> +\t\t\t\"Note: Your `pull.rebase` configuration may also be set to 'preserve',\\n\"\n> +\t\t\t\"which is no longer supported; use 'merges' instead\"));\n\n\"be set\" -> \"be set\".\n\nI am not sure how this helps anybody, though. \n\nWhen pull.rebase is parsed, rebase.c::rebase_parse_value() is called\nfrom builtin/pull.c::parse_config_rebase() and would trigger an\nerror, whether it comes from the pull.rebase or the branch.*.rebase\nconfiguration variable. An error() message already said that\n'preserve' was removed and 'merges' would be a replacement when it\nhappened.\n\nIf the user has *not* reached this die() due to a configuration\nvariable, then there is not much point giving this new message,\neither.\n"},{"id":"457063","messageId":"4cac8a13-a075-544e-8c10-e58bbf0dd73d@iee.email","threadId":"57926","inReplyTo":"xmqq1qw18yk2.fsf@gitster.g","subject":"Re: [PATCH v2 3/4] rebase: note `preserve` merges may be a pull config option","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-06-11T14:03:25Z","receivedAt":"2022-06-11T14:03:35Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Sorry for delay, I had other family priorities to attend to.\n\nOn 06/06/2022 18:57, Junio C Hamano wrote:\n> \"Philip Oakley via GitGitGadget\" writes:\n>\n>> From: Philip Oakley \n>>\n>> The `--preserve-merges` option was removed by v2.34.0. However\n>> users may not be aware that it is also a Pull configuration option,\n>> which is still offered by major IDE vendors such as Visual Studio.\n>>\n>> Extend the `--preserve-merges` die message to also direct users to\n>> the possible use of the `preserve` option in the `pull.rebase` config.\n>> This is an additional 'belt and braces' information statement.\n>>\n>> Signed-off-by: Philip Oakley \n>> ---\n>> builtin/rebase.c | 4 +++-\n>> 1 file changed, 3 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>> index 17cc776b4b1..5f8921551e1 100644\n>> --- a/builtin/rebase.c\n>> +++ b/builtin/rebase.c\n>> @@ -1205,7 +1205,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>> \t\t\t builtin_rebase_usage, 0);\n>> \n>> \tif (preserve_merges_selected)\n>> -\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\"));\n>> +\t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\\n\"\n>> +\t\t\t\"Note: Your `pull.rebase` configuration may also be set to 'preserve',\\n\"\n>> +\t\t\t\"which is no longer supported; use 'merges' instead\"));\n> \"be set\" -> \"be set\".\nNoted. I see that the series is now in `next` [Thank you], so not worth\nthe churn of a patch, unless folks start noticing..\n\n>\n> I am not sure how this helps anybody, though. \n\nIt's the Catch 22 problem for deleted capabilities, which we rarely see\nbecause we normally have backward compatibility.\n \n>\n> When pull.rebase is parsed, rebase.c::rebase_parse_value() is called\n> from builtin/pull.c::parse_config_rebase() and would trigger an\n> error, whether it comes from the pull.rebase or the branch.*.rebase\n> configuration variable. An error() message already said that\n> 'preserve' was removed and 'merges' would be a replacement when it\n> happened.\n>\n> If the user has *not* reached this die() due to a configuration\n> variable, then there is not much point giving this new message,\n> either.\n\nFrom my perspective, users should then be purging _all_ their `preserve`\nconfigurations once they hit such errors. As the v2.34.0 change\npropagates through the Git ecosystem, hopefully it'll be a sufficient\nprompt for those who haven't realised that the option can be 'hidden' in\ntheir configuration options.\n\nTime will tell.\n\nThanks\n\nPhilip\n"},{"id":"457066","messageId":"3800fa9c-50b4-2967-2f00-036c1edf5e52@iee.email","threadId":"57926","inReplyTo":"4cac8a13-a075-544e-8c10-e58bbf0dd73d@iee.email","subject":"Re: [PATCH v2 3/4] rebase: note `preserve` merges may be a pull config option","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-06-11T15:38:20Z","receivedAt":"2022-06-11T15:38:30Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"small clarification,\n\nOn 11/06/2022 15:03, Philip Oakley wrote:\n>> When pull.rebase is parsed, rebase.c::rebase_parse_value() is called\n>> from builtin/pull.c::parse_config_rebase() and would trigger an\n>> error, whether it comes from the pull.rebase or the branch.*.rebase\n>> configuration variable. An error() message already said that\n>> 'preserve' was removed and 'merges' would be a replacement when it\n>> happened.\n>>\n>> If the user has *not* reached this die() due to a configuration\n>> variable, then there is not much point giving this new message,\n>> either.\n> From my perspective, users should then\n\nThat is, when users hit any of the `preserve-merges` error message, ... \n> be purging _all_ their `preserve`\n> configurations once they hit such errors. As the v2.34.0 change\n> propagates through the Git ecosystem, hopefully it'll be a sufficient\n> prompt for those who haven't realised that the option can be 'hidden' in\n> their configuration options.\n\n"},{"id":"457070","messageId":"xmqqh74rattu.fsf@gitster.g","threadId":"57926","inReplyTo":"3800fa9c-50b4-2967-2f00-036c1edf5e52@iee.email","subject":"Re: [PATCH v2 3/4] rebase: note `preserve` merges may be a pull config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-11T19:22:37Z","receivedAt":"2022-06-11T19:22:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> small clarification,\n>\n> On 11/06/2022 15:03, Philip Oakley wrote:\n>>> When pull.rebase is parsed, rebase.c::rebase_parse_value() is called\n>>> from builtin/pull.c::parse_config_rebase() and would trigger an\n>>> error, whether it comes from the pull.rebase or the branch.*.rebase\n>>> configuration variable. An error() message already said that\n>>> 'preserve' was removed and 'merges' would be a replacement when it\n>>> happened.\n>>>\n>>> If the user has *not* reached this die() due to a configuration\n>>> variable, then there is not much point giving this new message,\n>>> either.\n>> From my perspective, users should then\n>\n> That is, when users hit any of the `preserve-merges` error message, ... \n\nYes, but configuration parsing happens way earlier than the actual\nuse of the option (which is decided after configuration and then\ncommand line is read), so the users would probably have hit the\nerror message and corrected their configuration before they can even\nsee this error message, no?\n\nI guess I am repeating myself, so there may be some case where a\nstale variable can still be in the user's configuration file and the\nuser can hit this error message without seeing the other error\nmessage about the stale configuration variable that I am not seeing?\n\n>> be purging _all_ their `preserve`\n>> configurations once they hit such errors. As the v2.34.0 change\n>> propagates through the Git ecosystem, hopefully it'll be a sufficient\n>> prompt for those who haven't realised that the option can be 'hidden' in\n>> their configuration options.\n"}]}