* Re: RFC: using supersedes: trailer to indicate patch/series revision flow
2019-11-07 20:43 RFC: using supersedes: trailer to indicate patch/series revision flow Konstantin Ryabitsev
@ 2019-11-07 23:45 ` Rafael J. Wysocki
2019-11-08 8:30 ` Han-Wen Nienhuys
2019-11-08 0:09 ` Andrew Donnellan
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2019-11-07 23:45 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: workflows
On Thursday, November 7, 2019 9:43:49 PM CET Konstantin Ryabitsev wrote:
> Hi, all:
>
> The only mechanism we currently have for patch/series versioning is
> subject suffixes. I think it would be useful to have a way to more
> explicitly mark that a series obsoletes a previous version, and I
> propose this is done with a `supersedes:` trailer at the end of the
> cover letter or in the first patch of the series:
All such things are a pain for patches generated by quilt, say.
> E.g.:
>
> Initial patch:
>
> ---8<---
> From: Dev Eloper <dev.eloper@example.com>
> Message-Id: <1572991351-86061-1-git-send-email-dev.eloper@example.com>
> Subject: [PATCH] Change foo
>
> Foo is no good. Use Bar.
>
> Signed-off-by: Dev Eloper <dev.eloper@example.com>
> ---
>
> foo | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/foo b/foo
> ...
>
> base-commit: 23fdb198ae81f47a574296dab5167c5e136a02ba
> --
> 2.24.0
> ---8<---
>
> Follow-up patch:
>
> ---8<---
> From: Dev Eloper <dev.eloper@example.com>
> Message-Id: <1572991352-86062-2-git-send-email-dev.eloper@example.com>
> Subject: [PATCH,v2] Change foo
>
> Foo is no good. Use Bar. Also use baz.
>
> Signed-off-by: Dev Eloper <dev.eloper@example.com>
> ---
>
> foo | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/foo b/foo
> ...
>
> supersedes: <1572991351-86061-1-git-send-email-dev.eloper@example.com>
> base-commit: 23fdb198ae81f47a574296dab5167c5e136a02ba
> --
> 2.24.0
> ---8<---
>
> Questions:
>
> 1. Should this be exposed via git-format-patch flags, or just used by
> specialized tooling?
I'd vote for specialized tooling (if anything).
> 2. Should supersedes: link to the previous version of the patch, or the
> first ever version of the patch? I am leaning towards the latter,
And then how do you know that version 2 was superseded by version 3?
> even though in this case the message-id largely becomes identical in
> usage to Gerrit's Change-Id.
>
> 3. Should the supersedes trailer have:
> a. message-id without brackets
> b. message-id with brackets
> c. https://lore.kernel.org/r/message-id
> My preference is b, to match with The Message-Id header usage.
An id of the exact patch (or series) superseded by this one IMO.
Cheers!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC: using supersedes: trailer to indicate patch/series revision flow
2019-11-07 23:45 ` Rafael J. Wysocki
@ 2019-11-08 8:30 ` Han-Wen Nienhuys
2019-11-08 9:59 ` Geert Uytterhoeven
0 siblings, 1 reply; 10+ messages in thread
From: Han-Wen Nienhuys @ 2019-11-08 8:30 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Konstantin Ryabitsev, workflows
On Fri, Nov 8, 2019 at 12:45 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > 2. Should supersedes: link to the previous version of the patch, or the
> > first ever version of the patch? I am leaning towards the latter,
>
> And then how do you know that version 2 was superseded by version 3?
You throw the message ID into a search engine, and see what it returns.
The advantage of keeping the patch series ID stable is that you can
consider a patchseries as a document and then easily index it inside a
service (say, patchwork) using Lucene, ElasticSearch or some other
common technology.
If you make the "supersedes" refer to specific versions, a workflow
service will be more susceptible to errors if messages were lost, and
the service has to work harder to aggregate the different versions of
a patchseries together.
Is it common for different authors to superseed each other's patch
series? If yes, "superseeds: precise version" is more precise, if not,
you get the same information from the timestamp of the cover letter.
--
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC: using supersedes: trailer to indicate patch/series revision flow
2019-11-08 8:30 ` Han-Wen Nienhuys
@ 2019-11-08 9:59 ` Geert Uytterhoeven
2019-11-08 10:48 ` Laurent Pinchart
2019-11-08 11:00 ` Rafael J. Wysocki
0 siblings, 2 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2019-11-08 9:59 UTC (permalink / raw)
To: Han-Wen Nienhuys; +Cc: Rafael J. Wysocki, Konstantin Ryabitsev, workflows
Hi Han-Wen,
On Fri, Nov 8, 2019 at 9:31 AM Han-Wen Nienhuys <hanwen@google.com> wrote:
> On Fri, Nov 8, 2019 at 12:45 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > > 2. Should supersedes: link to the previous version of the patch, or the
> > > first ever version of the patch? I am leaning towards the latter,
> >
> > And then how do you know that version 2 was superseded by version 3?
>
> You throw the message ID into a search engine, and see what it returns.
>
> The advantage of keeping the patch series ID stable is that you can
> consider a patchseries as a document and then easily index it inside a
> service (say, patchwork) using Lucene, ElasticSearch or some other
> common technology.
>
> If you make the "supersedes" refer to specific versions, a workflow
> service will be more susceptible to errors if messages were lost, and
> the service has to work harder to aggregate the different versions of
> a patchseries together.
>
> Is it common for different authors to superseed each other's patch
> series? If yes, "superseeds: precise version" is more precise, if not,
> you get the same information from the timestamp of the cover letter.
All/most of the above applies only to versioning of patch series that
implement a fixed feature, with a fixed scope.
So Vn+1 is really an improved version of Vn, and nothing more or less.
But this does not apply to many patch series in Linux kernel development.
Patch series may
- grow (more patches added),
- shrink (some patches rejected, or already applied independently),
- be split in multiple series,
- dropped but some parts reused in other series (possibly by someone
else),
- ...
That's the hard work in patch tracking.
So series IDs do not cope well with this, and "superseded" really should
apply to individual patches, not series, too.
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC: using supersedes: trailer to indicate patch/series revision flow
2019-11-08 9:59 ` Geert Uytterhoeven
@ 2019-11-08 10:48 ` Laurent Pinchart
2019-11-08 11:00 ` Rafael J. Wysocki
1 sibling, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2019-11-08 10:48 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Han-Wen Nienhuys, Rafael J. Wysocki, Konstantin Ryabitsev, workflows
Hi Geert,
On Fri, Nov 08, 2019 at 10:59:55AM +0100, Geert Uytterhoeven wrote:
> On Fri, Nov 8, 2019 at 9:31 AM Han-Wen Nienhuys <hanwen@google.com> wrote:
> > On Fri, Nov 8, 2019 at 12:45 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > > > 2. Should supersedes: link to the previous version of the patch, or the
> > > > first ever version of the patch? I am leaning towards the latter,
> > >
> > > And then how do you know that version 2 was superseded by version 3?
> >
> > You throw the message ID into a search engine, and see what it returns.
> >
> > The advantage of keeping the patch series ID stable is that you can
> > consider a patchseries as a document and then easily index it inside a
> > service (say, patchwork) using Lucene, ElasticSearch or some other
> > common technology.
> >
> > If you make the "supersedes" refer to specific versions, a workflow
> > service will be more susceptible to errors if messages were lost, and
> > the service has to work harder to aggregate the different versions of
> > a patchseries together.
> >
> > Is it common for different authors to superseed each other's patch
> > series? If yes, "superseeds: precise version" is more precise, if not,
> > you get the same information from the timestamp of the cover letter.
>
> All/most of the above applies only to versioning of patch series that
> implement a fixed feature, with a fixed scope.
> So Vn+1 is really an improved version of Vn, and nothing more or less.
>
> But this does not apply to many patch series in Linux kernel development.
> Patch series may
> - grow (more patches added),
This shouldn't be a problem, as the new patches get posted for the first
time, and thus don't supersede anything else than what the previous
version of the series contained.
> - shrink (some patches rejected, or already applied independently),
This should also not be an issue, as the patches that are dropped or
that are already applied will not be resubmitted separately.
> - be split in multiple series,
> - dropped but some parts reused in other series (possibly by someone
> else),
> - ...
These are the difficult cases :-(
> That's the hard work in patch tracking.
>
> So series IDs do not cope well with this, and "superseded" really should
> apply to individual patches, not series, too.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC: using supersedes: trailer to indicate patch/series revision flow
2019-11-08 9:59 ` Geert Uytterhoeven
2019-11-08 10:48 ` Laurent Pinchart
@ 2019-11-08 11:00 ` Rafael J. Wysocki
1 sibling, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2019-11-08 11:00 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Han-Wen Nienhuys, Konstantin Ryabitsev, workflows
On Friday, November 8, 2019 10:59:55 AM CET Geert Uytterhoeven wrote:
> Hi Han-Wen,
>
> On Fri, Nov 8, 2019 at 9:31 AM Han-Wen Nienhuys <hanwen@google.com> wrote:
> > On Fri, Nov 8, 2019 at 12:45 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > > > 2. Should supersedes: link to the previous version of the patch, or the
> > > > first ever version of the patch? I am leaning towards the latter,
> > >
> > > And then how do you know that version 2 was superseded by version 3?
> >
> > You throw the message ID into a search engine, and see what it returns.
> >
> > The advantage of keeping the patch series ID stable is that you can
> > consider a patchseries as a document and then easily index it inside a
> > service (say, patchwork) using Lucene, ElasticSearch or some other
> > common technology.
> >
> > If you make the "supersedes" refer to specific versions, a workflow
> > service will be more susceptible to errors if messages were lost, and
> > the service has to work harder to aggregate the different versions of
> > a patchseries together.
> >
> > Is it common for different authors to superseed each other's patch
> > series? If yes, "superseeds: precise version" is more precise, if not,
> > you get the same information from the timestamp of the cover letter.
>
> All/most of the above applies only to versioning of patch series that
> implement a fixed feature, with a fixed scope.
> So Vn+1 is really an improved version of Vn, and nothing more or less.
>
> But this does not apply to many patch series in Linux kernel development.
> Patch series may
> - grow (more patches added),
> - shrink (some patches rejected, or already applied independently),
> - be split in multiple series,
> - dropped but some parts reused in other series (possibly by someone
> else),
> - ...
> That's the hard work in patch tracking.
>
> So series IDs do not cope well with this, and "superseded" really should
> apply to individual patches, not series, too.
Right.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC: using supersedes: trailer to indicate patch/series revision flow
2019-11-07 20:43 RFC: using supersedes: trailer to indicate patch/series revision flow Konstantin Ryabitsev
2019-11-07 23:45 ` Rafael J. Wysocki
@ 2019-11-08 0:09 ` Andrew Donnellan
2019-11-08 9:19 ` Vegard Nossum
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Andrew Donnellan @ 2019-11-08 0:09 UTC (permalink / raw)
To: workflows
On 8/11/19 7:43 am, Konstantin Ryabitsev wrote:
> Questions:
>
> 1. Should this be exposed via git-format-patch flags, or just used by
> specialized tooling?
I guess we might want to expose it through git format-patch for the sake
of completeness? Though if people actually use it with git format-patch,
then the answer to Q2 is irrelevant because people will use it to link
to whatever the heck they feel like at the time...
Though what do you envision the user workflow actually looking like with
a specialised tool?
> 2. Should supersedes: link to the previous version of the patch, or the
> first ever version of the patch? I am leaning towards the latter,
> even though in this case the message-id largely becomes identical in
> usage to Gerrit's Change-Id.
If we link to the first version of the patch, we can't tell that V3
supersedes V2. Though if we link to only the previous version, we can't
tell that V3 supersedes V1 without looking at V2.
From a Patchwork perspective, if we were to use this to track patch
versions I think we could work with either, except for some corner cases
where mail is not received or received in the wrong order and there's
not enough subject metadata to work things out.
You could also include multiple trailers e.g. the first version and last
version, or all previous versions that are superseded, though that seems
a bit verbose.
> 3. Should the supersedes trailer have:
> a. message-id without brackets
> b. message-id with brackets
> c. https://lore.kernel.org/r/message-id
> My preference is b, to match with The Message-Id header usage.
A or B makes the most sense.
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC: using supersedes: trailer to indicate patch/series revision flow
2019-11-07 20:43 RFC: using supersedes: trailer to indicate patch/series revision flow Konstantin Ryabitsev
2019-11-07 23:45 ` Rafael J. Wysocki
2019-11-08 0:09 ` Andrew Donnellan
@ 2019-11-08 9:19 ` Vegard Nossum
2019-11-08 9:46 ` Geert Uytterhoeven
2019-11-14 6:29 ` Eric Wong
4 siblings, 0 replies; 10+ messages in thread
From: Vegard Nossum @ 2019-11-08 9:19 UTC (permalink / raw)
To: workflows
On 11/7/19 9:43 PM, Konstantin Ryabitsev wrote:
> Hi, all:
>
> The only mechanism we currently have for patch/series versioning is
> subject suffixes. I think it would be useful to have a way to more
> explicitly mark that a series obsoletes a previous version, and I
> propose this is done with a `supersedes:` trailer at the end of the
> cover letter or in the first patch of the series:
[...]
I think it's better to put it in the changelog itself (similar to S-O-B,
R-B, A-B, etc. lines) because it allows you to look the information up
straight from a commit once it has been merged.
This is what I suggested under "Step 3", second point here:
https://lore.kernel.org/workflows/b9fb52b8-8168-6bf0-9a72-1e6c44a281a5@oracle.com/
In the scheme I proposed, you would additionally refer to the previous
version by SHA1, which means diffing the patchsets would be as easy as
typing 'git diff' and then pasting the two SHA1s (which would both be
right there in front of you -- no need to follow lore links and applying
patches by hand to compare them).
This is just one data point, but in the team I work in this would be
incredibly useful to have -- we frequently look at upstream/merged
commits and sometimes want to compare them to mailing list submissions
and running git diff/log/range-diff is FAR easier than searching
mailing lists and applying series by hand or trying to figure out by
eyeballing email patches.
> Questions:
>
[...]
> 2. Should supersedes: link to the previous version of the patch, or the
> first ever version of the patch? I am leaning towards the latter,
> even though in this case the message-id largely becomes identical in
> usage to Gerrit's Change-Id.
If you add the info to each patch, then it makes sense to link to the
previous version of that patch, since you can always find the first/last
patch from that.
If we also used the merge trick I described to refer to a whole patchset
with a single SHA1 then you can additionally link to the previous
version of that patchset from the merge commit, meaning you effectively
get both (one link in each patch to the previous version of _that_
patch, plus one link in the final merge commit to the previous version
of the full patchset).
Vegard
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC: using supersedes: trailer to indicate patch/series revision flow
2019-11-07 20:43 RFC: using supersedes: trailer to indicate patch/series revision flow Konstantin Ryabitsev
` (2 preceding siblings ...)
2019-11-08 9:19 ` Vegard Nossum
@ 2019-11-08 9:46 ` Geert Uytterhoeven
2019-11-14 6:29 ` Eric Wong
4 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2019-11-08 9:46 UTC (permalink / raw)
To: workflows
Hi Konstantin,
On Thu, Nov 7, 2019 at 9:44 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
> The only mechanism we currently have for patch/series versioning is
> subject suffixes. I think it would be useful to have a way to more
> explicitly mark that a series obsoletes a previous version, and I
> propose this is done with a `supersedes:` trailer at the end of the
> cover letter or in the first patch of the series:
In theory, this sounds like a good idea to me.
Whether it's practical to implement it, is to be seen...
> Questions:
>
> 1. Should this be exposed via git-format-patch flags, or just used by
> specialized tooling?
While I switched to always applying bundles from patchwork to have the
Link: added automatically, I'm not aware of a simple way to add a
superseded header automatically.
> 2. Should supersedes: link to the previous version of the patch, or the
> first ever version of the patch? I am leaning towards the latter,
> even though in this case the message-id largely becomes identical in
> usage to Gerrit's Change-Id.
The previous version, as that's the one being superseded, and doing it
this way allows to iterate backwards through history.
The first ever version has been superseded already by the previous
version (if any).
> 3. Should the supersedes trailer have:
> a. message-id without brackets
> b. message-id with brackets
> c. https://lore.kernel.org/r/message-id
> My preference is b, to match with The Message-Id header usage.
I think c makes most sense, as it allows to follow the Link: in a commit, and
iterate backwards using the same method.
Else we have to convert manually from message-id to lore link.
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC: using supersedes: trailer to indicate patch/series revision flow
2019-11-07 20:43 RFC: using supersedes: trailer to indicate patch/series revision flow Konstantin Ryabitsev
` (3 preceding siblings ...)
2019-11-08 9:46 ` Geert Uytterhoeven
@ 2019-11-14 6:29 ` Eric Wong
4 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2019-11-14 6:29 UTC (permalink / raw)
To: workflows
Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> Hi, all:
>
> The only mechanism we currently have for patch/series versioning is
> subject suffixes. I think it would be useful to have a way to more
> explicitly mark that a series obsoletes a previous version, and I
> propose this is done with a `supersedes:` trailer at the end of the
> cover letter or in the first patch of the series:
<snip>
> Subject: [PATCH,v2] Change foo
>
> Foo is no good. Use Bar. Also use baz.
>
> Signed-off-by: Dev Eloper <dev.eloper@example.com>
> ---
> foo | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/foo b/foo
> ...
>
> supersedes: <1572991351-86061-1-git-send-email-dev.eloper@example.com>
This would works badly with limited bandwidth and storage
situations.
--interdiff and --range-diff options already give plenty of
useful info inline; and --range-diff can be enhanced to work
better in scenarios where there's enough storage/bandwidth for a
powerful search engine (--interdiff already does):
Here's a quick PoC which allows --range-diff to be more useful
to searcn engines (but the current regression is it's too
noisy for human eyes, and making it less so requires more work):
https://public-inbox.org/git/20191017121045.GA15364@dcvr/
("range-diff: show old/new blob OIDs in comments")
And search engine tooling still needs to be built around search
for better interdiff and range-diff blob expansion (since it
already exists for regular patches).
^ permalink raw reply [flat|nested] 10+ messages in thread