Re: [RFC/PATCH 1/1] format-patch: add an option to record base tree info

[Junio C Hamano]

Fengguang Wu <fengguang.wu@xxxxxxxxx> writes:

>> >> I have a mixed feeling about this one, primarily because this was
>> >> already tried quite early in the life of "format-patch" command.
>> >> 
>> >>     http://thread.gmane.org/gmane.comp.version-control.git/9694/focus=9757
>> >> 
>> >> Only the name is different (it was called "applies-to" and named a
>> >> tree object).
>> >
>> > Either commit or tree object will work for us. We can use it in
>> > v2 if you prefer tree object.
>> 
>> Sorry, I think you misunderstood.  By "only the name is different", I
>> didn't mean to say that the tree object name should be shown as the
>> old proposal did.  What I meant but didn't explicitly say, as I
>> thought it was sufficient to point at an old discussion thread, was
>> that this was already tried and rejected.  This round uses different
>> name but does essentially the same thing as the old proposal, and I
>> do not think I heard anything new that supports this patch against
>> earlier rejection by Linus.  That is what gave me a mixed feeling.
>
> I can understand the rejection by Linus in development process POV.
>
> However we are facing a new situation: in test robot POV, IMHO there
> are values to test exactly the same tree as the patch submitter.
> Otherwise the robot risks
>
> - false negative: failing to apply and test some patches
> - false positive: sending wrong bug reports due to guessed wrong base tree

I always get negatives and positives confused, so let me think aloud
with an example.  Let's say that somebody worked on adding a new
feature based on v4.2 codebase and sent in a patch series.  The
series touched files in quiescent part of the system, these files
are identical between v4.2 and the current codebase at v4.5-rc5, and
the series applies cleanly to a "wrong" base tree at the tip of
'master'.  But it turns out that the series uses an old API that was
removed in the meantime.  The test robot may say "the result of
applying the series does not even build" and the developer would
complain to you saying "You tested with a wrong version".

I've already said that I can see the value this approach has for
you.  By having the developer state which commit the series was
based on, it will shield you from such a complaint, because you
would not use closer-to-tip 'master' as the base, but instead use
v4.2 codebase for the test.

As I said, what is unclear to me is what value this apporach gives
to the project.

>> I can see that recording the exact commit object name allows you to
>> claim that you identified the exact commit to apply the patch, and
>> that you tested the exact tree contents.  It however is unclear what
>> the value of such a claim would be to the project or to the
>> integrator.
>
> The value of base commit info is: providing a solid ground to the
> tester, to reliably avoid false positive/negatives.

It is valuable for a testing organization to say "We tested this
series on top of version X.  We know it works, we have tested on a
lot more hardware than the original developer had, we know this is
good to go."  It is a valuable service.

But that is valuable only if version X is still relevant, isn't it?

Is the relevance of a version something that is decided by a
developer who submits a patch series, or is it more of an attribute
of the project and where the current integration is happening?
Judging from the responses from Dan to this thread, I think the
answer is the latter, and for the purpose of identifying the
relevant version(s), the project does not even care about the exact
commit, but it wants to know more about which branch the series is
targetted to.

With that understanding, I find it hard to believe that it buys the
project much for the "base" commit to be recorded in a patch series
and automated testing is done by applying the patches to that exact
commit, which possibly is no-longer-relevant, even though it may
help shielding the testing machinery from "you tested with a wrong
version" complaints.

Isn't it more valuable for the test robot to say "this may or may
not have worked well with whatever old version the patch series was
based on, but it no longer is useful to the current tip of the
'master'"?  If you consider what benefit the project would gain by
having such a robot, that is the conclusion I have to draw.

So I still am not convinced that this "record base commit" is a
useful thing to do.