All of lore.kernel.org
 help / color / mirror / Atom feed
* b4-0.9.0 available
@ 2022-06-17 19:01 Konstantin Ryabitsev
  2022-06-20  8:40 ` Geert Uytterhoeven
  2022-06-21 15:20 ` Geert Uytterhoeven
  0 siblings, 2 replies; 34+ messages in thread
From: Konstantin Ryabitsev @ 2022-06-17 19:01 UTC (permalink / raw)
  To: tools, users

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

Hello:

After a bit of a hiatus, I am finally able to work on b4 again, so here's
version 0.9.0 that implements one major new feature and adds some other
improvements to existing features.

# new: b4 shazam

This command is very similar to "b4 am", but it allows you to apply patches
directly to the git repository without the need pipe the output of "b4 am" to
"git am".

The shazam command has three modes of operation:

1. default mode that applies patches directly to your git repository, just
   like passing the .mbox file to git-am
2. with -H, shazam will try to prepare the patch series into FETCH_HEAD, which
   can then be checked out into a branch or merged with git-merge. This
   effectively makes patch series work similar to pull requests.
3. with -M, shazam will do the same as with -H, but will also exec git-merge
   with the cover letter prepared as part of the merge commit message. You can
   see it in action here: https://asciinema.org/a/502421

Please trial it out and report any problems or improvements you would like to
see.

# improved: --no-parent mode for b4 am/shazam

Per popular request, you are now able to break threads at the message-id
specified using the --no-parent flag to am/shazam subcommands. In this case,
b4 should ignore any parent messages in the thread and only operate on the
message itself and any of its descendants. This should help unconfuse b4 when
someone submits an auxiliary patch in the middle of a thread and you want to
be able to get *just* that patch and not the parent series.

# improved: send mail with b4 ty -S

If you're using b4 ty, you can use the -S switch to have b4 send mail directly
instead of just writing .thanks files. Additionally, b4 ty gained --dry-run to
help make sure that things are looking good before you actually send mail.

# improved: mailmap support for b4 ty

If you're using b4 ty, it will now properly check the .mailmap file when
creating reply messages. You can also exclude entire domains from ever being
included in to/cc when generating thank-you replies using the email-exclude
setting, e.g.:

    [b4]
        email-exclude = *@codeaurora.org

In addition to the above, there are fixes and improvements to many aspects of
b4.

# Upgrading

To upgrade from pip, run:

    pip install --upgrade b4

Alternatively, wait for distros to pick up the new release.

If you want to run b4 from your git checkout, check the README that documents
this option.

# Thanks

Thank you for your patience while I was unable to dedicate a lot of time to
b4. I had to spend a lot of time migrating and upgrading hardware at LF, and
then a certain dictator started a bloody invasion in Ukraine, which
significantly affected my ability to concentrate on most other things. If you
can, please support Ukraine's fight against the invading fascist regime
through your preferred humanitarian org.

I would also like to thank the following people who helped with this release,
in the reverse git log order:

- David Vernet <void@manifault.com>
- Mark Brown <broonie@kernel.org>
- Rob Herring <robh@kernel.org>
- Jens Axboe <axboe@kernel.dk>
- Kees Cook <keescook@chromium.org>

Best regards,
Konstantin

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-17 19:01 b4-0.9.0 available Konstantin Ryabitsev
@ 2022-06-20  8:40 ` Geert Uytterhoeven
  2022-06-21 23:38   ` Linus Walleij
  2022-06-21 15:20 ` Geert Uytterhoeven
  1 sibling, 1 reply; 34+ messages in thread
From: Geert Uytterhoeven @ 2022-06-20  8:40 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools, users

Hi Konstantin,

On Fri, Jun 17, 2022 at 9:02 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
> After a bit of a hiatus, I am finally able to work on b4 again, so here's
> version 0.9.0 that implements one major new feature and adds some other
> improvements to existing features.

Thanks for your continued support on improving our workflow!

> # new: b4 shazam
>
> This command is very similar to "b4 am", but it allows you to apply patches
> directly to the git repository without the need pipe the output of "b4 am" to
> "git am".
>
> The shazam command has three modes of operation:
>
> 1. default mode that applies patches directly to your git repository, just
>    like passing the .mbox file to git-am
> 2. with -H, shazam will try to prepare the patch series into FETCH_HEAD, which
>    can then be checked out into a branch or merged with git-merge. This
>    effectively makes patch series work similar to pull requests.
> 3. with -M, shazam will do the same as with -H, but will also exec git-merge
>    with the cover letter prepared as part of the merge commit message. You can
>    see it in action here: https://asciinema.org/a/502421

So if I want to run checkpatch, I have to keep on using "b4 am"
and "git am" separately, right?  I never apply patches without running
checkpatch first[1].
Perhaps "b4 shazam" could gain some --checker option to support this?

Thanks!

[1] Currently I use "cat *mbx | formail -s scripts/checkpatch.pl".

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] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-17 19:01 b4-0.9.0 available Konstantin Ryabitsev
  2022-06-20  8:40 ` Geert Uytterhoeven
@ 2022-06-21 15:20 ` Geert Uytterhoeven
  2022-06-21 15:29   ` Konstantin Ryabitsev
  1 sibling, 1 reply; 34+ messages in thread
From: Geert Uytterhoeven @ 2022-06-21 15:20 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools, users

Hi Konstantin,

On Fri, Jun 17, 2022 at 9:02 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
> # new: b4 shazam
>
> This command is very similar to "b4 am", but it allows you to apply patches
> directly to the git repository without the need pipe the output of "b4 am" to
> "git am".
>
> The shazam command has three modes of operation:
>
> 1. default mode that applies patches directly to your git repository, just
>    like passing the .mbox file to git-am

I gave it a try, and noticed a difference: with b4 am + git am, the
Link tag is added first, followed by my SoB, while b4 shazam does
the reverse, and adds my SoB first.
I believe the b4 am + git am behavior is correct.

Thanks!

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] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 15:20 ` Geert Uytterhoeven
@ 2022-06-21 15:29   ` Konstantin Ryabitsev
  2022-06-21 15:41     ` Geert Uytterhoeven
  2022-06-21 15:53     ` Jason A. Donenfeld
  0 siblings, 2 replies; 34+ messages in thread
From: Konstantin Ryabitsev @ 2022-06-21 15:29 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: tools, users

On Tue, Jun 21, 2022 at 05:20:02PM +0200, Geert Uytterhoeven wrote:
> > 1. default mode that applies patches directly to your git repository, just
> >    like passing the .mbox file to git-am
> 
> I gave it a try, and noticed a difference: with b4 am + git am, the
> Link tag is added first, followed by my SoB, while b4 shazam does
> the reverse, and adds my SoB first.
> I believe the b4 am + git am behavior is correct.

I'm guessing you were adding the SoB with "git am -s" as opposed to letting b4
do it?

I'm not sure there's an established "correct" behaviour here, as various
maintainers either stick to arbitrary ordering, chain-of-custody ordering, or
have a very strict "I like all my trailers sorted in a very particular order"
approach.

I can swap the Link: and S-o-b: trailer order, or you can just turn off the
Link: trailer, since the latest recommendation is to restrict them to merge
commits.

-K

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 15:29   ` Konstantin Ryabitsev
@ 2022-06-21 15:41     ` Geert Uytterhoeven
  2022-06-21 15:55       ` Linus Torvalds
  2022-06-21 15:53     ` Jason A. Donenfeld
  1 sibling, 1 reply; 34+ messages in thread
From: Geert Uytterhoeven @ 2022-06-21 15:41 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools, users

Hi Konstantin,

On Tue, Jun 21, 2022 at 5:29 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
> On Tue, Jun 21, 2022 at 05:20:02PM +0200, Geert Uytterhoeven wrote:
> > > 1. default mode that applies patches directly to your git repository, just
> > >    like passing the .mbox file to git-am
> >
> > I gave it a try, and noticed a difference: with b4 am + git am, the
> > Link tag is added first, followed by my SoB, while b4 shazam does
> > the reverse, and adds my SoB first.
> > I believe the b4 am + git am behavior is correct.
>
> I'm guessing you were adding the SoB with "git am -s" as opposed to letting b4
> do it?

Exactly.

> I'm not sure there's an established "correct" behaviour here, as various
> maintainers either stick to arbitrary ordering, chain-of-custody ordering, or
> have a very strict "I like all my trailers sorted in a very particular order"
> approach.

FWIW, I follow the chain-of-custody ordering.

> I can swap the Link: and S-o-b: trailer order, or you can just turn off the
> Link: trailer, since the latest recommendation is to restrict them to merge
> commits.

I still find them useful, as they e.g. allow me to navigate easily to the cover
letter (which might link to the previous version(s), and to whatever else),
even if I applied only one or only a few patches from a series.

Thanks!

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] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 15:29   ` Konstantin Ryabitsev
  2022-06-21 15:41     ` Geert Uytterhoeven
@ 2022-06-21 15:53     ` Jason A. Donenfeld
  1 sibling, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-06-21 15:53 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: Geert Uytterhoeven, tools, users

On 6/21/22, Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Tue, Jun 21, 2022 at 05:20:02PM +0200, Geert Uytterhoeven wrote:
>> > 1. default mode that applies patches directly to your git repository,
>> > just
>> >    like passing the .mbox file to git-am
>>
>> I gave it a try, and noticed a difference: with b4 am + git am, the
>> Link tag is added first, followed by my SoB, while b4 shazam does
>> the reverse, and adds my SoB first.
>> I believe the b4 am + git am behavior is correct.
>
> I'm guessing you were adding the SoB with "git am -s" as opposed to letting
> b4
> do it?
>
> I'm not sure there's an established "correct" behaviour here, as various
> maintainers either stick to arbitrary ordering, chain-of-custody ordering,
> or
> have a very strict "I like all my trailers sorted in a very particular
> order"
> approach.

Chain of custody is the only order that has any applicable meaning for
S-o-b. Otherwise what's the point of S-o-b in the first place? These
tools should be encouraging the right ways to do things rather than
catering to a potpourri of wrong ways.

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 15:41     ` Geert Uytterhoeven
@ 2022-06-21 15:55       ` Linus Torvalds
  2022-06-21 16:03         ` Jason A. Donenfeld
  0 siblings, 1 reply; 34+ messages in thread
From: Linus Torvalds @ 2022-06-21 15:55 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Konstantin Ryabitsev, tools, users

On Tue, Jun 21, 2022 at 10:41 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> FWIW, I follow the chain-of-custody ordering.

I don't think different kinds of links have any particular ordering,
but yes,the "Signed-off-by:" ones should basically be sorted by
"author -> email chain -> committer" order so that you can see which
actual "path" the patch took.

               Linus

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 15:55       ` Linus Torvalds
@ 2022-06-21 16:03         ` Jason A. Donenfeld
  2022-06-21 16:59           ` Konstantin Ryabitsev
  0 siblings, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-06-21 16:03 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Geert Uytterhoeven, Konstantin Ryabitsev, tools, users

On 6/21/22, Linus Torvalds <torvalds@linuxfoundation.org> wrote:
> On Tue, Jun 21, 2022 at 10:41 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>>
>> FWIW, I follow the chain-of-custody ordering.
>
> I don't think different kinds of links have any particular ordering,
> but yes,the "Signed-off-by:" ones should basically be sorted by
> "author -> email chain -> committer" order so that you can see which
> actual "path" the patch took.
>

Very much so. But if you look at various commits from the last 2 or 3
years, this is now disregarded pretty regularly. It'd be nice to see
the tooling preventing this kind of error rather than encouraging it.

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 16:03         ` Jason A. Donenfeld
@ 2022-06-21 16:59           ` Konstantin Ryabitsev
  2022-06-21 17:49             ` Jason A. Donenfeld
  0 siblings, 1 reply; 34+ messages in thread
From: Konstantin Ryabitsev @ 2022-06-21 16:59 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Linus Torvalds, Geert Uytterhoeven, tools, users

On Tue, Jun 21, 2022 at 06:03:30PM +0200, Jason A. Donenfeld wrote:
> Very much so. But if you look at various commits from the last 2 or 3
> years, this is now disregarded pretty regularly. It'd be nice to see
> the tooling preventing this kind of error rather than encouraging it.

The default for b4 since version 0.6 (2020) has been not to preserve the
existing trailer order and append any new ones at the bottom.

-K

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 16:59           ` Konstantin Ryabitsev
@ 2022-06-21 17:49             ` Jason A. Donenfeld
  2022-06-21 18:29               ` Konstantin Ryabitsev
  0 siblings, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-06-21 17:49 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: Linus Torvalds, Geert Uytterhoeven, tools, users

Hi Konstantin,

On Tue, Jun 21, 2022 at 12:59:53PM -0400, Konstantin Ryabitsev wrote:
> On Tue, Jun 21, 2022 at 06:03:30PM +0200, Jason A. Donenfeld wrote:
> > Very much so. But if you look at various commits from the last 2 or 3
> > years, this is now disregarded pretty regularly. It'd be nice to see
> > the tooling preventing this kind of error rather than encouraging it.
> 
> The default for b4 since version 0.6 (2020) has been not to preserve the
> existing trailer order and append any new ones at the bottom.

Huh, interesting. Is this a stable-distro or Debian thing, I wonder,
where maintainers are using an old b4? Or perhaps more likely, b4 still
has that option, and so people are using that option, even if it's not
the default?

IMO, you should remove both the "sort S-o-b into meaninglessness" and
the "add a useless Link: trailer that wastes my time when I middle click
it" options, so that it's harder to make these mistakes.

Looking at the man page, it seems like you make these mistakes very easy
and give people some example configs even:

     # When processing thread trailers, sort them in this order.
     # Can use shell-globbing and must end with ,*
     # Some sorting orders:
     #trailer-order=link*,fixes*,cc*,reported*,suggested*,original*,co-*,tested*,reviewed*,acked*,signed-off*,*
     #trailer-order = fixes*,reported*,suggested*,original*,co-*,signed-off*,tested*,reviewed*,acked*,cc*,link*,*
     trailer-order = _preserve_

Just remove the functionality?

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 17:49             ` Jason A. Donenfeld
@ 2022-06-21 18:29               ` Konstantin Ryabitsev
  2022-06-21 18:45                 ` Jason A. Donenfeld
  0 siblings, 1 reply; 34+ messages in thread
From: Konstantin Ryabitsev @ 2022-06-21 18:29 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Linus Torvalds, Geert Uytterhoeven, tools, users

On Tue, Jun 21, 2022 at 07:49:18PM +0200, Jason A. Donenfeld wrote:
> IMO, you should remove both the "sort S-o-b into meaninglessness" and
> the "add a useless Link: trailer that wastes my time when I middle click
> it" options, so that it's harder to make these mistakes.

I mean, I'm happy to make any changes, but I implemented these in the first
place as a response to requests or established practices. Some maintainers do
very much like to sort their trailers, and others like to link to the list
origins of the patch.

If there is someone who is willing to compile a definitive list of "Linux
Kernel commit do's and dont's", then I'll happily stick to that.

For now, the default operation is already exactly as you ask -- we neither
sort the trailers nor add the link unless both of these are explicitly
requested.

-K

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 18:29               ` Konstantin Ryabitsev
@ 2022-06-21 18:45                 ` Jason A. Donenfeld
  2022-06-21 19:27                   ` Konstantin Ryabitsev
  2022-06-21 21:25                   ` Bjorn Andersson
  0 siblings, 2 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-06-21 18:45 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: Linus Torvalds, Geert Uytterhoeven, tools, users

Hi Konstantin,

On Tue, Jun 21, 2022 at 02:29:53PM -0400, Konstantin Ryabitsev wrote:
> If there is someone who is willing to compile a definitive list of "Linux
> Kernel commit do's and dont's", then I'll happily stick to that.

I dunno about a "definitive list". But it seems like a lot of
conventions get solidified by way of tools that are ironed out over
time. For example, every time a code style discussion comes up, Joe
Perches arrives on the scene to make sure checkpatch.pl is current. And
now the clang-format file is starting to also accumulate collective
preferences. It seems like b4 is pretty widely used and will eventually
serve a similar purpose if it doesn't already do so.

Anyway, in lieu of a complete and thorough list of all of the things, it
seems like in the last week or so we at least have two things that can
be be reflected in the tooling:

- Don't add `Link:` if the URL hasn't been hand selected as being
  particularly relevant; the lkml patch email alone doesn't cut it. This
  suggests that automatically adding `Link:` is invariably wrong, since
  automatic != "hand selected", so maybe there's no point in
  `-l,--add-link`, and you can just remove that option.

- Don't reorder `Signed-off-by:`. The simplest thing to do would be to
  remove the `trailer-order` option all together. But some people do
  want to sort the trailers (within each S-o-b segment) for aesthetic
  purposes. So if you don't want to take that away from them, maybe
  S-o-b needs to be unconditionally special-cased so that it forms a
  reorder barrier.

  #define sob(name) do { smp_mb(); __sob(name); } while (0)

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 18:45                 ` Jason A. Donenfeld
@ 2022-06-21 19:27                   ` Konstantin Ryabitsev
  2022-06-21 19:42                     ` Jason A. Donenfeld
  2022-06-21 19:43                     ` Geert Uytterhoeven
  2022-06-21 21:25                   ` Bjorn Andersson
  1 sibling, 2 replies; 34+ messages in thread
From: Konstantin Ryabitsev @ 2022-06-21 19:27 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Linus Torvalds, Geert Uytterhoeven, tools, users

On Tue, Jun 21, 2022 at 08:45:46PM +0200, Jason A. Donenfeld wrote:
> Anyway, in lieu of a complete and thorough list of all of the things, it
> seems like in the last week or so we at least have two things that can
> be be reflected in the tooling:
> 
> - Don't add `Link:` if the URL hasn't been hand selected as being
>   particularly relevant; the lkml patch email alone doesn't cut it. This
>   suggests that automatically adding `Link:` is invariably wrong, since
>   automatic != "hand selected", so maybe there's no point in
>   `-l,--add-link`, and you can just remove that option.

I think the discussion veered towards "don't call it Link:", not "don't link
to it at all". Without "BugLink" in the running, the next winning option was
"Archived-at:" and I am considering towards making -l add that trailer
instead, with the custom msgid.link domain, to further distinguish it from
"hand selected" lore links:

    Archived-at: https://msgid.link/20220617190153.tvi5lkzlvemeqou5@meerkat.local

I am not currently working on that, though, so I'm happy to go either way,
including making -l a "does nothing" switch.

> - Don't reorder `Signed-off-by:`. The simplest thing to do would be to
>   remove the `trailer-order` option all together. But some people do
>   want to sort the trailers (within each S-o-b segment) for aesthetic
>   purposes. So if you don't want to take that away from them, maybe
>   S-o-b needs to be unconditionally special-cased so that it forms a
>   reorder barrier.

This is already the case for all trailers -- we *group* them by trailer name,
but the *order* in which they are written is preserved. E.g. with the config
option trailer-order="reported-by,reviewed-by,signed-off-by,*', we go from:

    Signed-off-by: Ze Developer <ze@example.dev>
    Reviewed-by: Some Rando <rando@example.dev>
    Reviewed-by: Managing Rando <mgmt@example.dev>
    Signed-off-by: Awesome Maintainer <aw@example.dev>
    Reported-by: Bug Finder <bug@example.dev>


to:

    Reported-by: Bug Finder <bug@example.dev>
    Reviewed-by: Some Rando <rando@example.dev>
    Reviewed-by: Managing Rando <mgmt@example.dev>
    Signed-off-by: Ze Developer <ze@example.dev>
    Signed-off-by: Awesome Maintainer <aw@example.dev>

Perhaps I should call the setting "trailer-grouping" instead of
"trailer-order" (it's called that because we did reorder them alphabetically
at some point in the distant past when the feature was first implemented).

-K

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 19:27                   ` Konstantin Ryabitsev
@ 2022-06-21 19:42                     ` Jason A. Donenfeld
  2022-06-21 19:43                     ` Geert Uytterhoeven
  1 sibling, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-06-21 19:42 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: Linus Torvalds, Geert Uytterhoeven, tools, users

Hi Konstantin,

On Tue, Jun 21, 2022 at 03:27:24PM -0400, Konstantin Ryabitsev wrote:
> I think the discussion veered towards "don't call it Link:", not "don't link
> to it at all". Without "BugLink" in the running, the next winning option was
> "Archived-at:" and I am considering towards making -l add that trailer
> instead, with the custom msgid.link domain, to further distinguish it from
> "hand selected" lore links:
> 
>     Archived-at: https://msgid.link/20220617190153.tvi5lkzlvemeqou5@meerkat.local
> 
> I am not currently working on that, though, so I'm happy to go either way,
> including making -l a "does nothing" switch.

Oh. I thought it wound up concluding that none of this was necessary and
instead the tooling could be fixed server-side, so that you could have
patchwork query "hey which mailing list post is patch 0123456789abcdef?"
and it'll tell you that, or vice versa, especially since it's not always
clear which patch version is the most relevant, as sometimes things are
tweaked after the fact, and so some fuzzy sorting is usually required.
In other words, no need to store this inline with commits, and a service
can handle that more flexibly anyway.

> > - Don't reorder `Signed-off-by:`. The simplest thing to do would be to
> >   remove the `trailer-order` option all together. But some people do
> >   want to sort the trailers (within each S-o-b segment) for aesthetic
> >   purposes. So if you don't want to take that away from them, maybe
> >   S-o-b needs to be unconditionally special-cased so that it forms a
> >   reorder barrier.
> 
> This is already the case for all trailers -- we *group* them by trailer name,
> but the *order* in which they are written is preserved. E.g. with the config
> option trailer-order="reported-by,reviewed-by,signed-off-by,*', we go from:
> 
>     Signed-off-by: Ze Developer <ze@example.dev>
>     Reviewed-by: Some Rando <rando@example.dev>
>     Reviewed-by: Managing Rando <mgmt@example.dev>
>     Signed-off-by: Awesome Maintainer <aw@example.dev>
>     Reported-by: Bug Finder <bug@example.dev>
> 
> 
> to:
> 
>     Reported-by: Bug Finder <bug@example.dev>
>     Reviewed-by: Some Rando <rando@example.dev>
>     Reviewed-by: Managing Rando <mgmt@example.dev>
>     Signed-off-by: Ze Developer <ze@example.dev>
>     Signed-off-by: Awesome Maintainer <aw@example.dev>
> 
> Perhaps I should call the setting "trailer-grouping" instead of
> "trailer-order" (it's called that because we did reorder them alphabetically
> at some point in the distant past when the feature was first implemented).

I don't know what that first block is, since the Reported-by comes after
a Signed-off-by line, which means it's already ambiguous and wrong. But
even if we throw away that line, your new snippet is still wrong, since
now it shows Ze Developer having collected those Reviewed-bys, when
Awesome Maintainer is the one who did it.

In other words, if we start with this:

     Signed-off-by: Ze Developer <ze@example.dev>
     Reviewed-by: Some Rando <rando@example.dev>
     Reported-by: Bug Finder <bug@example.dev>
     Reviewed-by: Managing Rando <mgmt@example.dev>
     Signed-off-by: Awesome Maintainer <aw@example.dev>

A valid reordering is this:

     Signed-off-by: Ze Developer <ze@example.dev>
     Reviewed-by: Some Rando <rando@example.dev>
     Reviewed-by: Managing Rando <mgmt@example.dev>
     Reported-by: Bug Finder <bug@example.dev>
     Signed-off-by: Awesome Maintainer <aw@example.dev>

But NOT:

     Reviewed-by: Some Rando <rando@example.dev>
     Reviewed-by: Managing Rando <mgmt@example.dev>
     Reported-by: Bug Finder <bug@example.dev>
     Signed-off-by: Ze Developer <ze@example.dev>
     Signed-off-by: Awesome Maintainer <aw@example.dev>

Since the bad one obscures who collected the tags.

The prevailing idea is that each hop in the custody chain has their
"S-o-b area", which is the zone above their S-o-b and below the previous
S-o-b. Sometimes such areas accumulate text too, such as:

  [ Replaced turboencabulator jet fuel with diesel. ]

So if you sort, fine, but don't sort across S-o-b lines. Those form an
ordering barrier.

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 19:27                   ` Konstantin Ryabitsev
  2022-06-21 19:42                     ` Jason A. Donenfeld
@ 2022-06-21 19:43                     ` Geert Uytterhoeven
  2022-06-21 19:50                       ` Jason A. Donenfeld
  1 sibling, 1 reply; 34+ messages in thread
From: Geert Uytterhoeven @ 2022-06-21 19:43 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: Jason A. Donenfeld, Linus Torvalds, tools, users

Hi Konstantin,

On Tue, Jun 21, 2022 at 9:27 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
> On Tue, Jun 21, 2022 at 08:45:46PM +0200, Jason A. Donenfeld wrote:
> > Anyway, in lieu of a complete and thorough list of all of the things, it
> > seems like in the last week or so we at least have two things that can
> > be be reflected in the tooling:
> >
> > - Don't add `Link:` if the URL hasn't been hand selected as being
> >   particularly relevant; the lkml patch email alone doesn't cut it. This

I disagree: when bisecting a bug, I can just follow the Link: tag,
and check if anyone already replied there with a newer review or
bug report (and hopefully with a fix, too).
Without the Link: tag, it's sometimes surprisingly difficult to find
out where the patch has been posted and discussed before.

> >   suggests that automatically adding `Link:` is invariably wrong, since
> >   automatic != "hand selected", so maybe there's no point in
> >   `-l,--add-link`, and you can just remove that option.
>
> I think the discussion veered towards "don't call it Link:", not "don't link
> to it at all". Without "BugLink" in the running, the next winning option was
> "Archived-at:" and I am considering towards making -l add that trailer

Please don't add any new variants.

> instead, with the custom msgid.link domain, to further distinguish it from
> "hand selected" lore links:
>
>     Archived-at: https://msgid.link/20220617190153.tvi5lkzlvemeqou5@meerkat.local

... which breaks the system we've been using for a few years now.

> > - Don't reorder `Signed-off-by:`. The simplest thing to do would be to
> >   remove the `trailer-order` option all together. But some people do
> >   want to sort the trailers (within each S-o-b segment) for aesthetic
> >   purposes. So if you don't want to take that away from them, maybe
> >   S-o-b needs to be unconditionally special-cased so that it forms a
> >   reorder barrier.
>
> This is already the case for all trailers -- we *group* them by trailer name,
> but the *order* in which they are written is preserved. E.g. with the config
> option trailer-order="reported-by,reviewed-by,signed-off-by,*', we go from:
>
>     Signed-off-by: Ze Developer <ze@example.dev>
>     Reviewed-by: Some Rando <rando@example.dev>
>     Reviewed-by: Managing Rando <mgmt@example.dev>
>     Signed-off-by: Awesome Maintainer <aw@example.dev>
>     Reported-by: Bug Finder <bug@example.dev>
>
>
> to:
>
>     Reported-by: Bug Finder <bug@example.dev>
>     Reviewed-by: Some Rando <rando@example.dev>
>     Reviewed-by: Managing Rando <mgmt@example.dev>

IMHO this is wrong, as it no longer shows that the review happened
on the version submitted by Ze Developer.

>     Signed-off-by: Ze Developer <ze@example.dev>
>     Signed-off-by: Awesome Maintainer <aw@example.dev>
>
> Perhaps I should call the setting "trailer-grouping" instead of
> "trailer-order" (it's called that because we did reorder them alphabetically
> at some point in the distant past when the feature was first implemented).

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] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 19:43                     ` Geert Uytterhoeven
@ 2022-06-21 19:50                       ` Jason A. Donenfeld
  2022-06-21 20:06                         ` Konstantin Ryabitsev
  0 siblings, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-06-21 19:50 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Konstantin Ryabitsev, Linus Torvalds, tools, users

Hi Geert,

On Tue, Jun 21, 2022 at 09:43:41PM +0200, Geert Uytterhoeven wrote:
> I disagree: when bisecting a bug, I can just follow the Link: tag,
> and check if anyone already replied there with a newer review or
> bug report (and hopefully with a fix, too).
> Without the Link: tag, it's sometimes surprisingly difficult to find
> out where the patch has been posted and discussed before.

Linus spelled this out pretty clearly last week I thought:

https://lore.kernel.org/all/CAHk-=wgzRUT1fBpuz3xcN+YdsX0SxqOzHWRtj0ReHpUBb5TKbA@mail.gmail.com/
> Honestly, I still think that patch submission link should probably
> just not exist at all.
> 
> Are there other links that might be worthwhilte?
> 
> Yes: if people want to make patch submission give more information,
> then I think b4 should encourage taking the *COVER*LETTER* and make it
> into a merge commit, and at that point it's probably worth having the
> link to the patch series in that merge commit.
> 
> Because then maybe you get discussion about the whole series, which is
> more likely to be interesting and worthwhile.
> 
> There really is a big difference between "data" and "information". I
> think some per-patch link to a random patch submission that is just
> the same thing as the commit is worthless ("data"), but something
> higher-level may actually give you something useful ("information").

This seems spot on. So much so, that it used to be that when there was a
link in kernel commits, I'd middle click and read. But now it's usually
just crap that doesn't add anything, and so it wastes time and obscures
the good information when it actually is there, since I grow wary of
clicking. I spend a lot of time reading random commits, and this is an
annoyance.

On the contrary, the thing you want is a service where you paste a
commit and you get a LKML thread. That sounds like something lore could
grow, or any other service that has that database handy. And it could do
it *better* too, since it could show multiple matches sorted by
relevance and matching distance to the actual git patch. This will be so
much better than polluting the human-written git commit log with
automated stuff.

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 19:50                       ` Jason A. Donenfeld
@ 2022-06-21 20:06                         ` Konstantin Ryabitsev
  2022-06-21 20:29                           ` Jason A. Donenfeld
  0 siblings, 1 reply; 34+ messages in thread
From: Konstantin Ryabitsev @ 2022-06-21 20:06 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Geert Uytterhoeven, Linus Torvalds, tools, users

On Tue, Jun 21, 2022 at 09:50:18PM +0200, Jason A. Donenfeld wrote:
> On the contrary, the thing you want is a service where you paste a
> commit and you get a LKML thread. That sounds like something lore could
> grow, or any other service that has that database handy. And it could do
> it *better* too, since it could show multiple matches sorted by
> relevance and matching distance to the actual git patch.

FWIW, this is something we should be able to do on git.kernel.org soon -- a
commit view will add a link to look up that patch on lore. The backend changes
for public-inbox to add automatic indexing on "git-patch-id --stable" just
landed in the past few days. I just need to figure out if I can do this sanely
with cgit -- and if I cannot, then at least all the guilty parties are already
here to add the necessary bits in. :)

-K

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 20:06                         ` Konstantin Ryabitsev
@ 2022-06-21 20:29                           ` Jason A. Donenfeld
  0 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-06-21 20:29 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: Geert Uytterhoeven, Linus Torvalds, tools, users

Hi Konstantin,

On Tue, Jun 21, 2022 at 04:06:40PM -0400, Konstantin Ryabitsev wrote:
> On Tue, Jun 21, 2022 at 09:50:18PM +0200, Jason A. Donenfeld wrote:
> > On the contrary, the thing you want is a service where you paste a
> > commit and you get a LKML thread. That sounds like something lore could
> > grow, or any other service that has that database handy. And it could do
> > it *better* too, since it could show multiple matches sorted by
> > relevance and matching distance to the actual git patch.
> 
> FWIW, this is something we should be able to do on git.kernel.org soon -- a
> commit view will add a link to look up that patch on lore. The backend changes
> for public-inbox to add automatic indexing on "git-patch-id --stable" just
> landed in the past few days. I just need to figure out if I can do this sanely
> with cgit -- and if I cannot, then at least all the guilty parties are already
> here to add the necessary bits in. :)

Oh excellent. Keep me posted if you have trouble on the cgit side. I
could use a bit of new motivation to pick up some maintenance tasks
there, and this sounds like a good impetus.

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 18:45                 ` Jason A. Donenfeld
  2022-06-21 19:27                   ` Konstantin Ryabitsev
@ 2022-06-21 21:25                   ` Bjorn Andersson
  2022-06-23 23:33                     ` Theodore Ts'o
  2022-06-24  8:34                     ` Nicolas Ferre
  1 sibling, 2 replies; 34+ messages in thread
From: Bjorn Andersson @ 2022-06-21 21:25 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Konstantin Ryabitsev, Linus Torvalds, Geert Uytterhoeven, tools, users

On Tue 21 Jun 11:45 PDT 2022, Jason A. Donenfeld wrote:

> Hi Konstantin,
> 
> On Tue, Jun 21, 2022 at 02:29:53PM -0400, Konstantin Ryabitsev wrote:
> > If there is someone who is willing to compile a definitive list of "Linux
> > Kernel commit do's and dont's", then I'll happily stick to that.
> 
> I dunno about a "definitive list". But it seems like a lot of
> conventions get solidified by way of tools that are ironed out over
> time. For example, every time a code style discussion comes up, Joe
> Perches arrives on the scene to make sure checkpatch.pl is current. And
> now the clang-format file is starting to also accumulate collective
> preferences. It seems like b4 is pretty widely used and will eventually
> serve a similar purpose if it doesn't already do so.
> 
> Anyway, in lieu of a complete and thorough list of all of the things, it
> seems like in the last week or so we at least have two things that can
> be be reflected in the tooling:
> 
> - Don't add `Link:` if the URL hasn't been hand selected as being
>   particularly relevant; the lkml patch email alone doesn't cut it. This
>   suggests that automatically adding `Link:` is invariably wrong, since
>   automatic != "hand selected", so maybe there's no point in
>   `-l,--add-link`, and you can just remove that option.
> 

FWIW I find it very helpful to have the lore-Link in patches, as that
allow me to quickly find and share a pointer to already landed
patch/patchset - in particular when the recipient doesn't have
linux-next synced and checked out locally.

And I don't mind there being multiple Link: tags to different resources,
as the various automation use cases I've run into have been easily
solved by just filtering the tags by domain...

Regards,
Bjorn

> - Don't reorder `Signed-off-by:`. The simplest thing to do would be to
>   remove the `trailer-order` option all together. But some people do
>   want to sort the trailers (within each S-o-b segment) for aesthetic
>   purposes. So if you don't want to take that away from them, maybe
>   S-o-b needs to be unconditionally special-cased so that it forms a
>   reorder barrier.
> 
>   #define sob(name) do { smp_mb(); __sob(name); } while (0)
> 
> Jason
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-20  8:40 ` Geert Uytterhoeven
@ 2022-06-21 23:38   ` Linus Walleij
  2022-06-22  5:49     ` Vinod Koul
  0 siblings, 1 reply; 34+ messages in thread
From: Linus Walleij @ 2022-06-21 23:38 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Konstantin Ryabitsev, tools, users

On Mon, Jun 20, 2022 at 10:41 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:

> So if I want to run checkpatch, I have to keep on using "b4 am"
> and "git am" separately, right?  I never apply patches without running
> checkpatch first[1].
> Perhaps "b4 shazam" could gain some --checker option to support this?

This is what the DRM "dim" tool does as well, it runs checkpatch
and then tries to apply the patch regardless, but outputs any
complaints from checkpatch to the console.

This would be really helpful if b4 was run inside a kernel
directory (i.e. it can find scripts/checkpatch.pl).

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 23:38   ` Linus Walleij
@ 2022-06-22  5:49     ` Vinod Koul
  0 siblings, 0 replies; 34+ messages in thread
From: Vinod Koul @ 2022-06-22  5:49 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Geert Uytterhoeven, Konstantin Ryabitsev, tools, users

On 22-06-22, 01:38, Linus Walleij wrote:
> On Mon, Jun 20, 2022 at 10:41 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> 
> > So if I want to run checkpatch, I have to keep on using "b4 am"
> > and "git am" separately, right?  I never apply patches without running
> > checkpatch first[1].
> > Perhaps "b4 shazam" could gain some --checker option to support this?
> 
> This is what the DRM "dim" tool does as well, it runs checkpatch
> and then tries to apply the patch regardless, but outputs any
> complaints from checkpatch to the console.
> 
> This would be really helpful if b4 was run inside a kernel
> directory (i.e. it can find scripts/checkpatch.pl).

checkpatch should be run, that helps. Along with that I have a script
to check Fixes tag, so either that should be added or we add an option
for us to run our script...

-- 
~Vinod

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 21:25                   ` Bjorn Andersson
@ 2022-06-23 23:33                     ` Theodore Ts'o
  2022-06-24 13:51                       ` Jason Gunthorpe
  2022-06-24  8:34                     ` Nicolas Ferre
  1 sibling, 1 reply; 34+ messages in thread
From: Theodore Ts'o @ 2022-06-23 23:33 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Jason A. Donenfeld, Konstantin Ryabitsev, Linus Torvalds,
	Geert Uytterhoeven, tools, users

On Tue, Jun 21, 2022 at 02:25:47PM -0700, Bjorn Andersson wrote:
> On Tue 21 Jun 11:45 PDT 2022, Jason A. Donenfeld wrote:
> > - Don't add `Link:` if the URL hasn't been hand selected as being
> >   particularly relevant; the lkml patch email alone doesn't cut it. This
> >   suggests that automatically adding `Link:` is invariably wrong, since
> >   automatic != "hand selected", so maybe there's no point in
> >   `-l,--add-link`, and you can just remove that option.
> 
> FWIW I find it very helpful to have the lore-Link in patches, as that
> allow me to quickly find and share a pointer to already landed
> patch/patchset - in particular when the recipient doesn't have
> linux-next synced and checked out locally.

I'll also note that a number of maintainers (including myself) are
adding the Link to the lore via a git hook:

#!/bin/sh
# For .git/hooks/applypatch-msg
#
. git-sh-setup
perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
test -x "$GIT_DIR/hooks/commit-msg" &&
        exec "$GIT_DIR/hooks/commit-msg" ${1+"$@"}
:

This predates my use of b4, because it means that I can save a message
from my inbox, and then run "git am -s /tmp/patch-mbox" and the Link
line is added.  No one has really complained, and I've found it useful
on occasion when doing code archeology when I'm trying to figure out
why a patch was added, or to pick up discussion *about* the patch that
happened during the code review.

It's not perfect, especially for large patch series where there is
say, a v27 patch series before it finally gets accepted, unless the
cover letter of v27 has a URL link to all of the previous patch
series.  In some ways, this is why the Gerrit scheme of having a
ChangeId trailer is so helpful.  It's an invariant across different
versions of the patch, and it means that assuming the Gerrit server is
still up, you can see the entire code review history in a single web
page.

It would be ***great*** if the lore/b4/git workflow had a similar
functionality.  Perhaps that means some kind of tool to automate
generation of the cover letter, or hiding the previous URL link after
the --- line in the e-mailed version of the patch?

> And I don't mind there being multiple Link: tags to different resources,
> as the various automation use cases I've run into have been easily
> solved by just filtering the tags by domain...

I don't mind because when I *have* had to go back in time to do code
archeology ("what *was* I thinking two years ago...") having an easy
way to do that has been worth its weight in gold.

That being said, it isn't easy to distinguish between a Link to a bug
report that was submitted to a vger mailing list, and as I've
mentioned, there isn't a good way to find the previous versions of the
patch, since previous versions of the patch or patch series tend be on
separate e-mail threads, and some of the more critical code review
discussions happen on earler versions of the patch/patch series, not
the final version where it's mostly about fixing the final nits, as
opposed to the design/architectural discussions.

Cheers,

						- Ted

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-21 21:25                   ` Bjorn Andersson
  2022-06-23 23:33                     ` Theodore Ts'o
@ 2022-06-24  8:34                     ` Nicolas Ferre
  1 sibling, 0 replies; 34+ messages in thread
From: Nicolas Ferre @ 2022-06-24  8:34 UTC (permalink / raw)
  To: Bjorn Andersson, Jason A. Donenfeld, Konstantin Ryabitsev
  Cc: Linus Torvalds, Geert Uytterhoeven, tools, users

On 21/06/2022 at 23:25, Bjorn Andersson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue 21 Jun 11:45 PDT 2022, Jason A. Donenfeld wrote:
> 
>> Hi Konstantin,
>>
>> On Tue, Jun 21, 2022 at 02:29:53PM -0400, Konstantin Ryabitsev wrote:
>>> If there is someone who is willing to compile a definitive list of "Linux
>>> Kernel commit do's and dont's", then I'll happily stick to that.
>>
>> I dunno about a "definitive list". But it seems like a lot of
>> conventions get solidified by way of tools that are ironed out over
>> time. For example, every time a code style discussion comes up, Joe
>> Perches arrives on the scene to make sure checkpatch.pl is current. And
>> now the clang-format file is starting to also accumulate collective
>> preferences. It seems like b4 is pretty widely used and will eventually
>> serve a similar purpose if it doesn't already do so.
>>
>> Anyway, in lieu of a complete and thorough list of all of the things, it
>> seems like in the last week or so we at least have two things that can
>> be be reflected in the tooling:
>>
>> - Don't add `Link:` if the URL hasn't been hand selected as being
>>    particularly relevant; the lkml patch email alone doesn't cut it. This
>>    suggests that automatically adding `Link:` is invariably wrong, since
>>    automatic != "hand selected", so maybe there's no point in
>>    `-l,--add-link`, and you can just remove that option.

Please no...

> FWIW I find it very helpful to have the lore-Link in patches, as that
> allow me to quickly find and share a pointer to already landed
> patch/patchset - in particular when the recipient doesn't have
> linux-next synced and checked out locally.
> 
> And I don't mind there being multiple Link: tags to different resources,
> as the various automation use cases I've run into have been easily
> solved by just filtering the tags by domain...

+1

"Link: " addition has been very useful also for me several times when 
coming back to applied patches, even simple one carrying no real 
information at first sight.

Even the lack of discussion associated to a patch is sometimes a good 
indicator that the merged patch was not questioned or exercised enough, 
and we can easily realize this by following the "Link:" tag.

Not only it links to email thread at the time of patch merging, but as 
time passes, it could even link to a discussion that happened *after* 
that the patch had been merged. This discussion can address regressions, 
remarks or request for follow-up. As developers, *easily* following this 
"Link:" brings value in my opinion.

[..]

Best regards,
   Nicolas

-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-23 23:33                     ` Theodore Ts'o
@ 2022-06-24 13:51                       ` Jason Gunthorpe
  2022-06-24 15:29                         ` Theodore Ts'o
  0 siblings, 1 reply; 34+ messages in thread
From: Jason Gunthorpe @ 2022-06-24 13:51 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Bjorn Andersson, Jason A. Donenfeld, Konstantin Ryabitsev,
	Linus Torvalds, Geert Uytterhoeven, tools, users

On Thu, Jun 23, 2022 at 07:33:00PM -0400, Theodore Ts'o wrote:

> It's not perfect, especially for large patch series where there is
> say, a v27 patch series before it finally gets accepted, unless the
> cover letter of v27 has a URL link to all of the previous patch
> series.  

IMHO this is best-practice anyhow..

For example:

https://lore.kernel.org/kvm/20220224142024.147653-1-yishaih@nvidia.com/

Which is perhaps a strong case for including meaningful cover letters
like the above as a merge commit - but git needs some easier tooling
to show the cover when starting from a commit id: 'git show --cover
<comitish>'

With b4 having the range-diff functionality it also allows reviewers
an easier way to generate their own diffs between versions of the
series, and allows people splunking in the git history to discover the
entire conversation and all prior versions (Konstantin, it would be
nice if 'b4 diff' would take a pair of lore URLs)

Unfortunately it is a bit of a PITA for the submitter so not too many
people seem to do it.

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-24 13:51                       ` Jason Gunthorpe
@ 2022-06-24 15:29                         ` Theodore Ts'o
  2022-06-24 15:34                           ` Mark Brown
                                             ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Theodore Ts'o @ 2022-06-24 15:29 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Bjorn Andersson, Jason A. Donenfeld, Konstantin Ryabitsev,
	Linus Torvalds, Geert Uytterhoeven, tools, users

On Fri, Jun 24, 2022 at 10:51:47AM -0300, Jason Gunthorpe wrote:
> 
> Unfortunately it is a bit of a PITA for the submitter so not too many
> people seem to do it.

Yeah, "git notes" in newer versions of git makes it easier to compose
the cover letter, but tracking the lore URL's of the previous versions
of the patches that was submitted has to be done completely manually.
And because it's a PITA, very few people do it.

It would be great if we had some kind of tooling where when someone
sends a new version of the patch series, the tooling opened an editor
and allowed the submitted to enter a quick changelog of what changed
between the v25 and v26 version of the patch, with an option to also
edit the body of the cover letter.  And then then when the patch is
sent via e-mail, all of this would be appended to the cummulative
version of the cover letter.

I think if we had this, then it would *significantly* improve code
archeology for patch series.  Would it be enough that people would
tolerate a cover letter for one-off commits?  Probably not --- but the
same infrastructure could be used to keep track of the inter-version
change logs, which could then be included after the --- line which
delimits the end of the commit description, and that would solve the
problem for single patch or smaller patch series that tend not to have
cover letters today.

				- Ted

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-24 15:29                         ` Theodore Ts'o
@ 2022-06-24 15:34                           ` Mark Brown
  2022-06-24 16:05                             ` Theodore Ts'o
  2022-06-24 15:52                           ` Konstantin Ryabitsev
  2022-06-24 17:51                           ` Chuck Lever
  2 siblings, 1 reply; 34+ messages in thread
From: Mark Brown @ 2022-06-24 15:34 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Jason Gunthorpe, Bjorn Andersson, Jason A. Donenfeld,
	Konstantin Ryabitsev, Linus Torvalds, Geert Uytterhoeven, tools,
	users

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

On Fri, Jun 24, 2022 at 11:29:44AM -0400, Theodore Ts'o wrote:

> It would be great if we had some kind of tooling where when someone
> sends a new version of the patch series, the tooling opened an editor
> and allowed the submitted to enter a quick changelog of what changed
> between the v25 and v26 version of the patch, with an option to also
> edit the body of the cover letter.  And then then when the patch is
> sent via e-mail, all of this would be appended to the cummulative
> version of the cover letter.

I gather some people use patman for that already:

   https://gitlab.com/u-boot/u-boot/tree/master/tools/patman

(I've got my own thing which predates me knowing that patman exists).

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-24 15:29                         ` Theodore Ts'o
  2022-06-24 15:34                           ` Mark Brown
@ 2022-06-24 15:52                           ` Konstantin Ryabitsev
  2022-06-24 16:05                             ` James Bottomley
  2022-06-24 16:06                             ` Jason Gunthorpe
  2022-06-24 17:51                           ` Chuck Lever
  2 siblings, 2 replies; 34+ messages in thread
From: Konstantin Ryabitsev @ 2022-06-24 15:52 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Jason Gunthorpe, Bjorn Andersson, Jason A. Donenfeld,
	Linus Torvalds, Geert Uytterhoeven, tools, users

On Fri, Jun 24, 2022 at 11:29:44AM -0400, Theodore Ts'o wrote:
> It would be great if we had some kind of tooling where when someone
> sends a new version of the patch series, the tooling opened an editor
> and allowed the submitted to enter a quick changelog of what changed
> between the v25 and v26 version of the patch, with an option to also
> edit the body of the cover letter.  And then then when the patch is
> sent via e-mail, all of this would be appended to the cummulative
> version of the cover letter.

This feature is in-flight for "b4 submit". I wrote about it here:
https://lore.kernel.org/git/20211217183942.npvkb3ajnx6p5cbp@meerkat.local/

Here's the relevant part, slightly updated with the latest info:

1. b4 submit --new: this will create a new tracking branch and define
   some metadata to go with it, such as a cover letter template. The cover
   letter can be edited using `b4 submit --edit-cover` at any time.

2. b4 submit --send: will generate a patch series from any commits created
   from the topical branch fork point and use the cover letter from the
   previous step. It will be able to send the patches using the traditional
   SMTP way, OR it will allow using a web-based submission service that will
   eventually be set up at kernel.org:

   - submitters will be required to register their ed25519 patch attestation
     key with the submission endpoint and cryptographically sign their
     patches (email roundtrip confirmation required to register).
   - on the receiving end, the patches will be written to a dedicated
     lore.kernel.org feed *as-is*, but also sent to the recipients after doing
     the usual From/Reply-To substitution and moving the original From into
     the in-body git headers (this is required for DMARC validation).

3. b4 submit will properly include base-commit information to all submitted
   patches, as well as a unique change-id trailer (but in the basement of the
   cover letter, not in the commit message trailers as Gerrit does).

4. b4 submit --sync: will retrieve any received code review trailers
   (reviewed-by, acked-by, etc) and amend the corresponding commits in the
   topical branch, assuming we can match patch-id's (I've not tackled this
   yet, so I may be unduly optimistic here).

5. b4 submit --reroll: will prepare a v2 (v3, v4) of the series, reusing the
   same change-id trailer and adding a templated "Changes in v2" entry to the
   cover letter (that must be edited before --send works again).

6. b4 submit --send: will send the new version of the series.

I'm hoping that this will improve the experience of patch submitters *and*
help ensure that CI can be incorporated into the process by streamlining the
submission procedure and providing a public-inbox feed that can be easily
monitored. The important goal is to keep development fully decentralized so
that even if the web submission endpoint provided by kernel.org becomes
unavailable, plain old SMTP submission mechanisms can still be used as a
fallback.

-K

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-24 15:34                           ` Mark Brown
@ 2022-06-24 16:05                             ` Theodore Ts'o
  0 siblings, 0 replies; 34+ messages in thread
From: Theodore Ts'o @ 2022-06-24 16:05 UTC (permalink / raw)
  To: Mark Brown
  Cc: Jason Gunthorpe, Bjorn Andersson, Jason A. Donenfeld,
	Konstantin Ryabitsev, Linus Torvalds, Geert Uytterhoeven, tools,
	users

On Fri, Jun 24, 2022 at 04:34:50PM +0100, Mark Brown wrote:
> 
> I gather some people use patman for that already:
> 
>    https://gitlab.com/u-boot/u-boot/tree/master/tools/patman
> 
> (I've got my own thing which predates me knowing that patman exists).

Oh wow, cool, this looks nice!  Thanks for pointing this out!

I see it has patchwork integration, although apparently it doesn't
have automated handling for lore.kernel.org links (at least, not yet).

					- Ted



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-24 15:52                           ` Konstantin Ryabitsev
@ 2022-06-24 16:05                             ` James Bottomley
  2022-06-24 16:16                               ` Konstantin Ryabitsev
  2022-06-24 16:06                             ` Jason Gunthorpe
  1 sibling, 1 reply; 34+ messages in thread
From: James Bottomley @ 2022-06-24 16:05 UTC (permalink / raw)
  To: Konstantin Ryabitsev, Theodore Ts'o
  Cc: Jason Gunthorpe, Bjorn Andersson, Jason A. Donenfeld,
	Linus Torvalds, Geert Uytterhoeven, tools, users

On Fri, 2022-06-24 at 11:52 -0400, Konstantin Ryabitsev wrote:
>    - submitters will be required to register their ed25519 patch
> attestation key with the submission endpoint and cryptographically
> sign their patches (email roundtrip confirmation required to
> register).

Can we please be less pejorative about crypto choices?  ed25519 is a
fine curve, but lots of crypto HSMs which do elliptic curve don't do
Edwards signatures because they're very different from standard ECDSA
signatures.  Those of us who take security seriously tend to use HSMs
for all our private keys and thus won't do ed25519 if the HSM we use
can't do it.

I was under the impression b4 used gpg on the backend, which can do a
variety of elliptic curves and now, bonus, can use a laptop TPM as the
HSM to protect the keys (which means pretty much everyone has no excuse
not to do hardware key protection)? (TPMs are one of the crypto devices
that can't do Edwards signatures).

Regards,

James



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-24 15:52                           ` Konstantin Ryabitsev
  2022-06-24 16:05                             ` James Bottomley
@ 2022-06-24 16:06                             ` Jason Gunthorpe
  2022-06-24 16:16                               ` Mark Brown
  1 sibling, 1 reply; 34+ messages in thread
From: Jason Gunthorpe @ 2022-06-24 16:06 UTC (permalink / raw)
  To: Konstantin Ryabitsev
  Cc: Theodore Ts'o, Bjorn Andersson, Jason A. Donenfeld,
	Linus Torvalds, Geert Uytterhoeven, tools, users

On Fri, Jun 24, 2022 at 11:52:13AM -0400, Konstantin Ryabitsev wrote:
> On Fri, Jun 24, 2022 at 11:29:44AM -0400, Theodore Ts'o wrote:
> > It would be great if we had some kind of tooling where when someone
> > sends a new version of the patch series, the tooling opened an editor
> > and allowed the submitted to enter a quick changelog of what changed
> > between the v25 and v26 version of the patch, with an option to also
> > edit the body of the cover letter.  And then then when the patch is
> > sent via e-mail, all of this would be appended to the cummulative
> > version of the cover letter.
> 
> This feature is in-flight for "b4 submit". I wrote about it here:
> https://lore.kernel.org/git/20211217183942.npvkb3ajnx6p5cbp@meerkat.local/
> 
> Here's the relevant part, slightly updated with the latest info:
> 
> 1. b4 submit --new: this will create a new tracking branch and define
>    some metadata to go with it, such as a cover letter template. The cover
>    letter can be edited using `b4 submit --edit-cover` at any time.

It would be nice if this could store the cover letter in an empty git
commit at the top of the branch.

At least here we always require cover letters to be reviewed (eg in
gerrit/gitlab/whatever) along with patches and having them stored as
notes or in the branch description that is not git pushable is not so
useful.

> 2. b4 submit --send: will generate a patch series from any commits created
>    from the topical branch fork point and use the cover letter from the
>    previous step. It will be able to send the patches using the traditional
>    SMTP way, OR it will allow using a web-based submission service that will
>    eventually be set up at kernel.org:

In my tooling I build a git commit that contains the raw final emails
sent, the message-id's that were used, a link to lore, and the git
commit range that bounds what was sent, then log this commit as an
dated archival branch so I can have a good idea of where everything is
and recover back to the exact git commits sent if I need to. Sort of
an enhanced 'reflog'.

I find it is really important be able to go back to the lore link of
the historical version(s) easially.

> 4. b4 submit --sync: will retrieve any received code review trailers
>    (reviewed-by, acked-by, etc) and amend the corresponding commits in the
>    topical branch, assuming we can match patch-id's (I've not tackled this
>    yet, so I may be unduly optimistic here).

Yay, I hope this is generally usable :)
 
> 5. b4 submit --reroll: will prepare a v2 (v3, v4) of the series, reusing the
>    same change-id trailer and adding a templated "Changes in v2" entry to the
>    cover letter (that must be edited before --send works again).

With an automatic lore url to the prior version?

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-24 16:06                             ` Jason Gunthorpe
@ 2022-06-24 16:16                               ` Mark Brown
  0 siblings, 0 replies; 34+ messages in thread
From: Mark Brown @ 2022-06-24 16:16 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Konstantin Ryabitsev, Theodore Ts'o, Bjorn Andersson,
	Jason A. Donenfeld, Linus Torvalds, Geert Uytterhoeven, tools,
	users

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

On Fri, Jun 24, 2022 at 01:06:28PM -0300, Jason Gunthorpe wrote:
> On Fri, Jun 24, 2022 at 11:52:13AM -0400, Konstantin Ryabitsev wrote:

> > 1. b4 submit --new: this will create a new tracking branch and define
> >    some metadata to go with it, such as a cover letter template. The cover
> >    letter can be edited using `b4 submit --edit-cover` at any time.

> It would be nice if this could store the cover letter in an empty git
> commit at the top of the branch.

> At least here we always require cover letters to be reviewed (eg in
> gerrit/gitlab/whatever) along with patches and having them stored as
> notes or in the branch description that is not git pushable is not so
> useful.

Or at least something that's easy to move between machines (eg, desktop
and laptop).

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-24 16:05                             ` James Bottomley
@ 2022-06-24 16:16                               ` Konstantin Ryabitsev
  2022-06-24 16:29                                 ` James Bottomley
  0 siblings, 1 reply; 34+ messages in thread
From: Konstantin Ryabitsev @ 2022-06-24 16:16 UTC (permalink / raw)
  To: James Bottomley
  Cc: Theodore Ts'o, Jason Gunthorpe, Bjorn Andersson,
	Jason A. Donenfeld, Linus Torvalds, Geert Uytterhoeven, tools,
	users

On Fri, Jun 24, 2022 at 12:05:35PM -0400, James Bottomley wrote:
> On Fri, 2022-06-24 at 11:52 -0400, Konstantin Ryabitsev wrote:
> >    - submitters will be required to register their ed25519 patch
> > attestation key with the submission endpoint and cryptographically
> > sign their patches (email roundtrip confirmation required to
> > register).
> 
> Can we please be less pejorative about crypto choices?  ed25519 is a
> fine curve, but lots of crypto HSMs which do elliptic curve don't do
> Edwards signatures because they're very different from standard ECDSA
> signatures.  Those of us who take security seriously tend to use HSMs
> for all our private keys and thus won't do ed25519 if the HSM we use
> can't do it.

This will be only required on the web submission endpoint, and only to begin
with. Shelling out to call gpg or openssh from a web process is a lot more
hairy than checking ed25519 signatures using PyNaCl, which is the only
reason behind this restriction.

> I was under the impression b4 used gpg on the backend, which can do a
> variety of elliptic curves and now, bonus, can use a laptop TPM as the
> HSM to protect the keys (which means pretty much everyone has no excuse
> not to do hardware key protection)? (TPMs are one of the crypto devices
> that can't do Edwards signatures).

If you're not planning to use the web submission endpoint (and I expect that
anyone already using git-send-email will not be interested in the web
endpoint), you're certainly free to continue to use gpg.

-K

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-24 16:16                               ` Konstantin Ryabitsev
@ 2022-06-24 16:29                                 ` James Bottomley
  0 siblings, 0 replies; 34+ messages in thread
From: James Bottomley @ 2022-06-24 16:29 UTC (permalink / raw)
  To: Konstantin Ryabitsev
  Cc: Theodore Ts'o, Jason Gunthorpe, Bjorn Andersson,
	Jason A. Donenfeld, Linus Torvalds, Geert Uytterhoeven, tools,
	users

On Fri, 2022-06-24 at 12:16 -0400, Konstantin Ryabitsev wrote:
> On Fri, Jun 24, 2022 at 12:05:35PM -0400, James Bottomley wrote:
> > On Fri, 2022-06-24 at 11:52 -0400, Konstantin Ryabitsev wrote:
> > >    - submitters will be required to register their ed25519 patch
> > > attestation key with the submission endpoint and
> > > cryptographically sign their patches (email roundtrip
> > > confirmation required to register).
> > 
> > Can we please be less pejorative about crypto choices?  ed25519 is
> > a fine curve, but lots of crypto HSMs which do elliptic curve don't
> > do Edwards signatures because they're very different from standard
> > ECDSA signatures.  Those of us who take security seriously tend to
> > use HSMs for all our private keys and thus won't do ed25519 if the
> > HSM we use can't do it.
> 
> This will be only required on the web submission endpoint, and only
> to begin with. Shelling out to call gpg or openssh from a web process
> is a lot more hairy than checking ed25519 signatures using PyNaCl,
> which is the only reason behind this restriction.

Sure, I won't use it, but it would be really nice to encourage
newcomers who do use it to also be more mindful of cryptographic
security ... particularly when this is a long lived identity token.

Using overly opinionated crypto libraries is inimical to general
security for two reasons: firstly because the crypto they mandate might
not be available to tokens and secondly (and NaCl is guilty of this)
because doing token support is hard so they mostly don't bother.  It
always amazes me how otherwise perfectly reasonable crypto people have
a plank in their own eye about this.

James



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: b4-0.9.0 available
  2022-06-24 15:29                         ` Theodore Ts'o
  2022-06-24 15:34                           ` Mark Brown
  2022-06-24 15:52                           ` Konstantin Ryabitsev
@ 2022-06-24 17:51                           ` Chuck Lever
  2 siblings, 0 replies; 34+ messages in thread
From: Chuck Lever @ 2022-06-24 17:51 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Jason Gunthorpe, Bjorn Andersson, Jason A. Donenfeld,
	Konstantin Ryabitsev, Linus Torvalds, Geert Uytterhoeven, tools,
	users

On Fri, Jun 24, 2022 at 11:30 AM Theodore Ts'o <tytso@mit.edu> wrote:
>
> On Fri, Jun 24, 2022 at 10:51:47AM -0300, Jason Gunthorpe wrote:
> >
> > Unfortunately it is a bit of a PITA for the submitter so not too many
> > people seem to do it.
>
> Yeah, "git notes" in newer versions of git makes it easier to compose
> the cover letter, but tracking the lore URL's of the previous versions
> of the patches that was submitted has to be done completely manually.
> And because it's a PITA, very few people do it.
>
> It would be great if we had some kind of tooling where when someone
> sends a new version of the patch series, the tooling opened an editor
> and allowed the submitted to enter a quick changelog of what changed
> between the v25 and v26 version of the patch, with an option to also
> edit the body of the cover letter.  And then then when the patch is
> sent via e-mail, all of this would be appended to the cummulative
> version of the cover letter.

Interestingly, this kind of per-branch cover letter tracking has also
recently been requested for Stacked Git.

https://github.com/stacked-git/stgit/discussions/186

It would be great if the basic approach could be the same for git,
stgit, and b4.

-- 
"We cannot take our next breath without the exhale."
 -- Ellen Scott Grable

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2022-06-24 17:52 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17 19:01 b4-0.9.0 available Konstantin Ryabitsev
2022-06-20  8:40 ` Geert Uytterhoeven
2022-06-21 23:38   ` Linus Walleij
2022-06-22  5:49     ` Vinod Koul
2022-06-21 15:20 ` Geert Uytterhoeven
2022-06-21 15:29   ` Konstantin Ryabitsev
2022-06-21 15:41     ` Geert Uytterhoeven
2022-06-21 15:55       ` Linus Torvalds
2022-06-21 16:03         ` Jason A. Donenfeld
2022-06-21 16:59           ` Konstantin Ryabitsev
2022-06-21 17:49             ` Jason A. Donenfeld
2022-06-21 18:29               ` Konstantin Ryabitsev
2022-06-21 18:45                 ` Jason A. Donenfeld
2022-06-21 19:27                   ` Konstantin Ryabitsev
2022-06-21 19:42                     ` Jason A. Donenfeld
2022-06-21 19:43                     ` Geert Uytterhoeven
2022-06-21 19:50                       ` Jason A. Donenfeld
2022-06-21 20:06                         ` Konstantin Ryabitsev
2022-06-21 20:29                           ` Jason A. Donenfeld
2022-06-21 21:25                   ` Bjorn Andersson
2022-06-23 23:33                     ` Theodore Ts'o
2022-06-24 13:51                       ` Jason Gunthorpe
2022-06-24 15:29                         ` Theodore Ts'o
2022-06-24 15:34                           ` Mark Brown
2022-06-24 16:05                             ` Theodore Ts'o
2022-06-24 15:52                           ` Konstantin Ryabitsev
2022-06-24 16:05                             ` James Bottomley
2022-06-24 16:16                               ` Konstantin Ryabitsev
2022-06-24 16:29                                 ` James Bottomley
2022-06-24 16:06                             ` Jason Gunthorpe
2022-06-24 16:16                               ` Mark Brown
2022-06-24 17:51                           ` Chuck Lever
2022-06-24  8:34                     ` Nicolas Ferre
2022-06-21 15:53     ` Jason A. Donenfeld

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.