* 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-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-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: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-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-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: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: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: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 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: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 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
* 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-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
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.