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

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

Ye Xiaolong <xiaolong.ye@intel.com> writes:

> On Wed, Mar 23, 2016 at 11:25:41AM -0700, Junio C Hamano wrote:
>>I also do not see the point of showing "parent id" which as far as I
>>can see is just a random commit object name and show different
>>output that is not even described what it is.  It would be better to
>
> Here is our consideration:
> There is high chance that branch_get_upstream will retrun NULL(thus we
> are not able to get exact base commit), since developers may checkout
> branch from a local branch or a commit and haven't set "--set-upstream-to"
> to track a remote branch, in this case, we want to provide likely useful
> info(here is parent commit id and patch id)

I do not agree with that reasoning at all, primarily because your
"likely useful" is not justfied.

Could you explain what makes you think that it is "likely" that the
commit that matches "parent commit id" is available to the recipient
of the patch?

Whatever the reason is, if it _is_ likely, then I do not see a point
in spending cycle to do get-upstream and merge-base, or giving an
option to the user to explicitly specify the base.  Given that this
series does these things, I'd have to conclude your "likely useful"
is "might possibly turn out to be useful in some cases if you are
lucky but is no way reliable" at best.

Rather than throwing an unreliable information in the output and
blindly proceeding, I'd think you would want to error out and tell
the user to explicitly give the base without producing the patch
output.  That way, you will not get patches with unreliable
information.

Suggesting to use set-upstream-to when you give that error message
may also be helpful.