{"thread":{"id":"28959","subject":"BUG. git rebase -i successfully continues (and also skips rewording) when pre-commit hook fails (exits with non-zero code)","startedAt":"2011-11-17T08:58:47Z","lastAt":"2011-11-30T15:52:51Z","messageCount":6,"participants":["Alexey Shumkin","Andrew Wong","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"179619","messageId":"20111117125847.190e9b25@ashu.dyn.rarus.ru","threadId":"28959","inReplyTo":null,"subject":"BUG. git rebase -i successfully continues (and also skips rewording) when pre-commit hook fails (exits with non-zero code)","fromName":"Alexey Shumkin","fromEmail":"alex.crezoff@gmail.com","sentAt":"2011-11-17T08:58:47Z","receivedAt":"2011-11-17T08:58:47Z","isPatch":false,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"For a project I have a pre-commit hook that monitors\nwhether files in a folder (scripts of DB) changed or not\nand fails if another special file (DB version) did not changed, too.\n\nSo, I did some commits and then I decided to change the order of them.\nOf course, I used a lovely \"git rebase -i\" command. I changed the order\nof the commits, then rebasing went ok. But I noticed that my pre-commit\nhook output failure message (one of the commits did not meet\nabove-mentioned condition). It's not too bad but ugly. But when I\ndecided to correct a message of that specific commit I ran\n\"git rebase -i\" again, marked that commit for rewording, rewording did\nnot start (because pre-commit hook failed, obviously) and rebasing went\non (commit had an unchanged message) and successfully finished. That is\nnot what I expected.\nI guess if any of hooks fail (which usually fail the commit), rebasing\nhave to be interrupted (as when there are conflicts)\n\nHere is a sample to reproduce the error\ngit init .\necho content > file\ngit add -fv file\ngit commit -a -m 'first commit'\necho line 2 >> file\ngit commit -a -m 'secont commit' # note a typo ;)\necho '#!/bin/bash\necho commit failed\nexit 1' > .git/hooks/pre-commit\nchmod +x .git/hooks/pre-commit\necho fail >> file\ngit commit -a -m 'failed commit' # to show that pre-commit hook fails\n# and outputs \"commit failed\"\ngit reset --hard\ngit rebase -i HEAD^\n# mark commit for rewording and exit an editor\n\nnote following output after all this:\n>commit fail/1)\n>Successfully rebased and updated refs/heads/master\n"},{"id":"180050","messageId":"1322496952-23819-1-git-send-email-andrew.kw.w@gmail.com","threadId":"28959","inReplyTo":"20111117125847.190e9b25@ashu.dyn.rarus.ru","subject":"Re: BUG. git rebase -i successfully continues (and also skips rewording) when pre-commit hook fails (exits with non-zero code)","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2011-11-28T16:15:51Z","receivedAt":"2011-11-28T16:15:51Z","isPatch":false,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"I actually have a patch to fix this sitting in my repo for a while. Thanks for bringing this issue up again.\n\nAndrew Wong (1):\n  rebase -i: interrupt rebase when \"commit --amend\" failed during\n    \"reword\"\n\n git-rebase--interactive.sh |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\n-- \n1.7.8.rc3.32.gb2fac\n"},{"id":"180051","messageId":"1322496952-23819-2-git-send-email-andrew.kw.w@gmail.com","threadId":"28959","inReplyTo":"1322496952-23819-1-git-send-email-andrew.kw.w@gmail.com","subject":"[PATCH] rebase -i: interrupt rebase when \"commit --amend\" failed during \"reword\"","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2011-11-28T16:15:52Z","receivedAt":"2011-11-28T16:15:52Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"\"commit --amend\" could fail in cases like the user empties the commit\nmessage, or pre-commit failed.  When it fails, rebase should be\ninterrupted, rather than ignoring the error and continue on rebasing.\nThis gives users a way to gracefully interrupt a \"reword\" if they\ndecided they actually want to do an \"edit\", or even \"rebase --abort\".\n\nSigned-off-by: Andrew Wong <andrew.kw.w@gmail.com>\n---\n git-rebase--interactive.sh |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 804001b..669f378 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -408,7 +408,8 @@ do_next () {\n \t\tmark_action_done\n \t\tpick_one $sha1 ||\n \t\t\tdie_with_patch $sha1 \"Could not apply $sha1... $rest\"\n-\t\tgit commit --amend --no-post-rewrite\n+\t\tgit commit --amend --no-post-rewrite ||\n+\t\t\tdie_with_patch $sha1 \"Cannot amend commit after successfully picking $sha1... $rest\"\n \t\trecord_in_rewritten $sha1\n \t\t;;\n \tedit|e)\n-- \n1.7.8.rc3.32.gb2fac\n"},{"id":"180108","messageId":"7vk46isncq.fsf@alter.siamese.dyndns.org","threadId":"28959","inReplyTo":"1322496952-23819-2-git-send-email-andrew.kw.w@gmail.com","subject":"Re: [PATCH] rebase -i: interrupt rebase when \"commit --amend\" failed during \"reword\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-29T20:08:37Z","receivedAt":"2011-11-29T20:08:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Wong <andrew.kw.w@gmail.com> writes:\n\n> \"commit --amend\" could fail in cases like the user empties the commit\n> message, or pre-commit failed.  When it fails, rebase should be\n> interrupted, rather than ignoring the error and continue on rebasing.\n> This gives users a way to gracefully interrupt a \"reword\" if they\n> decided they actually want to do an \"edit\", or even \"rebase --abort\".\n>\n> Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>\n\nMakes sense, especially if \"commit\" itself failed due to some unknown\nreason or a refusal from the pre-commit hook. Even though a user could\nhave been using the \"empty the commit log message and the original is\nkept\" as a trick to recover from a botched rewording attempt and this\nchange will regress for such use cases, I have a feeling that it does\nnot matter.\n\nIs there anything we should be saying more than \"fatal: Cannot amend\" to\nhelp users when this new \"die\" triggers? What is the recommended recovery\nprocedure? Run \"git commit --amend\" after doing whatever is needed to fix\nthe tree (e.g. if pre-commit refused because of a coding style violation,\nit may involve fixing the tree being committed; if it refused because of a\ntypo in the log message, the tree itself may be OK and nothing needs to be\ndone) and then \"git rebase --continue\"?\n\n>  git-rebase--interactive.sh |    3 ++-\n>  1 files changed, 2 insertions(+), 1 deletions(-)\n>\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 804001b..669f378 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -408,7 +408,8 @@ do_next () {\n>  \t\tmark_action_done\n>  \t\tpick_one $sha1 ||\n>  \t\t\tdie_with_patch $sha1 \"Could not apply $sha1... $rest\"\n> -\t\tgit commit --amend --no-post-rewrite\n> +\t\tgit commit --amend --no-post-rewrite ||\n> +\t\t\tdie_with_patch $sha1 \"Cannot amend commit after successfully picking $sha1... $rest\"\n>  \t\trecord_in_rewritten $sha1\n>  \t\t;;\n>  \tedit|e)\n"},{"id":"180156","messageId":"1322668371-21218-1-git-send-email-andrew.kw.w@gmail.com","threadId":"28959","inReplyTo":"7vk46isncq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] rebase -i: interrupt rebase when \"commit --amend\" failed during \"reword\"","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2011-11-30T15:52:50Z","receivedAt":"2011-11-30T15:52:50Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"On 11-11-29 3:08 PM, Junio C Hamano wrote:\n> Is there anything we should be saying more than \"fatal: Cannot amend\" to\n> help users when this new \"die\" triggers? \n\nAh, yes, that would be helpful.\n\nThe situation is actually very similar to an \"edit\", where a pick is successful\nbut requires user intervention. So I'm planning to refactor the behavior and\nmessage from \"edit\" into a function called \"exit_with_patch\". Then call the\nfunction from \"reword\" as well. Though it bothers me a bit that I have to pass\nin an exit code as well, since we want the exit status for \"reword\" to indicate\na failure, but \"edit\" needs to indicate a success. Is this acceptable? Or\nshould I just not bother with refactoring?\n\n\nAndrew Wong (1):\n  rebase -i: interrupt rebase when \"commit --amend\" failed during\n    \"reword\"\n\n git-rebase--interactive.sh |   36 +++++++++++++++++++++++-------------\n 1 files changed, 23 insertions(+), 13 deletions(-)\n\n-- \n1.7.8.rc3.32.gb0399.dirty\n"},{"id":"180157","messageId":"1322668371-21218-2-git-send-email-andrew.kw.w@gmail.com","threadId":"28959","inReplyTo":"1322668371-21218-1-git-send-email-andrew.kw.w@gmail.com","subject":"[PATCH] rebase -i: interrupt rebase when \"commit --amend\" failed during \"reword\"","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2011-11-30T15:52:51Z","receivedAt":"2011-11-30T15:52:51Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"\"commit --amend\" could fail in cases like the user empties the commit\nmessage, or pre-commit failed.  When it fails, rebase should be\ninterrupted and alert the user, rather than ignoring the error and\ncontinue on rebasing.  This also gives users a way to gracefully\ninterrupt a \"reword\" if they decided they actually want to do an \"edit\",\nor even \"rebase --abort\".\n\nSigned-off-by: Andrew Wong <andrew.kw.w@gmail.com>\n---\n git-rebase--interactive.sh |   36 +++++++++++++++++++++++-------------\n 1 files changed, 23 insertions(+), 13 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 804001b..5812222 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -143,6 +143,21 @@ die_with_patch () {\n \tdie \"$2\"\n }\n \n+exit_with_patch () {\n+\techo \"$1\" > \"$state_dir\"/stopped-sha\n+\tmake_patch $1\n+\tgit rev-parse --verify HEAD > \"$amend\"\n+\twarn \"You can amend the commit now, with\"\n+\twarn\n+\twarn \"\tgit commit --amend\"\n+\twarn\n+\twarn \"Once you are satisfied with your changes, run\"\n+\twarn\n+\twarn \"\tgit rebase --continue\"\n+\twarn\n+\texit $2\n+}\n+\n die_abort () {\n \trm -rf \"$state_dir\"\n \tdie \"$1\"\n@@ -408,7 +423,13 @@ do_next () {\n \t\tmark_action_done\n \t\tpick_one $sha1 ||\n \t\t\tdie_with_patch $sha1 \"Could not apply $sha1... $rest\"\n-\t\tgit commit --amend --no-post-rewrite\n+\t\tgit commit --amend --no-post-rewrite || {\n+\t\t\twarn \"Could not amend commit after successfully picking $sha1... $rest\"\n+\t\t\twarn \"This is most likely due to an empty commit message, or the pre-commit hook\"\n+\t\t\twarn \"failed. If the pre-commit hook failed, you may need to resolve the issue before\"\n+\t\t\twarn \"you are able to reword the commit.\"\n+\t\t\texit_with_patch $sha1 1\n+\t\t}\n \t\trecord_in_rewritten $sha1\n \t\t;;\n \tedit|e)\n@@ -417,19 +438,8 @@ do_next () {\n \t\tmark_action_done\n \t\tpick_one $sha1 ||\n \t\t\tdie_with_patch $sha1 \"Could not apply $sha1... $rest\"\n-\t\techo \"$sha1\" > \"$state_dir\"/stopped-sha\n-\t\tmake_patch $sha1\n-\t\tgit rev-parse --verify HEAD > \"$amend\"\n \t\twarn \"Stopped at $sha1... $rest\"\n-\t\twarn \"You can amend the commit now, with\"\n-\t\twarn\n-\t\twarn \"\tgit commit --amend\"\n-\t\twarn\n-\t\twarn \"Once you are satisfied with your changes, run\"\n-\t\twarn\n-\t\twarn \"\tgit rebase --continue\"\n-\t\twarn\n-\t\texit 0\n+\t\texit_with_patch $sha1 0\n \t\t;;\n \tsquash|s|fixup|f)\n \t\tcase \"$command\" in\n-- \n1.7.8.rc3.32.gb0399.dirty\n"}]}