From: Junio C Hamano <gitster@pobox•com>
To: Yuval Kogman <nothingmuch@woobling•org>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] Add --ff-only flag to git-merge
Date: Wed, 28 Jan 2009 13:12:57 -0800 [thread overview]
Message-ID: <7vk58fm8x2.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <1233147238-30082-1-git-send-email-nothingmuch@woobling.org> (Yuval Kogman's message of "Wed, 28 Jan 2009 14:53:58 +0200")
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.
> @@ -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).
> @@ -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.
next prev parent reply other threads:[~2009-01-28 21:14 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-28 12:53 [PATCH] Add --ff-only flag to git-merge Yuval Kogman
2009-01-28 13:26 ` Johannes Schindelin
2009-01-28 13:29 ` Yuval Kogman
2009-01-28 14:35 ` Sverre Rabbelier
2009-01-28 21:12 ` Junio C Hamano [this message]
2009-01-29 20:57 ` Yuval Kogman
2009-01-29 22:29 ` Junio C Hamano
2009-01-30 4:32 ` [PATCH] Add --ff-only flag to git-merge (take 2) Yuval Kogman
2009-01-30 4:32 ` [PATCH] Add --ff-only flag to git-merge Yuval Kogman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vk58fm8x2.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=nothingmuch@woobling$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox