[PATCH 0/8] Re-roll rr/rebase-checkout-reflog

[PATCH 0/8] Re-roll rr/rebase-checkout-reflog

[Ramkumar Ramachandra]

This is a re-roll for rr/rebase-checkout-reflog, which is queued in
`pu`.

I've reviewed the changes carefully, and I believe that they are all
correct.  I've added a [6/8], because I think it is the correct way to
unset GIT_REFLOG_ACTION.

Junio: there is no need to re-export GIT_REFLOG_ACTION every time we
set it.  I've also fixed up some of your commit messages.  Let me know
if any further tweaks are necessary.

Thanks.

Junio C Hamano (1):
  t/status-help: test "HEAD detached from"

Ramkumar Ramachandra (7):
  wt-status: remove unused field in grab_1st_switch_cbdata
  t/checkout-last: test "checkout -" after a rebase
  status: do not depend on rebase reflog messages
  checkout: respect GIT_REFLOG_ACTION
  sh-setup: make die_with_status clear GIT_REFLOG_ACTION
  rebase: write better reflog messages
  rebase -i: write better reflog messages

 builtin/checkout.c         | 11 ++++++++---
 git-rebase--am.sh          |  2 ++
 git-rebase--interactive.sh |  3 +++
 git-rebase.sh              |  8 +++++++-
 git-sh-setup.sh            |  1 +
 t/t2012-checkout-last.sh   | 34 +++++++++++++++++++++++++++++++++
 t/t7512-status-help.sh     | 47 +++++++++++++++++++++++++---------------------
 wt-status.c                |  7 ++++---
 8 files changed, 85 insertions(+), 28 deletions(-)

-- 
1.8.3.1.456.gb7f4cb6

[PATCH 1/8] t/status-help: test "HEAD detached from"

[Ramkumar Ramachandra]

From: Junio C Hamano <gitster@pobox.com>

b397ea (status: show more info than "currently not on any branch",
2013-03-13) wanted to make sure that after a checkout to detach HEAD,
the user can see where the HEAD was originally detached from.  The last
test added by that commit to t/status-help shows one example,
immediately after HEAD is detached via a checkout.  Enhance that test to
test the "HEAD detached from" message is displayed when the user further
resets to another commit.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t7512-status-help.sh | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index bf08d4e..bafa5e7 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -667,7 +667,7 @@ test_expect_success 'status when cherry-picking after resolving conflicts' '
 	test_i18ncmp expected actual
 '
 
-test_expect_success 'status showing detached from a tag' '
+test_expect_success 'status showing detached at and from a tag' '
 	test_commit atag tagging &&
 	git checkout atag &&
 	cat >expected <<-\EOF
@@ -675,6 +675,14 @@ test_expect_success 'status showing detached from a tag' '
 	nothing to commit (use -u to show untracked files)
 	EOF
 	git status --untracked-files=no >actual &&
+	test_i18ncmp expected actual &&
+
+	git reset --hard HEAD^ &&
+	cat >expected <<-\EOF
+	# HEAD detached from atag
+	nothing to commit (use -u to show untracked files)
+	EOF
+	git status --untracked-files=no >actual &&
 	test_i18ncmp expected actual
 '
 
-- 
1.8.3.1.456.gb7f4cb6

[PATCH 2/8] wt-status: remove unused field in grab_1st_switch_cbdata

[Ramkumar Ramachandra]

The struct grab_1st_switch_cbdata has the field "found", which is
set in grab_1st_switch() when a match is found.  This information is
redundant and unused by any code.  The return value of the function
serves to communicate this information anyway.

Remove the field.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wt-status.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index bf84a86..2511270 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1035,7 +1035,6 @@ got_nothing:
 }
 
 struct grab_1st_switch_cbdata {
-	int found;
 	struct strbuf buf;
 	unsigned char nsha1[20];
 };
@@ -1059,7 +1058,6 @@ static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
 	for (end = target; *end && *end != '\n'; end++)
 		;
 	strbuf_add(&cb->buf, target, end - target);
-	cb->found = 1;
 	return 1;
 }
 
-- 
1.8.3.1.456.gb7f4cb6

[PATCH 3/8] t/checkout-last: test "checkout -" after a rebase

[Ramkumar Ramachandra]

  $ git checkout -

does not work as expected after a rebase.  This is because the
reflog records "checkout" made by "rebase" as its implementation
detail the same way as end-user initiated "checkout", and makes it
count as the branch that was previously checked out.

Add four failing tests documenting this bug: two for a normal rebase,
and another two for an interactive rebase.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t2012-checkout-last.sh | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
index b44de9d..6ad6edf 100755
--- a/t/t2012-checkout-last.sh
+++ b/t/t2012-checkout-last.sh
@@ -116,4 +116,38 @@ test_expect_success 'master...' '
 	test "z$(git rev-parse --verify HEAD)" = "z$(git rev-parse --verify master^)"
 '
 
+test_expect_failure '"checkout -" works after a rebase A' '
+	git checkout master &&
+	git checkout other &&
+	git rebase master &&
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
+'
+
+test_expect_failure '"checkout -" works after a rebase A B' '
+	git branch moodle master~1 &&
+	git checkout master &&
+	git checkout other &&
+	git rebase master moodle &&
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
+'
+
+test_expect_failure '"checkout -" works after a rebase -i A' '
+	git checkout master &&
+	git checkout other &&
+	git rebase -i master &&
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
+'
+
+test_expect_failure '"checkout -" works after a rebase -i A B' '
+	git branch foodle master~1 &&
+	git checkout master &&
+	git checkout other &&
+	git rebase master foodle &&
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
+'
+
 test_done
-- 
1.8.3.1.456.gb7f4cb6

[PATCH 4/8] status: do not depend on rebase reflog messages

[Ramkumar Ramachandra]

b397ea4 (status: show more info than "currently not on any branch",
2013-03-13) attempted to make the output of 'git status' richer in
the case of a detached HEAD.  Before this patch, with a detached
HEAD, we saw:

  $ git status
  # Not currently on any branch.

But after the patch, we see:

  $ git checkout v1.8.2
  $ git status
  # HEAD detached at v1.8.2

It works by digging the reflog for the most recent message of the
form "checkout: moving from xxxx to yyyy".  It then asserts that
HEAD and "yyyy" are the same, and displays this message.  When they
aren't equal, it displays:

  $ git status
  # HEAD detached from fe11db

so that the user can see where the HEAD was first detached.

In case of a rebase [-i] operation in progress, this message depends on
the implementation of rebase writing "checkout: " messages to the
reflog, but that is an implementation detail of "rebase".  To remove
this dependency so that rebase can be updated to write better reflog
messages, replace this "HEAD detached from" message with:

  # rebase in progress; onto $ONTO

Changes to the commit object name in the expected output for some of the
tests shows that what the test expected "status" to show during "rebase
-i" was not consistent with the output during a vanilla "rebase", which
showed on top of what commit the series is being replayed.  Now we
consistently show something meaningful to the end user.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7512-status-help.sh | 37 +++++++++++++++++--------------------
 wt-status.c            |  5 ++++-
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index bafa5e7..d6c66d7 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -77,7 +77,7 @@ test_expect_success 'status when rebase in progress before resolving conflicts'
 	ONTO=$(git rev-parse --short HEAD^^) &&
 	test_must_fail git rebase HEAD^ --onto HEAD^^ &&
 	cat >expected <<-EOF &&
-	# HEAD detached at $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$ONTO'\''.
 	#   (fix conflicts and then run "git rebase --continue")
 	#   (use "git rebase --skip" to skip this patch)
@@ -104,7 +104,7 @@ test_expect_success 'status when rebase in progress before rebase --continue' '
 	echo three >main.txt &&
 	git add main.txt &&
 	cat >expected <<-EOF &&
-	# HEAD detached at $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$ONTO'\''.
 	#   (all conflicts fixed: run "git rebase --continue")
 	#
@@ -136,7 +136,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' '
 	ONTO=$(git rev-parse --short rebase_i_conflicts) &&
 	test_must_fail git rebase -i rebase_i_conflicts &&
 	cat >expected <<-EOF &&
-	# HEAD detached at $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''.
 	#   (fix conflicts and then run "git rebase --continue")
 	#   (use "git rebase --skip" to skip this patch)
@@ -162,7 +162,7 @@ test_expect_success 'status during rebase -i after resolving conflicts' '
 	test_must_fail git rebase -i rebase_i_conflicts &&
 	git add main.txt &&
 	cat >expected <<-EOF &&
-	# HEAD detached at $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''.
 	#   (all conflicts fixed: run "git rebase --continue")
 	#
@@ -188,10 +188,9 @@ test_expect_success 'status when rebasing -i in edit mode' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~2) &&
-	TGT=$(git rev-parse --short two_rebase_i) &&
 	git rebase -i HEAD~2 &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $TGT
+	# rebase in progress; onto $ONTO
 	# You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -216,9 +215,8 @@ test_expect_success 'status when splitting a commit' '
 	ONTO=$(git rev-parse --short HEAD~3) &&
 	git rebase -i HEAD~3 &&
 	git reset HEAD^ &&
-	TGT=$(git rev-parse --short HEAD) &&
 	cat >expected <<-EOF &&
-	# HEAD detached at $TGT
+	# rebase in progress; onto $ONTO
 	# You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -246,11 +244,10 @@ test_expect_success 'status after editing the last commit with --amend during a
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
-	TGT=$(git rev-parse --short three_amend) &&
 	git rebase -i HEAD~3 &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $TGT
+	# rebase in progress; onto $ONTO
 	# You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -280,7 +277,7 @@ test_expect_success 'status: (continue first edit) second edit' '
 	git rebase -i HEAD~3 &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -302,7 +299,7 @@ test_expect_success 'status: (continue first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -329,7 +326,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -351,7 +348,7 @@ test_expect_success 'status: (amend first edit) second edit' '
 	git commit --amend -m "a" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -374,7 +371,7 @@ test_expect_success 'status: (amend first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -402,7 +399,7 @@ test_expect_success 'status: (amend first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "d" &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -426,7 +423,7 @@ test_expect_success 'status: (split first edit) second edit' '
 	git commit -m "e" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -451,7 +448,7 @@ test_expect_success 'status: (split first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -481,7 +478,7 @@ test_expect_success 'status: (split first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "h" &&
 	cat >expected <<-EOF &&
-	# HEAD detached from $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -601,7 +598,7 @@ test_expect_success 'status when rebase conflicts with statushints disabled' '
 	ONTO=$(git rev-parse --short HEAD^^) &&
 	test_must_fail git rebase HEAD^ --onto HEAD^^ &&
 	cat >expected <<-EOF &&
-	# HEAD detached at $ONTO
+	# rebase in progress; onto $ONTO
 	# You are currently rebasing branch '\''statushints_disabled'\'' on '\''$ONTO'\''.
 	#
 	# Unmerged paths:
diff --git a/wt-status.c b/wt-status.c
index 2511270..85a00f1 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1174,7 +1174,10 @@ void wt_status_print(struct wt_status *s)
 			branch_name += 11;
 		else if (!strcmp(branch_name, "HEAD")) {
 			branch_status_color = color(WT_STATUS_NOBRANCH, s);
-			if (state.detached_from) {
+			if (state.rebase_in_progress || state.rebase_interactive_in_progress) {
+				on_what = _("rebase in progress; onto ");
+				branch_name = state.onto;
+			} else if (state.detached_from) {
 				unsigned char sha1[20];
 				branch_name = state.detached_from;
 				if (!get_sha1("HEAD", sha1) &&
-- 
1.8.3.1.456.gb7f4cb6

Re: [PATCH 4/8] status: do not depend on rebase reflog messages

[Peter Krefting]

Ramkumar Ramachandra:

> +				on_what = _("rebase in progress; onto ");

Could you please add a

   /* TRANSLATORS: Followed by branch name. */

or something similar here (and possibly to the other "on_what"s in the 
function)?

Ideally, the "%s" for the branch name should be inside that 
on_what string, but I guess that can be difficult since it is output 
in a different colour?

-- 
\\// Peter - http://www.softwolves.pp.se/

[PATCH 5/8] checkout: respect GIT_REFLOG_ACTION

[Ramkumar Ramachandra]

GIT_REFLOG_ACTION is an environment variable specifying the reflog
message to write after an action is completed.  Several other commands
including merge, reset, and commit respect it.

Fix the failing tests in t/checkout-last by making checkout respect it
too.  You can now expect

  $ git checkout -

to work as expected after any a rebase [-i].  It will also work with any
other scripts provided they set an appropriate GIT_REFLOG_ACTION if they
internally use "git checkout".

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/checkout.c       | 11 ++++++++---
 t/t2012-checkout-last.sh |  8 ++++----
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index f5b50e5..1e2af85 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -587,7 +587,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
 				   struct branch_info *new)
 {
 	struct strbuf msg = STRBUF_INIT;
-	const char *old_desc;
+	const char *old_desc, *reflog_msg;
 	if (opts->new_branch) {
 		if (opts->new_orphan_branch) {
 			if (opts->new_branch_log && !log_all_ref_updates) {
@@ -620,8 +620,13 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
 	old_desc = old->name;
 	if (!old_desc && old->commit)
 		old_desc = sha1_to_hex(old->commit->object.sha1);
-	strbuf_addf(&msg, "checkout: moving from %s to %s",
-		    old_desc ? old_desc : "(invalid)", new->name);
+
+	reflog_msg = getenv("GIT_REFLOG_ACTION");
+	if (!reflog_msg)
+		strbuf_addf(&msg, "checkout: moving from %s to %s",
+			old_desc ? old_desc : "(invalid)", new->name);
+	else
+		strbuf_insert(&msg, 0, reflog_msg, strlen(reflog_msg));
 
 	if (!strcmp(new->name, "HEAD") && !new->path && !opts->force_detach) {
 		/* Nothing to do. */
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
index 6ad6edf..e7ba8c5 100755
--- a/t/t2012-checkout-last.sh
+++ b/t/t2012-checkout-last.sh
@@ -116,7 +116,7 @@ test_expect_success 'master...' '
 	test "z$(git rev-parse --verify HEAD)" = "z$(git rev-parse --verify master^)"
 '
 
-test_expect_failure '"checkout -" works after a rebase A' '
+test_expect_success '"checkout -" works after a rebase A' '
 	git checkout master &&
 	git checkout other &&
 	git rebase master &&
@@ -124,7 +124,7 @@ test_expect_failure '"checkout -" works after a rebase A' '
 	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
 '
 
-test_expect_failure '"checkout -" works after a rebase A B' '
+test_expect_success '"checkout -" works after a rebase A B' '
 	git branch moodle master~1 &&
 	git checkout master &&
 	git checkout other &&
@@ -133,7 +133,7 @@ test_expect_failure '"checkout -" works after a rebase A B' '
 	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
 '
 
-test_expect_failure '"checkout -" works after a rebase -i A' '
+test_expect_success '"checkout -" works after a rebase -i A' '
 	git checkout master &&
 	git checkout other &&
 	git rebase -i master &&
@@ -141,7 +141,7 @@ test_expect_failure '"checkout -" works after a rebase -i A' '
 	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
 '
 
-test_expect_failure '"checkout -" works after a rebase -i A B' '
+test_expect_success '"checkout -" works after a rebase -i A B' '
 	git branch foodle master~1 &&
 	git checkout master &&
 	git checkout other &&
-- 
1.8.3.1.456.gb7f4cb6

Re: [PATCH 5/8] checkout: respect GIT_REFLOG_ACTION

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Fix the failing tests in t/checkout-last by making checkout respect it
> too.  You can now expect
>
>   $ git checkout -
>
> to work as expected after any a rebase [-i].  It will also work with any
> other scripts provided they set an appropriate GIT_REFLOG_ACTION if they
> internally use "git checkout".

Yes, it is better to clearly say that this just opens the door to
fix others and at this step "rebase" is the primary thing that is
fixed.

By the way, please stop doing "t/checkout-last" which I have to
manually fix-up every time to its actual prefix (i.e. I cannot
review with "less t/checkout-last*" to see what the log message is
talking about; I can with "less t/t2012-checkout-last*").

Re: [PATCH 5/8] checkout: respect GIT_REFLOG_ACTION

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> By the way, please stop doing "t/checkout-last" which I have to
> manually fix-up every time to its actual prefix (i.e. I cannot
> review with "less t/checkout-last*" to see what the log message is
> talking about; I can with "less t/t2012-checkout-last*").

Ah, I didn't realize you used it like that.  I was trying to be terse
and informative in the subject: the four test numbers don't mean
anything to a casual reader, while t/checkout-last conveys
information.

[PATCH 6/8] sh-setup: make die_with_status clear GIT_REFLOG_ACTION

[Ramkumar Ramachandra]

Several callers set GIT_REFLOG_ACTION via set_reflog_action(), but
nobody unsets it, leaving a potentially stray variable in the
environment.  Fix this by making die_with_status() unset it.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-sh-setup.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 2f78359..3297103 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -51,6 +51,7 @@ die () {
 }
 
 die_with_status () {
+	export GIT_REFLOG_ACTION=
 	status=$1
 	shift
 	echo >&2 "$*"
-- 
1.8.3.1.456.gb7f4cb6

Re: [PATCH 6/8] sh-setup: make die_with_status clear GIT_REFLOG_ACTION

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Several callers set GIT_REFLOG_ACTION via set_reflog_action(), but
> nobody unsets it, leaving a potentially stray variable in the
> environment.  Fix this by making die_with_status() unset it.

I am totally lost.

 - You can set your own environment variables, and they will affect
   your child processes' environment, but only _before_ you spawn
   them.  After you spawn them, changes to your environment will not
   affect your child processes' environment.

 - There is no mechanism for you to affect environment of your
   parent process.

 - And we are exiting the process by calling die_with_status.

Whose environment are you trying to affect (or not affect) with this
change?  This will not affect your children, this will not affect
your parent, and this will not help you, either.

Puzzled.

>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  git-sh-setup.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 2f78359..3297103 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -51,6 +51,7 @@ die () {
>  }
>  
>  die_with_status () {
> +	export GIT_REFLOG_ACTION=
>  	status=$1
>  	shift
>  	echo >&2 "$*"

Re: [PATCH 6/8] sh-setup: make die_with_status clear GIT_REFLOG_ACTION

[Ramkumar Ramachandra]

Junio C Hamano wrote:
>  - There is no mechanism for you to affect environment of your
>    parent process.

Please excuse my stupidity and drop this patch.  I got mislead by your
SQUASH??? patch which took care to set the environment variable and
call checkout in a subshell.

Re: [PATCH 6/8] sh-setup: make die_with_status clear GIT_REFLOG_ACTION

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Please excuse my stupidity and drop this patch.  

Heh, we always have brain-fart every once in a while.  Your
stupidity is always gladly excused ;-)

> I got mislead by your SQUASH??? patch which took care to set the
> environment variable and call checkout in a subshell.

To some, we could have done

	ENV_VAR="$ENV_VAR: some other string" command

but there were uses of "output" shell function in some instances,
and

	VAR=VAL shell_function

does not work, so that was the reason why

	(
        	VAR=VAL; export VAR
                shell_function
	)

form was used.

[PATCH 7/8] rebase: write better reflog messages

[Ramkumar Ramachandra]

Now that the "checkout" invoked internally from "rebase" knows to honor
GIT_REFLOG_ACTION, we can start to use it to write a better reflog
message when "rebase anotherbranch", "rebase --onto branch",
etc. internally checks out the new fork point.  We will write:

  rebase: checkout master

instead of the old

  rebase

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-rebase--am.sh | 2 ++
 git-rebase.sh     | 8 +++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 34e3102..d4bade8 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -40,9 +40,11 @@ else
 		rm -f "$GIT_DIR/rebased-patches"
 		case "$head_name" in
 		refs/heads/*)
+			GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $head_name"
 			git checkout -q "$head_name"
 			;;
 		*)
+			GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $orig_head"
 			git checkout -q "$orig_head"
 			;;
 		esac
diff --git a/git-rebase.sh b/git-rebase.sh
index d0c11a9..b43600d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -543,7 +543,11 @@ then
 	if test -z "$force_rebase"
 	then
 		# Lazily switch to the target branch if needed...
-		test -z "$switch_to" || git checkout "$switch_to" --
+		if test -n "$switch_to"
+		then
+			GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
+			git checkout "$switch_to" --
+		fi
 		say "$(eval_gettext "Current branch \$branch_name is up to date.")"
 		exit 0
 	else
@@ -568,6 +572,8 @@ test "$type" = interactive && run_specific_rebase
 
 # Detach HEAD and reset the tree
 say "$(gettext "First, rewinding head to replay your work on top of it...")"
+
+GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
 git checkout -q "$onto^0" || die "could not detach HEAD"
 git update-ref ORIG_HEAD $orig_head
 
-- 
1.8.3.1.456.gb7f4cb6

[PATCH 8/8] rebase -i: write better reflog messages

[Ramkumar Ramachandra]

Now that the "checkout" invoked internally from "rebase -i" knows to
honor GIT_REFLOG_ACTION, we can start to use it to write a better reflog
message when "rebase -i anotherbranch", "rebase -i --onto branch",
etc. internally checks out the new fork point.  We will write:

  rebase -i (start): checkout master

instead of the old

  rebase -i (start)

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-rebase--interactive.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f953d8d..5179500 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -326,6 +326,7 @@ pick_one_preserving_merges () {
 		if [ "$1" != "-n" ]
 		then
 			# detach HEAD to current parent
+			GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $first_parent"
 			output git checkout $first_parent 2> /dev/null ||
 				die "Cannot move HEAD to $first_parent"
 		fi
@@ -838,6 +839,7 @@ comment_for_reflog start
 
 if test ! -z "$switch_to"
 then
+	GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
 	output git checkout "$switch_to" -- ||
 		die "Could not checkout $switch_to"
 fi
@@ -981,6 +983,7 @@ has_action "$todo" ||
 
 test -d "$rewritten" || test -n "$force_rebase" || skip_unnecessary_picks
 
+GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
 output git checkout $onto || die_abort "could not detach HEAD"
 git update-ref ORIG_HEAD $orig_head
 do_rest
-- 
1.8.3.1.456.gb7f4cb6