Re: [PATCH v3 1/8] rebase --apply: remove duplicated code
From: Junio C Hamano <hidden>
Date: 2022-10-12 20:37:01
"Phillip Wood via GitGitGadget" [off-list ref] writes:
From: Phillip Wood <redacted> Use move_to_original_branch() when reattaching HEAD after a fast-forward rather than open coding a copy of that code. move_to_original_branch() does not call reset_head() if head_name is NULL but there should be no user visible changes even though we currently call reset_head() in that case.
move_to_original_branch() uses both .head_msg and .branch_msg and uses different messages for them, but the original code below only feeds .head_msg while .branch_msg leaves NULL, which leads reset.c::update_refs() to use the same message as .head_msg when it wants to use .branch_msg (i.e. the message recorded in the reflog of the branch). Doesn't this difference result in a different behaviour?
The reason for this is that the reset_head() call does not add a message to the reflog because we're not changing the commit that HEAD points to and so lock_ref_for_update() elides the update. When head_name is not NULL then reset_head() behaves like "git symbolic-ref" and so the reflog is updated.
Note that the removal of "strbuf_release(&msg)" is safe as there is an
The patch is removing strbuf_reset(), not _release(), here, though. We have released already so there is no point to reset it again, so the removal is still safe.
quoted hunk
identical call just above this hunk which can be seen by viewing the diff with -U6. Signed-off-by: Phillip Wood <redacted> --- builtin/rebase.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)diff --git a/builtin/rebase.c b/builtin/rebase.c index a2ca66b54be..51accb4fd61 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c@@ -1808,19 +1808,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) * If the onto is a proper descendant of the tip of the branch, then * we just fast-forwarded. */ - strbuf_reset(&msg); if (oideq(&branch_base, &options.orig_head->object.oid)) { printf(_("Fast-forwarded %s to %s.\n"), branch_name, options.onto_name); - strbuf_addf(&msg, "rebase finished: %s onto %s", - options.head_name ? options.head_name : "detached HEAD", - oid_to_hex(&options.onto->object.oid)); - memset(&ropts, 0, sizeof(ropts)); - ropts.branch = options.head_name; - ropts.flags = RESET_HEAD_REFS_ONLY; - ropts.head_msg = msg.buf; - reset_head(the_repository, &ropts); - strbuf_release(&msg); + move_to_original_branch(&options); ret = finish_rebase(&options); goto cleanup; }