{"thread":{"id":"25801","subject":"[PATCH] rebase --abort: do not update branch ref","startedAt":"2010-11-21T11:11:21Z","lastAt":"2010-11-22T12:23:27Z","messageCount":4,"participants":["Martin von Zweigbergk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"156290","messageId":"1290337881-26449-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"25801","inReplyTo":null,"subject":"[PATCH] rebase --abort: do not update branch ref","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-21T11:11:21Z","receivedAt":"2010-11-21T11:11:21Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"If a non-interactive rebase of a ref fails at commit X and is aborted by\nthe user, the ref will be updated twice. First to point at X (with the\nreflog message \"rebase finished: $head_name onto $onto\"), and then back\nto $orig_head. It should not be updated at all.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\nThis patch unfortunately duplicates some code from\nmove_to_original_branch, but since I'm refactoring the rebase code quite\na bit, I don't want to change too much in this patch. I will see what I\ncan do to reduce the duplication after some of the refactoring.\n\n git-rebase.sh           |    9 +++++++--\n t/t3407-rebase-abort.sh |   11 +++++++++++\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex ec08f9c..3d194b1 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -278,12 +278,17 @@ do\n \t\tif test -d \"$dotest\"\n \t\tthen\n \t\t\tGIT_QUIET=$(cat \"$dotest/quiet\")\n-\t\t\tmove_to_original_branch\n \t\telse\n \t\t\tdotest=\"$GIT_DIR\"/rebase-apply\n \t\t\tGIT_QUIET=$(cat \"$dotest/quiet\")\n-\t\t\tmove_to_original_branch\n \t\tfi\n+\t\thead_name=\"$(cat \"$dotest\"/head-name)\" &&\n+\t\tcase \"$head_name\" in\n+\t\trefs/*)\n+\t\t\tgit symbolic-ref HEAD $head_name ||\n+\t\t\tdie \"Could not move back to $head_name\"\n+\t\t\t;;\n+\t\tesac\n \t\tgit reset --hard $(cat \"$dotest/orig-head\")\n \t\trm -r \"$dotest\"\n \t\texit\ndiff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh\nindex fbb3f2e..f3250c3 100755\n--- a/t/t3407-rebase-abort.sh\n+++ b/t/t3407-rebase-abort.sh\n@@ -72,6 +72,17 @@ testrebase() {\n \t\ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase) &&\n \t\ttest ! -d \"$dotest\"\n \t'\n+\n+\ttest_expect_success \"rebase$type --abort does not update reflog\" '\n+\t\tcd \"$work_dir\" &&\n+\t\t# Clean up the state from the previous one\n+\t\tgit reset --hard pre-rebase &&\n+\t\treflog_entries_before=$(git reflog show to-rebase | wc -l) &&\n+\t\ttest_must_fail git rebase$type master &&\n+\t\tgit rebase --abort &&\n+\t\treflog_entries_after=$(git reflog show to-rebase | wc -l) &&\n+\t\ttest $reflog_entries_before -eq $reflog_entries_after\n+\t'\n }\n \n testrebase \"\" .git/rebase-apply\n-- \n1.7.3.2.190.gfb4ae\n"},{"id":"156300","messageId":"1290367234-8206-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"25801","inReplyTo":"1290337881-26449-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH] t3407: make reflog check stricter","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-21T19:20:34Z","receivedAt":"2010-11-21T19:20:34Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"---\nThis is hopefully an improvement to the test case. Please squash with\nprevious patch if you agree.\n\n t/t3407-rebase-abort.sh |    7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh\nindex f3250c3..e573dc8 100755\n--- a/t/t3407-rebase-abort.sh\n+++ b/t/t3407-rebase-abort.sh\n@@ -77,11 +77,12 @@ testrebase() {\n \t\tcd \"$work_dir\" &&\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n-\t\treflog_entries_before=$(git reflog show to-rebase | wc -l) &&\n+\t\tgit reflog show to-rebase > reflog_before &&\n \t\ttest_must_fail git rebase$type master &&\n \t\tgit rebase --abort &&\n-\t\treflog_entries_after=$(git reflog show to-rebase | wc -l) &&\n-\t\ttest $reflog_entries_before -eq $reflog_entries_after\n+\t\tgit reflog show to-rebase > reflog_after &&\n+\t\ttest_cmp reflog_before reflog_after &&\n+\t\trm reflog_before reflog_after\n \t'\n }\n \n-- \n1.7.3.2.190.gfb4ae\n"},{"id":"156302","messageId":"7v1v6evyq5.fsf@alter.siamese.dyndns.org","threadId":"25801","inReplyTo":"1290337881-26449-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH] rebase --abort: do not update branch ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-22T03:11:46Z","receivedAt":"2010-11-22T03:11:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n\n> diff --git a/git-rebase.sh b/git-rebase.sh\n> index ec08f9c..3d194b1 100755\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -278,12 +278,17 @@ do\n>  \t\tif test -d \"$dotest\"\n>  \t\tthen\n>  \t\t\tGIT_QUIET=$(cat \"$dotest/quiet\")\n> -\t\t\tmove_to_original_branch\n>  \t\telse\n>  \t\t\tdotest=\"$GIT_DIR\"/rebase-apply\n>  \t\t\tGIT_QUIET=$(cat \"$dotest/quiet\")\n> -\t\t\tmove_to_original_branch\n>  \t\tfi\n\nMicronit.  It appears that GIT_QUIET is set to the same value in either\ncase, so perhaps you would also want to move it outside of the if block,\ni.e.\n\n\ttest -d \"$dotest\" || dotest=\"$GIT_DIR/rebase-apply\"\n        GIT_QUIET=$(cat \"$dotest/quiet)\n        ... your rewrite to move_to_original_branch here ...\n\nno?  Staring at it further, I wonder who pays attention to GIT_QUIET in\nthis codepath that will soon exit, though.\n\n> +\t\thead_name=\"$(cat \"$dotest\"/head-name)\" &&\n> +\t\tcase \"$head_name\" in\n> +\t\trefs/*)\n> +\t\t\tgit symbolic-ref HEAD $head_name ||\n> +\t\t\tdie \"Could not move back to $head_name\"\n> +\t\t\t;;\n> +\t\tesac\n>  \t\tgit reset --hard $(cat \"$dotest/orig-head\")\n>  \t\trm -r \"$dotest\"\n>  \t\texit\n"},{"id":"156307","messageId":"AANLkTikwZTZhP5biBEsf62g+GTkZN7NwX=zeF2PKhmwO@mail.gmail.com","threadId":"25801","inReplyTo":"7v1v6evyq5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] rebase --abort: do not update branch ref","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-22T12:23:27Z","receivedAt":"2010-11-22T12:23:27Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Sun, Nov 21, 2010 at 10:11 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n>\n>> diff --git a/git-rebase.sh b/git-rebase.sh\n>> index ec08f9c..3d194b1 100755\n>> --- a/git-rebase.sh\n>> +++ b/git-rebase.sh\n>> @@ -278,12 +278,17 @@ do\n>>               if test -d \"$dotest\"\n>>               then\n>>                       GIT_QUIET=$(cat \"$dotest/quiet\")\n>> -                     move_to_original_branch\n>>               else\n>>                       dotest=\"$GIT_DIR\"/rebase-apply\n>>                       GIT_QUIET=$(cat \"$dotest/quiet\")\n>> -                     move_to_original_branch\n>>               fi\n>\n> Micronit.  It appears that GIT_QUIET is set to the same value in either\n> case, so perhaps you would also want to move it outside of the if block,\n> i.e.\n>\n>        test -d \"$dotest\" || dotest=\"$GIT_DIR/rebase-apply\"\n>        GIT_QUIET=$(cat \"$dotest/quiet)\n>        ... your rewrite to move_to_original_branch here ...\n>\n> no?  Staring at it further, I wonder who pays attention to GIT_QUIET in\n> this codepath that will soon exit, though.\n\nYes, I just didn't care to change it in this patch, because I'm changing\nthis code in the refactor I'm currently working on. OTOH, I don't mind\nif you do squash that in either. It might make sense since we don't yet\nknow if my refactoring series will accepted.\n\nBtw, I'm sure you noticed it, but also I sent a little fix for the test\nincluded in this patch. I have never used the In-Reply-To header, so I'm\nnot sure I used it right (judging by how the threaded Gmane view\npresents it, I did not).\n\nRegards,\nMartin\n"}]}