Yuval Kogman <nothingmuch@xxxxxxxxxxxx> 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@xxxxxxxxx>:
>
>> * 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
