Re: [PATCH v3 3/4] format-patch: introduce --base=auto option

[Ye Xiaolong <xiaolong.ye@intel.com>]

On Fri, Apr 01, 2016 at 09:06:20AM -0700, Junio C Hamano wrote:
>Ye Xiaolong <xiaolong.ye@intel.com> writes:
>
>> On Thu, Mar 31, 2016 at 10:43:48AM -0700, Junio C Hamano wrote:
>>>Xiaolong Ye <xiaolong.ye@intel.com> writes:
>>>
>>>> Introduce --base=auto to record the base commit info automatically, the base_commit
>>>> will be the merge base of tip commit of the upstream branch and revision-range
>>>> specified in cmdline.
>>>
>>>This line is probably a bit too long.
>>
>> How about simplifying it to "the base_commit is the merge base of upstream and
>> specified revision-range."?
>
>What I meant was not that profound.  I just wanted you to wrap your
>lines a bit shorter so that quoting in the discussion thread like
>this would not make the result overlong to fit on a 80-column
>terminal ;-)

Emm, get your point now, I'll shorten the lines to fit in 80-column.

>
>>>> +			base = base_list->item;
>>>> +			free_commit_list(base_list);
>>>
>>>What should happen when there are multiple merge bases?  The code
>>>picks one at random and ignores the remainder, if I am reading this
>>>correctly.
>>
>> If there is more than one merge base, commits in base_list should
>> be sorted by date, if I am understanding it correctly, so
>> base_list->item should be the lastest merge base commit, it should
>> be enough for us to used as base commit.
>
>By definition, when there are multiple merge bases, there is no
>latest one among them.
>
>When the history involves criss-cross merges, there can be more than
>one 'best' common ancestor for two commits.  For example, with this
>topology (note that X is not a commit; it merely denotes crossing of
>two lines):
>
>       ---1---o---A
>      /    \ /
>  ---O      X
>      \    / \
>       ---2---o---o---B
>
>both '1' and '2' are merge-bases of 'A' and 'B'.  And the timestamps
>on one (be it committer or author timestamp) being later than those
>of the other do not make it any more suitable than the other one.
>

For this criss-cross merges, as neither merge base(like 1) is better
than the other(both 1 and 2 are 'best' merge bases), I think it should
be fine to pick a random one as base commit(Or you prefer to show all of
them?) and I'll add this part of discusstion into documentation.

Thanks,
Xiaolong.