{"thread":{"id":"61836","subject":"bug/defaults: COMMIT_EDITMSG not reused after a failed commit","startedAt":"2024-07-24T11:29:05Z","lastAt":"2024-07-25T15:21:21Z","messageCount":6,"participants":["Robert Coup","Junio C Hamano","Konstantin Ryabitsev","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"499251","messageId":"CAFLLRpJgpjJpNRC_UpZmUXF2626e0BiH8CkOkoMrX3zcrOp7YA@mail.gmail.com","threadId":"61836","inReplyTo":null,"subject":"bug/defaults: COMMIT_EDITMSG not reused after a failed commit","fromName":"Robert Coup","fromEmail":"robert.coup@koordinates.com","sentAt":"2024-07-24T11:28:47Z","receivedAt":"2024-07-24T11:29:05Z","isPatch":false,"sender":{"key":"robert.coup@koordinates.com","avatar":"https://gravatar.com/avatar/d1a87d63ffb562b791992d8a119ebbdd742e703109d23333ca3fca51306ee95c?d=mp&s=160"},"body":"Hi,\n\n* What did you do before the bug happened? (Steps to reproduce your issue)\n\nI have commit signing configured, using 1Password as an ssh-signer.\n\n    [gpg]\n        format = ssh\n\n    [gpg \"ssh\"]\n        program = \"/Applications/1Password.app/Contents/MacOS/op-ssh-sign\"\n\nSometimes signing a commit fails, because 1PW has updated and needs restarted\nor something, I haven't been motivated to figure it out. But that's not the\nissue, there could be many reasons a commit fails.\n\n    $ git commit\n    <editor opens, write beautiful commit message prose>\n\n    error: 1Password: Could not connect to socket. Is the agent running?\n\n    fatal: failed to write commit object\n\nIf I restart 1Password, and do `git commit` again (signing works), my previous\ncommit message is wiped and I need to start afresh.\n\n* What did you expect to happen? (Expected behavior)\n\nIf a commit fails for whatever reason, I expect to be able to re-commit and it\nwould reuse my carefully crafted message.\n\n* What happened instead? (Actual behavior)\n\nIt got dropped on the floor and I needed to rewrite the whole thing.\n\n* What's different between what you expected and what actually happened?\n* Anything else you want to add:\n\nThe git-commit docs state:\n\n    $GIT_DIR/COMMIT_EDITMSG\n        This file contains the commit message of a commit in progress. If\n        git commit exits due to an error before creating a commit, any\n        commit message that has been provided by the user (e.g., in an\n        editor session) will be available in this file, but will be\n        overwritten by the next invocation of git commit.\n\nSo, yes, technically I can copy it or use `git commit -F .git/COMMIT_EDITMSG`\nbut this seems like a distinctly unhelpful default behaviour to me. Thoughts:\n\n- descriptive commit messages are good\n- most commits succeed\n- if a commit fails, the user _wanted_ to commit, so why would they _want_ to\n  drop the message they've previously written?\n- even if a commit fails and the user does something completely different and\n  now wants a different message, deleting a chunk of text is vastly easier than\n  rewriting something.\n\nWhile successful commits also leave the previous commit message in\n.git/COMMIT_EDITMSG, this is _not_ documented behaviour (it only talks about a\n\"commit in progress\"); so feels like we could change it:\n\n1. delete COMMIT_EDITMSG on success\n2. reopen COMMIT_EDITMSG on commit if it exists. Maybe logging something like\n   \"Restoring previous in-progress commit message...\" might explain what's\n   happening.\n3. if COMMIT_EDITMSG doesn't exist, re-populate from the template before opening\n   the editor. We could also do this for \"parsed-as-empty\" commit messages.\n\nI just don't see any particular upside to the current default behaviour?\n\nRob :)\n\n\n[System Info]\ngit version:\ngit version 2.39.3 (Apple Git-146)\ncpu: arm64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nfeature: fsmonitor--daemon\nuname: Darwin 23.5.0 Darwin Kernel Version 23.5.0: Wed May  1 20:12:58\nPDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T6000 arm64\ncompiler info: clang: 15.0.0 (clang-1500.3.9.4)\nlibc info: no libc information available\n$SHELL (typically, interactive shell): /opt/homebrew/bin/zsh\n\n[Enabled Hooks]\n"},{"id":"499281","messageId":"xmqq1q3iyceq.fsf@gitster.g","threadId":"61836","inReplyTo":"CAFLLRpJgpjJpNRC_UpZmUXF2626e0BiH8CkOkoMrX3zcrOp7YA@mail.gmail.com","subject":"Re: bug/defaults: COMMIT_EDITMSG not reused after a failed commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-24T16:37:01Z","receivedAt":"2024-07-24T16:37:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robert Coup <robert.coup@koordinates.com> writes:\n\n> 1. delete COMMIT_EDITMSG on success\n>\n> 2. reopen COMMIT_EDITMSG on commit if it exists. Maybe logging something like\n>    \"Restoring previous in-progress commit message...\" might explain what's\n>    happening.\n> 3. if COMMIT_EDITMSG doesn't exist, re-populate from the template before opening\n>    the editor. We could also do this for \"parsed-as-empty\" commit messages.\n\nUnconditionally doing this change would be disruptive to workflows\nof existing users.  To them, Git left COMMIT_EDITMSG available even\nafter the commit to them almost forever, but suddenly it stops doing\nso.  Like \"git cherry-pick|rebase|revert\" that got stopped can be\nrestarted _with_ some state information with \"--continue\", offering\nthis as an optional feature might be a possibility, but I haven't\nthought things through.\n\nAn obvious and a lot more lightweight first step is to make it clear\n(perhaps in the error message after a failed commit---after all,\nsuch a failure from \"git commit\" should be a rare event) where you\ncan resurrect the draft commit message from.  That is independent\nand orthogonal to the \"let's reuse COMMIT_EDITMSG file\" change.\n\nA similar issue was reported a few years ago but without any\nresponse or action.\n\nhttps://lore.kernel.org/git/CAJ2_uEOk8xoLvK8B8PYc0_=kA8W_LqKwGyhKghemQDdRzA2nFA@mail.gmail.com/\n\nLet's see if we find somebody interested in it this time.\n\nThanks.\n"},{"id":"499285","messageId":"20240724-cryptic-private-mustang-3f50aa@meerkat","threadId":"61836","inReplyTo":"xmqq1q3iyceq.fsf@gitster.g","subject":"Re: bug/defaults: COMMIT_EDITMSG not reused after a failed commit","fromName":"Konstantin Ryabitsev","fromEmail":"konstantin@linuxfoundation.org","sentAt":"2024-07-24T16:53:36Z","receivedAt":"2024-07-24T16:53:37Z","isPatch":false,"sender":{"key":"konstantin@linuxfoundation.org","avatar":"https://gravatar.com/avatar/7cb8827c6de56e1bd2dea16508c6708aa43feed3bf3813bcdacecdf96ceadd79?d=mp&s=160"},"body":"On Wed, Jul 24, 2024 at 09:37:01AM GMT, Junio C Hamano wrote:\n> > 1. delete COMMIT_EDITMSG on success\n> >\n> > 2. reopen COMMIT_EDITMSG on commit if it exists. Maybe logging something like\n> >    \"Restoring previous in-progress commit message...\" might explain what's\n> >    happening.\n> > 3. if COMMIT_EDITMSG doesn't exist, re-populate from the template before opening\n> >    the editor. We could also do this for \"parsed-as-empty\" commit messages.\n> \n> Unconditionally doing this change would be disruptive to workflows\n> of existing users.  To them, Git left COMMIT_EDITMSG available even\n> after the commit to them almost forever, but suddenly it stops doing\n> so.  Like \"git cherry-pick|rebase|revert\" that got stopped can be\n> restarted _with_ some state information with \"--continue\", offering\n> this as an optional feature might be a possibility, but I haven't\n> thought things through.\n> \n> An obvious and a lot more lightweight first step is to make it clear\n> (perhaps in the error message after a failed commit---after all,\n> such a failure from \"git commit\" should be a rare event) where you\n> can resurrect the draft commit message from.  That is independent\n> and orthogonal to the \"let's reuse COMMIT_EDITMSG file\" change.\n\nYes, I would say even doing the following would result in a better experience\nfor users who don't know about .git/COMMIT_EDITMSG:\n\n1. when git-commit fails, save the message as .git/FAILED_COMMIT_MSG\n2. output \"Commit message saved as .git/FAILED_COMMIT_MSG\"\n\n(exact wording/naming up for debate)\n\n-K\n"},{"id":"499299","messageId":"20240724210854.GB557365@coredump.intra.peff.net","threadId":"61836","inReplyTo":"20240724-cryptic-private-mustang-3f50aa@meerkat","subject":"Re: bug/defaults: COMMIT_EDITMSG not reused after a failed commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-07-24T21:08:54Z","receivedAt":"2024-07-24T21:08:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 24, 2024 at 12:53:36PM -0400, Konstantin Ryabitsev wrote:\n\n> Yes, I would say even doing the following would result in a better experience\n> for users who don't know about .git/COMMIT_EDITMSG:\n> \n> 1. when git-commit fails, save the message as .git/FAILED_COMMIT_MSG\n> 2. output \"Commit message saved as .git/FAILED_COMMIT_MSG\"\n\nI proposed something like (2) long ago. I'll reproduce the (rebased\nforward) patch below, but here's the original thread with a little bit\nof discussion:\n\n  https://lore.kernel.org/git/20120723185218.GC27588@sigill.intra.peff.net/\n\nIt just told you about COMMIT_EDITMSG, making it your responsibility to\nrecover it before running \"git commit\" again. Your (1) makes it a little\nnicer, in that you can run \"git commit\" and then pull the content from\nthe other file into your editor. Or we could even provide an option to\npre-populate the message with it.\n\nJunio was lukewarm on the original, so I'm not sure why I've been\nholding on to it all these years. But maybe it would help as a guide for\nanybody who wants to work on what you've proposed above.\n\n-- >8 --\nFrom: Jeff King <peff@peff.net>\nDate: Mon, 23 Jul 2012 14:52:18 -0400\nSubject: [PATCH] commit: give a hint when a commit message has been abandoned\n\nIf we launch an editor for the user to create a commit\nmessage, they may put significant work into doing so.\nTypically we try to check common mistakes that could cause\nthe commit to fail early, so that we die before the user\ngoes to the trouble.\n\nWe may still experience some errors afterwards, though; in\nthis case, the user is given no hint that their commit\nmessage has been saved. Let's tell them where it is.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/commit.c                          | 15 +++++++++++++++\n t/t7500-commit-template-squash-signoff.sh |  3 +--\n 2 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex dec78dfb86..42fefaa0e3 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -160,6 +160,16 @@ static int opt_parse_porcelain(const struct option *opt, const char *arg, int un\n \treturn 0;\n }\n \n+static int mention_abandoned_message;\n+static void maybe_mention_abandoned_message(void)\n+{\n+\tif (!mention_abandoned_message)\n+\t\treturn;\n+\tadvise(_(\"Your commit message has been saved in '%s' and will be\\n\"\n+\t\t \"overwritten by the next invocation of \\\"git commit\\\".\"),\n+\t       git_path_commit_editmsg());\n+}\n+\n static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n {\n \tstruct strbuf *buf = opt->value;\n@@ -1090,6 +1100,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\texit(1);\n \t\t}\n \t\tstrvec_clear(&env);\n+\t\tatexit(maybe_mention_abandoned_message);\n+\t\tmention_abandoned_message = 1;\n \t}\n \n \tif (!no_verify &&\n@@ -1813,11 +1825,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (message_is_empty(&sb, cleanup_mode) && !allow_empty_message) {\n \t\trollback_index_files();\n \t\tfprintf(stderr, _(\"Aborting commit due to empty commit message.\\n\"));\n+\t\tmention_abandoned_message = 0;\n \t\texit(1);\n \t}\n \tif (template_untouched(&sb, template_file, cleanup_mode) && !allow_empty_message) {\n \t\trollback_index_files();\n \t\tfprintf(stderr, _(\"Aborting commit; you did not edit the message.\\n\"));\n+\t\tmention_abandoned_message = 0;\n \t\texit(1);\n \t}\n \n@@ -1855,6 +1869,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tdie(\"%s\", err.buf);\n \t}\n \n+\tmention_abandoned_message = 0;\n \tsequencer_post_commit_cleanup(the_repository, 0);\n \tunlink(git_path_merge_head(the_repository));\n \tunlink(git_path_merge_msg(the_repository));\ndiff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh\nindex 4dca8d97a7..c476a26235 100755\n--- a/t/t7500-commit-template-squash-signoff.sh\n+++ b/t/t7500-commit-template-squash-signoff.sh\n@@ -396,13 +396,12 @@ test_expect_success 'consecutive amend! commits remove amend! line from commit m\n \n test_expect_success 'deny to create amend! commit if its commit msg body is empty' '\n \tcommit_for_rebase_autosquash_setup &&\n-\techo \"Aborting commit due to empty commit message body.\" >expected &&\n \t(\n \t\tset_fake_editor &&\n \t\ttest_must_fail env FAKE_COMMIT_MESSAGE=\"amend! target message subject line\" \\\n \t\t\tgit commit --fixup=amend:HEAD~ 2>actual\n \t) &&\n-\ttest_cmp expected actual\n+\tgrep \"Aborting commit due to empty commit message body\" actual\n '\n \n test_expect_success 'amend! commit allows empty commit msg body with --allow-empty-message' '\n-- \n2.46.0.rc1.447.g578b9b2b5c\n\n"},{"id":"499313","messageId":"CAFLLRpKqU7nBGsPsf=kdA9Z4F6QaZ91hsRvArRy0GaCfxUgsTg@mail.gmail.com","threadId":"61836","inReplyTo":"xmqq1q3iyceq.fsf@gitster.g","subject":"Re: bug/defaults: COMMIT_EDITMSG not reused after a failed commit","fromName":"Robert Coup","fromEmail":"robert.coup@koordinates.com","sentAt":"2024-07-25T08:15:14Z","receivedAt":"2024-07-25T08:15:31Z","isPatch":false,"sender":{"key":"robert.coup@koordinates.com","avatar":"https://gravatar.com/avatar/d1a87d63ffb562b791992d8a119ebbdd742e703109d23333ca3fca51306ee95c?d=mp&s=160"},"body":"Hi Junio,\n\nOn Wed, 24 Jul 2024 at 17:37, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Unconditionally doing this change would be disruptive to workflows\n> of existing users.  To them, Git left COMMIT_EDITMSG available even\n> after the commit to them almost forever, but suddenly it stops doing\n> so.\n\nA general question: how far down the \"I can imagine a hypothetical\nworkflow\" route do we need to go? Moreso when the behaviour is\ndocumented as doing something different, and it's noted in the list\narchive as a bug? I appreciate there's a lot of users out there who do\na lot of weird and wonderful things. Could it suffice for the\nhypothetical user to have an opt-in way to get to the old behaviour?\n\nSome experimenting reveals a simple `git commit -F\n.git/COMMIT_EDITMSG` doesn't work, since the comments get committed;\nand using `git commit --template .git/COMMIT_EDITMSG` repeats the\n#boilerplate, and results in an \"Aborting commit; you did not edit the\nmessage.\" error, even when you do. `git commit --edit -F\n.git/COMMIT_EDITMSG --cleanup=strip` works, except it also repeats the\n#boilerplate again, and it's getting unwieldy. I'll explore Jeff's\npatch too.\n\nThanks,\n\nRob :)\n"},{"id":"499347","messageId":"xmqqmsm5lcpd.fsf@gitster.g","threadId":"61836","inReplyTo":"20240724210854.GB557365@coredump.intra.peff.net","subject":"Re: bug/defaults: COMMIT_EDITMSG not reused after a failed commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-25T15:21:18Z","receivedAt":"2024-07-25T15:21:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> It just told you about COMMIT_EDITMSG, making it your responsibility to\n> recover it before running \"git commit\" again. Your (1) makes it a little\n> nicer, in that you can run \"git commit\" and then pull the content from\n> the other file into your editor. Or we could even provide an option to\n> pre-populate the message with it.\n>\n> Junio was lukewarm on the original, so I'm not sure why I've been\n> holding on to it all these years. But maybe it would help as a guide for\n> anybody who wants to work on what you've proposed above.\n\nI think it was only me being allergic of the use of atexit() for a\nnarrow single purpose, and perhaps I was hoping that we might be\nable to come up with a bit more generalized interface, possibly\nbased on atexit(), to register common cleanup \"hooks\", as back then\nwe only had a handful of calls to atexit() in mid 2012, and I was\nworried that we may add a lot more of them in an uncontrolled way.\n\n> -- >8 --\n> From: Jeff King <peff@peff.net>\n> Date: Mon, 23 Jul 2012 14:52:18 -0400\n> Subject: [PATCH] commit: give a hint when a commit message has been abandoned\n>\n> If we launch an editor for the user to create a commit\n> message, they may put significant work into doing so.\n> Typically we try to check common mistakes that could cause\n> the commit to fail early, so that we die before the user\n> goes to the trouble.\n>\n> We may still experience some errors afterwards, though; in\n> this case, the user is given no hint that their commit\n> message has been saved. Let's tell them where it is.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  builtin/commit.c                          | 15 +++++++++++++++\n>  t/t7500-commit-template-squash-signoff.sh |  3 +--\n>  2 files changed, 16 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index dec78dfb86..42fefaa0e3 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -160,6 +160,16 @@ static int opt_parse_porcelain(const struct option *opt, const char *arg, int un\n>  \treturn 0;\n>  }\n>  \n> +static int mention_abandoned_message;\n> +static void maybe_mention_abandoned_message(void)\n> +{\n> +\tif (!mention_abandoned_message)\n> +\t\treturn;\n> +\tadvise(_(\"Your commit message has been saved in '%s' and will be\\n\"\n> +\t\t \"overwritten by the next invocation of \\\"git commit\\\".\"),\n> +\t       git_path_commit_editmsg());\n> +}\n> +\n>  static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n>  {\n>  \tstruct strbuf *buf = opt->value;\n> @@ -1090,6 +1100,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \t\t\texit(1);\n>  \t\t}\n>  \t\tstrvec_clear(&env);\n> +\t\tatexit(maybe_mention_abandoned_message);\n> +\t\tmention_abandoned_message = 1;\n>  \t}\n>  \n>  \tif (!no_verify &&\n> @@ -1813,11 +1825,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \tif (message_is_empty(&sb, cleanup_mode) && !allow_empty_message) {\n>  \t\trollback_index_files();\n>  \t\tfprintf(stderr, _(\"Aborting commit due to empty commit message.\\n\"));\n> +\t\tmention_abandoned_message = 0;\n>  \t\texit(1);\n>  \t}\n>  \tif (template_untouched(&sb, template_file, cleanup_mode) && !allow_empty_message) {\n>  \t\trollback_index_files();\n>  \t\tfprintf(stderr, _(\"Aborting commit; you did not edit the message.\\n\"));\n> +\t\tmention_abandoned_message = 0;\n>  \t\texit(1);\n>  \t}\n>  \n> @@ -1855,6 +1869,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \t\tdie(\"%s\", err.buf);\n>  \t}\n>  \n> +\tmention_abandoned_message = 0;\n>  \tsequencer_post_commit_cleanup(the_repository, 0);\n>  \tunlink(git_path_merge_head(the_repository));\n>  \tunlink(git_path_merge_msg(the_repository));\n> diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh\n> index 4dca8d97a7..c476a26235 100755\n> --- a/t/t7500-commit-template-squash-signoff.sh\n> +++ b/t/t7500-commit-template-squash-signoff.sh\n> @@ -396,13 +396,12 @@ test_expect_success 'consecutive amend! commits remove amend! line from commit m\n>  \n>  test_expect_success 'deny to create amend! commit if its commit msg body is empty' '\n>  \tcommit_for_rebase_autosquash_setup &&\n> -\techo \"Aborting commit due to empty commit message body.\" >expected &&\n>  \t(\n>  \t\tset_fake_editor &&\n>  \t\ttest_must_fail env FAKE_COMMIT_MESSAGE=\"amend! target message subject line\" \\\n>  \t\t\tgit commit --fixup=amend:HEAD~ 2>actual\n>  \t) &&\n> -\ttest_cmp expected actual\n> +\tgrep \"Aborting commit due to empty commit message body\" actual\n>  '\n>  \n>  test_expect_success 'amend! commit allows empty commit msg body with --allow-empty-message' '\n"}]}