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

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

On Thu, Mar 24, 2016 at 10:01:58AM -0700, Junio C Hamano wrote:
>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?
>

Hi, Junio

Still want to discuss with you about the proposal that showing the "parent
commit-id as well as patch-id" when exact base commit is failed to get through
cmdline or upstream", from 0day robot's view, there would be 2 kinds of base tree
info appended at the bottom of patch message.

For the info starts with "base-commit: <xxx> ...", robot would know it
is reliable base commit, it would just checkout it and apply the prerequisite
patches and patchset for the work.

For the info starts with "parent-commit: <xxx>; parent-patch-id: <xxx>",
there are 3 situations 0day robot would need to handle:

1) parent commit is well-known public commit(e.g. one in Linus's tree),
   in this case, robot will just checkout the parent commit and apply the
   patchset accordingly.

2) parent commit is well-known in-flight patch, since 0day maintains the
   database of in-flight patches indexed by their patch-ids and
   commit-ids(of the git tree mentioned below), it also exports a git tree
   which holds all in-flight patches, where each patchset map to a new branch:

   https://github.com/0day-ci/linux/branches

   0day will find that patch in database by parent patch id, then do the
   checkout and apply work.

3) parent commit/patch-id is unknown, maybe because it's a
   - private patch;
   - public patch that's slightly modified locally;
   - public patch that's not covered by the 0day database
   all should be rare cases.

In practice, most developers may not feed exact commit id by --base=<xxx>
regularly when using git format-patch, this "showing parent commit" solution
could save developers' energy and reach a high coverage in the meantime.

Thanks,
Xiaolong.

>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.