{"thread":{"id":"39399","subject":"[PATCH 0/2] fix post-rewrite hook with 'git rebase -i --exec'","startedAt":"2015-05-21T18:13:00Z","lastAt":"2015-05-22T06:35:25Z","messageCount":5,"participants":["Matthieu Moy","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"261825","messageId":"1432231982-31314-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"39399","inReplyTo":null,"subject":"[PATCH 0/2] fix post-rewrite hook with 'git rebase -i --exec'","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-05-21T18:13:00Z","receivedAt":"2015-05-21T18:13:00Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Matthieu Moy (2):\n  rebase -i: demonstrate incorrect behavior of post-rewrite hook with\n    exec\n  rebase -i: fix post-rewrite hook with failed exec command\n\n git-rebase--interactive.sh   | 10 +++++-----\n t/t5407-post-rewrite-hook.sh | 17 +++++++++++++++++\n 2 files changed, 22 insertions(+), 5 deletions(-)\n\n-- \n2.4.1.171.g060e6ae.dirty\n"},{"id":"261826","messageId":"1432231982-31314-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"39399","inReplyTo":"1432231982-31314-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 1/2] rebase -i: demonstrate incorrect behavior of post-rewrite hook with exec","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-05-21T18:13:01Z","receivedAt":"2015-05-21T18:13:01Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The 'exec' command is sending the current commit to stopped-sha, which is\nsupposed to contain the original commit (before rebase). As a result, if\nan 'exec' command fails, the next 'git rebase --continue' will send the\ncurrent commit as <old-sha1> to the post-rewrite hook.\n\nThe test currently fails with :\n\n--- expected.data       2015-05-21 17:55:29.000000000 +0000\n+++ [...]post-rewrite.data      2015-05-21 17:55:29.000000000 +0000\n@@ -1,2 +1,3 @@\n 2362ae8e1b1b865e6161e6f0e165ffb974abf018 488028e9fac0b598b70cbeb594258a917e3f6fab\n+488028e9fac0b598b70cbeb594258a917e3f6fab 488028e9fac0b598b70cbeb594258a917e3f6fab\n babc8a4c7470895886fc129f1a015c486d05a351 8edffcc4e69a4e696a1d4bab047df450caf99507\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n t/t5407-post-rewrite-hook.sh | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh\nindex ea2e0d4..ecef820 100755\n--- a/t/t5407-post-rewrite-hook.sh\n+++ b/t/t5407-post-rewrite-hook.sh\n@@ -212,4 +212,21 @@ EOF\n \tverify_hook_input\n '\n \n+test_expect_failure 'git rebase -i (exec)' '\n+\tgit reset --hard D &&\n+\tclear_hook_input &&\n+\tFAKE_LINES=\"edit 1 exec_false 2\" git rebase -i B\n+\techo something >bar &&\n+\tgit add bar &&\n+\t# Fail because of exec false\n+\ttest_must_fail git rebase --continue &&\n+\tgit rebase --continue &&\n+\techo rebase >expected.args &&\n+\tcat >expected.data <<EOF &&\n+$(git rev-parse C) $(git rev-parse HEAD^)\n+$(git rev-parse D) $(git rev-parse HEAD)\n+EOF\n+\tverify_hook_input\n+'\n+\n test_done\n-- \n2.4.1.171.g060e6ae.dirty\n"},{"id":"261827","messageId":"1432231982-31314-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"39399","inReplyTo":"1432231982-31314-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/2] rebase -i: fix post-rewrite hook with failed exec command","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-05-21T18:13:02Z","receivedAt":"2015-05-21T18:13:02Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Usually, when 'git rebase' stops before completing the rebase, it is to\ngive the user an opportunity to edit a commit (e.g. with the 'edit'\ncommand). In such cases, 'git rebase' leaves the sha1 of the commit being\nrewritten in \"$state_dir\"/stopped-sha, and subsequent 'git rebase\n--continue' will call the post-rewrite hook with this sha1 as <old-sha1>\nargument to the post-rewrite hook.\n\nThe case of 'git rebase' stopping because of a failed 'exec' command is\ndifferent: it gives the opportunity to the user to examine or fix the\nfailure, but does not stop saying \"here's a commit to edit, use\n--continue when you're done\". So, there's no reason to call the\npost-rewrite hook for 'exec' commands. If the user did rewrite the\ncommit, it would be with 'git commit --amend' which already called the\npost-rewrite hook.\n\nFix the behavior to leave no stopped-sha file in case of failed exec\ncommand, and teach 'git rebase --continue' to skip record_in_rewritten if\nno stopped-sha file is found.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nI'm afraid I'm the one to blame for the bug :-(.\n\n git-rebase--interactive.sh   | 10 +++++-----\n t/t5407-post-rewrite-hook.sh |  2 +-\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 08e5d86..1c321e4 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -486,7 +486,7 @@ do_pick () {\n }\n \n do_next () {\n-\trm -f \"$msg\" \"$author_script\" \"$amend\" || exit\n+\trm -f \"$msg\" \"$author_script\" \"$amend\" \"$state_dir\"/stopped-sha || exit\n \tread -r command sha1 rest < \"$todo\"\n \tcase \"$command\" in\n \t\"$comment_char\"*|''|noop)\n@@ -576,9 +576,6 @@ do_next () {\n \t\tread -r command rest < \"$todo\"\n \t\tmark_action_done\n \t\tprintf 'Executing: %s\\n' \"$rest\"\n-\t\t# \"exec\" command doesn't take a sha1 in the todo-list.\n-\t\t# => can't just use $sha1 here.\n-\t\tgit rev-parse --verify HEAD > \"$state_dir\"/stopped-sha\n \t\t${SHELL:-@SHELL_PATH@} -c \"$rest\" # Actual execution\n \t\tstatus=$?\n \t\t# Run in subshell because require_clean_work_tree can die.\n@@ -874,7 +871,10 @@ first and then run 'git rebase --continue' again.\"\n \t\tfi\n \tfi\n \n-\trecord_in_rewritten \"$(cat \"$state_dir\"/stopped-sha)\"\n+\tif test -r \"$state_dir\"/stopped-sha\n+\tthen\n+\t\trecord_in_rewritten \"$(cat \"$state_dir\"/stopped-sha)\"\n+\tfi\n \n \trequire_clean_work_tree \"rebase\"\n \tdo_rest\ndiff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh\nindex ecef820..24ba796 100755\n--- a/t/t5407-post-rewrite-hook.sh\n+++ b/t/t5407-post-rewrite-hook.sh\n@@ -212,7 +212,7 @@ EOF\n \tverify_hook_input\n '\n \n-test_expect_failure 'git rebase -i (exec)' '\n+test_expect_success 'git rebase -i (exec)' '\n \tgit reset --hard D &&\n \tclear_hook_input &&\n \tFAKE_LINES=\"edit 1 exec_false 2\" git rebase -i B\n-- \n2.4.1.171.g060e6ae.dirty\n"},{"id":"261853","messageId":"CAPig+cRFwruC8DwPhH_=Jb8YAZPB5J2e0F2NBob3zLPGoMLWgA@mail.gmail.com","threadId":"39399","inReplyTo":"1432231982-31314-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 1/2] rebase -i: demonstrate incorrect behavior of post-rewrite hook with exec","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-21T21:12:35Z","receivedAt":"2015-05-21T21:12:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 21, 2015 at 2:13 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:\n> The 'exec' command is sending the current commit to stopped-sha, which is\n> supposed to contain the original commit (before rebase). As a result, if\n> an 'exec' command fails, the next 'git rebase --continue' will send the\n> current commit as <old-sha1> to the post-rewrite hook.\n> [...]\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n> diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh\n> index ea2e0d4..ecef820 100755\n> --- a/t/t5407-post-rewrite-hook.sh\n> +++ b/t/t5407-post-rewrite-hook.sh\n> @@ -212,4 +212,21 @@ EOF\n>         verify_hook_input\n>  '\n>\n> +test_expect_failure 'git rebase -i (exec)' '\n> +       git reset --hard D &&\n> +       clear_hook_input &&\n> +       FAKE_LINES=\"edit 1 exec_false 2\" git rebase -i B\n\nBroken &&-chain.\n\n> +       echo something >bar &&\n> +       git add bar &&\n> +       # Fail because of exec false\n> +       test_must_fail git rebase --continue &&\n> +       git rebase --continue &&\n> +       echo rebase >expected.args &&\n> +       cat >expected.data <<EOF &&\n> +$(git rev-parse C) $(git rev-parse HEAD^)\n> +$(git rev-parse D) $(git rev-parse HEAD)\n> +EOF\n> +       verify_hook_input\n> +'\n> +\n>  test_done\n> --\n> 2.4.1.171.g060e6ae.dirty\n"},{"id":"261883","messageId":"vpqoald2k7m.fsf@anie.imag.fr","threadId":"39399","inReplyTo":"CAPig+cRFwruC8DwPhH_=Jb8YAZPB5J2e0F2NBob3zLPGoMLWgA@mail.gmail.com","subject":"Re: [PATCH 1/2] rebase -i: demonstrate incorrect behavior of post-rewrite hook with exec","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-22T06:35:25Z","receivedAt":"2015-05-22T06:35:25Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> +test_expect_failure 'git rebase -i (exec)' '\n>> +       git reset --hard D &&\n>> +       clear_hook_input &&\n>> +       FAKE_LINES=\"edit 1 exec_false 2\" git rebase -i B\n>\n> Broken &&-chain.\n\nThanks, will add in v2.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}