{"thread":{"id":"31084","subject":"[PATCH] rebase -i: handle fixup of root commit correctly","startedAt":"2012-07-24T12:17:03Z","lastAt":"2012-07-31T22:47:05Z","messageCount":7,"participants":["Chris Webb","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"195624","messageId":"20120724121703.GG26014@arachsys.com","threadId":"31084","inReplyTo":null,"subject":"[PATCH] rebase -i: handle fixup of root commit correctly","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-07-24T12:17:03Z","receivedAt":"2012-07-24T12:17:03Z","isPatch":true,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"There is a bug with git rebase -i --root when a fixup or squash line is\napplied to the new root. We attempt to amend the commit onto which they\napply with git reset --soft HEAD^ followed by a normal commit. Unlike a\nreal commit --amend, this sequence will fail against a root commit as it\nhas no parent.\n\nFix rebase -i to use commit --amend for fixup and squash instead, and\nadd a test for the case of a fixup of the root commit.\n\nSigned-off-by: Chris Webb <chris@arachsys.com>\n---\n\nSorry, I should have spotted this issue when I did the original root-rebase\nseries. I've checked that this patch doesn't break any of the existing\ntests, as well as satisfying the newly introduced check for the root-fixup\ncase.\n\n git-rebase--interactive.sh    | 25 +++++++++++++------------\n t/t3404-rebase-interactive.sh |  8 ++++++++\n 2 files changed, 21 insertions(+), 12 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex bef7bc0..0d2056f 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -493,25 +493,28 @@ do_next () {\n \t\tauthor_script_content=$(get_author_ident_from_commit HEAD)\n \t\techo \"$author_script_content\" > \"$author_script\"\n \t\teval \"$author_script_content\"\n-\t\toutput git reset --soft HEAD^\n-\t\tpick_one -n $sha1 || die_failed_squash $sha1 \"$rest\"\n+\t\tif ! pick_one -n $sha1\n+\t\tthen\n+\t\t\tgit rev-parse --verify HEAD >\"$amend\"\n+\t\t\tdie_failed_squash $sha1 \"$rest\"\n+\t\tfi\n \t\tcase \"$(peek_next_command)\" in\n \t\tsquash|s|fixup|f)\n \t\t\t# This is an intermediate commit; its message will only be\n \t\t\t# used in case of trouble.  So use the long version:\n-\t\t\tdo_with_author output git commit --no-verify -F \"$squash_msg\" ||\n+\t\t\tdo_with_author output git commit --amend --no-verify -F \"$squash_msg\" ||\n \t\t\t\tdie_failed_squash $sha1 \"$rest\"\n \t\t\t;;\n \t\t*)\n \t\t\t# This is the final command of this squash/fixup group\n \t\t\tif test -f \"$fixup_msg\"\n \t\t\tthen\n-\t\t\t\tdo_with_author git commit --no-verify -F \"$fixup_msg\" ||\n+\t\t\t\tdo_with_author git commit --amend --no-verify -F \"$fixup_msg\" ||\n \t\t\t\t\tdie_failed_squash $sha1 \"$rest\"\n \t\t\telse\n \t\t\t\tcp \"$squash_msg\" \"$GIT_DIR\"/SQUASH_MSG || exit\n \t\t\t\trm -f \"$GIT_DIR\"/MERGE_MSG\n-\t\t\t\tdo_with_author git commit --no-verify -e ||\n+\t\t\t\tdo_with_author git commit --amend --no-verify -F \"$GIT_DIR\"/SQUASH_MSG -e ||\n \t\t\t\t\tdie_failed_squash $sha1 \"$rest\"\n \t\t\tfi\n \t\t\trm -f \"$squash_msg\" \"$fixup_msg\"\n@@ -748,7 +751,6 @@ In both case, once you're done, continue with:\n \t\tfi\n \t\t. \"$author_script\" ||\n \t\t\tdie \"Error trying to find the author identity to amend commit\"\n-\t\tcurrent_head=\n \t\tif test -f \"$amend\"\n \t\tthen\n \t\t\tcurrent_head=$(git rev-parse --verify HEAD)\n@@ -756,13 +758,12 @@ In both case, once you're done, continue with:\n \t\t\tdie \"\\\n You have uncommitted changes in your working tree. Please, commit them\n first and then run 'git rebase --continue' again.\"\n-\t\t\tgit reset --soft HEAD^ ||\n-\t\t\tdie \"Cannot rewind the HEAD\"\n+\t\t\tdo_with_author git commit --amend --no-verify -F \"$msg\" -e ||\n+\t\t\t\tdie \"Could not commit staged changes.\"\n+\t\telse\n+\t\t\tdo_with_author git commit --no-verify -F \"$msg\" -e ||\n+\t\t\t\tdie \"Could not commit staged changes.\"\n \t\tfi\n-\t\tdo_with_author git commit --no-verify -F \"$msg\" -e || {\n-\t\t\ttest -n \"$current_head\" && git reset --soft $current_head\n-\t\t\tdie \"Could not commit staged changes.\"\n-\t\t}\n \tfi\n \n \trecord_in_rewritten \"$(cat \"$state_dir\"/stopped-sha)\"\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 8078db6..3f75d32 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -903,4 +903,12 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n \tgit rebase --abort\n '\n \n+test_expect_success 'rebase -i --root fixup root commit' '\n+\tgit checkout B &&\n+\tFAKE_LINES=\"1 fixup 2\" git rebase -i --root &&\n+\ttest A = $(git cat-file commit HEAD | sed -ne \\$p) &&\n+\ttest B = $(git show HEAD:file1) &&\n+\ttest 0 = $(git cat-file commit HEAD | grep -c ^parent\\ )\n+'\n+\n test_done\n-- \n1.7.11.2.251.g6a928a6\n"},{"id":"195649","messageId":"7vlii9ezgl.fsf@alter.siamese.dyndns.org","threadId":"31084","inReplyTo":"20120724121703.GG26014@arachsys.com","subject":"Re: [PATCH] rebase -i: handle fixup of root commit correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-24T19:22:34Z","receivedAt":"2012-07-24T19:22:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Webb <chris@arachsys.com> writes:\n\n> There is a bug with git rebase -i --root when a fixup or squash line is\n> applied to the new root. We attempt to amend the commit onto which they\n> apply with git reset --soft HEAD^ followed by a normal commit. Unlike a\n> real commit --amend, this sequence will fail against a root commit as it\n> has no parent.\n>\n> Fix rebase -i to use commit --amend for fixup and squash instead, and\n> add a test for the case of a fixup of the root commit.\n>\n> Signed-off-by: Chris Webb <chris@arachsys.com>\n> ---\n>\n> Sorry, I should have spotted this issue when I did the original root-rebase\n> series. I've checked that this patch doesn't break any of the existing\n> tests, as well as satisfying the newly introduced check for the root-fixup\n> case.\n\nOK, so instead of \"reset --soft HEAD^ && pick -n && commit -F msg\"\nto back up one step and then build on top of it, the new sequence\n\"pick -n && commit --amend -F msg\" modifies and then amends, whose\nend result should be the same but the important difference is that\nthe latter would work even if the current commit is a root one.\n\nMakes sense.  Thanks for catching and fixing it.\n\n>\n>  git-rebase--interactive.sh    | 25 +++++++++++++------------\n>  t/t3404-rebase-interactive.sh |  8 ++++++++\n>  2 files changed, 21 insertions(+), 12 deletions(-)\n>\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index bef7bc0..0d2056f 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -493,25 +493,28 @@ do_next () {\n>  \t\tauthor_script_content=$(get_author_ident_from_commit HEAD)\n>  \t\techo \"$author_script_content\" > \"$author_script\"\n>  \t\teval \"$author_script_content\"\n> -\t\toutput git reset --soft HEAD^\n> -\t\tpick_one -n $sha1 || die_failed_squash $sha1 \"$rest\"\n> +\t\tif ! pick_one -n $sha1\n> +\t\tthen\n> +\t\t\tgit rev-parse --verify HEAD >\"$amend\"\n> +\t\t\tdie_failed_squash $sha1 \"$rest\"\n> +\t\tfi\n>  \t\tcase \"$(peek_next_command)\" in\n>  \t\tsquash|s|fixup|f)\n>  \t\t\t# This is an intermediate commit; its message will only be\n>  \t\t\t# used in case of trouble.  So use the long version:\n> -\t\t\tdo_with_author output git commit --no-verify -F \"$squash_msg\" ||\n> +\t\t\tdo_with_author output git commit --amend --no-verify -F \"$squash_msg\" ||\n>  \t\t\t\tdie_failed_squash $sha1 \"$rest\"\n>  \t\t\t;;\n>  \t\t*)\n>  \t\t\t# This is the final command of this squash/fixup group\n>  \t\t\tif test -f \"$fixup_msg\"\n>  \t\t\tthen\n> -\t\t\t\tdo_with_author git commit --no-verify -F \"$fixup_msg\" ||\n> +\t\t\t\tdo_with_author git commit --amend --no-verify -F \"$fixup_msg\" ||\n>  \t\t\t\t\tdie_failed_squash $sha1 \"$rest\"\n>  \t\t\telse\n>  \t\t\t\tcp \"$squash_msg\" \"$GIT_DIR\"/SQUASH_MSG || exit\n>  \t\t\t\trm -f \"$GIT_DIR\"/MERGE_MSG\n> -\t\t\t\tdo_with_author git commit --no-verify -e ||\n> +\t\t\t\tdo_with_author git commit --amend --no-verify -F \"$GIT_DIR\"/SQUASH_MSG -e ||\n>  \t\t\t\t\tdie_failed_squash $sha1 \"$rest\"\n>  \t\t\tfi\n>  \t\t\trm -f \"$squash_msg\" \"$fixup_msg\"\n> @@ -748,7 +751,6 @@ In both case, once you're done, continue with:\n>  \t\tfi\n>  \t\t. \"$author_script\" ||\n>  \t\t\tdie \"Error trying to find the author identity to amend commit\"\n> -\t\tcurrent_head=\n>  \t\tif test -f \"$amend\"\n>  \t\tthen\n>  \t\t\tcurrent_head=$(git rev-parse --verify HEAD)\n> @@ -756,13 +758,12 @@ In both case, once you're done, continue with:\n>  \t\t\tdie \"\\\n>  You have uncommitted changes in your working tree. Please, commit them\n>  first and then run 'git rebase --continue' again.\"\n> -\t\t\tgit reset --soft HEAD^ ||\n> -\t\t\tdie \"Cannot rewind the HEAD\"\n> +\t\t\tdo_with_author git commit --amend --no-verify -F \"$msg\" -e ||\n> +\t\t\t\tdie \"Could not commit staged changes.\"\n> +\t\telse\n> +\t\t\tdo_with_author git commit --no-verify -F \"$msg\" -e ||\n> +\t\t\t\tdie \"Could not commit staged changes.\"\n>  \t\tfi\n> -\t\tdo_with_author git commit --no-verify -F \"$msg\" -e || {\n> -\t\t\ttest -n \"$current_head\" && git reset --soft $current_head\n> -\t\t\tdie \"Could not commit staged changes.\"\n> -\t\t}\n>  \tfi\n>  \n>  \trecord_in_rewritten \"$(cat \"$state_dir\"/stopped-sha)\"\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 8078db6..3f75d32 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -903,4 +903,12 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n>  \tgit rebase --abort\n>  '\n>  \n> +test_expect_success 'rebase -i --root fixup root commit' '\n> +\tgit checkout B &&\n> +\tFAKE_LINES=\"1 fixup 2\" git rebase -i --root &&\n> +\ttest A = $(git cat-file commit HEAD | sed -ne \\$p) &&\n> +\ttest B = $(git show HEAD:file1) &&\n> +\ttest 0 = $(git cat-file commit HEAD | grep -c ^parent\\ )\n> +'\n> +\n>  test_done\n"},{"id":"196220","messageId":"5017A1E4.1070800@kdbg.org","threadId":"31084","inReplyTo":"20120724121703.GG26014@arachsys.com","subject":"Re: [PATCH] rebase -i: handle fixup of root commit correctly","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-07-31T09:14:12Z","receivedAt":"2012-07-31T09:14:12Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 24.07.2012 14:17, schrieb Chris Webb:\n> There is a bug with git rebase -i --root when a fixup or squash line is\n> applied to the new root. We attempt to amend the commit onto which they\n> apply with git reset --soft HEAD^ followed by a normal commit. Unlike a\n> real commit --amend, this sequence will fail against a root commit as it\n> has no parent.\n>\n> Fix rebase -i to use commit --amend for fixup and squash instead, and\n> add a test for the case of a fixup of the root commit.\n>\n> Signed-off-by: Chris Webb<chris@arachsys.com>\n> ---\n>\n> Sorry, I should have spotted this issue when I did the original root-rebase\n> series. I've checked that this patch doesn't break any of the existing\n> tests, as well as satisfying the newly introduced check for the root-fixup\n> case.\n>\n>   git-rebase--interactive.sh    | 25 +++++++++++++------------\n>   t/t3404-rebase-interactive.sh |  8 ++++++++\n>   2 files changed, 21 insertions(+), 12 deletions(-)\n>\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index bef7bc0..0d2056f 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -493,25 +493,28 @@ do_next () {\n>   \t\tauthor_script_content=$(get_author_ident_from_commit HEAD)\n>   \t\techo \"$author_script_content\">  \"$author_script\"\n>   \t\teval \"$author_script_content\"\n> -\t\toutput git reset --soft HEAD^\n> -\t\tpick_one -n $sha1 || die_failed_squash $sha1 \"$rest\"\n> +\t\tif ! pick_one -n $sha1\n> +\t\tthen\n> +\t\t\tgit rev-parse --verify HEAD>\"$amend\"\n> +\t\t\tdie_failed_squash $sha1 \"$rest\"\n> +\t\tfi\n>   \t\tcase \"$(peek_next_command)\" in\n>   \t\tsquash|s|fixup|f)\n>   \t\t\t# This is an intermediate commit; its message will only be\n>   \t\t\t# used in case of trouble.  So use the long version:\n> -\t\t\tdo_with_author output git commit --no-verify -F \"$squash_msg\" ||\n> +\t\t\tdo_with_author output git commit --amend --no-verify -F \"$squash_msg\" ||\n>   \t\t\t\tdie_failed_squash $sha1 \"$rest\"\n\nThis new sequence looks *VERY* suspicious. It makes a HUGE difference in \nwhat is left behind if the cherry-pick fails. Did you think about what \nhappens when the cherry-pick fails in a squash+squash+fixup+fixup sequence \n(or any combination thereof) and then the rebase is continued (after a \nmanual resolution)?\n\n>   \t\t\t;;\n>   \t\t*)\n>   \t\t\t# This is the final command of this squash/fixup group\n>   \t\t\tif test -f \"$fixup_msg\"\n>   \t\t\tthen\n> -\t\t\t\tdo_with_author git commit --no-verify -F \"$fixup_msg\" ||\n> +\t\t\t\tdo_with_author git commit --amend --no-verify -F \"$fixup_msg\" ||\n>   \t\t\t\t\tdie_failed_squash $sha1 \"$rest\"\n>   \t\t\telse\n>   \t\t\t\tcp \"$squash_msg\" \"$GIT_DIR\"/SQUASH_MSG || exit\n>   \t\t\t\trm -f \"$GIT_DIR\"/MERGE_MSG\n> -\t\t\t\tdo_with_author git commit --no-verify -e ||\n> +\t\t\t\tdo_with_author git commit --amend --no-verify -F \"$GIT_DIR\"/SQUASH_MSG -e ||\n>   \t\t\t\t\tdie_failed_squash $sha1 \"$rest\"\n>   \t\t\tfi\n>   \t\t\trm -f \"$squash_msg\" \"$fixup_msg\"\n> @@ -748,7 +751,6 @@ In both case, once you're done, continue with:\n>   \t\tfi\n>   \t\t. \"$author_script\" ||\n>   \t\t\tdie \"Error trying to find the author identity to amend commit\"\n> -\t\tcurrent_head=\n>   \t\tif test -f \"$amend\"\n>   \t\tthen\n>   \t\t\tcurrent_head=$(git rev-parse --verify HEAD)\n> @@ -756,13 +758,12 @@ In both case, once you're done, continue with:\n>   \t\t\tdie \"\\\n>   You have uncommitted changes in your working tree. Please, commit them\n>   first and then run 'git rebase --continue' again.\"\n> -\t\t\tgit reset --soft HEAD^ ||\n> -\t\t\tdie \"Cannot rewind the HEAD\"\n> +\t\t\tdo_with_author git commit --amend --no-verify -F \"$msg\" -e ||\n> +\t\t\t\tdie \"Could not commit staged changes.\"\n> +\t\telse\n> +\t\t\tdo_with_author git commit --no-verify -F \"$msg\" -e ||\n> +\t\t\t\tdie \"Could not commit staged changes.\"\n>   \t\tfi\n> -\t\tdo_with_author git commit --no-verify -F \"$msg\" -e || {\n> -\t\t\ttest -n \"$current_head\"&&  git reset --soft $current_head\n> -\t\t\tdie \"Could not commit staged changes.\"\n> -\t\t}\n>   \tfi\n>\n>   \trecord_in_rewritten \"$(cat \"$state_dir\"/stopped-sha)\"\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 8078db6..3f75d32 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -903,4 +903,12 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n>   \tgit rebase --abort\n>   '\n>\n> +test_expect_success 'rebase -i --root fixup root commit' '\n> +\tgit checkout B&&\n> +\tFAKE_LINES=\"1 fixup 2\" git rebase -i --root&&\n> +\ttest A = $(git cat-file commit HEAD | sed -ne \\$p)&&\n> +\ttest B = $(git show HEAD:file1)&&\n> +\ttest 0 = $(git cat-file commit HEAD | grep -c ^parent\\ )\n> +'\n> +\n>   test_done\n\n-- Hannes\n"},{"id":"196224","messageId":"20120731111938.GD19416@arachsys.com","threadId":"31084","inReplyTo":"5017A1E4.1070800@kdbg.org","subject":"Re: [PATCH] rebase -i: handle fixup of root commit correctly","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-07-31T11:19:39Z","receivedAt":"2012-07-31T11:19:39Z","isPatch":true,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 24.07.2012 14:17, schrieb Chris Webb:\n> >diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> >index bef7bc0..0d2056f 100644\n> >--- a/git-rebase--interactive.sh\n> >+++ b/git-rebase--interactive.sh\n> >@@ -493,25 +493,28 @@ do_next () {\n> >  \t\tauthor_script_content=$(get_author_ident_from_commit HEAD)\n> >  \t\techo \"$author_script_content\" >\"$author_script\"\n> >  \t\teval \"$author_script_content\"\n> >-\t\toutput git reset --soft HEAD^\n> >-\t\tpick_one -n $sha1 || die_failed_squash $sha1 \"$rest\"\n> >+\t\tif ! pick_one -n $sha1\n> >+\t\tthen\n> >+\t\t\tgit rev-parse --verify HEAD >\"$amend\"\n> >+\t\t\tdie_failed_squash $sha1 \"$rest\"\n> >+\t\tfi\n> >  \t\tcase \"$(peek_next_command)\" in\n> >  \t\tsquash|s|fixup|f)\n> >  \t\t\t# This is an intermediate commit; its message will only be\n> >  \t\t\t# used in case of trouble.  So use the long version:\n> >-\t\t\tdo_with_author output git commit --no-verify -F \"$squash_msg\" ||\n> >+\t\t\tdo_with_author output git commit --amend --no-verify -F \"$squash_msg\" ||\n> >  \t\t\t\tdie_failed_squash $sha1 \"$rest\"\n> \n> This new sequence looks *VERY* suspicious. It makes a HUGE\n> difference in what is left behind if the cherry-pick fails. Did you\n> think about what happens when the cherry-pick fails in a\n> squash+squash+fixup+fixup sequence (or any combination thereof) and\n> then the rebase is continued (after a manual resolution)?\n\nI had to deal with the case where there's a conflict while picking the\nsquash/fixup, and we have to ensure we commit --amend in rebase --continue.\nThis is why I've written\n\n  git rev-parse --verify HEAD >\"$amend\"\n\nin the above, to use the pre-existing support for amending the HEAD commit\nin rebase --continue. (We test for this fixup-conflict case in various ways\nin t3404 and not doing an amend there would result in double commits and\nspectacular test breakage.)\n\nIs this the issue you mean here, or is it something more subtle which I'm\nnot properly following?\n\nIf we have a conflict in the middle of a chain of fixup/squashes, as far as\nI can see, we have a HEAD with all the previous successful fixups applied,\nconflict markers for the current failed pick, and when the conflict has been\nresolved, git rebase --continue will commit --amend the resolution and\ncontinue? Isn't that the correct behaviour here?\n\nCheers,\n\nChris.\n"},{"id":"196230","messageId":"20120731124824.GC14028@arachsys.com","threadId":"31084","inReplyTo":"20120731111938.GD19416@arachsys.com","subject":"Re: [PATCH] rebase -i: handle fixup of root commit correctly","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-07-31T12:48:25Z","receivedAt":"2012-07-31T12:48:25Z","isPatch":true,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"Chris Webb <chris@arachsys.com> writes:\n\n> If we have a conflict in the middle of a chain of fixup/squashes, as far as\n> I can see, we have a HEAD with all the previous successful fixups applied,\n> conflict markers for the current failed pick, and when the conflict has been\n> resolved, git rebase --continue will commit --amend the resolution and\n> continue? Isn't that the correct behaviour here?\n\nAs an explicit test, I've just tried a chain of four squashed commits, each\nof which deliberately resulted in a conflict to manually resolve. For each\nsquash, I was left with conflict markers on top of what had already been\nsquashed in the expected way, and when I continued after resolving these,\nthe resolution was 'commit --amend'ed in the expected way, with the same\nbehaviour and resulting commit at the end of the rebase -i as I get with a\ncopy of git without this patch.\n\nCheers,\n\nChris.\n"},{"id":"196259","messageId":"50183A4C.9080706@kdbg.org","threadId":"31084","inReplyTo":"20120731124824.GC14028@arachsys.com","subject":"Re: [PATCH] rebase -i: handle fixup of root commit correctly","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-07-31T20:04:28Z","receivedAt":"2012-07-31T20:04:28Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 31.07.2012 14:48, schrieb Chris Webb:\n> Chris Webb<chris@arachsys.com>  writes:\n>\n>> If we have a conflict in the middle of a chain of fixup/squashes, as far as\n>> I can see, we have a HEAD with all the previous successful fixups applied,\n>> conflict markers for the current failed pick, and when the conflict has been\n>> resolved, git rebase --continue will commit --amend the resolution and\n>> continue? Isn't that the correct behaviour here?\n>\n> As an explicit test, I've just tried a chain of four squashed commits, each\n> of which deliberately resulted in a conflict to manually resolve. For each\n> squash, I was left with conflict markers on top of what had already been\n> squashed in the expected way, and when I continued after resolving these,\n> the resolution was 'commit --amend'ed in the expected way, with the same\n> behaviour and resulting commit at the end of the rebase -i as I get with a\n> copy of git without this patch.\n\nOK, good. One subtlety to watch out for is when commit messages are \nedited. That is, if you edit the proposed message at 'rebase --continue' \nafter the first squash failed, is the new text preserved until the last \nsquash? I *think* that previously that was the case.\n\nThat said, I do appreciate the new modus operandi. The state when a rebase \nis interrupted is much clearer than earlier: now HEAD contains everything \nthat was successfully replayed so far, and the index anything that failed.\n\n-- Hannes\n"},{"id":"196269","messageId":"20120731224704.GD2823@arachsys.com","threadId":"31084","inReplyTo":"50183A4C.9080706@kdbg.org","subject":"Re: [PATCH] rebase -i: handle fixup of root commit correctly","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-07-31T22:47:05Z","receivedAt":"2012-07-31T22:47:05Z","isPatch":true,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> One subtlety to watch out for is when commit messages are edited. That is,\n> if you edit the proposed message at 'rebase --continue' after the first\n> squash failed, is the new text preserved until the last squash? I *think*\n> that previously that was the case.\n\nHi. Yes, doing this seems to work fine both in the original code, and after\nmy patch. I've just checked to be certain using my previous test case of\nfour conflicting squashes again, editing the message at each stage and\nensuring the edits are all retained in the final commit.\n\nBest wishes,\n\nChris.\n"}]}