Re: [PATCH] Add --ff-only flag to git-merge

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

Yuval Kogman <nothingmuch@woobling.org> writes:

> I started incorperating your feedback but before I send a new patch I
> have several questions about the trickier bits:
>
> 2009/1/28 Junio C Hamano <gitster@pobox.com>:
>
>>  * The placement of this misses the case where a merge of two unrelated
>>   histories is attempted.  You would need to also have a check at "No
>>   common ancestors found. We need a real merge." part.
>
> Won't that fall through? The if (!common) is above, and this is
> eventually an else if for it (see line 978)

When "if (!common)" is true, the empty statement ";" is executed, and all
its "else if" conditional arms will be skipped.  Is that what you want to
happen?

>> The octopus
>>   codepath could also end up with a fast forward or up-to-date.
>
> So this case is obviously more convoluted... If an octopus merge is
> chosen should it just pass --ff-only to git-merge-octopus? Or maybe it
> should always pass --ff-only and the various different strategies
> would just die unconditionally?

I was referring to this part:

	} else {
		/*
		 * An octopus.  If we can reach all the remote we are up
		 * to date.
		 */
		int up_to_date = 1;
		...
		if (up_to_date) {
			finish_up_to_date("Already up-to-date. Yeeah!");
			return 0;
		}
	}

You do not want to fail this case, where you tried to merge others that
have already been merged, when --ff-only is given, do you?  After all, all
that you are interested in is "do not create a new merge commit".

If you scroll down a bit from there, you will see:


	/* We are going to make a new commit. */
	git_committer_info(IDENT_ERROR_ON_NO_NAME);

	/*
	 * At this point, we need a real merge.  No matter what strategy
	 * we use, it would operate on the index, possibly affecting the

This is where "if (!common) ;" will fall through to.

If your goal is to prevent the user from creating a new merge commit,
the logical place to do so would be immediately before that comment,
independent from all the if..elseif..fi conditional arms that come before
it, I think.

You also need to disable allow_trivial when ff-only is given, but I think
that goes without saying.  If you do not want to allow creating a new
merge commit, you do not want even a trivial merge to happen.