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

From: Fengguang Wu
Date: Tue Feb 23 2016 - 22:13:36 EST


On Tue, Feb 23, 2016 at 11:51:31AM -0800, Junio C Hamano wrote:
> 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.

Problem arises when a developer based his work on a maintainer's topic
branch. The robot doesn't know that and tests the patch on v4.5-rc5,
which may trigger a false error because the patch depends on some
changes in that maintainer's topic branch. In that case, the error
report will be pure noise.

Thanks,
Fengguang