RFC: Superseded-by: follow-up trailer

RFC: Superseded-by: follow-up trailer

[Konstantin Ryabitsev]

Hi, all:

B4 (and git-patchwork-bot) try to deal with series revisions, but the process
is not infallible due to needing to guess whether one series obsoletes another
via weak string matches (subject + author). If v2 arrives with a modified
subject than in v1 (even if it's a typo fix), or from a different email
address, then auto-matching will fail.

I would like to propose a "Superseded-by" follow-up trailer to help strengthen
the link between series revisions. The trailer needs to be sent as a follow-up
to the cover letter or first patch of the previous series.

    Superseded-by: [url-ending-with/message-id-of-next-version]

Example:

After submitting a new series revision, a contributor sends a follow-up email
to the previous series submission, adding a single follow-up trailer, e.g.:


    Subject: Re: [PATCH v2 0/2] frobdrv: reduce frob latency

    [...]
    > Please submit a new revision that fixes this problem.
    >
    > Nacked-by: Maine Tainer <mtainer@example.com>

    Hi,

    I just submitted a v3 with these fixes.
    
    Superseded-by: https://lore.kernel.org/r/foo-bar-3333-1-git@example.net

Of course, I don't expect a lot of folks to remember to do this, but I hope to
incorporate this into the upcoming "auto-pr-exploder" tool that helps folks
turn pull requests into well-formed patch series. In the same vein, when
git-patchwork-bot recognizes that a series is superseded by a newer version,
it can send an automated follow-up to the list to help indicate to the
maintainer that a newer version of the patch/series is available. 

When b4 encounters this trailer, it can issue a warning to the maintainer that
they aren't retrieving the latest series revision (and can automatically grab
the new one if instructed to do so).

Please let me know if this sounds reasonable.

-K

Re: RFC: Superseded-by: follow-up trailer

[Alex Elder]

On 4/13/21 3:49 PM, Konstantin Ryabitsev wrote:
> Hi, all:
> 
> B4 (and git-patchwork-bot) try to deal with series revisions, but the process
> is not infallible due to needing to guess whether one series obsoletes another
> via weak string matches (subject + author). If v2 arrives with a modified
> subject than in v1 (even if it's a typo fix), or from a different email
> address, then auto-matching will fail.
> 
> I would like to propose a "Superseded-by" follow-up trailer to help strengthen
> the link between series revisions. The trailer needs to be sent as a follow-up
> to the cover letter or first patch of the previous series.
> 
>      Superseded-by: [url-ending-with/message-id-of-next-version]

Could "Supercedes: [url]" posted with the new version work?	-Alex

> 
> Example:
> 
> After submitting a new series revision, a contributor sends a follow-up email
> to the previous series submission, adding a single follow-up trailer, e.g.:
> 
> 
>      Subject: Re: [PATCH v2 0/2] frobdrv: reduce frob latency
> 
>      [...]
>      > Please submit a new revision that fixes this problem.
>      >
>      > Nacked-by: Maine Tainer <mtainer@example.com>
> 
>      Hi,
> 
>      I just submitted a v3 with these fixes.
>      
>      Superseded-by: https://lore.kernel.org/r/foo-bar-3333-1-git@example.net
> 
> Of course, I don't expect a lot of folks to remember to do this, but I hope to
> incorporate this into the upcoming "auto-pr-exploder" tool that helps folks
> turn pull requests into well-formed patch series. In the same vein, when
> git-patchwork-bot recognizes that a series is superseded by a newer version,
> it can send an automated follow-up to the list to help indicate to the
> maintainer that a newer version of the patch/series is available.
> 
> When b4 encounters this trailer, it can issue a warning to the maintainer that
> they aren't retrieving the latest series revision (and can automatically grab
> the new one if instructed to do so).
> 
> Please let me know if this sounds reasonable.
> 
> -K
> 

Re: RFC: Superseded-by: follow-up trailer

[Jason A. Donenfeld]

On Tue, Apr 13, 2021 at 2:57 PM Alex Elder <elder@ieee.org> wrote:
>
> On 4/13/21 3:49 PM, Konstantin Ryabitsev wrote:
> > Hi, all:
> >
> > B4 (and git-patchwork-bot) try to deal with series revisions, but the process
> > is not infallible due to needing to guess whether one series obsoletes another
> > via weak string matches (subject + author). If v2 arrives with a modified
> > subject than in v1 (even if it's a typo fix), or from a different email
> > address, then auto-matching will fail.
> >
> > I would like to propose a "Superseded-by" follow-up trailer to help strengthen
> > the link between series revisions. The trailer needs to be sent as a follow-up
> > to the cover letter or first patch of the previous series.
> >
> >      Superseded-by: [url-ending-with/message-id-of-next-version]
>
> Could "Supercedes: [url]" posted with the new version work?     -Alex

But in that case, the metadata is kind of useless once committed, and
it increases the overhead of submitting new patchsets, which means
fewer people will actually use it. With Konstantine's original
proposal, after you send your new version, you can lazily follow up
with a reply saying what the new one is, which helps inform everyone
who was hanging out on that old thread too. And folks other than the
original submitter can help provide that kind of thing too. In other
words, I think K's original idea seems like a sensible low-overhead
thing, while yours is a bit more laborious.

Re: RFC: Superseded-by: follow-up trailer

[Steven Rostedt]

On Tue, 13 Apr 2021 15:02:53 -0600
"Jason A. Donenfeld" <Jason@zx2c4.com> wrote:

> On Tue, Apr 13, 2021 at 2:57 PM Alex Elder <elder@ieee.org> wrote:
> >
> > On 4/13/21 3:49 PM, Konstantin Ryabitsev wrote:  
> > > Hi, all:
> > >
> > > B4 (and git-patchwork-bot) try to deal with series revisions, but the process
> > > is not infallible due to needing to guess whether one series obsoletes another
> > > via weak string matches (subject + author). If v2 arrives with a modified
> > > subject than in v1 (even if it's a typo fix), or from a different email
> > > address, then auto-matching will fail.
> > >
> > > I would like to propose a "Superseded-by" follow-up trailer to help strengthen
> > > the link between series revisions. The trailer needs to be sent as a follow-up
> > > to the cover letter or first patch of the previous series.
> > >
> > >      Superseded-by: [url-ending-with/message-id-of-next-version]  
> >
> > Could "Supercedes: [url]" posted with the new version work?     -Alex  
> 
> But in that case, the metadata is kind of useless once committed, and

Not at all.

> it increases the overhead of submitting new patchsets, which means
> fewer people will actually use it. With Konstantine's original
> proposal, after you send your new version, you can lazily follow up
> with a reply saying what the new one is, which helps inform everyone
> who was hanging out on that old thread too. And folks other than the
> original submitter can help provide that kind of thing too. In other
> words, I think K's original idea seems like a sensible low-overhead
> thing, while yours is a bit more laborious.

I would actually be less likely do the follow up on the previous patch set,
as usually writing the next series, you don't think about the previous
ones, except maybe to implement the comments. But after testing the final
work and sending it, the old version may be long out of memory. I would then
need to go in my inbox (or lore) find the old series and then reply to it.
That to me is more work, and likely not to be done.

Now, I would really like the Supercedes: tag, as its just a link you need,
and not a email sent. Ideally this would be in all the patches (with a
direct link to what it Supercedes. I may update my scripts to do exactly
this, as I always add a "Link: " to the last version, and that's in my
repo. Or perhaps b4 could add this too!

About why it's not useless from the comment above. The history of why
changes are made are usually mostly described in the early versions of a
patch series. But even with Link tags, which usually just point to the last
version of a patch, it's hard to find that useful discussion.

When I do a git blame, I'll look at the commit. Then I want to know more
details about why that change happened, and if there's a Link tag in the
commit, I go there. Then I see the link is to v32 of some patch series,
with just a bunch of Acks and Reviewed by tags attached. Rather useless.

Then its an exercise of doing searches based on the subjects. But to have
direct links to the previous versions of the series, then you can easily
find the rationale behind the changes as you will see more of the
discussions that took place to make those decisions.

-- Steve

Re: RFC: Superseded-by: follow-up trailer

[Konstantin Ryabitsev]

On Tue, Apr 13, 2021 at 05:29:01PM -0400, Steven Rostedt wrote:
> Now, I would really like the Supercedes: tag, as its just a link you need,
> and not a email sent. Ideally this would be in all the patches (with a
> direct link to what it Supercedes. I may update my scripts to do exactly
> this, as I always add a "Link: " to the last version, and that's in my
> repo. Or perhaps b4 could add this too!

I'd rather folks used the following notation in the cover letter/patch
basement:

    Previous versions:

    Link: https://lore.kernel.org/r/foo # v2
    Link: https://lore.kernel.org/r/bar # v1

This is much more succinct, indicates the version number in the trailer, and
removes the need to remember how to spell "Superseded", as there seems to be
little consensus. :)

The tooling on the kernel.org side can automatically send the required
follow-ups to previous threads when it finds these trailers in newer
submissions.

-K

Re: RFC: Superseded-by: follow-up trailer

[Steven Rostedt]

On Tue, 13 Apr 2021 17:40:31 -0400
Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:

> I'd rather folks used the following notation in the cover letter/patch
> basement:
> 
>     Previous versions:
> 
>     Link: https://lore.kernel.org/r/foo # v2
>     Link: https://lore.kernel.org/r/bar # v1
> 
> This is much more succinct, indicates the version number in the trailer, and
> removes the need to remember how to spell "Superseded", as there seems to be
> little consensus. :)
> 
> The tooling on the kernel.org side can automatically send the required
> follow-ups to previous threads when it finds these trailers in newer
> submissions.

Sounds good.

-- Steve

Re: RFC: Superseded-by: follow-up trailer

[Peter Zijlstra]

On Tue, Apr 13, 2021 at 05:40:31PM -0400, Konstantin Ryabitsev wrote:
> On Tue, Apr 13, 2021 at 05:29:01PM -0400, Steven Rostedt wrote:
> > Now, I would really like the Supercedes: tag, as its just a link you need,
> > and not a email sent. Ideally this would be in all the patches (with a
> > direct link to what it Supercedes. I may update my scripts to do exactly
> > this, as I always add a "Link: " to the last version, and that's in my
> > repo. Or perhaps b4 could add this too!
> 
> I'd rather folks used the following notation in the cover letter/patch
> basement:
> 
>     Previous versions:
> 
>     Link: https://lore.kernel.org/r/foo # v2
>     Link: https://lore.kernel.org/r/bar # v1
> 
> This is much more succinct, indicates the version number in the trailer, and
> removes the need to remember how to spell "Superseded", as there seems to be
> little consensus. :)

How would you handle the difference between the above and:

Depends on:

Link: https://lkml.kernel.org/r/foo

Which is also a semi common thing to do with larger work that comes in
multiple dependent series.

That is; I supppse; a long winded way of saying that for parsing (both
human and machine) it might be useful to have distinct tag names. Now I
agree with you that Supersedes is a crap word to spell, but it has the
distinction of being clear on intent.

So my 2ct go to adding Supercedes: and Depends-on:

(Alternatively, Replaces: ?)

Re: RFC: Superseded-by: follow-up trailer

[Jason Gunthorpe]

On Wed, Apr 14, 2021 at 09:30:57AM +0200, Peter Zijlstra wrote:
> On Tue, Apr 13, 2021 at 05:40:31PM -0400, Konstantin Ryabitsev wrote:
> > On Tue, Apr 13, 2021 at 05:29:01PM -0400, Steven Rostedt wrote:
> > > Now, I would really like the Supercedes: tag, as its just a link you need,
> > > and not a email sent. Ideally this would be in all the patches (with a
> > > direct link to what it Supercedes. I may update my scripts to do exactly
> > > this, as I always add a "Link: " to the last version, and that's in my
> > > repo. Or perhaps b4 could add this too!
> > 
> > I'd rather folks used the following notation in the cover letter/patch
> > basement:
> > 
> >     Previous versions:
> > 
> >     Link: https://lore.kernel.org/r/foo # v2
> >     Link: https://lore.kernel.org/r/bar # v1
> > 
> > This is much more succinct, indicates the version number in the trailer, and
> > removes the need to remember how to spell "Superseded", as there seems to be
> > little consensus. :)
> 
> How would you handle the difference between the above and:
> 
> Depends on:
> 
> Link: https://lkml.kernel.org/r/foo
> 
> Which is also a semi common thing to do with larger work that comes in
> multiple dependent series.
> 
> That is; I supppse; a long winded way of saying that for parsing (both
> human and machine) it might be useful to have distinct tag names. Now I
> agree with you that Supersedes is a crap word to spell, but it has the
> distinction of being clear on intent.

Most people I see already providing the above links in their cover
letters are integrating it in the change log, something like:

v3:
  - blah blah
v2: https://lore.kernel.org/r/foo
  - blah blah
v1: https://lore.kernel.org/r/bar

Which is pleasingly readable but probably not precise enough to
mechanically parse.

As getting the links back is somewhat annoying I don't see people do
it so often :(

b4 does a nice job helping the maintainer, but the submitter has no
helpful common tooling currently.

Jason

Re: RFC: Superseded-by: follow-up trailer

[James Bottomley]

On Wed, 2021-04-14 at 08:49 -0300, Jason Gunthorpe wrote:
> b4 does a nice job helping the maintainer, but the submitter has no
> helpful common tooling currently.

We have git-format-patch/git-send-email.  I like the --cover-from-
description of git-format-email but I really think we should use more
fields and make it automatic.  It would certainly be helpful if it
tracked the last version sent of the same branch so I didn't have to
add --version.  It can't, unfortunately, track message id's because
that's generated by git-send-email (has to be that way because resends
need different message id's).  but suppose git-send-email used fields
to track the cover letter message id and version (provided it's called
from the git tree), git-format-patch could use them to add a link to
the prior version.

The problem with the above is it's very fragile when done by two tools
the latter of which might not be executed from the tree.  It really
sounds like we need a combined tool for this, which includes the
ability to resend patch series as well as to increment the version.

James

Re: RFC: Superseded-by: follow-up trailer

[Jason Gunthorpe]

On Wed, Apr 14, 2021 at 06:57:13AM -0700, James Bottomley wrote:
> On Wed, 2021-04-14 at 08:49 -0300, Jason Gunthorpe wrote:
> > b4 does a nice job helping the maintainer, but the submitter has no
> > helpful common tooling currently.
> 
> We have git-format-patch/git-send-email. 

I think it is very poor. 

If you look at what a typical submitter has to deal with:

 - Make a series and write a cover letter. Version control the cover
   letter. In corporate cases many submitters should be getting internal
   review of everything, including the cover letter before sending.
   (Mostly I see people store the cover letter in git as an empty
   commit, easiest way to share for review)
 - Formulate the defined series and cover letter into a patch series
 - Annotate required To and CC's for the series and
   maintainers/reviewers. 

   Our tooling here is mediocre, we have get_maintainers but it is
   very hard to review/revise its output in the typical usage of just
   jamming it into git-send-email

   Little details like 'add the author and reviewers of the patches in
   the Fixes: lines' to the CC are often overlooked and are not
   automated.

   People seem to struggle to both narrow the CC list of individual
   patches series and still CC things like the cover letter to
   everyone.
 - Upload it to some public git so people can git fetch it
 - Submit it to SMTP, including using now required OAUTH. (Good luck!)
 - Keep a local copy of everything that was done.

 For v2:
 - Pull in all tags from the mailing list and update the git commits
   (b4 downloads the tags but doesn't automate fixing the local git
   branch)
 - Make required changes
 - Generate a change log and don't forget anything.

   git range-diff from the local copy of v1 is super helpful, but it
   is annoying to invoke git range-diff by hand.
 - Figure out the lore links of v1 to include in the cover letter
 - Revise the cover letter
 - Update the cc/to list to include discussion participants, people
   who gave reviewed-by/etc.
 - Send it again as v2

In all steps don't botch anything. Keep all the "state" related to the
submission in your head over a multiple weeks/months review cycle.

IMHO every single peice of information to drive the conversion of the
patch series to email should be stored in git itself, otherwise it
gets messed up/lost when it is revised/resent, and can't be reviewed
prior to submission.

git even makes very simple steps like editing the commit messages to
annotate tags/fix spelling overly difficult. Rebase with reword is a
pretty painfully way to do this.

The whole thing is quite hard and really time consuming - 'git
send-email' does almost none of what is needed and its operation of
relying on a huge list of unique command line arguments is not
effective.

I know people who send a lot of patches have their own custom work
flow scripts, but more casual people skip steps and do way too much
manual work, IMHO.

> description of git-format-email but I really think we should use more
> fields and make it automatic.  It would certainly be helpful if it
> tracked the last version sent of the same branch so I didn't have to
> add --version.  It can't, unfortunately, track message id's because
> that's generated by git-send-email (has to be that way because resends
> need different message id's).  

The thing invoking git-send-email can specify the message id, and it
can generate and track them using an acceptable algorithm. Other than
lacking OAUTH, git-send-email is an effective way to marshal a
directory of near-email format patches into SMTP.

For instance I've been using this algorithmic format for message id:

0-v1-6730d4ee0d32+40e6-gup_combine_put_jgg@nvidia.com
                       ^^^^^^^^^^^^^^^ my topic name
                  ^^^^ current time expressed as hex seconds since the
		       commit date
     ^^^^^^^^^^^^^ git commit in my tree that is at the top of what is
                   being sent
  ^^ Series sersion number
^  Patch number

Revises get new version numbers and new commit ids, resends get new
time stamps.

> The problem with the above is it's very fragile when done by two tools
> the latter of which might not be executed from the tree.  It really
> sounds like we need a combined tool for this, which includes the
> ability to resend patch series as well as to increment the version.

A single tool defaulting to supporting a narrow well defined
"acceptable" workflow would go along way to help the myriad of less
frequent submitters be more efficient.

Jason

Re: RFC: Superseded-by: follow-up trailer

[Konstantin Ryabitsev]

On Wed, Apr 14, 2021 at 11:36:18AM -0300, Jason Gunthorpe wrote:

I think submitter tooling is a separate discussion that should happen in a
different thread, which I'll start shortly (it slots into my auto-pr-exploder
work). I'll just comment on the below:

>  For v2:
>  - Pull in all tags from the mailing list and update the git commits
>    (b4 downloads the tags but doesn't automate fixing the local git
>    branch)

The best workflow I can recommend here is:

1. create a new branch from the latest upstream, e.g. myfoo_v1
2. "b4 am/git am" your own v1 (this has an added bonus of rebasing it)
3. create a new branch from myfoo_v1 as myfoo_v2
4. make any changes to myfoo_v2

This lets you easily run range-diff between myfoo_v1 and myfoo_v2 and lets you
reuse the cover letter you retrieved for v1 as part of "b4 am".

-K

Re: RFC: Superseded-by: follow-up trailer

[Jason Gunthorpe]

On Wed, Apr 14, 2021 at 10:56:19AM -0400, Konstantin Ryabitsev wrote:
> On Wed, Apr 14, 2021 at 11:36:18AM -0300, Jason Gunthorpe wrote:
> 
> I think submitter tooling is a separate discussion that should happen in a
> different thread, which I'll start shortly (it slots into my auto-pr-exploder
> work). I'll just comment on the below:

I'm nervous asking submitters to change their workflows without
providing any tooling to help :(

> >  For v2:
> >  - Pull in all tags from the mailing list and update the git commits
> >    (b4 downloads the tags but doesn't automate fixing the local git
> >    branch)
> 
> The best workflow I can recommend here is:
> 
> 1. create a new branch from the latest upstream, e.g. myfoo_v1
> 2. "b4 am/git am" your own v1 (this has an added bonus of rebasing it)
> 3. create a new branch from myfoo_v1 as myfoo_v2
> 4. make any changes to myfoo_v2

This isn't workable - it assumes the user will only change the patches
after all the tags have been sent because it resets the whole branch
to the as-sent condition.

Usually I would make changes to the series as the discussion evolves
and resync the tags only before resending the next version.

Jason

Re: RFC: Superseded-by: follow-up trailer

[Konstantin Ryabitsev]

On Wed, Apr 14, 2021 at 12:57:33PM -0300, Jason Gunthorpe wrote:
> > The best workflow I can recommend here is:
> > 
> > 1. create a new branch from the latest upstream, e.g. myfoo_v1
> > 2. "b4 am/git am" your own v1 (this has an added bonus of rebasing it)
> > 3. create a new branch from myfoo_v1 as myfoo_v2
> > 4. make any changes to myfoo_v2
> 
> This isn't workable - it assumes the user will only change the patches
> after all the tags have been sent because it resets the whole branch
> to the as-sent condition.
> 
> Usually I would make changes to the series as the discussion evolves
> and resync the tags only before resending the next version.

Okay, this is fair. Is this any better:

1. "b4 am" learns a --newrev flag that populates "Obsoletes:" trailers 
   for your own patches it grabs from the list
2. when you're ready to start working on the new version, you run "b4 am
   --newrev" that prepares a mbox file with Obsoletes: trailers, and you
   "git am" it to a new branch (similar to what I described above)
3. b4 additionally learns a command to grab new trailers from the list by
   matching the Obsoletes: message-ids and auto-amending commits with any new
   trailers

This would allow you to start working on a new revision whenever you're ready
for it and give b4 a precise match it can rely on when tracking new trailers.

-K

Re: RFC: Superseded-by: follow-up trailer

[Jason Gunthorpe]

On Wed, Apr 14, 2021 at 01:07:51PM -0400, Konstantin Ryabitsev wrote:
> On Wed, Apr 14, 2021 at 12:57:33PM -0300, Jason Gunthorpe wrote:
> > > The best workflow I can recommend here is:
> > > 
> > > 1. create a new branch from the latest upstream, e.g. myfoo_v1
> > > 2. "b4 am/git am" your own v1 (this has an added bonus of rebasing it)
> > > 3. create a new branch from myfoo_v1 as myfoo_v2
> > > 4. make any changes to myfoo_v2
> > 
> > This isn't workable - it assumes the user will only change the patches
> > after all the tags have been sent because it resets the whole branch
> > to the as-sent condition.
> > 
> > Usually I would make changes to the series as the discussion evolves
> > and resync the tags only before resending the next version.
> 
> Okay, this is fair. Is this any better:
> 
> 1. "b4 am" learns a --newrev flag that populates "Obsoletes:" trailers 
>    for your own patches it grabs from the list
> 2. when you're ready to start working on the new version, you run "b4 am
>    "git am" it to a new branch (similar to what I described above)
> 3. b4 additionally learns a command to grab new trailers from the list by
>    matching the Obsoletes: message-ids and auto-amending commits with any new
>    trailers
> 
> This would allow you to start working on a new revision whenever you're ready
> for it and give b4 a precise match it can rely on when tracking new trailers.

While I understand the interest in exact matching patches, I'm not
super keen on adding more headers to every patch - this feels alot
like the gerrit change-id header that everone hates, just in another
format.

Jason

Re: RFC: Superseded-by: follow-up trailer

[Konstantin Ryabitsev]

On Wed, Apr 14, 2021 at 02:24:49PM -0300, Jason Gunthorpe wrote:
> > 1. "b4 am" learns a --newrev flag that populates "Obsoletes:" trailers 
> >    for your own patches it grabs from the list
> > 2. when you're ready to start working on the new version, you run "b4 am
> >    "git am" it to a new branch (similar to what I described above)
> > 3. b4 additionally learns a command to grab new trailers from the list by
> >    matching the Obsoletes: message-ids and auto-amending commits with any new
> >    trailers
> > 
> > This would allow you to start working on a new revision whenever you're ready
> > for it and give b4 a precise match it can rely on when tracking new trailers.
> 
> While I understand the interest in exact matching patches, I'm not
> super keen on adding more headers to every patch - this feels alot
> like the gerrit change-id header that everone hates, just in another
> format.

I know what you mean, but we can also take steps to make sure that it doesn't
leak into final commits:

1. a sendmail-validate hook can move Obsoletes: trailers below the ---
2. regular "b4 am" runs can drop any Obsoletes: trailers they find
3. even if it does leak into the final commits, it's not as bad as Change-Id
   because it's not just an obtuse string, but a link leading to a mailing
   list discussion

I think just setting up a sendmail-validate hook will be sufficient to prevent
any negative problems with this strategy.

-K

Re: RFC: Superseded-by: follow-up trailer

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

On Wed, Apr 14, 2021 at 02:24:49PM -0300, Jason Gunthorpe wrote:
> On Wed, Apr 14, 2021 at 01:07:51PM -0400, Konstantin Ryabitsev wrote:

> > 3. b4 additionally learns a command to grab new trailers from the list by
> >    matching the Obsoletes: message-ids and auto-amending commits with any new
> >    trailers

> > This would allow you to start working on a new revision whenever you're ready
> > for it and give b4 a precise match it can rely on when tracking new trailers.

> While I understand the interest in exact matching patches, I'm not
> super keen on adding more headers to every patch - this feels alot
> like the gerrit change-id header that everone hates, just in another
> format.

The big issue people generally have with the gerrit change IDs is that
they're only meaningful in the context of a gerrit instance that knows
about that particular change ID, usually one on some corporate network,
and no tooling other than gerrit uses them.  Message IDs are much more
generally useful.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: RFC: Superseded-by: follow-up trailer

[Jason Gunthorpe]

On Wed, Apr 14, 2021 at 07:46:59PM +0100, Mark Brown wrote:
> On Wed, Apr 14, 2021 at 02:24:49PM -0300, Jason Gunthorpe wrote:
> > On Wed, Apr 14, 2021 at 01:07:51PM -0400, Konstantin Ryabitsev wrote:
> 
> > > 3. b4 additionally learns a command to grab new trailers from the list by
> > >    matching the Obsoletes: message-ids and auto-amending commits with any new
> > >    trailers
> 
> > > This would allow you to start working on a new revision whenever you're ready
> > > for it and give b4 a precise match it can rely on when tracking new trailers.
> 
> > While I understand the interest in exact matching patches, I'm not
> > super keen on adding more headers to every patch - this feels alot
> > like the gerrit change-id header that everone hates, just in another
> > format.
> 
> The big issue people generally have with the gerrit change IDs is that
> they're only meaningful in the context of a gerrit instance that knows
> about that particular change ID, usually one on some corporate network,
> and no tooling other than gerrit uses them.  Message IDs are much more
> generally useful.

Well, IMHO, a chain of "obsoletes" is a fairly hacky way to create a
stable ID for a single commit to assist tooling. The gerrit idea of
applying a stable id to every commit is much more useful. If we had
that lore could index those stable IDs and the problem Konstantin is
chasing here becomes fairly simple to resolve by searching the index
for other messages related to the stable ID.

In turn if lore indexes the stable ID then storing the stable ID in
the commits becomes useful because lore can turn up everything related
to it, not just the single message that was used to make the commit.

Jason

Re: RFC: Superseded-by: follow-up trailer

[Steven Rostedt]

On Wed, 14 Apr 2021 11:36:18 -0300
Jason Gunthorpe <jgg@ziepe.ca> wrote:

>    People seem to struggle to both narrow the CC list of individual
>    patches series and still CC things like the cover letter to
>    everyone.

As someone that still uses quilt send --mail (I'm much more comfortable
that it will send the right thing than I am with git send-email), it
struggles with the above. I haven't had time to fix it to allow you to Cc
everyone the cover letter but narrow the individual patches.

It only sends all patches to anyone on the cover letter :-(

-- Steve

Re: RFC: Superseded-by: follow-up trailer

[Peter Zijlstra]

On Wed, Apr 14, 2021 at 11:01:02AM -0400, Steven Rostedt wrote:
> On Wed, 14 Apr 2021 11:36:18 -0300
> Jason Gunthorpe <jgg@ziepe.ca> wrote:
> 
> >    People seem to struggle to both narrow the CC list of individual
> >    patches series and still CC things like the cover letter to
> >    everyone.
> 
> As someone that still uses quilt send --mail (I'm much more comfortable
> that it will send the right thing than I am with git send-email), it
> struggles with the above. I haven't had time to fix it to allow you to Cc
> everyone the cover letter but narrow the individual patches.

If you really want to do daft things like that, just edit the mbox. I
mostly just send everybody everything. We all get too much email as is,
we should be able to ignore a few more emails a day.

Re: RFC: Superseded-by: follow-up trailer

[Willy Tarreau]

On Wed, Apr 14, 2021 at 05:36:01PM +0200, Peter Zijlstra wrote:
> I mostly just send everybody everything. We all get too much email as is,
> we should be able to ignore a few more emails a day.

For quite some time I've been very careful to refine the Cc lists to
avoid spamming people, just to get complaints afterwards when a discussion
started, because some didn't have all the context. So now I'm also in
favor of sending the whole series to everyone as long as it's a small
series. And this process is less error-prone.

Willy

Re: RFC: Superseded-by: follow-up trailer

[Theodore Ts'o]

On Wed, Apr 14, 2021 at 08:49:50AM -0300, Jason Gunthorpe wrote:
> 
> v3:
>   - blah blah
> v2: https://lore.kernel.org/r/foo
>   - blah blah
> v1: https://lore.kernel.org/r/bar

> b4 does a nice job helping the maintainer, but the submitter has no
> helpful common tooling currently.

It would be nice (and I think possible) if finding the mapping between
v1 and https://lore.kernel.org/r/bar could be automated --- and if we
had an appropriate hook in git send-email, it should be possible to
capture the message-id and stash it in a state file somewhere so the
developer could either fetch it manually, or if we had something like
debian's "debchange" command to make it easy to compose the changelog,
it could fetch it and stash it into the template file before opening
up an editor for the developer to update the changelog.

What do folks think?

				- Ted

Re: RFC: Superseded-by: follow-up trailer

[Geert Uytterhoeven]

Hi Ted et al,

On Wed, Apr 14, 2021 at 4:46 PM Theodore Ts'o <tytso@mit.edu> wrote:
> On Wed, Apr 14, 2021 at 08:49:50AM -0300, Jason Gunthorpe wrote:
> > v3:
> >   - blah blah
> > v2: https://lore.kernel.org/r/foo
> >   - blah blah
> > v1: https://lore.kernel.org/r/bar
>
> > b4 does a nice job helping the maintainer, but the submitter has no
> > helpful common tooling currently.
>
> It would be nice (and I think possible) if finding the mapping between
> v1 and https://lore.kernel.org/r/bar could be automated --- and if we
> had an appropriate hook in git send-email, it should be possible to
> capture the message-id and stash it in a state file somewhere so the
> developer could either fetch it manually, or if we had something like
> debian's "debchange" command to make it easy to compose the changelog,
> it could fetch it and stash it into the template file before opening
> up an editor for the developer to update the changelog.
>
> What do folks think?

I recently switched from letting "git send-email" generate Message-IDs
to letting "git format-patch" generate them (by setting
format.thread=true in git config). This means the Message-IDs are now
preserved in the cover letter and individual patch files, which can be
useful for reporting, or for creating backlinks to the previous version.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: RFC: Superseded-by: follow-up trailer

[Konstantin Ryabitsev]

On Wed, Apr 14, 2021 at 09:30:57AM +0200, Peter Zijlstra wrote:
> > I'd rather folks used the following notation in the cover letter/patch
> > basement:
> > 
> >     Previous versions:
> > 
> >     Link: https://lore.kernel.org/r/foo # v2
> >     Link: https://lore.kernel.org/r/bar # v1
> > 
> > This is much more succinct, indicates the version number in the trailer, and
> > removes the need to remember how to spell "Superseded", as there seems to be
> > little consensus. :)
> 
> How would you handle the difference between the above and:
> 
> Depends on:
> 
> Link: https://lkml.kernel.org/r/foo
> 
> Which is also a semi common thing to do with larger work that comes in
> multiple dependent series.

It's the "# vX" at the end that we'd be looking for. There's no other reason
to put it there if you aren't linking to a previous revision of the series, so
it's a very unambiguous pattern. However, read below.

> That is; I supppse; a long winded way of saying that for parsing (both
> human and machine) it might be useful to have distinct tag names. Now I
> agree with you that Supersedes is a crap word to spell, but it has the
> distinction of being clear on intent.
> 
> So my 2ct go to adding Supercedes: and Depends-on:

Hmm... there is an officially sanctioned way to indicate patch
interdependencies using "prerequisite-patch-id:" footers (see
https://git-scm.com/docs/git-format-patch#_base_tree_information).

That said, I've never seen anyone actually use them, I guess for two reasons:

1. it's per-patch, not per-series
2. it requires running "git patch-id" on each patch in the dependencies

We could build upon that using "prerequisite-series-link", but I find that
unwieldy, especially if it will be followed by a long URL.

How about we borrow RPM-spec lingo, for stuff to go into the cover letter or
below ---, e.g.:

    Subject: [PATCH v3] frobdrv: improve frob latency

    Commit message here

    Signed-off-by: Dave Elloper <dave@example.net>
    ---
    Requires: https://link.to/required-patch
    Obsoletes: https://link.to/v1 # v1
    Obsoletes: https://link.to/v2 # v2
    ---
    Changes in v3:
    Style tweaks.

    [diffstat, patch, etc]

or:

    Subject: [PATCH v3 0/2] frobdrv: improve frob latency

    This series improves frob latency by 20%.

    Requires: https://link.to/required-patch
    Obsoletes: https://link.to/v1 # v1
    Obsoletes: https://link.to/v2 # v2

    [diffstat, etc]

Then I can add automation that will send "Obsoleted-by:" and maybe
"Required-by:" follow-ups to the linked series when it encounters these tags.

Does this work for others?

-K

Re: RFC: Superseded-by: follow-up trailer

[Peter Zijlstra]

On Wed, Apr 14, 2021 at 10:35:04AM -0400, Konstantin Ryabitsev wrote:
> Hmm... there is an officially sanctioned way to indicate patch
> interdependencies using "prerequisite-patch-id:" footers (see
> https://git-scm.com/docs/git-format-patch#_base_tree_information).
> 
> That said, I've never seen anyone actually use them, I guess for two reasons:
> 
> 1. it's per-patch, not per-series
> 2. it requires running "git patch-id" on each patch in the dependencies

I'm not using git, not for making patches, not for sending patches, and
reading this thread makes me glad I'm not. It looks super painful in all
respects.

As to the original proposal, I'm with Steve, I'd never bother to dig out
the old thread (in fact, it's not even in my Sent folder).

As to the pain mentioned elsewhere, I'm not having that, I have an mbox
of the old submission laying about that I use as a basis for the new
submission, so I have all Msg-IDs right there.

Maybe the new-fangled tool you all need on the submission side already
exists, it's called quilt :-)

Re: RFC: Superseded-by: follow-up trailer

[Alex Elder]

On 4/13/21 4:40 PM, Konstantin Ryabitsev wrote:
> On Tue, Apr 13, 2021 at 05:29:01PM -0400, Steven Rostedt wrote:
>> Now, I would really like the Supercedes: tag, as its just a link you need,
>> and not a email sent. Ideally this would be in all the patches (with a
>> direct link to what it Supercedes. I may update my scripts to do exactly
>> this, as I always add a "Link: " to the last version, and that's in my
>> repo. Or perhaps b4 could add this too!
> 
> I'd rather folks used the following notation in the cover letter/patch
> basement:
> 
>      Previous versions:
> 
>      Link: https://lore.kernel.org/r/foo # v2
>      Link: https://lore.kernel.org/r/bar # v1

I had a few thoughts about this last night.  I hope this
isn't too rambly.

I haven't been using this to tie together versions of a
series.  But I think it's a good idea and I will start
(possibly after the dust settles on this discussion).
It implements at least part of what Doug Anderson sought
when he advocated including a Change-Id tag to track
related versions of patch series.

This practice is the developer's responsibility, not the
maintainer's.  I don't find that tracking the link down is a
very big burden.  And checkpatch.pl could be taught to look
for such links and suggest them for any version after the
first.  This probably isn't the "helpful common tooling"
that Jason Gunthorpe mentioned, but using checkpatch.pl is
a common expectation of developers.

I assume these links are pointing to the first message in
the series--which would be the cover letter assuming there
is one.  I have no interest in finding (or seeing) links
like this for every commit.  This suggests that developers
should always include a cover page for multi-patch series;
is that now required?

If this information is in the cover page, then maintainers
should preserve it by including the cover letter in the
merge commit.  I know this was a recent topic, but I didn't
follow it close enough to know whether that was the conclusion.

For single patches, I'd rather have the link be under the
"---", and have the link field added by the maintainer be
the only one committed.  I think I'd also prefer just
having a single link to the previous version, rather than
including a full and growing set of them in each version
(but that's only a slight preference).

> This is much more succinct, indicates the version number in the trailer, and
> removes the need to remember how to spell "Superseded", as there seems to be
> little consensus. :)

I retract my earlier spelling of that word.  Here too,
checkpatch.pl could ridicule anyone who spells it wrong and
we'll all figure it out eventually.

I like the succinctness of "Link:".  It is flexible enough
to be interpreted as a "see also" reference rather than
just "the patch submission correspondence is here."

On the other hand, it lacks specificity.  It's possible
that having distinct tags would be helpful for tools.

And finally I hope this practice (however it ends up looking)
is optional, to avoid adding yet another learning hurdle for
new developers.  Individual maintainers can require it or not
as a matter of their own policy.  I realize that reduces its
value.

					-Alex

> The tooling on the kernel.org side can automatically send the required
> follow-ups to previous threads when it finds these trailers in newer
> submissions.
> 
> -K
> 

Re: RFC: Superseded-by: follow-up trailer

[Konstantin Ryabitsev]

On Tue, Apr 13, 2021 at 03:53:50PM -0500, Alex Elder wrote:
> > I would like to propose a "Superseded-by" follow-up trailer to help strengthen
> > the link between series revisions. The trailer needs to be sent as a follow-up
> > to the cover letter or first patch of the previous series.
> > 
> >      Superseded-by: [url-ending-with/message-id-of-next-version]
> 
> Could "Supercedes: [url]" posted with the new version work?	-Alex

It doesn't achieve quite what we want -- specifically, a way to indicate to
someone that the patch series they have retrieved has an updated version.

E.g. if someone uses "b4 am" to retrieve a patch series, they wouldn't know
that there's an updated version -- we try to check for it by searching
lore.kernel.org based on subject + author, but this is a weak check.

If we only used the "Supersedes:" (or Link: [url] # v2) in newer revisions,
we'd have to already be aware of their existence. Patchwork can possibly do
this, since it parses all content, but b4 cannot, since it works within the
confines of a single thread.

-K