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

[Michael J Gruber]

Stefan Beller venit, vidit, dixit 23.02.2016 23:21:
> On Tue, Feb 23, 2016 at 12:46 PM, H. Peter Anvin <hpa@xxxxxxxxx> wrote:
>> On February 23, 2016 12:35:12 PM PST, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>> ebiederm@xxxxxxxxxxxx (Eric W. Biederman) writes:
>>>
>>>> Junio C Hamano <gitster@xxxxxxxxx> 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

Thanks for mentioning pull-requests. In fact: Is there any problem (in
the OP's use case) that would not be solved by pull-requests?

In other words: Are we talking "pull-request by format-patch"?

[Still trying to figure out what problem to solve exactly, and how
generic that is.]

Michael