{"thread":{"id":"34162","subject":"[PATCH v2 0/6] Fix checkout-dash to work with rebase","startedAt":"2013-06-16T08:45:10Z","lastAt":"2013-06-18T16:51:07Z","messageCount":15,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":6},"messages":[{"id":"220985","messageId":"1371372316-16059-1-git-send-email-artagnon@gmail.com","threadId":"34162","inReplyTo":null,"subject":"[PATCH v2 0/6] Fix checkout-dash to work with rebase","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-16T08:45:10Z","receivedAt":"2013-06-16T08:45:10Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"So after extensive discussions with Junio, I have updated [5/6] to\nspecial-case rebase and rebase -i instead of dropping the \"HEAD\ndetached from\" message altogether.  Also, [1/6] includes two more\ntests, as suggested by Junio.\n\nJunio: The message is now the constant \"rebase in progress; onto\n$ONTO\".  Feel free to tweak it before applying.\n\nThanks.\n\nRamkumar Ramachandra (6):\n  t/checkout-last: checkout - doesn't work after rebase\n  rebase: prepare to write reflog message for checkout\n  rebase -i: prepare to write reflog message for checkout\n  wt-status: remove unused field in grab_1st_switch_cbdata\n  status: do not depend on rebase reflog messages\n  checkout: respect GIT_REFLOG_ACTION\n\n builtin/checkout.c         | 11 ++++++++---\n git-rebase--interactive.sh |  2 ++\n git-rebase.sh              |  2 ++\n t/t2012-checkout-last.sh   | 34 ++++++++++++++++++++++++++++++++++\n t/t7512-status-help.sh     | 37 +++++++++++++++++--------------------\n wt-status.c                |  7 ++++---\n 6 files changed, 67 insertions(+), 26 deletions(-)\n\n-- \n1.8.3.1.443.g4fd77b9\n"},{"id":"220986","messageId":"1371372316-16059-2-git-send-email-artagnon@gmail.com","threadId":"34162","inReplyTo":"1371372316-16059-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 1/6] t/checkout-last: checkout - doesn't work after rebase","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-16T08:45:11Z","receivedAt":"2013-06-16T08:45:11Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The following command\n\n  $ git checkout -\n\ndoes not work as expected after a rebase.  Every kind of rebase must\nbehave in the exactly same way: for the purposes of checkout -, the\nrebase event should be inconsequential.\n\nAdd four failing tests documenting this bug: two for a normal rebase,\nand another two for an interactive rebase.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t2012-checkout-last.sh | 34 ++++++++++++++++++++++++++++++++++\n 1 file changed, 34 insertions(+)\n\ndiff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh\nindex b44de9d..6ad6edf 100755\n--- a/t/t2012-checkout-last.sh\n+++ b/t/t2012-checkout-last.sh\n@@ -116,4 +116,38 @@ test_expect_success 'master...' '\n \ttest \"z$(git rev-parse --verify HEAD)\" = \"z$(git rev-parse --verify master^)\"\n '\n \n+test_expect_failure '\"checkout -\" works after a rebase A' '\n+\tgit checkout master &&\n+\tgit checkout other &&\n+\tgit rebase master &&\n+\tgit checkout - &&\n+\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/master\"\n+'\n+\n+test_expect_failure '\"checkout -\" works after a rebase A B' '\n+\tgit branch moodle master~1 &&\n+\tgit checkout master &&\n+\tgit checkout other &&\n+\tgit rebase master moodle &&\n+\tgit checkout - &&\n+\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/master\"\n+'\n+\n+test_expect_failure '\"checkout -\" works after a rebase -i A' '\n+\tgit checkout master &&\n+\tgit checkout other &&\n+\tgit rebase -i master &&\n+\tgit checkout - &&\n+\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/master\"\n+'\n+\n+test_expect_failure '\"checkout -\" works after a rebase -i A B' '\n+\tgit branch foodle master~1 &&\n+\tgit checkout master &&\n+\tgit checkout other &&\n+\tgit rebase master foodle &&\n+\tgit checkout - &&\n+\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/master\"\n+'\n+\n test_done\n-- \n1.8.3.1.443.g4fd77b9\n"},{"id":"220989","messageId":"1371372316-16059-3-git-send-email-artagnon@gmail.com","threadId":"34162","inReplyTo":"1371372316-16059-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 2/6] rebase: prepare to write reflog message for checkout","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-16T08:45:12Z","receivedAt":"2013-06-16T08:45:12Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The branch-flipping rebase internally does is not 'checkout' as far as\nthe end-user is concerned; therefore, rebase should never write\n\"checkout: \" messages to the reflog.  To achieve this, set a sensible\nGIT_REFLOG_ACTION; checkout does not respect this variable yet, but a\nfuture patch will change this.\n\nAfter that patch, rebase will write the following line to the reflog\nwhen started:\n\n  rebase: checkout master\n\nThis is much better than the confusing message it currently writes:\n\n  checkout: moving from master to 1462b67\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n git-rebase.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex d0c11a9..6587019 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -568,6 +568,8 @@ test \"$type\" = interactive && run_specific_rebase\n \n # Detach HEAD and reset the tree\n say \"$(gettext \"First, rewinding head to replay your work on top of it...\")\"\n+\n+GIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n git checkout -q \"$onto^0\" || die \"could not detach HEAD\"\n git update-ref ORIG_HEAD $orig_head\n \n-- \n1.8.3.1.443.g4fd77b9\n"},{"id":"220987","messageId":"1371372316-16059-4-git-send-email-artagnon@gmail.com","threadId":"34162","inReplyTo":"1371372316-16059-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 3/6] rebase -i: prepare to write reflog message for checkout","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-16T08:45:13Z","receivedAt":"2013-06-16T08:45:13Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The branch-flipping rebase -i internally does is not 'checkout' as far\nas the end-user is concerned; therefore, rebase -i should never write\n\"checkout: \" messages to the reflog.  To achieve this, set a sensible\nGIT_REFLOG_ACTION; checkout does not respect this variable yet, but a\nfuture patch will change this.\n\nAfter that patch, rebase -i will write the following line to the reflog\nwhen started:\n\n  rebase -i (start): checkout master\n\nThis is much better than the confusing message it currently writes:\n\n  checkout: moving from master to 1462b67\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n git-rebase--interactive.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex f953d8d..0f04425 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -838,6 +838,7 @@ comment_for_reflog start\n \n if test ! -z \"$switch_to\"\n then\n+\tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $switch_to\"\n \toutput git checkout \"$switch_to\" -- ||\n \t\tdie \"Could not checkout $switch_to\"\n fi\n@@ -981,6 +982,7 @@ has_action \"$todo\" ||\n \n test -d \"$rewritten\" || test -n \"$force_rebase\" || skip_unnecessary_picks\n \n+GIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n output git checkout $onto || die_abort \"could not detach HEAD\"\n git update-ref ORIG_HEAD $orig_head\n do_rest\n-- \n1.8.3.1.443.g4fd77b9\n"},{"id":"220991","messageId":"1371372316-16059-5-git-send-email-artagnon@gmail.com","threadId":"34162","inReplyTo":"1371372316-16059-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 4/6] wt-status: remove unused field in grab_1st_switch_cbdata","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-16T08:45:14Z","receivedAt":"2013-06-16T08:45:14Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The struct grab_1st_switch_cbdata has the field \"found\", which is set in\ngrab_1st_switch() when a match is found.  This information is redundant\nand unused by any caller: the return value of the function serves to\ncommunicate this information anyway.  Remove the field.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n wt-status.c | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex bf84a86..2511270 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1035,7 +1035,6 @@ got_nothing:\n }\n \n struct grab_1st_switch_cbdata {\n-\tint found;\n \tstruct strbuf buf;\n \tunsigned char nsha1[20];\n };\n@@ -1059,7 +1058,6 @@ static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,\n \tfor (end = target; *end && *end != '\\n'; end++)\n \t\t;\n \tstrbuf_add(&cb->buf, target, end - target);\n-\tcb->found = 1;\n \treturn 1;\n }\n \n-- \n1.8.3.1.443.g4fd77b9\n"},{"id":"220990","messageId":"1371372316-16059-6-git-send-email-artagnon@gmail.com","threadId":"34162","inReplyTo":"1371372316-16059-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 5/6] status: do not depend on rebase reflog messages","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-16T08:45:15Z","receivedAt":"2013-06-16T08:45:15Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"b397ea4 (status: show more info than \"currently not on any branch\",\n2013-03-13) made the output of 'git status' richer in the case of a\ndetached HEAD.  Before this patch, with a detached HEAD:\n\n  $ git status\n  # Not currently on any branch.\n\nAfter the patch:\n\n  $ git checkout v1.8.2\n  $ git status\n  # HEAD detached at v1.8.2.\n\nIt works by digging the reflog for the most recent message of the form\n\"checkout: moving from xxxx to yyyy\".  It then asserts that HEAD and\n\"yyyy\" are the same, and displays this message.  When they aren't equal,\nit displays:\n\n  $ git status\n  # HEAD detached from fe11db.\n\nIn case of a rebase [-i] operation in progress, this message depends on\nthe implementation of rebase writing \"checkout: \" messages to the\nreflog.  To remove this dependency so that rebase can be updated to\nwrite better reflog messages, replace this \"HEAD detached from\" message\nwith the constant:\n\n  # rebase in progress; onto $ONTO\n\nThe issue is not isolated to rebase at all.  Several other scripts like\nbisect write \"checkout: \" messages to the reflog, and the tests in\nt/status-help depend on them.  Fixing them is left as an exercise to\nother contributors.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t7512-status-help.sh | 37 +++++++++++++++++--------------------\n wt-status.c            |  5 ++++-\n 2 files changed, 21 insertions(+), 21 deletions(-)\n\ndiff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh\nindex bf08d4e..739624e 100755\n--- a/t/t7512-status-help.sh\n+++ b/t/t7512-status-help.sh\n@@ -77,7 +77,7 @@ test_expect_success 'status when rebase in progress before resolving conflicts'\n \tONTO=$(git rev-parse --short HEAD^^) &&\n \ttest_must_fail git rebase HEAD^ --onto HEAD^^ &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached at $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently rebasing branch '\\''rebase_conflicts'\\'' on '\\''$ONTO'\\''.\n \t#   (fix conflicts and then run \"git rebase --continue\")\n \t#   (use \"git rebase --skip\" to skip this patch)\n@@ -104,7 +104,7 @@ test_expect_success 'status when rebase in progress before rebase --continue' '\n \techo three >main.txt &&\n \tgit add main.txt &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached at $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently rebasing branch '\\''rebase_conflicts'\\'' on '\\''$ONTO'\\''.\n \t#   (all conflicts fixed: run \"git rebase --continue\")\n \t#\n@@ -136,7 +136,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' '\n \tONTO=$(git rev-parse --short rebase_i_conflicts) &&\n \ttest_must_fail git rebase -i rebase_i_conflicts &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached at $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently rebasing branch '\\''rebase_i_conflicts_second'\\'' on '\\''$ONTO'\\''.\n \t#   (fix conflicts and then run \"git rebase --continue\")\n \t#   (use \"git rebase --skip\" to skip this patch)\n@@ -162,7 +162,7 @@ test_expect_success 'status during rebase -i after resolving conflicts' '\n \ttest_must_fail git rebase -i rebase_i_conflicts &&\n \tgit add main.txt &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached at $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently rebasing branch '\\''rebase_i_conflicts_second'\\'' on '\\''$ONTO'\\''.\n \t#   (all conflicts fixed: run \"git rebase --continue\")\n \t#\n@@ -188,10 +188,9 @@ test_expect_success 'status when rebasing -i in edit mode' '\n \texport FAKE_LINES &&\n \ttest_when_finished \"git rebase --abort\" &&\n \tONTO=$(git rev-parse --short HEAD~2) &&\n-\tTGT=$(git rev-parse --short two_rebase_i) &&\n \tgit rebase -i HEAD~2 &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $TGT\n+\t# rebase in progress; onto $ONTO\n \t# You are currently editing a commit while rebasing branch '\\''rebase_i_edit'\\'' on '\\''$ONTO'\\''.\n \t#   (use \"git commit --amend\" to amend the current commit)\n \t#   (use \"git rebase --continue\" once you are satisfied with your changes)\n@@ -216,9 +215,8 @@ test_expect_success 'status when splitting a commit' '\n \tONTO=$(git rev-parse --short HEAD~3) &&\n \tgit rebase -i HEAD~3 &&\n \tgit reset HEAD^ &&\n-\tTGT=$(git rev-parse --short HEAD) &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached at $TGT\n+\t# rebase in progress; onto $ONTO\n \t# You are currently splitting a commit while rebasing branch '\\''split_commit'\\'' on '\\''$ONTO'\\''.\n \t#   (Once your working directory is clean, run \"git rebase --continue\")\n \t#\n@@ -246,11 +244,10 @@ test_expect_success 'status after editing the last commit with --amend during a\n \texport FAKE_LINES &&\n \ttest_when_finished \"git rebase --abort\" &&\n \tONTO=$(git rev-parse --short HEAD~3) &&\n-\tTGT=$(git rev-parse --short three_amend) &&\n \tgit rebase -i HEAD~3 &&\n \tgit commit --amend -m \"foo\" &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $TGT\n+\t# rebase in progress; onto $ONTO\n \t# You are currently editing a commit while rebasing branch '\\''amend_last'\\'' on '\\''$ONTO'\\''.\n \t#   (use \"git commit --amend\" to amend the current commit)\n \t#   (use \"git rebase --continue\" once you are satisfied with your changes)\n@@ -280,7 +277,7 @@ test_expect_success 'status: (continue first edit) second edit' '\n \tgit rebase -i HEAD~3 &&\n \tgit rebase --continue &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently editing a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n \t#   (use \"git commit --amend\" to amend the current commit)\n \t#   (use \"git rebase --continue\" once you are satisfied with your changes)\n@@ -302,7 +299,7 @@ test_expect_success 'status: (continue first edit) second edit and split' '\n \tgit rebase --continue &&\n \tgit reset HEAD^ &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently splitting a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n \t#   (Once your working directory is clean, run \"git rebase --continue\")\n \t#\n@@ -329,7 +326,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' '\n \tgit rebase --continue &&\n \tgit commit --amend -m \"foo\" &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently editing a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n \t#   (use \"git commit --amend\" to amend the current commit)\n \t#   (use \"git rebase --continue\" once you are satisfied with your changes)\n@@ -351,7 +348,7 @@ test_expect_success 'status: (amend first edit) second edit' '\n \tgit commit --amend -m \"a\" &&\n \tgit rebase --continue &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently editing a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n \t#   (use \"git commit --amend\" to amend the current commit)\n \t#   (use \"git rebase --continue\" once you are satisfied with your changes)\n@@ -374,7 +371,7 @@ test_expect_success 'status: (amend first edit) second edit and split' '\n \tgit rebase --continue &&\n \tgit reset HEAD^ &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently splitting a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n \t#   (Once your working directory is clean, run \"git rebase --continue\")\n \t#\n@@ -402,7 +399,7 @@ test_expect_success 'status: (amend first edit) second edit and amend' '\n \tgit rebase --continue &&\n \tgit commit --amend -m \"d\" &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently editing a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n \t#   (use \"git commit --amend\" to amend the current commit)\n \t#   (use \"git rebase --continue\" once you are satisfied with your changes)\n@@ -426,7 +423,7 @@ test_expect_success 'status: (split first edit) second edit' '\n \tgit commit -m \"e\" &&\n \tgit rebase --continue &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently editing a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n \t#   (use \"git commit --amend\" to amend the current commit)\n \t#   (use \"git rebase --continue\" once you are satisfied with your changes)\n@@ -451,7 +448,7 @@ test_expect_success 'status: (split first edit) second edit and split' '\n \tgit rebase --continue &&\n \tgit reset HEAD^ &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently splitting a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n \t#   (Once your working directory is clean, run \"git rebase --continue\")\n \t#\n@@ -481,7 +478,7 @@ test_expect_success 'status: (split first edit) second edit and amend' '\n \tgit rebase --continue &&\n \tgit commit --amend -m \"h\" &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached from $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently editing a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n \t#   (use \"git commit --amend\" to amend the current commit)\n \t#   (use \"git rebase --continue\" once you are satisfied with your changes)\n@@ -601,7 +598,7 @@ test_expect_success 'status when rebase conflicts with statushints disabled' '\n \tONTO=$(git rev-parse --short HEAD^^) &&\n \ttest_must_fail git rebase HEAD^ --onto HEAD^^ &&\n \tcat >expected <<-EOF &&\n-\t# HEAD detached at $ONTO\n+\t# rebase in progress; onto $ONTO\n \t# You are currently rebasing branch '\\''statushints_disabled'\\'' on '\\''$ONTO'\\''.\n \t#\n \t# Unmerged paths:\ndiff --git a/wt-status.c b/wt-status.c\nindex 2511270..85a00f1 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1174,7 +1174,10 @@ void wt_status_print(struct wt_status *s)\n \t\t\tbranch_name += 11;\n \t\telse if (!strcmp(branch_name, \"HEAD\")) {\n \t\t\tbranch_status_color = color(WT_STATUS_NOBRANCH, s);\n-\t\t\tif (state.detached_from) {\n+\t\t\tif (state.rebase_in_progress || state.rebase_interactive_in_progress) {\n+\t\t\t\ton_what = _(\"rebase in progress; onto \");\n+\t\t\t\tbranch_name = state.onto;\n+\t\t\t} else if (state.detached_from) {\n \t\t\t\tunsigned char sha1[20];\n \t\t\t\tbranch_name = state.detached_from;\n \t\t\t\tif (!get_sha1(\"HEAD\", sha1) &&\n-- \n1.8.3.1.443.g4fd77b9\n"},{"id":"220988","messageId":"1371372316-16059-7-git-send-email-artagnon@gmail.com","threadId":"34162","inReplyTo":"1371372316-16059-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 6/6] checkout: respect GIT_REFLOG_ACTION","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-16T08:45:16Z","receivedAt":"2013-06-16T08:45:16Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"GIT_REFLOG_ACTION is an environment variable specifying the reflog\nmessage to write after an action is completed.  Several other commands\nincluding merge, reset, and commit respect it.\n\nFix the failing tests in t/checkout-last by making checkout respect it\ntoo.  You can now expect\n\n  $ git checkout -\n\nto work as expected after any rebase operation.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n builtin/checkout.c       | 11 ++++++++---\n t/t2012-checkout-last.sh |  8 ++++----\n 2 files changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex f5b50e5..1e2af85 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -587,7 +587,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \t\t\t\t   struct branch_info *new)\n {\n \tstruct strbuf msg = STRBUF_INIT;\n-\tconst char *old_desc;\n+\tconst char *old_desc, *reflog_msg;\n \tif (opts->new_branch) {\n \t\tif (opts->new_orphan_branch) {\n \t\t\tif (opts->new_branch_log && !log_all_ref_updates) {\n@@ -620,8 +620,13 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \told_desc = old->name;\n \tif (!old_desc && old->commit)\n \t\told_desc = sha1_to_hex(old->commit->object.sha1);\n-\tstrbuf_addf(&msg, \"checkout: moving from %s to %s\",\n-\t\t    old_desc ? old_desc : \"(invalid)\", new->name);\n+\n+\treflog_msg = getenv(\"GIT_REFLOG_ACTION\");\n+\tif (!reflog_msg)\n+\t\tstrbuf_addf(&msg, \"checkout: moving from %s to %s\",\n+\t\t\told_desc ? old_desc : \"(invalid)\", new->name);\n+\telse\n+\t\tstrbuf_insert(&msg, 0, reflog_msg, strlen(reflog_msg));\n \n \tif (!strcmp(new->name, \"HEAD\") && !new->path && !opts->force_detach) {\n \t\t/* Nothing to do. */\ndiff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh\nindex 6ad6edf..e7ba8c5 100755\n--- a/t/t2012-checkout-last.sh\n+++ b/t/t2012-checkout-last.sh\n@@ -116,7 +116,7 @@ test_expect_success 'master...' '\n \ttest \"z$(git rev-parse --verify HEAD)\" = \"z$(git rev-parse --verify master^)\"\n '\n \n-test_expect_failure '\"checkout -\" works after a rebase A' '\n+test_expect_success '\"checkout -\" works after a rebase A' '\n \tgit checkout master &&\n \tgit checkout other &&\n \tgit rebase master &&\n@@ -124,7 +124,7 @@ test_expect_failure '\"checkout -\" works after a rebase A' '\n \ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/master\"\n '\n \n-test_expect_failure '\"checkout -\" works after a rebase A B' '\n+test_expect_success '\"checkout -\" works after a rebase A B' '\n \tgit branch moodle master~1 &&\n \tgit checkout master &&\n \tgit checkout other &&\n@@ -133,7 +133,7 @@ test_expect_failure '\"checkout -\" works after a rebase A B' '\n \ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/master\"\n '\n \n-test_expect_failure '\"checkout -\" works after a rebase -i A' '\n+test_expect_success '\"checkout -\" works after a rebase -i A' '\n \tgit checkout master &&\n \tgit checkout other &&\n \tgit rebase -i master &&\n@@ -141,7 +141,7 @@ test_expect_failure '\"checkout -\" works after a rebase -i A' '\n \ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/master\"\n '\n \n-test_expect_failure '\"checkout -\" works after a rebase -i A B' '\n+test_expect_success '\"checkout -\" works after a rebase -i A B' '\n \tgit branch foodle master~1 &&\n \tgit checkout master &&\n \tgit checkout other &&\n-- \n1.8.3.1.443.g4fd77b9\n"},{"id":"221033","messageId":"7v1u81ibjp.fsf@alter.siamese.dyndns.org","threadId":"34162","inReplyTo":"1371372316-16059-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v2 0/6] Fix checkout-dash to work with rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-17T04:52:26Z","receivedAt":"2013-06-17T04:52:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> So after extensive discussions with Junio, I have updated [5/6] to\n> special-case rebase and rebase -i instead of dropping the \"HEAD\n> detached from\" message altogether.  Also, [1/6] includes two more\n> tests, as suggested by Junio.\n>\n> Junio: The message is now the constant \"rebase in progress; onto\n> $ONTO\".\n\nThat message is better than I was anticipating.\n\nI was actually assuming that you would leave them as they are\ni.e. \"# HEAD detached at xxx\", as we agreed that we will see them\nimproved anyway by the other topic.\n\nI am still puzzled to see that the change to checkout comes at the\nvery end.  With \"do not depend on rebase reflog messages\" done\nbefore the \"checkout: respect reflog-action\", I was hoping that we\ncan have changes to the actual reflog message made to rebase (patch\n2 and 3) can be done at the very end.  Was I missing something else?\n\nIn other words, the order I was anticipating to see after the\ndiscussion (this is different from saying \"A series that is not\nordered like this is unacceptable\") was:\n\n>   wt-status: remove unused field in grab_1st_switch_cbdata\n\nThis is an unrelated clean-up, and can be done before anything else.\n\n>   t/checkout-last: checkout - doesn't work after rebase\n\nThis spells out what we want to happen at the end and marks the\ncurrent breakage.\n\n>   status: do not depend on rebase reflog messages\n\nThis compensates for fallouts from the next change.\n\n>   checkout: respect GIT_REFLOG_ACTION\n\nAnd this is the fix, the most important step.\n\n>   rebase: prepare to write reflog message for checkout\n>   rebase -i: prepare to write reflog message for checkout\n\nAnd these are icing on the cake, but that cannot be done before\nstatus is fixed.\n"},{"id":"221080","messageId":"7vr4g0ei6n.fsf@alter.siamese.dyndns.org","threadId":"34162","inReplyTo":"1371372316-16059-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v2 5/6] status: do not depend on rebase reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-17T17:54:56Z","receivedAt":"2013-06-17T17:54:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh\n> index bf08d4e..739624e 100755\n> --- a/t/t7512-status-help.sh\n> +++ b/t/t7512-status-help.sh\n> @@ -188,10 +188,9 @@ test_expect_success 'status when rebasing -i in edit mode' '\n>  \texport FAKE_LINES &&\n>  \ttest_when_finished \"git rebase --abort\" &&\n>  \tONTO=$(git rev-parse --short HEAD~2) &&\n> -\tTGT=$(git rev-parse --short two_rebase_i) &&\n>  \tgit rebase -i HEAD~2 &&\n>  \tcat >expected <<-EOF &&\n> -\t# HEAD detached from $TGT\n> +\t# rebase in progress; onto $ONTO\n>  \t# You are currently editing a commit while rebasing branch '\\''rebase_i_edit'\\'' on '\\''$ONTO'\\''.\n>  \t#   (use \"git commit --amend\" to amend the current commit)\n>  \t#   (use \"git rebase --continue\" once you are satisfied with your changes)\n\nThis hunk (and others that used to use $TGT) shows that the original\ntest was depending too much on the implementation detail (i.e. that\nrebase/rebase-i internally uses checkout to affect the reflog, making\nthe info shown as \"detached from/at\" not useful during \"rebase\").\nThis step may have started as a workaround against the fallout from 6/6,\nbut I think this is a good change by itself independent of \"checkut -\"\nfix, which is the main topic of this series.\n\n> diff --git a/wt-status.c b/wt-status.c\n> index 2511270..85a00f1 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1174,7 +1174,10 @@ void wt_status_print(struct wt_status *s)\n>  \t\t\tbranch_name += 11;\n>  \t\telse if (!strcmp(branch_name, \"HEAD\")) {\n>  \t\t\tbranch_status_color = color(WT_STATUS_NOBRANCH, s);\n> -\t\t\tif (state.detached_from) {\n> +\t\t\tif (state.rebase_in_progress || state.rebase_interactive_in_progress) {\n> +\t\t\t\ton_what = _(\"rebase in progress; onto \");\n> +\t\t\t\tbranch_name = state.onto;\n\nThe part after \"||\" is what I missed in the \"how about an approach\nalong this line\" patch.\n\nGood job.\n\n> +\t\t\t} else if (state.detached_from) {\n>  \t\t\t\tunsigned char sha1[20];\n>  \t\t\t\tbranch_name = state.detached_from;\n>  \t\t\t\tif (!get_sha1(\"HEAD\", sha1) &&\n"},{"id":"221081","messageId":"7vk3lsei4i.fsf@alter.siamese.dyndns.org","threadId":"34162","inReplyTo":"7v1u81ibjp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/6] Fix checkout-dash to work with rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-17T17:56:13Z","receivedAt":"2013-06-17T17:56:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> In other words, the order I was anticipating to see after the\n> discussion (this is different from saying \"A series that is not\n> ordered like this is unacceptable\") was:\n>\n>>   wt-status: remove unused field in grab_1st_switch_cbdata\n>\n> This is an unrelated clean-up, and can be done before anything else.\n>\n>>   t/checkout-last: checkout - doesn't work after rebase\n>\n> This spells out what we want to happen at the end and marks the\n> current breakage.\n>\n>>   status: do not depend on rebase reflog messages\n>\n> This compensates for fallouts from the next change.\n>\n>>   checkout: respect GIT_REFLOG_ACTION\n>\n> And this is the fix, the most important step.\n>\n>>   rebase: prepare to write reflog message for checkout\n>>   rebase -i: prepare to write reflog message for checkout\n>\n> And these are icing on the cake, but that cannot be done before\n> status is fixed.\n\nI actually tried to reorder the patches and they seem to work OK as\nexpected.  And I think it makes sense to order them the way I've\nbeen suggesting, so I'll tentatively queue the result of reordering\non 'rr/rebase-checkout-reflog' and push it out as a part of 'pu'.\n\nPlease check to see if I introduced a new bug while doing so.\n\nRegardless of the ordering, however, I suspect two patches that\nchange the message recorded in reflog in \"rebase\" need further\nfixing.\n\nFor example, the one in \"git reabse\" does this:\n\n    GIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n    git checkout -q \"$onto^0\" || die \"could not detach HEAD\"\n    git update-ref ORIG_HEAD $orig_head\n    ...\n    run_specific_rebase\n\nBut the specific rebase, e.g. git-rebase--interactive, does this:\n\n\tcase $head_name in\n\trefs/*)\n\t\tmessage=\"$GIT_REFLOG_ACTION: $head_name onto $onto\" &&\n\t\tgit update-ref -m \"$message\" $head_name $newhead $orig_head &&\n\t\tgit symbolic-ref \\\n\t\t  -m \"$GIT_REFLOG_ACTION: returning to $head_name\" \\\n\t\t  HEAD $head_name\n\t\t;;\n\tesac && {\n\nI think the message you added to \"git reabse\" is only meant for that\nspecific \"checkout $onto\", but it is set globally.  Wouldn't it\naffect later use, which expected it to be \"rebase\" and nothing else?\n\nPerhaps something like this on top of the entire series may be\nsufficient (which will be queued as \"SQUASH???\" at the tip).\n\nHint for anybody (not limited to Ram):\n\nThere also are other 'git checkout' invocations in git-rebase\\*.sh\nthat are not yet covered by these \"nicer reflog message\" patches,\nwhich may want to be fixed up before these two icing-on-cake patches\nbecome ready to be merged; see output of\n\n\tgit grep -C2 'git checkout' -- git-rebase\\*.sh\n\nThanks.\n\n git-rebase--interactive.sh    | 15 ++++++++++-----\n git-rebase.sh                 |  4 ++--\n t/t3404-rebase-interactive.sh | 15 +++++++++++++++\n 3 files changed, 27 insertions(+), 7 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex a05a6e4..f21ff7c 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -837,9 +837,11 @@ comment_for_reflog start\n \n if test ! -z \"$switch_to\"\n then\n-\tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $switch_to\"\n-\toutput git checkout \"$switch_to\" -- ||\n-\t\tdie \"Could not checkout $switch_to\"\n+\t(\n+\t\tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $switch_to\"\n+\t\texport GIT_REFLOG_ACTION\n+\t\toutput git checkout \"$switch_to\" --\n+\t) || die \"Could not checkout $switch_to\"\n fi\n \n orig_head=$(git rev-parse --verify HEAD) || die \"No HEAD?\"\n@@ -981,7 +983,10 @@ has_action \"$todo\" ||\n \n test -d \"$rewritten\" || test -n \"$force_rebase\" || skip_unnecessary_picks\n \n-GIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n-output git checkout $onto || die_abort \"could not detach HEAD\"\n+(\n+\tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n+\texport GIT_REFLOG_ACTION\n+\toutput git checkout $onto\n+) || die_abort \"could not detach HEAD\"\n git update-ref ORIG_HEAD $orig_head\n do_rest\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 7d55372..2344eef 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -523,8 +523,8 @@ test \"$type\" = interactive && run_specific_rebase\n # Detach HEAD and reset the tree\n say \"$(gettext \"First, rewinding head to replay your work on top of it...\")\"\n \n-GIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n-git checkout -q \"$onto^0\" || die \"could not detach HEAD\"\n+GIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\" \\\n+\tgit checkout -q \"$onto^0\" || die \"could not detach HEAD\"\n git update-ref ORIG_HEAD $orig_head\n \n # If the $onto is a proper descendant of the tip of the branch, then\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex a58406d..c175ef1 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -934,6 +934,21 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '\n \ttest L = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n+test_expect_success 'rebase -i produces readable reflog' '\n+\tgit reset --hard &&\n+\tgit branch -f branch1 H &&\n+\tgit rebase -i --onto I F branch1 &&\n+\tcat >expect <<-\\EOF &&\n+\trebase -i (start): checkout I\n+\trebase -i (pick): G\n+\trebase -i (pick): H\n+\trebase -i (finish): returning to refs/heads/branch1\n+\tEOF\n+\ttail -n 4 .git/logs/HEAD |\n+\tsed -e \"s/.*\t//\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'rebase -i respects core.commentchar' '\n \tgit reset --hard &&\n \tgit checkout E^0 &&\n"},{"id":"221168","messageId":"CALkWK0nGeyna_vxnN0RMULRj3sEj3v=Q61cXrHDYDHYv-nZ4sg@mail.gmail.com","threadId":"34162","inReplyTo":"7vk3lsei4i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/6] Fix checkout-dash to work with rebase","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-18T09:30:38Z","receivedAt":"2013-06-18T09:30:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> I actually tried to reorder the patches and they seem to work OK as\n> expected.  And I think it makes sense to order them the way I've\n> been suggesting, so I'll tentatively queue the result of reordering\n> on 'rr/rebase-checkout-reflog' and push it out as a part of 'pu'.\n>\n> Please check to see if I introduced a new bug while doing so.\n\nThanks for the reorder and commit message tweaks.  I'm working on the\nseries you put up on `pu` now.\n\n> For example, the one in \"git reabse\" does this:\n>\n>     GIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n>     git checkout -q \"$onto^0\" || die \"could not detach HEAD\"\n>     git update-ref ORIG_HEAD $orig_head\n>     ...\n>     run_specific_rebase\n>\n> But the specific rebase, e.g. git-rebase--interactive, does this:\n>\n>         case $head_name in\n>         refs/*)\n>                 message=\"$GIT_REFLOG_ACTION: $head_name onto $onto\" &&\n>                 git update-ref -m \"$message\" $head_name $newhead $orig_head &&\n>                 git symbolic-ref \\\n>                   -m \"$GIT_REFLOG_ACTION: returning to $head_name\" \\\n>                   HEAD $head_name\n>                 ;;\n>         esac && {\n>\n> I think the message you added to \"git reabse\" is only meant for that\n> specific \"checkout $onto\", but it is set globally.  Wouldn't it\n> affect later use, which expected it to be \"rebase\" and nothing else?\n\nBoth rebase.sh and rebase--interactive.sh set a sane GIT_REFLOG_ACTION\nright on top (using set_reflog_action), so no worries.  I'll just\ndouble-check to make sure that no bogus/ incorrect messages are\nprinted.\n\n> Perhaps something like this on top of the entire series may be\n> sufficient (which will be queued as \"SQUASH???\" at the tip).\n\nI think this takes the wrong approach to the problem.  In my opinion,\nthe correct approach is to actually overshadow die() with a function\nthat clears GIT_REFLOG_ACTION before calling die().\n\n>         git grep -C2 'git checkout' -- git-rebase\\*.sh\n\nUgh.  I'll check all the codepaths thoroughly before submitting a re-roll.\n\nThanks.\n"},{"id":"221170","messageId":"CALkWK0nO6fVh2jg9WC_4zN1rKtXRQCsMbfOgmw-g3pvrwBfTZw@mail.gmail.com","threadId":"34162","inReplyTo":"CALkWK0nGeyna_vxnN0RMULRj3sEj3v=Q61cXrHDYDHYv-nZ4sg@mail.gmail.com","subject":"Re: [PATCH v2 0/6] Fix checkout-dash to work with rebase","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-18T09:50:36Z","receivedAt":"2013-06-18T09:50:36Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Thanks for the reorder and commit message tweaks.  I'm working on the\n> series you put up on `pu` now.\n\nThat said, I do not agree with one of your commit message updates:\n\n    checkout: respect GIT_REFLOG_ACTION\n\n    GIT_REFLOG_ACTION is an environment variable specifying the reflog\n    message to write after an action is completed.  Several other commands\n    including merge, reset, and commit respect it.\n\n    Fix the failing tests in t/checkout-last by making checkout respect it\n    too.  You can now expect\n\n      $ git checkout -\n\n    to work as expected after any operation that internally uses \"checkout\"\n    as its implementation detail, e.g. \"rebase\".\n\nAfter the patch you _cannot_ expect checkout-dash to work after any\noperation that internally uses checkout; it's limited to rebase.  Many\nother scripts (eg. bisect) do _not_ set GIT_REFLOG_ACTION at all.\n"},{"id":"221179","messageId":"CALkWK0n2AvL49DNp1t7ty0Dneprd7r_V2p=brS0qMKz=wdZz6A@mail.gmail.com","threadId":"34162","inReplyTo":"7vk3lsei4i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/6] Fix checkout-dash to work with rebase","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-18T10:36:03Z","receivedAt":"2013-06-18T10:36:03Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index a58406d..c175ef1 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -934,6 +934,21 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '\n>         test L = $(git cat-file commit HEAD | sed -ne \\$p)\n>  '\n>\n> +test_expect_success 'rebase -i produces readable reflog' '\n\nI don't know if this test is a good idea at all.  We never directly\ncheck the messages written by a command to the reflog, and I suspect\nthat there is a good reason for this: the format of .git/logs is not\nguaranteed to be stable, and the messages written by various commands\nare not guaranteed to be stable either; the only machine-parsing of\nreflogs we do is very minimal: interpret_nth_prior_checkout() and\ngrab_1st_switch().\n\nA quick\n\n  $ git grep .git/logs -- t\n\nshows that I'm mostly right about this.\n\nWhy make an exception in the case of rebase -i?  In the worst case,\nthe patch atleast needs to be discussed as an independent patch: it's\ncertainly not obvious enough to sneak into this series.  I'll submit a\nre-roll without this hunk.\n"},{"id":"221248","messageId":"7vip1b75uc.fsf@alter.siamese.dyndns.org","threadId":"34162","inReplyTo":"CALkWK0n2AvL49DNp1t7ty0Dneprd7r_V2p=brS0qMKz=wdZz6A@mail.gmail.com","subject":"Re: [PATCH v2 0/6] Fix checkout-dash to work with rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-18T16:15:39Z","receivedAt":"2013-06-18T16:15:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n>> index a58406d..c175ef1 100755\n>> --- a/t/t3404-rebase-interactive.sh\n>> +++ b/t/t3404-rebase-interactive.sh\n>> @@ -934,6 +934,21 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '\n>>         test L = $(git cat-file commit HEAD | sed -ne \\$p)\n>>  '\n>>\n>> +test_expect_success 'rebase -i produces readable reflog' '\n>\n> I don't know if this test is a good idea at all.\n\nIt is.\n\nThis catches \"a change to GIT_REFLOG_ACTION that is meant to only\napply to checkout done internally in git-rebase leaks to the later\ncodepath and affects the reflog message left by rebase--interactive\".\n\nApply the test change without the \"do not leak\" part in the fix-up\n(queued as a single \"SQUASH???\" commit on 'pu') to what you posted\nearlier and see how it breaks.\n\n    # rr/rebase-checkout-reflog with fix-up\n    $ git checkout 33b1cdeb \n    # revert the fix but keep the test\n    $ git checkout HEAD^ -- git-rebase--interactive.sh git-rebase.sh\n    # run the test\n    $ make && cd t && sh ./t3404-*.sh -v -i\n\nThe test fails with this:\n\n    --- expect      2013-06-18 16:09:21.000000000 +0000\n    +++ actual      2013-06-18 16:09:21.000000000 +0000\n    @@ -1,4 +1,4 @@\n    -rebase -i (start): checkout I\n    +rebase -i (start): checkout branch-reflog-test: checkout I\n     rebase -i (pick): G\n     rebase -i (pick): H\n     rebase -i (finish): returning to refs/heads/branch-reflog-test\n\n\"checkout branch-reflog-test\" part is leaking from your setting of\nGIT_REFLOG_ACTION for \"git checkout\" in \"git rebase\"; it is only\nnecessary to affect that \"checkout\" and it should not affect later\ncommands that write reflog entries, but we see the breakage.\n\nAnd that is why I did the fix-up to make sure your changes only\napply to \"git checkout\".\n\nIf you apply the test part of that fixup to what you posted today,\nwhat would you see?  I didn't look at the patches very closely, but\nI wouldn't be surprised if they are still leaking the change meant\nonly to \"checkout\" to the later stages and breaking that test (the\nreason why I would not be surprised is because the impression I am\ngetting from reading your responses is that you do not understand\nwhy it is bad not to limit the setting only to checkout).\n\n> Why make an exception in the case of rebase -i?\n\nBecause the whole point of last two patches are \"word reflog\nmessages better for rebase\", the improvements made by these two\npatches are better protected from future changes; it also makes sure\nthat deliberate improvements will show up as changes to the tests.\n"},{"id":"221259","messageId":"CALkWK0m90Z7Qsxr6ai7t-D2LW6WP6sVaQYcM0eCa1R2xYNk9Jw@mail.gmail.com","threadId":"34162","inReplyTo":"7vip1b75uc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/6] Fix checkout-dash to work with rebase","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-18T16:51:07Z","receivedAt":"2013-06-18T16:51:07Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Apply the test change without the \"do not leak\" part in the fix-up\n> (queued as a single \"SQUASH???\" commit on 'pu') to what you posted\n> earlier and see how it breaks.\n>\n>     --- expect      2013-06-18 16:09:21.000000000 +0000\n>     +++ actual      2013-06-18 16:09:21.000000000 +0000\n>     @@ -1,4 +1,4 @@\n>     -rebase -i (start): checkout I\n>     +rebase -i (start): checkout branch-reflog-test: checkout I\n>      rebase -i (pick): G\n>      rebase -i (pick): H\n>      rebase -i (finish): returning to refs/heads/branch-reflog-test\n>\n> If you apply the test part of that fixup to what you posted today,\n> what would you see?\n\nDamn it!  I didn't realize that your subshell was for a do-not-leak.\nI'm feeling especially stupid now.\n\nWill post a re-roll in a bit.\n"}]}