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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 28, 2009, 21:12 UTC
Message-ID: <7vk58fm8x2.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1233147238-30082-1-git-send-email-nothingmuch@woobling.org>

Yuval Kogman <nothingmuch@woobling.org> writes:

> This patch adds an --ff-only flag to git-merge. When specified git-merge
> will exit with an error whenver a merge will not be a simple fast
> forward, similar to the default behavior of git push.
> ---

Lacks a sign-off.

Show 9 quoted lines

> @@ -167,6 +168,8 @@ static struct option builtin_merge_options[] = {
>  		"perform a commit if the merge succeeds (default)"),
>  	OPT_BOOLEAN(0, "ff", &allow_fast_forward,
>  		"allow fast forward (default)"),
> +	OPT_BOOLEAN(0, "ff-only", &only_fast_forward,
> +		"allow only fast forward"),
>  	OPT_CALLBACK('s', "strategy", &use_strategies, "strategy",
>  		"merge strategy to use", option_parse_strategy),
>  	OPT_CALLBACK('m', "message", &merge_msg, "message",

After parse_options() returns, there are a few existing tests to reject nonsensical combination of options. I think there are some combinations you would want to reject (for example, "--no-ff --ff-only" is an obvious such nonsensical combination, but there may be others, such as giving a strategy explicitly).

Show 9 quoted lines

> @@ -1012,6 +1015,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		finish(o->sha1, msg.buf);
>  		drop_save();
>  		return 0;
> +	} else if ( only_fast_forward ) {
> +		printf("Merge is non fast forward, aborting.\n");
> +		return 1;
>  	} else if (!remoteheads->next && common->next)
>  		;

This is wrong for at least three reasons:

 * Style of "if (expression) {", somebody else already pointed out.

 * Only this case exits with 1, while others die() that exits with 128.

 * 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.  The octopus
   codepath could also end up with a fast forward or up-to-date.