[09/11] rebase --apply: fix reflog

Commit Message

Phillip Wood Oct. 1, 2021, 10:05 a.m. UTC

From: Phillip Wood <[e-mail]>

move_to_original_branch() passes the message intended for the branch
reflog as `orig_head_msg`. Fix this by adding a `branch_msg` member to
struct reset_head_opts and add a regression test.  Note that these
reflog messages do not respect GIT_REFLOG_ACTION. They are not alone
in that and will be fixed in a future series.

The "merge" backend already has tests that check both the branch and
HEAD reflogs.

Signed-off-by: Phillip Wood <[e-mail]>
---
 builtin/rebase.c          |  8 ++++----
 reset.c                   | 12 ++++++++++--
 reset.h                   |  2 ++
 t/t3406-rebase-message.sh | 23 +++++++++++++++++++++++
 4 files changed, 39 insertions(+), 6 deletions(-)

Comments

Junio C Hamano Oct. 1, 2021, 9:12 p.m. UTC | #1

"Phillip Wood via GitGitGadget" <[e-mail]> writes:

> From: Phillip Wood <[e-mail]>
>
> move_to_original_branch() passes the message intended for the branch
> reflog as `orig_head_msg`. Fix this by adding a `branch_msg` member to
> struct reset_head_opts and add a regression test.
> ...
> The "merge" backend already has tests that check both the branch and
> HEAD reflogs.

OK.  This sounds like a good idea.

> +test_expect_success 'rebase --apply reflog' '
> +	git checkout -b reflog-apply start &&
> +	old_head_reflog="$(git log -g --format=%gs -1 HEAD)" &&
> +
> +	git rebase --apply Y &&
> +
> +	git log -g --format=%gs -4 HEAD >actual &&
> +	cat >expect <<-EOF &&
> +	rebase finished: returning to refs/heads/reflog-apply
> +	rebase: Z
> +	rebase: checkout Y
> +	$old_head_reflog
> +	EOF
> +	test_cmp expect actual &&
> +
> +	git log -g --format=%gs -2 reflog-apply >actual &&
> +	cat >expect <<-EOF &&
> +	rebase finished: refs/heads/reflog-apply onto $(git rev-parse Y)
> +	branch: Created from start
> +	EOF
> +	test_cmp expect actual
> +'

Thanks.

Patch

diff --git a/builtin/rebase.c b/builtin/rebase.c
index ae5de271e9a..c40a8b843f4 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -776,7 +776,7 @@  static void add_var(struct strbuf *buf, const char *name, const char *value)
 
 static int move_to_original_branch(struct rebase_options *opts)
 {
-	struct strbuf orig_head_reflog = STRBUF_INIT, head_reflog = STRBUF_INIT;
+	struct strbuf branch_reflog = STRBUF_INIT, head_reflog = STRBUF_INIT;
 	struct reset_head_opts ropts = { 0 };
 	int ret;
 
@@ -786,17 +786,17 @@  static int move_to_original_branch(struct rebase_options *opts)
 	if (!opts->onto)
 		BUG("move_to_original_branch without onto");
 
-	strbuf_addf(&orig_head_reflog, "rebase finished: %s onto %s",
+	strbuf_addf(&branch_reflog, "rebase finished: %s onto %s",
 		    opts->head_name, oid_to_hex(&opts->onto->object.oid));
 	strbuf_addf(&head_reflog, "rebase finished: returning to %s",
 		    opts->head_name);
 	ropts.branch = opts->head_name;
 	ropts.flags = RESET_HEAD_REFS_ONLY;
-	ropts.orig_head_msg = orig_head_reflog.buf;
+	ropts.branch_msg = branch_reflog.buf;
 	ropts.head_msg = head_reflog.buf;
 	ret = reset_head(the_repository, &ropts);
 
-	strbuf_release(&orig_head_reflog);
+	strbuf_release(&branch_reflog);
 	strbuf_release(&head_reflog);
 	return ret;
 }
diff --git a/reset.c b/reset.c
index e77a8ac423d..2c32600234d 100644
--- a/reset.c
+++ b/reset.c
@@ -15,6 +15,7 @@  static int update_refs(const struct reset_head_opts *opts,
 	unsigned run_hook = opts->flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
 	unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
 	const char *switch_to_branch = opts->branch;
+	const char *reflog_branch = opts->branch_msg;
 	const char *reflog_head = opts->head_msg;
 	const char *reflog_orig_head = opts->orig_head_msg;
 	const char *default_reflog_action = opts->default_reflog_action;
@@ -58,8 +59,9 @@  static int update_refs(const struct reset_head_opts *opts,
 				 detach_head ? REF_NO_DEREF : 0,
 				 UPDATE_REFS_MSG_ON_ERR);
 	else {
-		ret = update_ref(reflog_head, switch_to_branch, oid,
-				 NULL, 0, UPDATE_REFS_MSG_ON_ERR);
+		ret = update_ref(reflog_branch ? reflog_branch : reflog_head,
+				 switch_to_branch, oid, NULL, 0,
+				 UPDATE_REFS_MSG_ON_ERR);
 		if (!ret)
 			ret = create_symref("HEAD", switch_to_branch,
 					    reflog_head);
@@ -90,6 +92,12 @@  int reset_head(struct repository *r, const struct reset_head_opts *opts)
 	if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
 		BUG("Not a fully qualified branch: '%s'", switch_to_branch);
 
+	if (opts->orig_head_msg && !update_orig_head)
+		BUG("ORIG_HEAD reflog message given without updating ORIG_HEAD");
+
+	if (opts->branch_msg && !opts->branch)
+		BUG("branch reflog message given without a branch");
+
 	if (!refs_only && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
 		ret = -1;
 		goto leave_reset_head;
diff --git a/reset.h b/reset.h
index 996d7f569b6..c0f4e99a3be 100644
--- a/reset.h
+++ b/reset.h
@@ -19,6 +19,8 @@  struct reset_head_opts {
 	const char *branch;
 	/* Flags defined above */
 	unsigned flags;
+	 /* Optional reflog message for branch, defaults to head_msg. */
+	const char *branch_msg;
 	 /*
 	  * Optional reflog message for HEAD, if this is not set then
 	  * default_reflog_action must be.
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 77a313f62eb..d17b450e811 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -105,6 +105,29 @@  test_expect_success 'GIT_REFLOG_ACTION' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rebase --apply reflog' '
+	git checkout -b reflog-apply start &&
+	old_head_reflog="$(git log -g --format=%gs -1 HEAD)" &&
+
+	git rebase --apply Y &&
+
+	git log -g --format=%gs -4 HEAD >actual &&
+	cat >expect <<-EOF &&
+	rebase finished: returning to refs/heads/reflog-apply
+	rebase: Z
+	rebase: checkout Y
+	$old_head_reflog
+	EOF
+	test_cmp expect actual &&
+
+	git log -g --format=%gs -2 reflog-apply >actual &&
+	cat >expect <<-EOF &&
+	rebase finished: refs/heads/reflog-apply onto $(git rev-parse Y)
+	branch: Created from start
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'rebase -i onto unrelated history' '
 	git init unrelated &&
 	test_commit -C unrelated 1 &&