Hi Junio and All,
CC more relevant people. FYI this thread starts here:
http://thread.gmane.org/gmane.comp.version-control.git
On Mon, Feb 22, 2016 at 10:54:38PM -0800, Junio C Hamano wrote:
> Fengguang Wu <fengguang.wu@xxxxxxxxx> writes:
>
> > Hi Junio,
> >
> > On Sun, Feb 21, 2016 at 08:19:56PM -0800, Junio C Hamano wrote:
> >> Xiaolong Ye <xiaolong.ye@xxxxxxxxx> writes:
> >>
> >> > It would be helpful for maintainers or reviewers to know the base tree
> >> > info of the patches created by git format-patch. Teach git format-patch
> >> > a --base-tree-info option to record these info.
> >> >
> >> > Signed-off-by: Xiaolong Ye <xiaolong.ye@xxxxxxxxx>
> >> > ---
> >>
> >> 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
> >> Is it your goal to insist on one exact commit the patch is applied
> >> to?
> >
> > Right. Our goal is fully automated patch testing, where the base tree
> > info is required for *reliably* avoid reporting false positives.
> >
> > A clean git-apply does not guarantee the resulted code is logically
> > consistent and hence testable by 3rd party. For a 3rd party tester to
> > provide useful and trustable test reports, he must apply the patch to
> > exactly the same base as the patch submitter.
>
> The patch submitter (or you as a third party tester) is not in the
> position to dictate the integrator to apply the patch to one
> specific commit and use it from there. The integrator would pick an
> appropriate base that would be different from the commit where the
> patch was taken from, apply it there, and merge the result to the
> tip of the mainline, or apply the patch directly to the tip of the
> mainline. Even if the integrator picked the commit the patch was
> taken from, the result would not be used alone without any other
> changes, i.e. before getting merged into the integration branch.
Yeah. Per my understanding the base commit info will be mainly parsed
by test robots instead of integrators.
> So in that sense, any test that is done by the patch submitter and
> the third party tester would not be what will be released to the
> wild *anyway*. The resulting code will be exercised in a context
> that *is* different from the context the original author had.
That's right. But no worry, when the patch is merged by maintainer,
we'll test it once again in the maintainer tree.
Pre-merge patch testing is useful in 2 ways:
- shift left testing to early review stage
- maintainer trees are typically not rebaseable. When errors are
discovered there, it's a bit too late: the error will likely remain
in git history for ever. Which will hurt bisects.
> 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.
> So I dunno.
FYI, the 0day test robot will be able to work better if provided the
base commit info. It'll work a bit more sophisticated than simply
relying on the base commit info: if it's sure about the tree the patch
is targeted for (or the maintainer would apply to), it'll use that as
base tree[*]; otherwise it'll fall back to using the base commit info
included in the patchset.
[*] For examples,
[PATCH -mm] ...
[PATCH net] ...
For such patches we are sure they are targeted for the well known
mm/net trees.
Anyway the worst case of not adopting the discussed patch is, the 0day
test robot continue to work in current heuristic way.
Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html