{"thread":{"id":"57457","subject":"[PATCH 0/2] Update the die() preserve-merges messages to help some users","startedAt":"2022-02-22T10:34:02Z","lastAt":"2022-03-09T12:18:30Z","messageCount":18,"participants":["Johannes Schindelin via GitGitGadget","Philip Oakley via GitGitGadget","Ævar Arnfjörð Bjarmason","Philip Oakley","Phillip Wood","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"449102","messageId":"pull.1155.git.1645526016.gitgitgadget@gmail.com","threadId":"57457","inReplyTo":null,"subject":"[PATCH 0/2] Update the die() preserve-merges messages to help some users","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-22T10:33:34Z","receivedAt":"2022-02-22T10:34:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This small update to the die() preserve-merges messages is a response to the\nreported edge case in the Git-for-Windows googlegroups thread\n[https://groups.google.com/g/git-for-windows/c/3jMWbBlXXHM] where even git\nrebase --continue would die.\n\nIt is most relevant for Windows because Visual Studio still offers the\noption to run git pull --preserve, therefore Git for Windows already applied\nthese patches. The improvements are not specific to Windows, though, and\nshould therefore also get into core Git, albeit at a more leisurely pace.\n\nThis is a companion patch series to\nhttps://github.com/git-for-windows/git/pull/3708\n\nPhilip Oakley (2):\n  rebase: help user when dying with preserve-merges`\n  rebase: `preserve` is also a pull option, tell dying users\n\n builtin/rebase.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\n\nbase-commit: e6ebfd0e8cbbd10878070c8a356b5ad1b3ca464e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1155%2Fdscho%2Fdie_preserve-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1155/dscho/die_preserve-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1155\n-- \ngitgitgadget\n"},{"id":"449103","messageId":"cd06aa68c2fc65551cd810a1c2c0941c51433163.1645526016.git.gitgitgadget@gmail.com","threadId":"57457","inReplyTo":"pull.1155.git.1645526016.gitgitgadget@gmail.com","subject":"[PATCH 1/2] rebase: help user when dying with preserve-merges`","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-22T10:33:35Z","receivedAt":"2022-02-22T10:34:03Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: Philip Oakley <philipoakley@iee.email>\n\nGit will die if a \"rebase --preserve-merges\" is in progress.\nUsers cannot --quit, --abort or --continue the rebase.\n\nThis sceario can occur if the user updates their Git, or switches\nto another newer version, after starting a preserve-merges rebase,\ncommonly via the pull setting.\n\nOne trigger is 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 cause, i.e. the existence of the directory.\nThe problem must be resolved manually, `git rebase --<option>`\ncommands will die, or the user must downgrade. Also, note that\nthe deleted options are no longer shown in the documentation.\n\nSigned-off-by: Philip Oakley <philipoakley@iee.email>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\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 d858add3fe8..07221d0ae41 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1181,7 +1181,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 (is_directory(buf.buf)) {\n-\t\t\tdie(\"`rebase -p` is no longer supported\");\n+\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n+\t\t\t\"You still have a `.git/rebase-merge/rewritten` directory, \\n\"\n+\t\t\t\"indicating a `rebase preserve-merge` is still in progress.\\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":"449104","messageId":"eb5871db95b12500cc0a6b8b0e3a82ed9e8fcfbd.1645526016.git.gitgitgadget@gmail.com","threadId":"57457","inReplyTo":"pull.1155.git.1645526016.gitgitgadget@gmail.com","subject":"[PATCH 2/2] rebase: `preserve` is also a pull option, tell dying users","fromName":"Philip Oakley via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-22T10:33:36Z","receivedAt":"2022-02-22T10:34:06Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: Philip Oakley <philipoakley@iee.email>\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 direct users to\nthis option and it's locations.\n\nSigned-off-by: Philip Oakley <philipoakley@iee.email>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/rebase.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 07221d0ae41..97f704bb297 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1205,7 +1205,10 @@ 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\"Also, check your `pull` configuration settings\\n\"\n+\t\t\t\"`git config --show-scope --show-origin --get-regexp 'pull.*'`\\n\"\n+\t\t\t\"which 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":"449140","messageId":"220222.86czje7w4i.gmgdl@evledraar.gmail.com","threadId":"57457","inReplyTo":"cd06aa68c2fc65551cd810a1c2c0941c51433163.1645526016.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] rebase: help user when dying with preserve-merges`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-02-22T15:32:59Z","receivedAt":"2022-02-22T15:35:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Feb 22 2022, Philip Oakley via GitGitGadget wrote:\n\n> From: Philip Oakley <philipoakley@iee.email>\n>\n> Git will die if a \"rebase --preserve-merges\" is in progress.\n> Users cannot --quit, --abort or --continue the rebase.\n>\n> This sceario can occur if the user updates their Git, or switches\n> to another newer version, after starting a preserve-merges rebase,\n> commonly via the pull setting.\n>\n> One trigger is 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 cause, i.e. the existence of the directory.\n> The problem must be resolved manually, `git rebase --<option>`\n> commands will die, or the user must downgrade. Also, note that\n> the deleted options are no longer shown in the documentation.\n\nI can go and read the linked thread for the answer, but:\n\n>  \t\tif (is_directory(buf.buf)) {\n> -\t\t\tdie(\"`rebase -p` is no longer supported\");\n> +\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n> +\t\t\t\"You still have a `.git/rebase-merge/rewritten` directory, \\n\"\n> +\t\t\t\"indicating a `rebase preserve-merge` is still in progress.\\n\");\n>  \t\t} else {\n>  \t\t\tstrbuf_reset(&buf);\n>  \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n\nAs much of an improvement this is, I'd be no closer to knowing what I\nshould do at this point.\n\nShould I \"rm -rf\" that directory, downgrade my version of git if I'd\nlike to recover my work (as the message alludes to).\n\nIn either case I'd think that this is getting a bit past the length\nwhere we'd have just a die() v.s. splitting it into a die()/advise()\npair. I.e. to have the advise() carry some bullet-point list about X/Y/Z\nsolutions, with the die() being a brief ~\"we did because xyz dir is\nstill here\".\n\n\n"},{"id":"449141","messageId":"220222.868ru27vtt.gmgdl@evledraar.gmail.com","threadId":"57457","inReplyTo":"eb5871db95b12500cc0a6b8b0e3a82ed9e8fcfbd.1645526016.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] rebase: `preserve` is also a pull option, tell dying users","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-02-22T15:34:56Z","receivedAt":"2022-02-22T15:41:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Feb 22 2022, Philip Oakley via GitGitGadget wrote:\n\n> From: Philip Oakley <philipoakley@iee.email>\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 direct users to\n> this option and it's locations.\n>\n> Signed-off-by: Philip Oakley <philipoakley@iee.email>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>  builtin/rebase.c | 5 ++++-\n>  1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 07221d0ae41..97f704bb297 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1205,7 +1205,10 @@ 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\"Also, check your `pull` configuration settings\\n\"\n> +\t\t\t\"`git config --show-scope --show-origin --get-regexp 'pull.*'`\\n\"\n> +\t\t\t\"which may also invoke this option.\"));\n\nI may be missing some subtlety, but how is the user ever going to need\nto check their config?\n\nAfter 52f1e82178e (pull: remove support for `--rebase=preserve`,\n2021-09-07) we:\n\n    $ git -c pull.rebase=preserve pull\n    error: rebase.c:29: preserve: 'preserve' superseded by 'merges'\n    fatal: builtin/pull.c:45: Invalid value for pull.rebase: preserve\n\nI.e. we'd error before this, and the \"preserve_merges_selected\" variable\nbeing checked here is not affected by config, i.e. we only ever got to\nthis \"via config\" route if \"pull\" et al was invoking us.\n\nBut now that command dies.\n\nIf there is still a codepath where we call \"rebase --preserve-merges\" on\nthe basis of config that I've missed, shouldn't this die() be happening\nthere?\n"},{"id":"449146","messageId":"3e144b59-dce2-ce10-cd9b-eca92eee922c@iee.email","threadId":"57457","inReplyTo":"220222.868ru27vtt.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/2] rebase: `preserve` is also a pull option, tell dying users","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-02-22T15:56:05Z","receivedAt":"2022-02-22T15:56:15Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 22/02/2022 15:34, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Feb 22 2022, Philip Oakley via GitGitGadget wrote:\n>\n>> From: Philip Oakley <philipoakley@iee.email>\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 direct users to\n>> this option and it's locations.\n>>\n>> Signed-off-by: Philip Oakley <philipoakley@iee.email>\n>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>> ---\n>>  builtin/rebase.c | 5 ++++-\n>>  1 file changed, 4 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>> index 07221d0ae41..97f704bb297 100644\n>> --- a/builtin/rebase.c\n>> +++ b/builtin/rebase.c\n>> @@ -1205,7 +1205,10 @@ 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\"Also, check your `pull` configuration settings\\n\"\n>> +\t\t\t\"`git config --show-scope --show-origin --get-regexp 'pull.*'`\\n\"\n>> +\t\t\t\"which may also invoke this option.\"));\n> I may be missing some subtlety, but how is the user ever going to need\n> to check their config?\n>\n> After 52f1e82178e (pull: remove support for `--rebase=preserve`,\n> 2021-09-07) we:\n>\n>     $ git -c pull.rebase=preserve pull\n>     error: rebase.c:29: preserve: 'preserve' superseded by 'merges'\n>     fatal: builtin/pull.c:45: Invalid value for pull.rebase: preserve\n>\n> I.e. we'd error before this, and the \"preserve_merges_selected\" variable\n> being checked here is not affected by config, i.e. we only ever got to\n> this \"via config\" route if \"pull\" et al was invoking us.\n>\n> But now that command dies.\n>\n> If there is still a codepath where we call \"rebase --preserve-merges\" on\n> the basis of config that I've missed, shouldn't this die() be happening\n> there?\nHi Ævar,\n\nThe preserve merges is a pull config item, still supported by Visual\nStudio, so could easily be set for many users. When they click on the\n\"Pull\" menu and get the old die() fatal: message they won't know why\nthis happened or how to resolve it. Hence the extra info.\n\nIf the user was actually running a rebase command then, yes, they\nwould/should be reasonably OK with the original error message telling\nthem what was wrong with their command line. But a `git pull`?\nespecially as the manual says nothing about preserve merges any more.\n\nIf we are to remove a capability the `give up and die` messages should\nbe helpful in the edge cases. The issue (needing to check the pull, as\nwell as the rebase configs) has already shown on the git-users list. It\ntook a while to workout how it all happened.\n\nNot sure if I've covered your concerns properly, but this is one of the\nplaces that can and warn the user.\n\nPhilip\n"},{"id":"449148","messageId":"c04eba64-0357-cfdd-2ffe-3905ce4f4cb9@iee.email","threadId":"57457","inReplyTo":"220222.86czje7w4i.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/2] rebase: help user when dying with preserve-merges`","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-02-22T16:06:10Z","receivedAt":"2022-02-22T16:06:15Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 22/02/2022 15:32, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Feb 22 2022, Philip Oakley via GitGitGadget wrote:\n>\n>> From: Philip Oakley <philipoakley@iee.email>\n>>\n>> Git will die if a \"rebase --preserve-merges\" is in progress.\n>> Users cannot --quit, --abort or --continue the rebase.\n>>\n>> This sceario can occur if the user updates their Git, or switches\n>> to another newer version, after starting a preserve-merges rebase,\n>> commonly via the pull setting.\n>>\n>> One trigger is 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 cause, i.e. the existence of the directory.\n>> The problem must be resolved manually, `git rebase --<option>`\n>> commands will die, or the user must downgrade. Also, note that\n>> the deleted options are no longer shown in the documentation.\n> I can go and read the linked thread for the answer, but:\n>\n>>  \t\tif (is_directory(buf.buf)) {\n>> -\t\t\tdie(\"`rebase -p` is no longer supported\");\n>> +\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n>> +\t\t\t\"You still have a `.git/rebase-merge/rewritten` directory, \\n\"\n>> +\t\t\t\"indicating a `rebase preserve-merge` is still in progress.\\n\");\n>>  \t\t} else {\n>>  \t\t\tstrbuf_reset(&buf);\n>>  \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n> As much of an improvement this is, I'd be no closer to knowing what I\n> should do at this point.\n>\n> Should I \"rm -rf\" that directory, downgrade my version of git if I'd\n> like to recover my work (as the message alludes to).\n>\n> In either case I'd think that this is getting a bit past the length\n> where we'd have just a die() v.s. splitting it into a die()/advise()\n> pair. I.e. to have the advise() carry some bullet-point list about X/Y/Z\n> solutions, with the die() being a brief ~\"we did because xyz dir is\n> still here\".\n>\n>\nHi Ævar,\n\nExactly. This is a slightly special, but real, case. The previous\nmessage was essentially totally opaque to users. An \"If I were you I\nwouldn't start from here\" response is somewhat true, so we simply tell\nthe user how they got to receive the fatal message. They can then take\nany of the options they choose.\n\nUltimately the user downgraded and managed to use \"rebase --continue\",\nas advised by Git, without the response \"fatal:\" to complete their old\npreserve-merges rebase.\n\nThey'll hit a similar fault in short order because when they next `pull`\nthey'll be slipped into trying the preserve-merge rebase again - that's\nthe 2/2 patch, making sure they know why.\n\nPhilip\n"},{"id":"449174","messageId":"c5197f9d-11eb-5f91-ce33-c196069dc2d8@iee.email","threadId":"57457","inReplyTo":"3e144b59-dce2-ce10-cd9b-eca92eee922c@iee.email","subject":"Re: [PATCH 2/2] rebase: `preserve` is also a pull option, tell dying users","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-02-22T17:48:58Z","receivedAt":"2022-02-22T17:49:01Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Ævar,\n\nOn 22/02/2022 15:56, Philip Oakley wrote:\n> On 22/02/2022 15:34, Ævar Arnfjörð Bjarmason wrote:\n>> On Tue, Feb 22 2022, Philip Oakley via GitGitGadget wrote:\n>>\n>>> From: Philip Oakley <philipoakley@iee.email>\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 direct users to\n>>> this option and it's locations.\n>>>\n>>> Signed-off-by: Philip Oakley <philipoakley@iee.email>\n>>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>>> ---\n>>>  builtin/rebase.c | 5 ++++-\n>>>  1 file changed, 4 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>>> index 07221d0ae41..97f704bb297 100644\n>>> --- a/builtin/rebase.c\n>>> +++ b/builtin/rebase.c\n>>> @@ -1205,7 +1205,10 @@ 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\"Also, check your `pull` configuration settings\\n\"\n>>> +\t\t\t\"`git config --show-scope --show-origin --get-regexp 'pull.*'`\\n\"\n>>> +\t\t\t\"which may also invoke this option.\"));\n>> I may be missing some subtlety, but how is the user ever going to need\n>> to check their config?\n>>\n>> After 52f1e82178e (pull: remove support for `--rebase=preserve`,\n>> 2021-09-07) we:\n>>\n>>     $ git -c pull.rebase=preserve pull\n>>     error: rebase.c:29: preserve: 'preserve' superseded by 'merges'\n>>     fatal: builtin/pull.c:45: Invalid value for pull.rebase: preserve\n>>\n>> I.e. we'd error before this, and the \"preserve_merges_selected\" variable\n>> being checked here is not affected by config, i.e. we only ever got to\n>> this \"via config\" route if \"pull\" et al was invoking us.\n>>\n>> But now that command dies.\n>>\n>> If there is still a codepath where we call \"rebase --preserve-merges\" on\n>> the basis of config that I've missed, shouldn't this die() be happening\n>> there?\n> Hi Ævar,\n>\n> The preserve merges is a pull config item, still supported by Visual\n> Studio, so could easily be set for many users. When they click on the\n> \"Pull\" menu and get the old die() fatal: message they won't know why\n> this happened or how to resolve it. Hence the extra info.\n>\n> If the user was actually running a rebase command then, yes, they\n> would/should be reasonably OK with the original error message telling\n> them what was wrong with their command line. But a `git pull`?\n> especially as the manual says nothing about preserve merges any more.\n>\n> If we are to remove a capability the `give up and die` messages should\n> be helpful in the edge cases. The issue (needing to check the pull, as\n> well as the rebase configs) has already shown on the git-users list. It\n> took a while to workout how it all happened.\n>\n> Not sure if I've covered your concerns properly, but this is one of the\n> places that can and warn the user.\n>\n>\nI don't think that came out well. For this message, it's about reminding\nusers that they may *also* be using the `preserve` option within their\npull options, and not just within their rebase command's options.\n\nA case of \"don't use it here, and don't use it there either, please check\".\n\nPhilip\n"},{"id":"449187","messageId":"8b95ac6e-5e9d-38e4-4729-dbbe4b671ea8@gmail.com","threadId":"57457","inReplyTo":"pull.1155.git.1645526016.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Update the die() preserve-merges messages to help some users","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-02-22T18:55:14Z","receivedAt":"2022-02-22T18:55:24Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Philip\n\nOn 22/02/2022 10:33, Johannes Schindelin via GitGitGadget wrote:\n> This small update to the die() preserve-merges messages is a response to the\n> reported edge case in the Git-for-Windows googlegroups thread\n> [https://groups.google.com/g/git-for-windows/c/3jMWbBlXXHM] where even git\n> rebase --continue would die.\n> \n> It is most relevant for Windows because Visual Studio still offers the\n> option to run git pull --preserve, therefore Git for Windows already applied\n> these patches. The improvements are not specific to Windows, though, and\n> should therefore also get into core Git, albeit at a more leisurely pace.\n\nI think the new messages are an improvement, I was wondering how \ndifficult it would be to allow the user to run rebase --abort so they \ncan at least easily start again with --rebase-merges.\n\nBest Wishes\n\nPhillip\n\n> This is a companion patch series to\n> https://github.com/git-for-windows/git/pull/3708\n> \n> Philip Oakley (2):\n>    rebase: help user when dying with preserve-merges`\n>    rebase: `preserve` is also a pull option, tell dying users\n> \n>   builtin/rebase.c | 9 +++++++--\n>   1 file changed, 7 insertions(+), 2 deletions(-)\n> \n> \n> base-commit: e6ebfd0e8cbbd10878070c8a356b5ad1b3ca464e\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1155%2Fdscho%2Fdie_preserve-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1155/dscho/die_preserve-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1155\n\n"},{"id":"449193","messageId":"df47bb75-9684-6896-1e5b-21406c4a549a@iee.email","threadId":"57457","inReplyTo":"8b95ac6e-5e9d-38e4-4729-dbbe4b671ea8@gmail.com","subject":"Re: [PATCH 0/2] Update the die() preserve-merges messages to help some users","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-02-22T20:24:58Z","receivedAt":"2022-02-22T20:25:05Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Phillip,\n\nOn 22/02/2022 18:55, Phillip Wood wrote:\n> Hi Philip\n>\n> On 22/02/2022 10:33, Johannes Schindelin via GitGitGadget wrote:\n>> This small update to the die() preserve-merges messages is a response\n>> to the\n>> reported edge case in the Git-for-Windows googlegroups thread\n>> [https://groups.google.com/g/git-for-windows/c/3jMWbBlXXHM] where\n>> even git\n>> rebase --continue would die.\n>>\n>> It is most relevant for Windows because Visual Studio still offers the\n>> option to run git pull --preserve, therefore Git for Windows already\n>> applied\n>> these patches. The improvements are not specific to Windows, though, and\n>> should therefore also get into core Git, albeit at a more leisurely\n>> pace.\n>\n> I think the new messages are an improvement, I was wondering how\n> difficult it would be to allow the user to run rebase --abort so they\n> can at least easily start again with --rebase-merges.\n\nIn this case, the user (another Phillip), couldn't run `rebase\n--continue` without getting a `fatal:` report. A code inspection showed\nthat was one of the first tests so I don't believe they could run\n`--quit` or `abort` either!\n\nWe eventually nailed it down to being an update of Git, after getting\ninto a bad conflict resolution, so the update refused to do any rebase\ncommands! Later they tried downgrading and finishing the rebase that way\nin the usual 'hack & hope' way. Luckily they had a backup from the time\nof the update, which was able to confirm the presence of the indicative\ndirectory (which is an implementation detail).\n\nIf the `rebase abort` was moved earlier in the code, then it might work,\nbut we'd still need to keep the clean-up code for a non-existent option,\nwhich is less than ideal .\n\nPhilip\n\n>\n> Best Wishes\n>\n> Phillip\n>\n>> This is a companion patch series to\n>> https://github.com/git-for-windows/git/pull/3708\n>>\n>> Philip Oakley (2):\n>>    rebase: help user when dying with preserve-merges`\n>>    rebase: `preserve` is also a pull option, tell dying users\n>>\n>>   builtin/rebase.c | 9 +++++++--\n>>   1 file changed, 7 insertions(+), 2 deletions(-)\n>>\n>>\n>> base-commit: e6ebfd0e8cbbd10878070c8a356b5ad1b3ca464e\n>> Published-As:\n>> https://github.com/gitgitgadget/git/releases/tag/pr-1155%2Fdscho%2Fdie_preserve-v1\n>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git\n>> pr-1155/dscho/die_preserve-v1\n>> Pull-Request: https://github.com/gitgitgadget/git/pull/1155\n>\n\n"},{"id":"449232","messageId":"220223.86r17t6fvf.gmgdl@evledraar.gmail.com","threadId":"57457","inReplyTo":"c04eba64-0357-cfdd-2ffe-3905ce4f4cb9@iee.email","subject":"Re: [PATCH 1/2] rebase: help user when dying with preserve-merges`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-02-23T10:20:28Z","receivedAt":"2022-02-23T10:23:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Feb 22 2022, Philip Oakley wrote:\n\n> On 22/02/2022 15:32, Ævar Arnfjörð Bjarmason wrote:\n>> On Tue, Feb 22 2022, Philip Oakley via GitGitGadget wrote:\n>>\n>>> From: Philip Oakley <philipoakley@iee.email>\n>>>\n>>> Git will die if a \"rebase --preserve-merges\" is in progress.\n>>> Users cannot --quit, --abort or --continue the rebase.\n>>>\n>>> This sceario can occur if the user updates their Git, or switches\n>>> to another newer version, after starting a preserve-merges rebase,\n>>> commonly via the pull setting.\n>>>\n>>> One trigger is 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 cause, i.e. the existence of the directory.\n>>> The problem must be resolved manually, `git rebase --<option>`\n>>> commands will die, or the user must downgrade. Also, note that\n>>> the deleted options are no longer shown in the documentation.\n>> I can go and read the linked thread for the answer, but:\n>>\n>>>  \t\tif (is_directory(buf.buf)) {\n>>> -\t\t\tdie(\"`rebase -p` is no longer supported\");\n>>> +\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n>>> +\t\t\t\"You still have a `.git/rebase-merge/rewritten` directory, \\n\"\n>>> +\t\t\t\"indicating a `rebase preserve-merge` is still in progress.\\n\");\n>>>  \t\t} else {\n>>>  \t\t\tstrbuf_reset(&buf);\n>>>  \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n>> As much of an improvement this is, I'd be no closer to knowing what I\n>> should do at this point.\n>>\n>> Should I \"rm -rf\" that directory, downgrade my version of git if I'd\n>> like to recover my work (as the message alludes to).\n>>\n>> In either case I'd think that this is getting a bit past the length\n>> where we'd have just a die() v.s. splitting it into a die()/advise()\n>> pair. I.e. to have the advise() carry some bullet-point list about X/Y/Z\n>> solutions, with the die() being a brief ~\"we did because xyz dir is\n>> still here\".\n>>\n>>\n> Hi Ævar,\n>\n> Exactly. This is a slightly special, but real, case. The previous\n> message was essentially totally opaque to users. An \"If I were you I\n> wouldn't start from here\" response is somewhat true, so we simply tell\n> the user how they got to receive the fatal message. They can then take\n> any of the options they choose.\n>\n> Ultimately the user downgraded and managed to use \"rebase --continue\",\n> as advised by Git, without the response \"fatal:\" to complete their old\n> preserve-merges rebase.\n\nRight. I'm pointing out that in this proposed version of the die()\nmessage we stop just short of actually telling the user how to proceed.\n\nI.e. just that they have a X directory, not that they should either\nremove X and lose their work, or downgrade git, proceed, and then\nupgrade git.\n\n> They'll hit a similar fault in short order because when they next `pull`\n> they'll be slipped into trying the preserve-merge rebase again - that's\n> the 2/2 patch, making sure they know why.\n\nWell, this is \"rebase\". You can have been running rebases in a\nrepository without ever having any interactions with remotes.\n\nAnd even if you had interactions with remotes you might be doing so via\n\"git fetch\" followed by \"git rebase\", and might not ever invoke \"git\npull\".\n\nAnd even if you did a \"git pull\" later shouldn't the error you got here\nbe sufficiently stand-alone as to tell you what to do, without needing a\nlater \"pull\"?\n"},{"id":"449233","messageId":"220223.86mtih6fai.gmgdl@evledraar.gmail.com","threadId":"57457","inReplyTo":"c5197f9d-11eb-5f91-ce33-c196069dc2d8@iee.email","subject":"Re: [PATCH 2/2] rebase: `preserve` is also a pull option, tell dying users","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-02-23T10:27:32Z","receivedAt":"2022-02-23T10:36:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Feb 22 2022, Philip Oakley wrote:\n\n> Hi Ævar,\n>\n> On 22/02/2022 15:56, Philip Oakley wrote:\n>> On 22/02/2022 15:34, Ævar Arnfjörð Bjarmason wrote:\n>>> On Tue, Feb 22 2022, Philip Oakley via GitGitGadget wrote:\n>>>\n>>>> From: Philip Oakley <philipoakley@iee.email>\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 direct users to\n>>>> this option and it's locations.\n>>>>\n>>>> Signed-off-by: Philip Oakley <philipoakley@iee.email>\n>>>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>>>> ---\n>>>>  builtin/rebase.c | 5 ++++-\n>>>>  1 file changed, 4 insertions(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>>>> index 07221d0ae41..97f704bb297 100644\n>>>> --- a/builtin/rebase.c\n>>>> +++ b/builtin/rebase.c\n>>>> @@ -1205,7 +1205,10 @@ 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\"Also, check your `pull` configuration settings\\n\"\n>>>> +\t\t\t\"`git config --show-scope --show-origin --get-regexp 'pull.*'`\\n\"\n>>>> +\t\t\t\"which may also invoke this option.\"));\n>>> I may be missing some subtlety, but how is the user ever going to need\n>>> to check their config?\n>>>\n>>> After 52f1e82178e (pull: remove support for `--rebase=preserve`,\n>>> 2021-09-07) we:\n>>>\n>>>     $ git -c pull.rebase=preserve pull\n>>>     error: rebase.c:29: preserve: 'preserve' superseded by 'merges'\n>>>     fatal: builtin/pull.c:45: Invalid value for pull.rebase: preserve\n>>>\n>>> I.e. we'd error before this, and the \"preserve_merges_selected\" variable\n>>> being checked here is not affected by config, i.e. we only ever got to\n>>> this \"via config\" route if \"pull\" et al was invoking us.\n>>>\n>>> But now that command dies.\n>>>\n>>> If there is still a codepath where we call \"rebase --preserve-merges\" on\n>>> the basis of config that I've missed, shouldn't this die() be happening\n>>> there?\n>> Hi Ævar,\n>>\n>> The preserve merges is a pull config item, still supported by Visual\n>> Studio, so could easily be set for many users. When they click on the\n>> \"Pull\" menu and get the old die() fatal: message they won't know why\n>> this happened or how to resolve it. Hence the extra info.\n>>\n>> If the user was actually running a rebase command then, yes, they\n>> would/should be reasonably OK with the original error message telling\n>> them what was wrong with their command line. But a `git pull`?\n>> especially as the manual says nothing about preserve merges any more.\n>>\n>> If we are to remove a capability the `give up and die` messages should\n>> be helpful in the edge cases. The issue (needing to check the pull, as\n>> well as the rebase configs) has already shown on the git-users list. It\n>> took a while to workout how it all happened.\n>>\n>> Not sure if I've covered your concerns properly, but this is one of the\n>> places that can and warn the user.\n>>\n>>\n> I don't think that came out well. For this message, it's about reminding\n> users that they may *also* be using the `preserve` option within their\n> pull options, and not just within their rebase command's options.\n>\n> A case of \"don't use it here, and don't use it there either, please check\".\n\nI see. What I was confused with (and still find confusing) is that the\nerror here says \"[and the pull.rebase=preserve config] may also invoke\nthis option\".\n\nBut it doesn't do that at all, it *used to*, but now \"git pull\" will\npromptly fail on it as well.\n\nSo this is really just trying to tell us something like this, right?\n\n    Hey, you invoked --preserve-merges, bad mojo! Also: if you invoke\n    'git pull' and used the related config you may or may not have used,\n    that'll die too. So maybe check that out?\n\n:)\n\nAnyway, to the user the \"invoke this option\" isn't strictly accurate,\nbut they probably don't care/know the difference, so I don't mind\nper-se, I was just wondering if I was missing something.\n\nI do wonder if this part is needed at all. I.e. at this point this will\nonly benefit users who are *manually* invoking \"git rebase\n--preserve-merges\", everyone else (including that \"VS\" example you\nmentioned) will go through \"git pull\" and see the other error.\n\nIOW maybe just saying \"this option is an error now\" when we intercept\nthe option is enough, and ditto for \"this config is an error now\" when\nwe intercept the config, and not assume that a user using the option\nmust also be using the config is simpler/sufficient.\n\nJust my 0.02. For my part whahtever you decide to do here is fine by me,\nI was just wondering if I'd misundestood the whole flow around this\nlegacy option and how we invoke the small parts of die() boilerplate\nleft of it.\n"},{"id":"449277","messageId":"7cb42ab5-92c5-0a48-ffba-4f6b55ef130c@iee.email","threadId":"57457","inReplyTo":"220223.86r17t6fvf.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/2] rebase: help user when dying with preserve-merges`","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-02-23T17:44:34Z","receivedAt":"2022-02-23T17:44:41Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 23/02/2022 10:20, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Feb 22 2022, Philip Oakley wrote:\n>\n>> On 22/02/2022 15:32, Ævar Arnfjörð Bjarmason wrote:\n>>> On Tue, Feb 22 2022, Philip Oakley via GitGitGadget wrote:\n>>>\n>>>> From: Philip Oakley <philipoakley@iee.email>\n>>>>\n>>>> Git will die if a \"rebase --preserve-merges\" is in progress.\n>>>> Users cannot --quit, --abort or --continue the rebase.\n>>>>\n>>>> This sceario can occur if the user updates their Git, or switches\n>>>> to another newer version, after starting a preserve-merges rebase,\n>>>> commonly via the pull setting.\n>>>>\n>>>> One trigger is 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 cause, i.e. the existence of the directory.\n>>>> The problem must be resolved manually, `git rebase --<option>`\n>>>> commands will die, or the user must downgrade. Also, note that\n>>>> the deleted options are no longer shown in the documentation.\n>>> I can go and read the linked thread for the answer, but:\n>>>\n>>>>  \t\tif (is_directory(buf.buf)) {\n>>>> -\t\t\tdie(\"`rebase -p` is no longer supported\");\n>>>> +\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n>>>> +\t\t\t\"You still have a `.git/rebase-merge/rewritten` directory, \\n\"\n>>>> +\t\t\t\"indicating a `rebase preserve-merge` is still in progress.\\n\");\n>>>>  \t\t} else {\n>>>>  \t\t\tstrbuf_reset(&buf);\n>>>>  \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n>>> As much of an improvement this is, I'd be no closer to knowing what I\n>>> should do at this point.\n>>>\n>>> Should I \"rm -rf\" that directory, downgrade my version of git if I'd\n>>> like to recover my work (as the message alludes to).\n>>>\n>>> In either case I'd think that this is getting a bit past the length\n>>> where we'd have just a die() v.s. splitting it into a die()/advise()\n>>> pair. I.e. to have the advise() carry some bullet-point list about X/Y/Z\n>>> solutions, with the die() being a brief ~\"we did because xyz dir is\n>>> still here\".\n>>>\n>>>\n>> Hi Ævar,\n>>\n>> Exactly. This is a slightly special, but real, case. The previous\n>> message was essentially totally opaque to users. An \"If I were you I\n>> wouldn't start from here\" response is somewhat true, so we simply tell\n>> the user how they got to receive the fatal message. They can then take\n>> any of the options they choose.\n>>\n>> Ultimately the user downgraded and managed to use \"rebase --continue\",\n>> as advised by Git, without the response \"fatal:\" to complete their old\n>> preserve-merges rebase.\n> Right. I'm pointing out that in this proposed version of the die()\n> message we stop just short of actually telling the user how to proceed.\n>\n> I.e. just that they have a X directory, not that they should either\n> remove X and lose their work, or downgrade git, proceed, and then\n> upgrade git.\nIn a sense, that is it. They are in a difficult place, but with at least\na little information to seek further information and start making their\nchoices. Before, they (users in difficulty) were rather uninformed.\n>> They'll hit a similar fault in short order because when they next `pull`\n>> they'll be slipped into trying the preserve-merge rebase again - that's\n>> the 2/2 patch, making sure they know why.\n> Well, this is \"rebase\". You can have been running rebases in a\n> repository without ever having any interactions with remotes.\nTrue. That is a possibility. But we have also removed the preserve\noption for interaction with remotes as well.\n>\n> And even if you had interactions with remotes you might be doing so via\n> \"git fetch\" followed by \"git rebase\", and might not ever invoke \"git\n> pull\".\n>\n> And even if you did a \"git pull\" later shouldn't the error you got here\n> be sufficiently stand-alone as to tell you what to do, without needing a\n> later \"pull\"?\nWhy are we delaying telling the user that they would have problems there\nas well? It shouldn't be a game about how many ways we can trip up the user.\n\nIt's a pity the problem has split into the different ways into disaster.\n\n"},{"id":"450400","messageId":"8bd89179-e94b-cc13-6373-1aa9fb539e59@iee.email","threadId":"57457","inReplyTo":"220223.86mtih6fai.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/2] rebase: `preserve` is also a pull option, tell dying users","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-03-04T14:29:44Z","receivedAt":"2022-03-04T14:30:03Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Sorry for the delay. I've got some on-going family matters to attend to\nwhich have limited my availablilty.\n\nOn 23/02/2022 10:27, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Feb 22 2022, Philip Oakley wrote:\n>\n>> Hi Ævar,\n>>\n>> On 22/02/2022 15:56, Philip Oakley wrote:\n>>> On 22/02/2022 15:34, Ævar Arnfjörð Bjarmason wrote:\n>>>> On Tue, Feb 22 2022, Philip Oakley via GitGitGadget wrote:\n>>>>\n>>>>> From: Philip Oakley <philipoakley@iee.email>\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 direct users to\n>>>>> this option and it's locations.\n>>>>>\n>>>>> Signed-off-by: Philip Oakley <philipoakley@iee.email>\n>>>>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>>>>> ---\n>>>>>  builtin/rebase.c | 5 ++++-\n>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>>>>> index 07221d0ae41..97f704bb297 100644\n>>>>> --- a/builtin/rebase.c\n>>>>> +++ b/builtin/rebase.c\n>>>>> @@ -1205,7 +1205,10 @@ 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\"Also, check your `pull` configuration settings\\n\"\n>>>>> +\t\t\t\"`git config --show-scope --show-origin --get-regexp 'pull.*'`\\n\"\n>>>>> +\t\t\t\"which may also invoke this option.\"));\n>>>> I may be missing some subtlety, but how is the user ever going to need\n>>>> to check their config?\n>>>>\n>>>> After 52f1e82178e (pull: remove support for `--rebase=preserve`,\n>>>> 2021-09-07) we:\n>>>>\n>>>>     $ git -c pull.rebase=preserve pull\n>>>>     error: rebase.c:29: preserve: 'preserve' superseded by 'merges'\n>>>>     fatal: builtin/pull.c:45: Invalid value for pull.rebase: preserve\n>>>>\n>>>> I.e. we'd error before this, and the \"preserve_merges_selected\" variable\n>>>> being checked here is not affected by config, i.e. we only ever got to\n>>>> this \"via config\" route if \"pull\" et al was invoking us.\n>>>>\n>>>> But now that command dies.\n>>>>\n>>>> If there is still a codepath where we call \"rebase --preserve-merges\" on\n>>>> the basis of config that I've missed, shouldn't this die() be happening\n>>>> there?\n>>> Hi Ævar,\n>>>\n>>> The preserve merges is a pull config item, still supported by Visual\n>>> Studio, so could easily be set for many users. When they click on the\n>>> \"Pull\" menu and get the old die() fatal: message they won't know why\n>>> this happened or how to resolve it. Hence the extra info.\n>>>\n>>> If the user was actually running a rebase command then, yes, they\n>>> would/should be reasonably OK with the original error message telling\n>>> them what was wrong with their command line. But a `git pull`?\n>>> especially as the manual says nothing about preserve merges any more.\n>>>\n>>> If we are to remove a capability the `give up and die` messages should\n>>> be helpful in the edge cases. The issue (needing to check the pull, as\n>>> well as the rebase configs) has already shown on the git-users list. It\n>>> took a while to workout how it all happened.\n>>>\n>>> Not sure if I've covered your concerns properly, but this is one of the\n>>> places that can and warn the user.\n>>>\n>>>\n>> I don't think that came out well. For this message, it's about reminding\n>> users that they may *also* be using the `preserve` option within their\n>> pull options, and not just within their rebase command's options.\n>>\n>> A case of \"don't use it here, and don't use it there either, please check\".\n> I see. What I was confused with (and still find confusing) is that the\n> error here says \"[and the pull.rebase=preserve config] may also invoke\n> this option\".\n>\n> But it doesn't do that at all, it *used to*, but now \"git pull\" will\n> promptly fail on it as well.\n\nOk, That's probably my local idiom that confusing the now of the error\nand the past when the config would have been set up. I'll see if I can\nclarify that better.\n> So this is really just trying to tell us something like this, right?\n>\n>     Hey, you invoked --preserve-merges, bad mojo! Also: if you invoke\n>     'git pull' and used the related config you may or may not have used,\n>     that'll die too. So maybe check that out?\n>\n> :)\n>\n> Anyway, to the user the \"invoke this option\" isn't strictly accurate,\n> but they probably don't care/know the difference, so I don't mind\n> per-se, I was just wondering if I was missing something.\n>\n> I do wonder if this part is needed at all. I.e. at this point this will\n> only benefit users who are *manually* invoking \"git rebase\n> --preserve-merges\", everyone else (including that \"VS\" example you\n> mentioned) will go through \"git pull\" and see the other error.\n>\n> IOW maybe just saying \"this option is an error now\" when we intercept\n> the option is enough, and ditto for \"this config is an error now\" when\n> we intercept the config, and not assume that a user using the option\n> must also be using the config is simpler/sufficient.\nError reporting is a tricky subject, especially when we add the human\nerror and misunderstanding aspects into the mix.\n>\n> Just my 0.02. For my part whahtever you decide to do here is fine by me,\n> I was just wondering if I'd misundestood the whole flow around this\n> legacy option and how we invoke the small parts of die() boilerplate\n> left of it.\nYep, it is tricky talking about disappeared options, the error messages,\nand the multiple ways that show themselves. In the originating edge case\nit wasn't helped by a Git advice message that contained the failing\ncommand line (`git rebase --continue`) suggestion. In general I was\ntrying to cover all the possible sources in all the places the 'error'\nmay occur.\n\nI'll have another look at the ways these edge cases could appear, and\ntry an improve the commit message explanations where the diff doesn't\nshow sufficient context. It'll be at least next week.\n\nPhilip\n"},{"id":"450610","messageId":"nycvar.QRO.7.76.6.2203071742240.11118@tvgsbejvaqbjf.bet","threadId":"57457","inReplyTo":"8bd89179-e94b-cc13-6373-1aa9fb539e59@iee.email","subject":"Re: [PATCH 2/2] rebase: `preserve` is also a pull option, tell dying users","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-03-07T16:43:13Z","receivedAt":"2022-03-07T16:43:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\nOn Fri, 4 Mar 2022, Philip Oakley wrote:\n\n> I'll have another look at the ways these edge cases could appear, and\n> try an improve the commit message explanations where the diff doesn't\n> show sufficient context. It'll be at least next week.\n\nCan I punt this patch series back to you? (I sent it upstream on your\nbehalf because I had assumed that you'd want me to, sorry for\nmisunderstanding your intentions.)\n\nCiao,\nDscho\n"},{"id":"450673","messageId":"fd18d7bf-53b6-50ca-12e7-ac66e8fe75da@iee.email","threadId":"57457","inReplyTo":"nycvar.QRO.7.76.6.2203071742240.11118@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/2] rebase: `preserve` is also a pull option, tell dying users","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-03-07T23:26:56Z","receivedAt":"2022-03-07T23:27:12Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Dscho,\n\nOn 07/03/2022 16:43, Johannes Schindelin wrote:\n> Hi Philip,\n>\n> On Fri, 4 Mar 2022, Philip Oakley wrote:\n>\n>> I'll have another look at the ways these edge cases could appear, and\n>> try an improve the commit message explanations where the diff doesn't\n>> show sufficient context. It'll be at least next week.\n> Can I punt this patch series back to you? (I sent it upstream on your\n> behalf because I had assumed that you'd want me to, sorry for\n> misunderstanding your intentions.)\n>\nI'm happy to continue to work on this series, and am grateful for your\nsupport in pushing it through GGG. How is it best to transfer the\n'ownership' at GGG?\n\nI've still got some family issues so it'll be later in the week, or even\nnext week before I can update the series.\n\nThanks.\nPhilip\n"},{"id":"450732","messageId":"220308.86zgm0wo1r.gmgdl@evledraar.gmail.com","threadId":"57457","inReplyTo":"7cb42ab5-92c5-0a48-ffba-4f6b55ef130c@iee.email","subject":"Re: [PATCH 1/2] rebase: help user when dying with preserve-merges`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-08T13:45:59Z","receivedAt":"2022-03-08T13:51:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Feb 23 2022, Philip Oakley wrote:\n\n> On 23/02/2022 10:20, Ævar Arnfjörð Bjarmason wrote:\n>> On Tue, Feb 22 2022, Philip Oakley wrote:\n>>\n>>> On 22/02/2022 15:32, Ævar Arnfjörð Bjarmason wrote:\n>>>> On Tue, Feb 22 2022, Philip Oakley via GitGitGadget wrote:\n>>>>\n>>>>> From: Philip Oakley <philipoakley@iee.email>\n>>>>>\n>>>>> Git will die if a \"rebase --preserve-merges\" is in progress.\n>>>>> Users cannot --quit, --abort or --continue the rebase.\n>>>>>\n>>>>> This sceario can occur if the user updates their Git, or switches\n>>>>> to another newer version, after starting a preserve-merges rebase,\n>>>>> commonly via the pull setting.\n>>>>>\n>>>>> One trigger is 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 cause, i.e. the existence of the directory.\n>>>>> The problem must be resolved manually, `git rebase --<option>`\n>>>>> commands will die, or the user must downgrade. Also, note that\n>>>>> the deleted options are no longer shown in the documentation.\n>>>> I can go and read the linked thread for the answer, but:\n>>>>\n>>>>>  \t\tif (is_directory(buf.buf)) {\n>>>>> -\t\t\tdie(\"`rebase -p` is no longer supported\");\n>>>>> +\t\t\tdie(\"`rebase --preserve-merges` (-p) is no longer supported.\\n\"\n>>>>> +\t\t\t\"You still have a `.git/rebase-merge/rewritten` directory, \\n\"\n>>>>> +\t\t\t\"indicating a `rebase preserve-merge` is still in progress.\\n\");\n>>>>>  \t\t} else {\n>>>>>  \t\t\tstrbuf_reset(&buf);\n>>>>>  \t\t\tstrbuf_addf(&buf, \"%s/interactive\", merge_dir());\n>>>> As much of an improvement this is, I'd be no closer to knowing what I\n>>>> should do at this point.\n>>>>\n>>>> Should I \"rm -rf\" that directory, downgrade my version of git if I'd\n>>>> like to recover my work (as the message alludes to).\n>>>>\n>>>> In either case I'd think that this is getting a bit past the length\n>>>> where we'd have just a die() v.s. splitting it into a die()/advise()\n>>>> pair. I.e. to have the advise() carry some bullet-point list about X/Y/Z\n>>>> solutions, with the die() being a brief ~\"we did because xyz dir is\n>>>> still here\".\n>>>>\n>>>>\n>>> Hi Ævar,\n>>>\n>>> Exactly. This is a slightly special, but real, case. The previous\n>>> message was essentially totally opaque to users. An \"If I were you I\n>>> wouldn't start from here\" response is somewhat true, so we simply tell\n>>> the user how they got to receive the fatal message. They can then take\n>>> any of the options they choose.\n>>>\n>>> Ultimately the user downgraded and managed to use \"rebase --continue\",\n>>> as advised by Git, without the response \"fatal:\" to complete their old\n>>> preserve-merges rebase.\n>> Right. I'm pointing out that in this proposed version of the die()\n>> message we stop just short of actually telling the user how to proceed.\n>>\n>> I.e. just that they have a X directory, not that they should either\n>> remove X and lose their work, or downgrade git, proceed, and then\n>> upgrade git.\n> In a sense, that is it. They are in a difficult place, but with at least\n> a little information to seek further information and start making their\n> choices. Before, they (users in difficulty) were rather uninformed.\n\nYes, it's definitely an improvement. I'm just wondering if we can tell\nthem a bit more so that they're not needing to search \"what do do about\n.git/rebase-merge/rewritten\" on Google/stackoverflow/whatever.\n\nI.e. can we just specifically say that they either need to downgrade,\ncontinue, upgrade, or alternatively rm -rf it, depending on wheher\nthey'd like to not lose their work, or if that's OK?\n\n>>> They'll hit a similar fault in short order because when they next `pull`\n>>> they'll be slipped into trying the preserve-merge rebase again - that's\n>>> the 2/2 patch, making sure they know why.\n>> Well, this is \"rebase\". You can have been running rebases in a\n>> repository without ever having any interactions with remotes.\n> True. That is a possibility. But we have also removed the preserve\n> option for interaction with remotes as well.\n\n*Nod*, presumably you mean its removal from \"builtin/pull.c\" etc.\n>>\n>> And even if you had interactions with remotes you might be doing so via\n>> \"git fetch\" followed by \"git rebase\", and might not ever invoke \"git\n>> pull\".\n>>\n>> And even if you did a \"git pull\" later shouldn't the error you got here\n>> be sufficiently stand-alone as to tell you what to do, without needing a\n>> later \"pull\"?\n> Why are we delaying telling the user that they would have problems there\n> as well? It shouldn't be a game about how many ways we can trip up the user.\n>\n> It's a pity the problem has split into the different ways into disaster.\n\nI'm just saying that the reason the message is a bit confusing is\nbecause we *are* delaying it. I.e. we could give a more specific error\nmessage in builtin/pull.c, but the \"preserve merges\" scaffolding was\nalready removed from it.\n\nWhich is why we're doing it in builtin/rebase.c, and suggesting \"tweak\nyour pull settings\", to a user who may or may not have arrived there\nafter a \"pull\" (and may not be using remotes at all).\n\nI think overall that's OK in this case, i.e. this fix shouldn't be held\nup by putting that scaffolding in place in builtin/pull.c again to\nslightly improve the error.\n\nBut perhaps in the 2/2 error just say instead: \"if you arrived here via\n'git pull' ....\". I.e. no extra code, just a slight tweak to the\nmessage, and any user seeing it by invoking \"rebase\" directly won't be\nfurther confused about why the error is saying something about \"pull\"\n(which they didn't run).\n"},{"id":"450833","messageId":"nycvar.QRO.7.76.6.2203091317220.357@tvgsbejvaqbjf.bet","threadId":"57457","inReplyTo":"fd18d7bf-53b6-50ca-12e7-ac66e8fe75da@iee.email","subject":"Re: [PATCH 2/2] rebase: `preserve` is also a pull option, tell dying users","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-03-09T12:18:22Z","receivedAt":"2022-03-09T12:18:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\nOn Mon, 7 Mar 2022, Philip Oakley wrote:\n\n> On 07/03/2022 16:43, Johannes Schindelin wrote:\n> > Hi Philip,\n> >\n> > On Fri, 4 Mar 2022, Philip Oakley wrote:\n> >\n> >> I'll have another look at the ways these edge cases could appear, and\n> >> try an improve the commit message explanations where the diff doesn't\n> >> show sufficient context. It'll be at least next week.\n> > Can I punt this patch series back to you? (I sent it upstream on your\n> > behalf because I had assumed that you'd want me to, sorry for\n> > misunderstanding your intentions.)\n> >\n> I'm happy to continue to work on this series, and am grateful for your\n> support in pushing it through GGG. How is it best to transfer the\n> 'ownership' at GGG?\n\nSadly, I don't know of any way how I could transfer ownership to you, but\nmaybe you can just open a new one and reference the first thread in the\ncover letter?\n\n> I've still got some family issues so it'll be later in the week, or even\n> next week before I can update the series.\n\nSorry to hear that you have issues. These are tough times, and I feel for\nyou.\n\nCiao,\nDscho\n"}]}