git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Michael J Gruber <git@drmicha.warpmail.net>
Date
Feb 24, 2016, 10:31 UTC
Message-ID
<56CD8681.9010308@drmicha.warpmail.net>
In-Reply-To
<CAGZ79kYVHJFeS41yj+JymKyfKpSW4y-Wpk6ZTT4yxzprC6UQTg@mail.gmail.com>
Stefan Beller venit, vidit, dixit 23.02.2016 23:21:
Show 113 quoted lines
> On Tue, Feb 23, 2016 at 12:46 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On February 23, 2016 12:35:12 PM PST, Junio C Hamano <gitster@pobox.com> wrote:
>>> ebiederm@xmission.com (Eric W. Biederman) writes:
>>>
>>>> Junio C Hamano <gitster@pobox.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

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
Previous: Stefan BellerNext: Junio C Hamano
Message 24 of 29 in “Add an option to git-format-patch to record base tree info”
  1. 0/1 Add an option to git-format-patch to record base tree infoXiaolong Ye, Feb 22, 2016
  2. 1/1 format-patch: add an option to record base tree infoXiaolong Ye, Feb 22, 2016
  3. Junio C HamanoFeb 22, 2016
  4. Jacob KellerFeb 22, 2016
  5. Fengguang WuFeb 23, 2016
  6. Junio C HamanoFeb 23, 2016
  7. Fengguang WuFeb 23, 2016
  8. H. Peter AnvinFeb 23, 2016
  9. Fengguang WuFeb 23, 2016
  10. Dan CarpenterFeb 23, 2016
  11. Fengguang WuFeb 23, 2016
  12. Dan CarpenterFeb 23, 2016
  13. Fengguang WuFeb 24, 2016
  14. Junio C HamanoFeb 24, 2016
  15. Fengguang WuFeb 24, 2016
  16. Junio C HamanoFeb 24, 2016
  17. Junio C HamanoFeb 23, 2016
  18. Eric W. BiedermanFeb 23, 2016
  19. Junio C HamanoFeb 23, 2016
  20. H. Peter AnvinFeb 23, 2016
  21. Eric W. BiedermanFeb 23, 2016
  22. H. Peter AnvinFeb 24, 2016
  23. Stefan BellerFeb 23, 2016
  24. Michael J GruberFeb 24, 2016
  25. Junio C HamanoFeb 24, 2016
  26. Fengguang WuFeb 24, 2016
  27. Fengguang WuFeb 24, 2016
  28. Eric W. BiedermanFeb 23, 2016
  29. Fengguang WuFeb 24, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.