Message-ID: <CAGZ79kYVHJFeS41yj+JymKyfKpSW4y-Wpk6ZTT4yxzprC6UQTg@mail.gmail.com>
Date:	Tue, 23 Feb 2016 14:21:59 -0800
From:	Stefan Beller <sbeller@...gle.com>
To:	"H. Peter Anvin" <hpa@...or.com>
Cc:	Junio C Hamano <gitster@...ox.com>, ebiederm@...ssion.com,
	Fengguang Wu <fengguang.wu@...el.com>,
	Xiaolong Ye <xiaolong.ye@...el.com>,
	"git@...r.kernel.org" <git@...r.kernel.org>, ying.huang@...el.com,
	philip.li@...el.com, julie.du@...el.com,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Christoph Hellwig <hch@....de>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC/PATCH 1/1] format-patch: add an option to record base tree info

On Tue, Feb 23, 2016 at 12:46 PM, H. Peter Anvin <hpa@...or.com> wrote:
> On February 23, 2016 12:35:12 PM PST, Junio C Hamano <gitster@...ox.com> wrote:
>>ebiederm@...ssion.com (Eric W. Biederman) writes:
>>
>>> Junio C Hamano <gitster@...ox.com> writes:
>>>
>>>> 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.
>>>
>>> So I don't know what value this has to the git project.
>>
>>Oh, don't worry, I wasn't talking about value this may have to the
>>Git project at all.  "The value to the project" I mentioned in my
>>response was all about the value to the kernel project.
>>
>>> The value of Fengguag's automated testing to the kernel project is to
>>> smoke out really stupid things.
>>
>>I'll snip your bullet points, but as I wrote, I do understand the
>>value of prescreening test.
>>
>>What I was questioning was what value it gives to that testing to
>>use "the developer based this patch on this exact commit" added to
>>each incoming patch, and have Fengguag's testing machinery test a
>>patch with a base version that may no longer be relevant in the
>>context of the project.  Compared to that, wouldn't "this no longer
>>applies to the branch the series targets" or "this still applies
>>cleanly, but no longer compiles because the surrounding API has
>>changed" be much more valuable?
>>
>>In your other message, you mentioned the "index $old..$new" lines,
>>and it is possible to use them to find a tree that the patch cleanly
>>applies to, but it will not uniquely identify _the_ version the
>>patch submitter used.  IMHO, finding such _a_ tree from the recent
>>history of the branch that the patch targets and testing the patch
>>on top of that tree (as opposed to testing the patch in the exact
>>context the developer worked on) would give the project a better
>>value.
>
> Personally, as a maintainer, I would love to see the tree ID and ideally also the commit ID a series is based on.  The commit ID is in some ways less useful since it is non-recreatable (and therefore will never match for anything but the first patch of a series), but could be useful to the maintainer.

As a contributor a commit id would also add value I would think. When
it is unclear
where a series is headed, contributors in Git land say things like:

    This applies cleanly on origin/master.

And usually this is the master branch from yesterday as Junio pushes
once a day. origin/master being a moving target, so it may not apply any more,
so a commit sha1 would help for finding out what to do in the maintainer role.

This discussion sounds to me as if we'd want to have some advantages of
the (kernel pull style, not github style) pull-request here for patch
submissions.

I don't remember the exact quote, but Linus said once upon a time about the
pull request workflow roughly:

    "Please pull from ... And by the way I tested this software for 2
month during development"
    (For kernels that makes sense as the contributor run the kernel
and it worked)

as pull requests have the new patches on top of the exact parents the
contributor put them
on, there can be more faith in the process to divide between the
problems the contributor
overlooked/introduced and problems as introduced by the merge of the maintainer.

Now when applying patches at another parent than the contributor
developed on, some
subtle problems may arise, which are not easily spotted by compile
tests or running the
test suite.

Maybe this could be solved by a convention (similar to a sign-off line
in each patch
which is not formally checked) in the cover letter, such that we have
a both machine
and human readable format where the contributor can suggest an anchor point?
(The maintainer may ignore it, but buildbots are free to follow strictly)

Thanks,
Stefan


>
> As far as putting in a bunch of metainformation that has to be manually or semimanually obtained, there are a lot of issues with that, as it way too easily turns into wrong information or potential information leaks that might make corporate contributors uncomfortable.
> --
> Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists