mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Fengguang Wu <fengguang.wu@intel.com>
Cc: Xiaolong Ye <xiaolong.ye@intel.com>,
	git@vger.kernel.org, ying.huang@intel.com, philip.li@intel.com,
	julie.du@intel.com,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Christoph Hellwig <hch@lst.de>, "H. Peter Anvin" <hpa@zytor.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
Date: Tue, 23 Feb 2016 11:51:31 -0800	[thread overview]
Message-ID: <xmqq1t8319z0.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160223091740.GA3830@wfg-t540p.sh.intel.com>

Fengguang Wu <fengguang.wu@intel.com> 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.

  parent reply	other threads:[~2016-02-23 19:51 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1456109938-8568-1-git-send-email-xiaolong.ye@intel.com>
     [not found] ` <1456109938-8568-2-git-send-email-xiaolong.ye@intel.com>
     [not found]   ` <xmqqmvqt8jgz.fsf@gitster.mtv.corp.google.com>
     [not found]     ` <20160223014741.GA21025@wfg-t540p.sh.intel.com>
     [not found]       ` <xmqqio1f3oi9.fsf@gitster.mtv.corp.google.com>
2016-02-23  9:17         ` Fengguang Wu
2016-02-23  9:23           ` H. Peter Anvin
2016-02-23  9:32             ` Fengguang Wu
2016-02-23 10:32           ` Dan Carpenter
2016-02-23 12:00             ` Fengguang Wu
2016-02-23 13:31               ` Dan Carpenter
2016-02-24  2:55                 ` Fengguang Wu
2016-02-24  6:30                   ` Junio C Hamano
2016-02-24  7:07                     ` Fengguang Wu
2016-02-24 18:34                       ` Junio C Hamano
2016-02-23 19:51           ` Junio C Hamano [this message]
2016-02-23 20:08             ` Eric W. Biederman
2016-02-23 20:35               ` Junio C Hamano
2016-02-23 20:46                 ` H. Peter Anvin
2016-02-23 21:49                   ` Eric W. Biederman
2016-02-24  1:40                     ` H. Peter Anvin
2016-02-23 22:21                   ` Stefan Beller
2016-02-24 10:31                     ` Michael J Gruber
2016-02-24  6:19                   ` Junio C Hamano
2016-02-24  3:36                 ` Fengguang Wu
2016-02-24  3:13             ` Fengguang Wu
2016-02-23 19:56           ` Eric W. Biederman
2016-02-24  2:30             ` Fengguang Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq1t8319z0.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=dan.carpenter@oracle.com \
    --cc=ebiederm@xmission.com \
    --cc=fengguang.wu@intel.com \
    --cc=git@vger.kernel.org \
    --cc=hch@lst.de \
    --cc=hpa@zytor.com \
    --cc=julie.du@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=philip.li@intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=xiaolong.ye@intel.com \
    --cc=ying.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome