Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> + /*
> + * If HEAD is not identical to the parent of the original merge commit,
> + * we cannot fast-forward.
> + */
> + can_fast_forward = commit && commit->parents &&
> + !oidcmp(&commit->parents->item->object.oid,
> + &head_commit->object.oid);
> +
I think this expression and assignment should better be done much
later. Are you going to update commit, commit->parents, etc. that
are involved in the computation in the meantime???
> strbuf_addf(&ref_name, "refs/rewritten/%.*s", merge_arg_len, arg);
> merge_commit = lookup_commit_reference_by_name(ref_name.buf);
> if (!merge_commit) {
> @@ -2164,6 +2172,17 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
> rollback_lock_file(&lock);
> return -1;
> }
> +
> + if (can_fast_forward && commit->parents->next &&
> + !commit->parents->next->next &&
> + !oidcmp(&commit->parents->next->item->object.oid,
> + &merge_commit->object.oid)) {
... Namely, here. Because the earlier one is computing "are we
replaying exactly the same commit on top of exactly the same
state?", which is merely one half of "can we fast-forward", and
storing it in a variable whose name is over-promising way before it
becomes necessary. The other half of "can we fast-forward?" logic
is the remainder of the if() condition we see above. IOW, when
fully spelled, this code can fast-forward when we are replaying a
commit on top of exactly the same first-parent and the commit being
replayed is a single parent merge.
We may even want to get rid of can_fast_forward variable.
> + strbuf_release(&ref_name);
> + rollback_lock_file(&lock);
> + return fast_forward_to(&commit->object.oid,
> + &head_commit->object.oid, 0, opts);
> + }
> +
> write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ,
> git_path_merge_head(), 0);
> write_message("no-ff", 5, git_path_merge_mode(), 0);
