Re: [PATCH v2 1/8] rebase --apply: remove duplicated code

[Junio C Hamano <gitster@pobox.com>]

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

> -	strbuf_reset(&msg);

This is unnecessary, because we have released immediately before.

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> When we are reattaching HEAD after a fast-forward we can use
> move_to_original_branch() rather than open coding a copy of that
> code. The old code appears to handle the case where the rebase is
> started from a detached HEAD but in fact in that case there is nothing
> to do as we have already updated HEAD.

At the beginning of move_to_original_branch(), there is an early
return to do nothing when the head is detached.  But the code before
the pre-context of the hunk already detached the head at the "onto",
and the fast-forward case wants to leave the detached head pointing
at that "onto" commit, so the early return without doing anything is
exactly what we want to see.

Does that mean that the original code had left two reflog entries
for HEAD, one to detach at the "onto" commit, and then in this block
to say "rebase finished" here?  With the new code, because we return
early from move_to_original_branch(), we no longer leave the "rebase
finished" record in the reflog of HEAD?

Is it done deliberately as an improvement, or an oversight that led
to a possible regression?

Or do we still add the reflog entry for HEAD that says "rebase finished"
somewhere else when we trigger the early return and I am misreading
the code?

> >  	if (oideq(&merge_base, &options.orig_head)) {
>  		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;
>  	}