{"thread":{"id":"11353","subject":"[PATCH 2/2] Catch and handle git-commit failures in git-rebase --interactive","startedAt":"2007-12-19T06:45:00Z","lastAt":"2007-12-19T12:21:21Z","messageCount":3,"participants":["Shawn O. Pearce","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"63717","messageId":"20071219064500.GB8915@spearce.org","threadId":"11353","inReplyTo":null,"subject":"[PATCH 2/2] Catch and handle git-commit failures in git-rebase --interactive","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-12-19T06:45:00Z","receivedAt":"2007-12-19T06:45:00Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If git-commit fails for any reason then git-rebase needs to stop\nand not plow through the rest of the series.  Its unlikely that\na future git-commit will succeed if the current attempt failed.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n\n Comments welcome.  This is on top of my 1/2 patch but we could\n drop my 1/2 and rewrite this to not use --no-verify, but handle\n the git-commit error correctly.\n\n However that would force users to fix whitespace errors in later\n patches in a series if they use -i, even though non-i wouldn't\n require that sort of fix-up.  So I think we should do both patches\n in the series.\n\n This has also bitten me a few times. :-\\\n\n git-rebase--interactive.sh |   17 +++++++++--------\n 1 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex b846ea8..0f7d939 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -289,22 +289,22 @@ do_next () {\n \t\toutput git reset --soft HEAD^\n \t\tpick_one -n $sha1 || failed=t\n \t\techo \"$author_script\" > \"$DOTEST\"/author-script\n-\t\tcase $failed in\n-\t\tf)\n+\t\tif test $failed = f\n+\t\tthen\n \t\t\t# This is like --amend, but with a different message\n \t\t\teval \"$author_script\"\n \t\t\tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\" \\\n \t\t\tGIT_AUTHOR_EMAIL=\"$GIT_AUTHOR_EMAIL\" \\\n \t\t\tGIT_AUTHOR_DATE=\"$GIT_AUTHOR_DATE\" \\\n-\t\t\t$USE_OUTPUT git commit --no-verify -F \"$MSG\" $EDIT_COMMIT\n-\t\t\t;;\n-\t\tt)\n+\t\t\t$USE_OUTPUT git commit --no-verify -F \"$MSG\" $EDIT_COMMIT || failed=t\n+\t\tfi\n+\t\tif test $failed = t\n+\t\tthen\n \t\t\tcp \"$MSG\" \"$GIT_DIR\"/MERGE_MSG\n \t\t\twarn\n \t\t\twarn \"Could not apply $sha1... $rest\"\n \t\t\tdie_with_patch $sha1 \"\"\n-\t\t\t;;\n-\t\tesac\n+\t\tfi\n \t\t;;\n \t*)\n \t\twarn \"Unknown command: $command $sha1 $rest\"\n@@ -372,7 +372,8 @@ do\n \t\t\ttest ! -f \"$DOTEST\"/amend || git reset --soft HEAD^\n \t\t} &&\n \t\texport GIT_AUTHOR_NAME GIT_AUTHOR_NAME GIT_AUTHOR_DATE &&\n-\t\tgit commit --no-verify -F \"$DOTEST\"/message -e\n+\t\tgit commit --no-verify -F \"$DOTEST\"/message -e ||\n+\t\t\tdie \"Could not commit staged changes.\"\n \n \t\trequire_clean_work_tree\n \t\tdo_rest\n-- \n1.5.4.rc0.1155.g12ed9\n"},{"id":"63718","messageId":"7vr6hjw4gw.fsf@gitster.siamese.dyndns.org","threadId":"11353","inReplyTo":"20071219064500.GB8915@spearce.org","subject":"Re: [PATCH 2/2] Catch and handle git-commit failures in git-rebase --interactive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-19T07:03:59Z","receivedAt":"2007-12-19T07:03:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n>  Comments welcome.  This is on top of my 1/2 patch but we could\n>  drop my 1/2 and rewrite this to not use --no-verify, but handle\n>  the git-commit error correctly.\n>\n>  However that would force users to fix whitespace errors in later\n>  patches in a series if they use -i, even though non-i wouldn't\n>  require that sort of fix-up.  So I think we should do both patches\n>  in the series.\n\nI agree with both patches.\n\nWhen one wants to use rebase to fix-up whitespace breakage in patches in\nbulk, you can set apply.whitespace to \"fix\".  One bad side effect of\nthis is that if you usually have apply.whitespace set to \"fix\" (because\nyou need to accept a lot of patches but your contributers tend to give\ncrappy patches), you need to temporarily change the configuration while\nrebasing if you do not want to preserve intentional whitespace breakages\n(e.g. ones in test scripts).  This can be argued either a feature or a\nmisfeature.\n\nBut rebase -i breaking and squashing upon commit failure (including\npre-commit safety) cannot be called either feature nor misfeature --- it\nis an outright bug.\n"},{"id":"63762","messageId":"Pine.LNX.4.64.0712191220520.23902@racer.site","threadId":"11353","inReplyTo":"20071219064500.GB8915@spearce.org","subject":"Re: [PATCH 2/2] Catch and handle git-commit failures in git-rebase --interactive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-19T12:21:21Z","receivedAt":"2007-12-19T12:21:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 19 Dec 2007, Shawn O. Pearce wrote:\n\n>  This has also bitten me a few times. :-\\\n\nSorry to hear that.  Both patches look sensible to me.\n\nCiao,\nDscho\n"}]}