b4 submit ready for beta testing

b4 submit ready for beta testing

[Konstantin Ryabitsev]

Hello, all:

I've committed the initial b4-submit features to the master branch, and I
need some volunteers to test it. If you routinely send patches to the Linux
kernel mailing lists, then I hope that volunteer is you. :)

## What it does

The goal behind "b4 submit" is simplifying the submitter's workflow by
automating away a lot of menial tasks, such as creating cover letters,
collecting trailers, rerolling new revisions, remembering all the flags to
git-format-patch and git-send-email, etc. The main workflow can be described
like this:

1. b4 submit --new my-new-feature [-f v5.19-rc6]

This will create a new branch called "b4/my-new-feature". If -f is not
specified, the branch will be created from the current HEAD (but this is
rarely what you want, so you'll want to pick a tag to work from).

After the branch is created, b4 will create an empty commit that will serve to
store the cover letter contents and some tracking information. You can view
the raw commit with "git log" just like any other commit, and you can even
edit it with "git rebase" but it's not recommended. It's much better to use
"b4 submit --edit-cover".

2. b4 submit --edit-cover

This allows you to work on the cover letter using $EDITOR or core.editor
as defined in git-config. Once the editing is complete, the empty commit will
be updated and the entire branch rebased using git-filter-repo.

3. use regular git operations for the rest of your work

You can apply, rebase, edit, squash, etc -- just avoid touching the cover
letter commit. You can also push the b4/my-new-feature branch to any remote
and pull it from anywhere else if you want to switch computers.

4. b4 submit --send

When you are ready to send off your series, you can review the resulting
patches by running:

  b4 submit --send --dry-run

or

  b4 submit --send -o /tmp/somedir

The first will print out the patches to send on stdout, exactly as they would
be fed to the SMTP server, and the second will output the patches into the
specified directory a-la git-format-patch -- useful for running "checkpatch"
or any other checks, as necessary.

When you are satisfied that the patches are looking sane, you can fire off the
actual --send command.

NOTE: when we find a scripts/get_maintainer.pl in the repo toplevel, we will
automatically run the most-recommended (that I could find) combination of
flags to automatically populate the To: and Cc: list in addition to whatever
we get out of the cover letter and commit trailers. You can turn this off with
--no-auto-to-cc.

NOTE: at this time, a working SMTP server and a valid [sendemail] section are
required for sending patches. The web submission endpoint work is not yet
completed.

5. b4 submit --reroll

After you send your series and when you're ready to start working on a new
revision, you'll want to --reroll to increment your tracked version. This will
automatically update the cover letter commit to add a "Changes in v[N]"
section that will also contain a link to the v[N-1] series, creating a
historical breadcrumb of revisions.

6. b4 submit --update-trailers [--signoff]

As you receive feedback on your patches, you can retrieve any received
trailers from lore.kernel.org and apply them directly to your branch. This is
accomplished by retrieving any threads matching the unique "change-id"
submitted as part of the cover letter.

One thing to keep in mind here is that actual trailer matching is done by
comparing the patch-id (see git-patch-id for details). This has the following
important bonuses and caveats:

  - You can rearrange patches or modify summaries without it affecting the
    match.
  - If you modify actual code (even as much as fixing a typo in a string), any
    received trailers will no longer match. This is as designed, because if
    you change the code, it's no longer reviewed (but we won't remove any
    existing trailers that were already applied).
  - when you use --signoff, this will move your own "Signed-off-by" trailer
    below any other trailers in all series commits, which is commonly
    recommended for the chain-of-custody purposes.

7. b4 submit --send

GOTO 4.

In addition to the above, please see "b4 submit --help" for other flags you
can use with these operations.

### Editing the cover letter commit manually

If you ever need to edit the cover letter commit manually (e.g. to fix
something about the tracking section json), you can use regular "git rebase
-i" with the "reword" action. When rebase bails complaining about the empty
commit, just run "git commit --amend --allow-empty" to edit the cover letter
and then "git rebase --continue".

Again, this is not recommended and it's best to always use "b4 submit
--edit-cover".

## Getting the b4 dev version

To install the b4 development version, you will need to run it from the git
checkout:

  git clone https://git.kernel.org/pub/scm/utils/b4/b4.git
  cd b4
  git submodule update --init
  alias b4="$PWD/b4.sh"

You can find out more details in the README file. You will also want to make
sure that git-filter-repo is installed (either from your distro packages or
from pip).

If you're already running b4 from the git checkout, make sure you run "git
submodule update" to pull in the latest unreleased patatt version.

## Feedback

I hope you have a positive experience using b4 submit. I will be happy to
receive any feedback, as I have to make a lot of assumptions and they aren't
always the right ones. :) The best place for this feedback is on the
tools@linux.kernel.org list.

Best regards,
Konstantin

Re: b4 submit ready for beta testing

[James Bottomley]

On Sat, 2022-07-16 at 10:29 -0400, Konstantin Ryabitsev wrote:
> After the branch is created, b4 will create an empty commit that will
> serve to store the cover letter contents and some tracking
> information. You can view the raw commit with "git log" just like any
> other commit, and you can even edit it with "git rebase" but it's not
> recommended. It's much better to use "b4 submit --edit-cover".

Could we please stop inventing stuff that's incompatible with a
standard git workflow ... or argue with the git maintainers that what
we want to do is better and persuade them to add it?  That way git and
b4 are fully interoperable ... for those of us who aren't dedicated to
one or the other.

The standard git way of storing cover letters isn't in an empty commit,
it's in the branch description.  To be honest, this does seem a more
logical way to do it (a commit can get lost within the branch), but I'd
be reasonably OK with either mechanism, provided both git and b4 agree
on it.

Regards,

James

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Sat, Jul 16, 2022 at 10:43:17AM -0400, James Bottomley wrote:
> On Sat, 2022-07-16 at 10:29 -0400, Konstantin Ryabitsev wrote:
> > After the branch is created, b4 will create an empty commit that will
> > serve to store the cover letter contents and some tracking
> > information. You can view the raw commit with "git log" just like any
> > other commit, and you can even edit it with "git rebase" but it's not
> > recommended. It's much better to use "b4 submit --edit-cover".
> 
> Could we please stop inventing stuff that's incompatible with a
> standard git workflow ... or argue with the git maintainers that what
> we want to do is better and persuade them to add it?  That way git and
> b4 are fully interoperable ... for those of us who aren't dedicated to
> one or the other.
> 
> The standard git way of storing cover letters isn't in an empty commit,
> it's in the branch description.  To be honest, this does seem a more
> logical way to do it (a commit can get lost within the branch), but I'd
> be reasonably OK with either mechanism, provided both git and b4 agree
> on it.

I hear you, but branch descriptions are confined to your local repository and
can't be pushed to a remote. So, if you work on a series, send it off, and
then pick it up again two weeks later from a different workstation, that's no
good for most of what b4 submit needs to do. :(

So, it was either using notes (which almost everyone forgets to push anyway),
or using an empty commit. I agree it's hurky, but it currently the only thing
that fits in the "least likely to end up lost on a rebase or workstation
switch" category.

-K

Re: b4 submit ready for beta testing

[Junio C Hamano]

I thought that on the receiving end you implemented a feature where
the cover letter material is kept and used as the log message for the
commit that merges the topic branch into whatever integration branch
it is meant to go in? Does it make sense to make this feature a (sort
of) mirror image of that feature? I haven't formed an expected
end-user interaction to work with that shape of the history on the
authoring end in my head yet, but as the end-goal to aim at, the
"individual commit on the topic forms a strand of pearls on a topic
branch, and there is a cover letter that marks its end at the end,
merging the topic back to the upstream" shape of the history looks
like a more appropriate form which (if I remember correctly) you
already invented and support.

On Sat, Jul 16, 2022 at 7:57 AM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> On Sat, Jul 16, 2022 at 10:43:17AM -0400, James Bottomley wrote:
> > On Sat, 2022-07-16 at 10:29 -0400, Konstantin Ryabitsev wrote:
> > > After the branch is created, b4 will create an empty commit that will
> > > serve to store the cover letter contents and some tracking
> > > information. You can view the raw commit with "git log" just like any
> > > other commit, and you can even edit it with "git rebase" but it's not
> > > recommended. It's much better to use "b4 submit --edit-cover".
> >
> > Could we please stop inventing stuff that's incompatible with a
> > standard git workflow ... or argue with the git maintainers that what
> > we want to do is better and persuade them to add it?  That way git and
> > b4 are fully interoperable ... for those of us who aren't dedicated to
> > one or the other.
> >
> > The standard git way of storing cover letters isn't in an empty commit,
> > it's in the branch description.  To be honest, this does seem a more
> > logical way to do it (a commit can get lost within the branch), but I'd
> > be reasonably OK with either mechanism, provided both git and b4 agree
> > on it.
>
> I hear you, but branch descriptions are confined to your local repository and
> can't be pushed to a remote. So, if you work on a series, send it off, and
> then pick it up again two weeks later from a different workstation, that's no
> good for most of what b4 submit needs to do. :(
>
> So, it was either using notes (which almost everyone forgets to push anyway),
> or using an empty commit. I agree it's hurky, but it currently the only thing
> that fits in the "least likely to end up lost on a rebase or workstation
> switch" category.
>
> -K
>

Re: b4 submit ready for beta testing

[James Bottomley]

On Sat, 2022-07-16 at 10:56 -0400, Konstantin Ryabitsev wrote:
[...]
> I hear you, but branch descriptions are confined to your local
> repository and can't be pushed to a remote.

Yes, I accept this, but:

>  So, if you work on a series, send it off, and then pick it up again
> two weeks later from a different workstation, that's no good for most
> of what b4 submit needs to do. :(

Are you so sure that's the common case.  For me, to connect to an SMTP
server, I need to be on a system running my VPN, which is only my
laptop.  Thus however much I transfer the series around for testing
etc., it always comes back to the same system for submission, so
keeping the cover letter in the branch description of that system works
just fine.

The point is you're introducing a divergence between b4 and git for
this ... are you so sure most developers submit from arbitrary systems
and that's the use case we need to optimize for even at the expense of
the incompatibility?

> So, it was either using notes (which almost everyone forgets to push
> anyway), or using an empty commit. I agree it's hurky, but it
> currently the only thing that fits in the "least likely to end up
> lost on a rebase or workstation switch" category.

I get what problem you're solving, but it isn't my problem and is never
likely to be because I'm not going to increase my attack surface by
running a VPN on anything but a single system where the TPM that holds
my cryptographic keys is.

James

Re: b4 submit ready for beta testing

[Conor Dooley]

On 16/07/2022 17:55, James Bottomley wrote:
> On Sat, 2022-07-16 at 10:56 -0400, Konstantin Ryabitsev wrote:
> [...]
>> I hear you, but branch descriptions are confined to your local
>> repository and can't be pushed to a remote.
> 
> 
>> So, it was either using notes (which almost everyone forgets to push
>> anyway), or using an empty commit. I agree it's hurky, but it
>> currently the only thing that fits in the "least likely to end up
>> lost on a rebase or workstation switch" category.


Is it silly to suggest that b4 could handle the a notes to
description & description to notes conversion in a "b4 push-series"
or "b4 fetch-series" type of command? 

I don't think it is much of a stretch to assume someone that is
going the whole b4 hog would be afraid of using a wrapper for git
push/fetch.

Thanks,
Conor.

Re: b4 submit ready for beta testing

[Geert Uytterhoeven]

Hi Konstantin,

On Sat, Jul 16, 2022 at 4:56 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
> On Sat, Jul 16, 2022 at 10:43:17AM -0400, James Bottomley wrote:
> > On Sat, 2022-07-16 at 10:29 -0400, Konstantin Ryabitsev wrote:
> > > After the branch is created, b4 will create an empty commit that will
> > > serve to store the cover letter contents and some tracking
> > > information. You can view the raw commit with "git log" just like any
> > > other commit, and you can even edit it with "git rebase" but it's not
> > > recommended. It's much better to use "b4 submit --edit-cover".
> >
> > Could we please stop inventing stuff that's incompatible with a
> > standard git workflow ... or argue with the git maintainers that what
> > we want to do is better and persuade them to add it?  That way git and
> > b4 are fully interoperable ... for those of us who aren't dedicated to
> > one or the other.
> >
> > The standard git way of storing cover letters isn't in an empty commit,
> > it's in the branch description.  To be honest, this does seem a more
> > logical way to do it (a commit can get lost within the branch), but I'd
> > be reasonably OK with either mechanism, provided both git and b4 agree
> > on it.
>
> I hear you, but branch descriptions are confined to your local repository and
> can't be pushed to a remote. So, if you work on a series, send it off, and
> then pick it up again two weeks later from a different workstation, that's no
> good for most of what b4 submit needs to do. :(
>
> So, it was either using notes (which almost everyone forgets to push anyway),
> or using an empty commit. I agree it's hurky, but it currently the only thing
> that fits in the "least likely to end up lost on a rebase or workstation
> switch" category.

Empty commits are very likely to be lost on a rebase, as the default is
to drop them while doing an interactive rebase (unless --keep-empty).

My local development trees have lots of visual separators which are
commits that add dummy files, to avoid losing these commits during
rebases.  I have a script to create such separators, but it's been
a while since I used it, as the separators are recyclable.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Sun, Jul 17, 2022 at 11:58:08AM +0200, Geert Uytterhoeven wrote:
> Empty commits are very likely to be lost on a rebase, as the default is
> to drop them while doing an interactive rebase (unless --keep-empty).

Geert:

I think this is no longer the case with the later git versions that use the
merge backend for rebase. I just tested "rebase -i" and the cover letter
commit was successfully kept without any extra flags being required.

-K

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Sat, Jul 16, 2022 at 06:14:37PM +0100, Conor Dooley wrote:
> >> So, it was either using notes (which almost everyone forgets to push
> >> anyway), or using an empty commit. I agree it's hurky, but it
> >> currently the only thing that fits in the "least likely to end up
> >> lost on a rebase or workstation switch" category.
> 
> 
> Is it silly to suggest that b4 could handle the a notes to
> description & description to notes conversion in a "b4 push-series"
> or "b4 fetch-series" type of command? 

It's not silly, but it's significantly more fragile. Pushing and fetching
requires reachable remotes and some kind of ability to handle conflicts, keys,
etc. I'd much rather avoid going down that road.

-K

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Sat, Jul 16, 2022 at 12:55:26PM -0400, James Bottomley wrote:
> >  So, if you work on a series, send it off, and then pick it up again
> > two weeks later from a different workstation, that's no good for most
> > of what b4 submit needs to do. :(
> 
> Are you so sure that's the common case.  For me, to connect to an SMTP
> server, I need to be on a system running my VPN, which is only my
> laptop.  Thus however much I transfer the series around for testing
> etc., it always comes back to the same system for submission, so
> keeping the cover letter in the branch description of that system works
> just fine.

Perhaps, but I also have other reasons to like using an empty commit for this.
For example, it makes it very easy to mark where exactly our series starts.
It's possible to do this with other tricks like tracking merge-base, using
tags, etc, but they are a lot more complicated and have other caveats (e.g.
when using merge-base tricks you have to also track the branch you forked
from, and when using tags you have to also consider that someone may push the
branch but not push the tags).

> The point is you're introducing a divergence between b4 and git for
> this ... are you so sure most developers submit from arbitrary systems
> and that's the use case we need to optimize for even at the expense of
> the incompatibility?

Well, at least two people on this list specifically asked to use empty commits
for cover letters specifically because they want to be able to easily push
their work to a remote either as an easy means of backup or to work on it from
a different system in the future.

> > So, it was either using notes (which almost everyone forgets to push
> > anyway), or using an empty commit. I agree it's hurky, but it
> > currently the only thing that fits in the "least likely to end up
> > lost on a rebase or workstation switch" category.
> 
> I get what problem you're solving, but it isn't my problem and is never
> likely to be because I'm not going to increase my attack surface by
> running a VPN on anything but a single system where the TPM that holds
> my cryptographic keys is.

The upside of "b4 submit" is that it doesn't replace any schemes that already
work for others -- if you have compelling reasons not to use it, then you can
just ignore its existence.

The goal is to make life easier for occasional contributors, plus also make it
suitable for the subset of developers who find that it works for them as-is.

-K

Re: b4 submit ready for beta testing

[Maxime Ripard]

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

Hi Konstantin,

Thanks a lot for working on this!

On Sat, Jul 16, 2022 at 10:29:54AM -0400, Konstantin Ryabitsev wrote:
> Hello, all:
> 
> I've committed the initial b4-submit features to the master branch, and I
> need some volunteers to test it. If you routinely send patches to the Linux
> kernel mailing lists, then I hope that volunteer is you. :)
> 
> ## What it does
> 
> The goal behind "b4 submit" is simplifying the submitter's workflow by
> automating away a lot of menial tasks, such as creating cover letters,
> collecting trailers, rerolling new revisions, remembering all the flags to
> git-format-patch and git-send-email, etc. The main workflow can be described
> like this:
> 
> 1. b4 submit --new my-new-feature [-f v5.19-rc6]
> 
> This will create a new branch called "b4/my-new-feature". If -f is not
> specified, the branch will be created from the current HEAD (but this is
> rarely what you want, so you'll want to pick a tag to work from).
> 
> After the branch is created, b4 will create an empty commit that will serve to
> store the cover letter contents and some tracking information. You can view
> the raw commit with "git log" just like any other commit, and you can even
> edit it with "git rebase" but it's not recommended. It's much better to use
> "b4 submit --edit-cover".

So, I've been using a lot git-publish since a couple of years
(https://github.com/stefanha/git-publish), and this creates arbitrary
constraints that aren't great from a usability point of view compared to
git-publish:

  - The fact that you need to create a branch beforehand prevents to
    easily use it for one-off series. I'm sure it's doable, but it's not
    clear to me why you would want a special branch in the first
    place...

  - ... Apart from that special commit. I'm not sure it's great either
    and am almost certain I'm going to screw up rebases all the time,
    dropping that special commit.

git-publish solves kind of both by storing the cover letter content and
it's metadata into a tag, one for each version sent (with the pattern
$branchname-$version), and a -staging one for the next version that is
going to be send.

This also allows to easily rollback to a previous series version and
it's transparent to others whether you're using it or not, the history
is clean. You can even send the PR directly from the branch if you want
to.

> 2. b4 submit --edit-cover
> 
> This allows you to work on the cover letter using $EDITOR or core.editor
> as defined in git-config. Once the editing is complete, the empty commit will
> be updated and the entire branch rebased using git-filter-repo.
> 
> 3. use regular git operations for the rest of your work
> 
> You can apply, rebase, edit, squash, etc -- just avoid touching the cover
> letter commit. You can also push the b4/my-new-feature branch to any remote
> and pull it from anywhere else if you want to switch computers.
> 
> 4. b4 submit --send
> 
> When you are ready to send off your series, you can review the resulting
> patches by running:
> 
>   b4 submit --send --dry-run
> 
> or
> 
>   b4 submit --send -o /tmp/somedir
> 
> The first will print out the patches to send on stdout, exactly as they would
> be fed to the SMTP server, and the second will output the patches into the
> specified directory a-la git-format-patch -- useful for running "checkpatch"
> or any other checks, as necessary.
> 
> When you are satisfied that the patches are looking sane, you can fire off the
> actual --send command.
> 
> NOTE: when we find a scripts/get_maintainer.pl in the repo toplevel, we will
> automatically run the most-recommended (that I could find) combination of
> flags to automatically populate the To: and Cc: list in addition to whatever
> we get out of the cover letter and commit trailers. You can turn this off with
> --no-auto-to-cc.
> 
> NOTE: at this time, a working SMTP server and a valid [sendemail] section are
> required for sending patches. The web submission endpoint work is not yet
> completed.
> 
> 5. b4 submit --reroll
> 
> After you send your series and when you're ready to start working on a new
> revision, you'll want to --reroll to increment your tracked version. This will
> automatically update the cover letter commit to add a "Changes in v[N]"
> section that will also contain a link to the v[N-1] series, creating a
> historical breadcrumb of revisions.

Similarly, I'm pretty sure I'm going to mess this up all the time :)

If you start tracking which version has been sent last, b4 submit can
just reuse it and add 1 by default, or something else if one needs to.

> 6. b4 submit --update-trailers [--signoff]
> 
> As you receive feedback on your patches, you can retrieve any received
> trailers from lore.kernel.org and apply them directly to your branch. This is
> accomplished by retrieving any threads matching the unique "change-id"
> submitted as part of the cover letter.
> 
> One thing to keep in mind here is that actual trailer matching is done by
> comparing the patch-id (see git-patch-id for details). This has the following
> important bonuses and caveats:
> 
>   - You can rearrange patches or modify summaries without it affecting the
>     match.
>   - If you modify actual code (even as much as fixing a typo in a string), any
>     received trailers will no longer match. This is as designed, because if
>     you change the code, it's no longer reviewed (but we won't remove any
>     existing trailers that were already applied).
>   - when you use --signoff, this will move your own "Signed-off-by" trailer
>     below any other trailers in all series commits, which is commonly
>     recommended for the chain-of-custody purposes.

Ok, that part is really awesome :)

Maxime

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

Re: b4 submit ready for beta testing

[Paolo Bonzini]

On 7/18/22 10:49, Maxime Ripard wrote:
>>
>> One thing to keep in mind here is that actual trailer matching is done by
>> comparing the patch-id (see git-patch-id for details). This has the following
>> important bonuses and caveats:
>>
>>    - You can rearrange patches or modify summaries without it affecting the
>>      match.
>>    - If you modify actual code (even as much as fixing a typo in a string), any
>>      received trailers will no longer match. This is as designed, because if
>>      you change the code, it's no longer reviewed (but we won't remove any
>>      existing trailers that were already applied).
>>    - when you use --signoff, this will move your own "Signed-off-by" trailer
>>      below any other trailers in all series commits, which is commonly
>>      recommended for the chain-of-custody purposes.
> Ok, that part is really awesome:)

Yeah, and it's the part that I'd like to have as a standalone tool in 
b4.  Can we have some kind of "b4 update-trailers <MSGID>" subcommand?

Paolo

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Mon, Jul 18, 2022 at 10:49:27AM +0200, Maxime Ripard wrote:
> > After the branch is created, b4 will create an empty commit that will serve to
> > store the cover letter contents and some tracking information. You can view
> > the raw commit with "git log" just like any other commit, and you can even
> > edit it with "git rebase" but it's not recommended. It's much better to use
> > "b4 submit --edit-cover".
> 
> So, I've been using a lot git-publish since a couple of years
> (https://github.com/stefanha/git-publish), and this creates arbitrary
> constraints that aren't great from a usability point of view compared to
> git-publish:
> 
>   - The fact that you need to create a branch beforehand prevents to
>     easily use it for one-off series. I'm sure it's doable, but it's not
>     clear to me why you would want a special branch in the first
>     place...

When approaching this work, I was trying to see it from the point of view of
someone who is an occasional contributor. This is easy for me, because I do,
very occasionally, contribute to the documentation tree. So, my choices were
dictated mainly by the following considerations:

1. it should be obvious where everything is stored and how to view it
2. we should use as little git magic as possible
3. it should be easy to blow away and start over

I was not aware of git-publish before you mentioned it, but I did consider
using annotated tags to store cover commit information, just like git-publish
does. My main worries about using this approach were:

1. Pushing tags is a separate step from pushing commits, which is not obvious
   to a lot of occasional contributors. Someone could do "git push origin" and
   assume their cover letters are preserved, but they won't be.
2. This requires us to assume (or track) the base branch in order to know
   where the series starts. For example, git-publish assumes it's "master" and
   allows you to override that with -b, but a lot of subtrees require that
   all series are based off of "next" or some other branch. Using a special
   empty commit allows me to completely ignore any other branches in the repo
   that can go away or get force-pulled into some weird state, resulting in a
   completely unpredictable patch series.
3. This creates a lot of tags that are detached from any heads, and they are
   probably never going to get cleaned up. This is mostly a nitpick, but I
   find this messy. :)

>   - ... Apart from that special commit. I'm not sure it's great either
>     and am almost certain I'm going to screw up rebases all the time,
>     dropping that special commit.

I know everyone worries about that, but I really think this is no longer the
case ever since git-rebase switched to the merge backend. It's really
difficult these days to unintentionally drop empty commits from your history.

> This also allows to easily rollback to a previous series version and
> it's transparent to others whether you're using it or not, the history
> is clean. You can even send the PR directly from the branch if you want
> to.

Yes, this is my main worry -- that these empty commits will somehow make their
way into a pull request and annoy Linus or other maintainers. This is
basically the only reason why I would consider abandoning them in favour of
something else.

> > After you send your series and when you're ready to start working on a new
> > revision, you'll want to --reroll to increment your tracked version. This will
> > automatically update the cover letter commit to add a "Changes in v[N]"
> > section that will also contain a link to the v[N-1] series, creating a
> > historical breadcrumb of revisions.
> 
> Similarly, I'm pretty sure I'm going to mess this up all the time :)
> 
> If you start tracking which version has been sent last, b4 submit can
> just reuse it and add 1 by default, or something else if one needs to.

I wanted to avoid situations like "it got sent, but the list bounced it, so I
should resend it now with the same revision", which is why I didn't want to
auto-increment after each time we send. I will add a check for "are we sending
again with the same revision number as last time", to help avoid confusing
collisions.

-K

Re: b4 submit ready for beta testing

[Maxime Ripard]

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

On Mon, Jul 18, 2022 at 10:33:47AM -0400, Konstantin Ryabitsev wrote:
> On Mon, Jul 18, 2022 at 10:49:27AM +0200, Maxime Ripard wrote:
> > > After the branch is created, b4 will create an empty commit that will serve to
> > > store the cover letter contents and some tracking information. You can view
> > > the raw commit with "git log" just like any other commit, and you can even
> > > edit it with "git rebase" but it's not recommended. It's much better to use
> > > "b4 submit --edit-cover".
> > 
> > So, I've been using a lot git-publish since a couple of years
> > (https://github.com/stefanha/git-publish), and this creates arbitrary
> > constraints that aren't great from a usability point of view compared to
> > git-publish:
> > 
> >   - The fact that you need to create a branch beforehand prevents to
> >     easily use it for one-off series. I'm sure it's doable, but it's not
> >     clear to me why you would want a special branch in the first
> >     place...
> 
> When approaching this work, I was trying to see it from the point of view of
> someone who is an occasional contributor. This is easy for me, because I do,
> very occasionally, contribute to the documentation tree. So, my choices were
> dictated mainly by the following considerations:
> 
> 1. it should be obvious where everything is stored and how to view it
> 2. we should use as little git magic as possible
> 3. it should be easy to blow away and start over

Yeah, full agree on this :)

However, I think trying to stick as close as we can to the regular git
workflow is part of those goals too, otherwise you will effectively ask
for the occasional user to learn some more b4 magic, that won't be
obvious at first.

> I was not aware of git-publish before you mentioned it, but I did consider
> using annotated tags to store cover commit information, just like git-publish
> does.

For the record, prior to git-publish I was using git-series:
https://github.com/git-series/git-series

That was also imposing a bunch of commands to maintain a series of
patches (to start a new branch, like b4, but also to rebase for
example). git-publish is, to me, much more convenient to use since I can
use any tool, frontend I want, and I don't have to remember all the time
whether I'm on a git-series branch, and if I need to use git series
rebase or git rebase.

> My main worries about using this approach were:
> 
> 1. Pushing tags is a separate step from pushing commits, which is not obvious
>    to a lot of occasional contributors. Someone could do "git push origin" and
>    assume their cover letters are preserved, but they won't be.

That's true, but when I'm pushing a branch for a PR or someone to test,
I don't really care about pushing the cover letter itself. That might be
just me though. It might be useful for archival when a PR is merged, but
for any other purpose it's not useful at all to me.

> 2. This requires us to assume (or track) the base branch in order to know
>    where the series starts. For example, git-publish assumes it's "master" and
>    allows you to override that with -b, but a lot of subtrees require that
>    all series are based off of "next" or some other branch. Using a special
>    empty commit allows me to completely ignore any other branches in the repo
>    that can go away or get force-pulled into some weird state, resulting in a
>    completely unpredictable patch series.

Yeah, but that info is one of the metadata stored in the tag, so if it's
set as long as you don't change it, you don't have to provide it.

> 3. This creates a lot of tags that are detached from any heads, and they are
>    probably never going to get cleaned up. This is mostly a nitpick, but I
>    find this messy. :)
> 
> >   - ... Apart from that special commit. I'm not sure it's great either
> >     and am almost certain I'm going to screw up rebases all the time,
> >     dropping that special commit.
> 
> I know everyone worries about that, but I really think this is no longer the
> case ever since git-rebase switched to the merge backend. It's really
> difficult these days to unintentionally drop empty commits from your history.

It's not just about the merge strategy (which I probably forced somehow,
since it definitely complains about empty commits here), but it's also
very easy to drop it if it's something you can handle. If you do a git
rebase $base --onto $newbase for example, you can definitely mess up the
base commit and drop the "metadata" commit in the process.

A tag is a separate item, and still more convenient than a note when it
comes to rebasing or pushing.

> > This also allows to easily rollback to a previous series version and
> > it's transparent to others whether you're using it or not, the history
> > is clean. You can even send the PR directly from the branch if you want
> > to.
> 
> Yes, this is my main worry -- that these empty commits will somehow make their
> way into a pull request and annoy Linus or other maintainers. This is
> basically the only reason why I would consider abandoning them in favour of
> something else.

We should definitely expect it to happen. I know it will happen to me at
some point ;)

> > > After you send your series and when you're ready to start working on a new
> > > revision, you'll want to --reroll to increment your tracked version. This will
> > > automatically update the cover letter commit to add a "Changes in v[N]"
> > > section that will also contain a link to the v[N-1] series, creating a
> > > historical breadcrumb of revisions.
> > 
> > Similarly, I'm pretty sure I'm going to mess this up all the time :)
> > 
> > If you start tracking which version has been sent last, b4 submit can
> > just reuse it and add 1 by default, or something else if one needs to.
> 
> I wanted to avoid situations like "it got sent, but the list bounced it, so I
> should resend it now with the same revision", which is why I didn't want to
> auto-increment after each time we send. I will add a check for "are we sending
> again with the same revision number as last time", to help avoid confusing
> collisions.

Sending an incremented version when we didn't mean too doesn't have any
consequence though. The worst that could happen is that maintainers will
complain about a new series that wasn't really necessary, but it will be
obvious to everyone which one should be picked, maintainers, lore/b4, etc.

Sending a new series with the same number though is very bad and will
confuse/piss off everyone.

If we can detect it reliably, we shouldn't rely on user action, we
should just do it.

Maxime

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

Re: b4 submit ready for beta testing

[Rob Herring]

On Sat, Jul 16, 2022 at 8:30 AM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> Hello, all:
>
> I've committed the initial b4-submit features to the master branch, and I
> need some volunteers to test it. If you routinely send patches to the Linux
> kernel mailing lists, then I hope that volunteer is you. :)
>
> ## What it does
>
> The goal behind "b4 submit" is simplifying the submitter's workflow by
> automating away a lot of menial tasks, such as creating cover letters,
> collecting trailers, rerolling new revisions, remembering all the flags to
> git-format-patch and git-send-email, etc. The main workflow can be described
> like this:
>
> 1. b4 submit --new my-new-feature [-f v5.19-rc6]
>
> This will create a new branch called "b4/my-new-feature". If -f is not
> specified, the branch will be created from the current HEAD (but this is
> rarely what you want, so you'll want to pick a tag to work from).

It would be nice if an existing branch could be used. Either as an
enroll this branch as-is with b4 submit or create a new branch based
on the existing branch. Often, I start working on something for a
while, and then start thinking about submitting, how to split patches,
and a cover letter some time later. I guess that would involve
rebasing the branch with the empty commit approach (which the branch
description approach would not need).


> After the branch is created, b4 will create an empty commit that will serve to
> store the cover letter contents and some tracking information. You can view
> the raw commit with "git log" just like any other commit, and you can even
> edit it with "git rebase" but it's not recommended. It's much better to use
> "b4 submit --edit-cover".
>
> 2. b4 submit --edit-cover
>
> This allows you to work on the cover letter using $EDITOR or core.editor
> as defined in git-config. Once the editing is complete, the empty commit will
> be updated and the entire branch rebased using git-filter-repo.
>
> 3. use regular git operations for the rest of your work
>
> You can apply, rebase, edit, squash, etc -- just avoid touching the cover
> letter commit. You can also push the b4/my-new-feature branch to any remote
> and pull it from anywhere else if you want to switch computers.
>
> 4. b4 submit --send
>
> When you are ready to send off your series, you can review the resulting
> patches by running:
>
>   b4 submit --send --dry-run
>
> or
>
>   b4 submit --send -o /tmp/somedir
>
> The first will print out the patches to send on stdout, exactly as they would
> be fed to the SMTP server, and the second will output the patches into the
> specified directory a-la git-format-patch -- useful for running "checkpatch"
> or any other checks, as necessary.
>
> When you are satisfied that the patches are looking sane, you can fire off the
> actual --send command.
>
> NOTE: when we find a scripts/get_maintainer.pl in the repo toplevel, we will
> automatically run the most-recommended (that I could find) combination of
> flags to automatically populate the To: and Cc: list in addition to whatever
> we get out of the cover letter and commit trailers. You can turn this off with
> --no-auto-to-cc.
>
> NOTE: at this time, a working SMTP server and a valid [sendemail] section are
> required for sending patches. The web submission endpoint work is not yet
> completed.
>
> 5. b4 submit --reroll
>
> After you send your series and when you're ready to start working on a new
> revision, you'll want to --reroll to increment your tracked version. This will
> automatically update the cover letter commit to add a "Changes in v[N]"
> section that will also contain a link to the v[N-1] series, creating a
> historical breadcrumb of revisions.

As you are rebasing anyways, an option for a new base would be useful here.

>
> 6. b4 submit --update-trailers [--signoff]
>
> As you receive feedback on your patches, you can retrieve any received
> trailers from lore.kernel.org and apply them directly to your branch. This is
> accomplished by retrieving any threads matching the unique "change-id"
> submitted as part of the cover letter.
>
> One thing to keep in mind here is that actual trailer matching is done by
> comparing the patch-id (see git-patch-id for details). This has the following
> important bonuses and caveats:
>
>   - You can rearrange patches or modify summaries without it affecting the
>     match.
>   - If you modify actual code (even as much as fixing a typo in a string), any
>     received trailers will no longer match. This is as designed, because if
>     you change the code, it's no longer reviewed (but we won't remove any
>     existing trailers that were already applied).

A Reviewed-by would typically stand for something like a typo fix.
Often minor changes are pointed out by the reviewer and a Reviewed-by
is given assuming those are fixed. At a minimum, I think we want an
override option on this perhaps with warnings or confirmation prompt.

Knowing what to do here is a common issue for new submitters. Both
just knowing that they need to run update-trailers and when to keep or
drop them. It's common enough that I run checks for not adding tags.

>   - when you use --signoff, this will move your own "Signed-off-by" trailer
>     below any other trailers in all series commits, which is commonly
>     recommended for the chain-of-custody purposes.

I would think that is more commonly desired than not, so I think that
should be the default.

>
> 7. b4 submit --send
>
> GOTO 4.

It seems like you are mixing sub-commands and options. Wouldn't 'b4
submit new|reroll|send|update-trailers ..." or a new executable/link
for 'b4 submit' be better? This is fairly disjoint from the maintainer
side of b4 functions.

Rob

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Sun, Jul 17, 2022 at 11:43:44AM -0400, Konstantin Ryabitsev wrote:
> On Sat, Jul 16, 2022 at 06:14:37PM +0100, Conor Dooley wrote:
> > >> So, it was either using notes (which almost everyone forgets to push
> > >> anyway), or using an empty commit. I agree it's hurky, but it
> > >> currently the only thing that fits in the "least likely to end up
> > >> lost on a rebase or workstation switch" category.
> > 
> > 
> > Is it silly to suggest that b4 could handle the a notes to
> > description & description to notes conversion in a "b4 push-series"
> > or "b4 fetch-series" type of command? 
> 
> It's not silly, but it's significantly more fragile. Pushing and fetching
> requires reachable remotes and some kind of ability to handle conflicts, keys,
> etc. I'd much rather avoid going down that road.

More importantly all these other suggestions don't become trivially
visible on places like gitweb, github, gitlab, gerrit, etc.

The cover letter is important, if I want to share the actual commits
with someone else for some reason I want them to reliably get it.

The empty commit scheme also lets you make multi-series branches and
reliably share them in a way that other people can understand the
structure.

Jason

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Sun, Jul 17, 2022 at 12:02:18PM -0400, Konstantin Ryabitsev wrote:

> Perhaps, but I also have other reasons to like using an empty commit for this.
> For example, it makes it very easy to mark where exactly our series
> starts.

It is a good point, but it is backwards to how alot of people have
been doing things already for a long time..

Putting the commit last makes it work a lot more like the usual
merge-commit approach to preserve the cover letter. Particularly if
you open the branch in any of the web viewers for git, or gitk you get
a very nice view of the cover letter explaining the branch followed by
the usual code in reverse patch order.

It would be nice to use merge commits to mark the series boundary, but
IMHO, the tooling is poor for this.

Jason

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Mon, Jul 18, 2022 at 02:38:48PM +0200, Paolo Bonzini wrote:
> On 7/18/22 10:49, Maxime Ripard wrote:
> > > 
> > > One thing to keep in mind here is that actual trailer matching is done by
> > > comparing the patch-id (see git-patch-id for details). This has the following
> > > important bonuses and caveats:
> > > 
> > >    - You can rearrange patches or modify summaries without it affecting the
> > >      match.
> > >    - If you modify actual code (even as much as fixing a typo in a string), any
> > >      received trailers will no longer match. This is as designed, because if
> > >      you change the code, it's no longer reviewed (but we won't remove any
> > >      existing trailers that were already applied).

IMHO, I don't like this. It means even minor changes that don't
invalidate a given "reviewed-by" will not be taken. It should only be
up to the user to make these decisions when tags should be dropped.

> > >    - when you use --signoff, this will move your own "Signed-off-by" trailer
> > >      below any other trailers in all series commits, which is commonly
> > >      recommended for the chain-of-custody purposes.
> > Ok, that part is really awesome:)
> 
> Yeah, and it's the part that I'd like to have as a standalone tool in b4.
> Can we have some kind of "b4 update-trailers <MSGID>" subcommand?

+1

Updating trailers by hand is a PITA..

Jason

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Mon, Jul 18, 2022 at 11:15:00AM -0600, Rob Herring wrote:

[skipped a lot due to ongoing work changing some underlying things.
Not ignoring, I promise.]

> It seems like you are mixing sub-commands and options. Wouldn't 'b4
> submit new|reroll|send|update-trailers ..." or a new executable/link
> for 'b4 submit' be better? This is fairly disjoint from the maintainer
> side of b4 functions.

Yeah, the current set of flags aren't great. Unfortunately, I'm limited by
what python's argparse is able to do, and it can only do a single level of
subcommands. Maybe I will use a separate command for submit, as you suggest.

-K

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Mon, Jul 18, 2022 at 03:20:46PM -0300, Jason Gunthorpe wrote:
> > > >    - If you modify actual code (even as much as fixing a typo in a string), any
> > > >      received trailers will no longer match. This is as designed, because if
> > > >      you change the code, it's no longer reviewed (but we won't remove any
> > > >      existing trailers that were already applied).
> 
> IMHO, I don't like this. It means even minor changes that don't
> invalidate a given "reviewed-by" will not be taken. It should only be
> up to the user to make these decisions when tags should be dropped.

Should we go with "patch-id OR commit subject"?

-K

> > Yeah, and it's the part that I'd like to have as a standalone tool in b4.
> > Can we have some kind of "b4 update-trailers <MSGID>" subcommand?
> 
> +1

Yes, this is easy -- just a matter of sanely identifying how far back we start
looking to match commits to patches. I will probably go with "find all commits
by the current user for the past month and match subjects/patch-ids."

-K

RE: b4 submit ready for beta testing

[Luck, Tony]

> The empty commit scheme also lets you make multi-series branches and
> reliably share them in a way that other people can understand the
> structure.

What's the advantage of an empty commit?  Couldn't you start with a commit
that creates a "cover-letter" file in the root of the project? Then users could add
as they go along by editing and committing changes to the cover letter (need
to take care not to have a commit that touches both the cover letter and other
"real" files ... but I expect tools could figure out how to fix that if it happened).

-Tony

Re: b4 submit ready for beta testing

[Geert Uytterhoeven]

Hi Jason,

On Mon, Jul 18, 2022 at 8:17 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Sun, Jul 17, 2022 at 12:02:18PM -0400, Konstantin Ryabitsev wrote:
> > Perhaps, but I also have other reasons to like using an empty commit for this.
> > For example, it makes it very easy to mark where exactly our series
> > starts.
>
> It is a good point, but it is backwards to how alot of people have
> been doing things already for a long time..
>
> Putting the commit last makes it work a lot more like the usual
> merge-commit approach to preserve the cover letter. Particularly if
> you open the branch in any of the web viewers for git, or gitk you get
> a very nice view of the cover letter explaining the branch followed by
> the usual code in reverse patch order.

So a tag does sound like a logical place to store the cover letter,
as merging the tag already pulls in the tag description into the
merge commit?

> It would be nice to use merge commits to mark the series boundary, but
> IMHO, the tooling is poor for this.

Tou can use "git tag --points-at" to find out if you already have a tag
pointing to a commit.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Mon, Jul 18, 2022 at 10:28:13PM +0200, Geert Uytterhoeven wrote:
> Hi Jason,
> 
> On Mon, Jul 18, 2022 at 8:17 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > On Sun, Jul 17, 2022 at 12:02:18PM -0400, Konstantin Ryabitsev wrote:
> > > Perhaps, but I also have other reasons to like using an empty commit for this.
> > > For example, it makes it very easy to mark where exactly our series
> > > starts.
> >
> > It is a good point, but it is backwards to how alot of people have
> > been doing things already for a long time..
> >
> > Putting the commit last makes it work a lot more like the usual
> > merge-commit approach to preserve the cover letter. Particularly if
> > you open the branch in any of the web viewers for git, or gitk you get
> > a very nice view of the cover letter explaining the branch followed by
> > the usual code in reverse patch order.
> 
> So a tag does sound like a logical place to store the cover letter,
> as merging the tag already pulls in the tag description into the
> merge commit?

See 'git-tag(1)' "On Re-tagging" for a discussion on why changing tags
that have been pushed is a bad idea. There are many troublesome
behaviors here.

Tags are not a solution to store the cover letter during development..

> > It would be nice to use merge commits to mark the series boundary, but
> > IMHO, the tooling is poor for this.
> 
> Tou can use "git tag --points-at" to find out if you already have a tag
> pointing to a commit.

tags are also bad because they don't auto track - when I rebase a
series with an empty cover commit everything stays in order
automatically. I can even shuffle around where patches are in a
multi-series work and break up a series with new cover letters rather
trivially with git rebase.

If I use a tag I have to remember to update the tag after every
rebase. It is much more likely to break down.

Jason

Re: b4 submit ready for beta testing

[Geert Uytterhoeven]

Hi Jason,

On Tue, Jul 19, 2022 at 1:10 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Mon, Jul 18, 2022 at 10:28:13PM +0200, Geert Uytterhoeven wrote:
> > On Mon, Jul 18, 2022 at 8:17 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > On Sun, Jul 17, 2022 at 12:02:18PM -0400, Konstantin Ryabitsev wrote:
> > > > Perhaps, but I also have other reasons to like using an empty commit for this.
> > > > For example, it makes it very easy to mark where exactly our series
> > > > starts.
> > >
> > > It is a good point, but it is backwards to how alot of people have
> > > been doing things already for a long time..
> > >
> > > Putting the commit last makes it work a lot more like the usual
> > > merge-commit approach to preserve the cover letter. Particularly if
> > > you open the branch in any of the web viewers for git, or gitk you get
> > > a very nice view of the cover letter explaining the branch followed by
> > > the usual code in reverse patch order.
> >
> > So a tag does sound like a logical place to store the cover letter,
> > as merging the tag already pulls in the tag description into the
> > merge commit?
>
> See 'git-tag(1)' "On Re-tagging" for a discussion on why changing tags
> that have been pushed is a bad idea. There are many troublesome
> behaviors here.
>
> Tags are not a solution to store the cover letter during development..

Oh, I always write my cover letters _after_ development, just before
sending out my series.

> > > It would be nice to use merge commits to mark the series boundary, but
> > > IMHO, the tooling is poor for this.
> >
> > Tou can use "git tag --points-at" to find out if you already have a tag
> > pointing to a commit.
>
> tags are also bad because they don't auto track - when I rebase a
> series with an empty cover commit everything stays in order
> automatically. I can even shuffle around where patches are in a
> multi-series work and break up a series with new cover letters rather
> trivially with git rebase.

True. But see above.

> If I use a tag I have to remember to update the tag after every
> rebase. It is much more likely to break down.

Why would you have to update the tag? The tag would represent what
was sent out, i.e. the tag name would include the revision number.
After rebase, you have to recreate the cover letter (with an
increased revision number), using the previous tag description as
a template.

It would be great if this could auto-insert a lore link to the previous
revision ("Changes since v${n-1} [https://lore...]").  Git can store
the commit hash into the Message-Id, but it also contains a timestamp,
so you can't go easily from tag to Message-Id.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: b4 submit ready for beta testing

[Mark Brown]

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

On Tue, Jul 19, 2022 at 09:02:40AM +0200, Geert Uytterhoeven wrote:
> On Tue, Jul 19, 2022 at 1:10 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:

> > See 'git-tag(1)' "On Re-tagging" for a discussion on why changing tags
> > that have been pushed is a bad idea. There are many troublesome
> > behaviors here.

> > Tags are not a solution to store the cover letter during development..

> Oh, I always write my cover letters _after_ development, just before
> sending out my series.

I tend to find that's less true for followup versions - when revising a
series I tend to add to the inter version changelog as I go through
making changes.

> > If I use a tag I have to remember to update the tag after every
> > rebase. It is much more likely to break down.

> Why would you have to update the tag? The tag would represent what
> was sent out, i.e. the tag name would include the revision number.

Yes, tagging versions as they go out can be very useful (I do as several
people have suggested here and tag things as I post them with the cover
letter in the tag).

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

Re: b4 submit ready for beta testing

[Mattijs Korpershoek]

Hi Konstantin,

First of all, thank you for this.
As an occasional kernel contributor I struggle with the current workflow.
I've never properly spend time automating any of this so I appreciate
the "b4 submit" initiative.

I think I've found a bug in the patch ordering result from
'b4 submit --send -o' or 'b4 submit --send --dry-run'.

The patch order is reverted when comparing with a classic "git format
patch" result.

Please find details below.

On Sat, Jul 16, 2022 at 10:29, Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:

> Hello, all:
>
> I've committed the initial b4-submit features to the master branch, and I
> need some volunteers to test it. If you routinely send patches to the Linux
> kernel mailing lists, then I hope that volunteer is you. :)
>
> ## What it does
>
> The goal behind "b4 submit" is simplifying the submitter's workflow by
> automating away a lot of menial tasks, such as creating cover letters,
> collecting trailers, rerolling new revisions, remembering all the flags to
> git-format-patch and git-send-email, etc. The main workflow can be described
> like this:
>
> 1. b4 submit --new my-new-feature [-f v5.19-rc6]
>
> This will create a new branch called "b4/my-new-feature". If -f is not
> specified, the branch will be created from the current HEAD (but this is
> rarely what you want, so you'll want to pick a tag to work from).
>
> After the branch is created, b4 will create an empty commit that will serve to
> store the cover letter contents and some tracking information. You can view
> the raw commit with "git log" just like any other commit, and you can even
> edit it with "git rebase" but it's not recommended. It's much better to use
> "b4 submit --edit-cover".
>
> 2. b4 submit --edit-cover
>
> This allows you to work on the cover letter using $EDITOR or core.editor
> as defined in git-config. Once the editing is complete, the empty commit will
> be updated and the entire branch rebased using git-filter-repo.
>
> 3. use regular git operations for the rest of your work
>
> You can apply, rebase, edit, squash, etc -- just avoid touching the cover
> letter commit. You can also push the b4/my-new-feature branch to any remote
> and pull it from anywhere else if you want to switch computers.
>
> 4. b4 submit --send
>
> When you are ready to send off your series, you can review the resulting
> patches by running:
>
>   b4 submit --send --dry-run
>
> or
>
>   b4 submit --send -o /tmp/somedir

revision of b4:
 41cded9122f2 ("submit: store tracking info in the cover letter header")

git submodule status:
 0eb41be65707a1e156a59fd25ea9824c1a9e95ce patatt (v0.5.0-1-g0eb41be65707)


Steps to reproduce:

## prepare branch/work
$ b4 submit --new patch-ordering -f next-20220718
$ b4 submit --edit-cover # to remove EDITME and be able to use --dry-run/-o
$ touch oldest_file && git add oldest_file && git commit -s -m "oldest commit"
$ touch new_file && git add new_file && git commit -s -m "newer commit"
$ touch newest_file && git add newest_file && git commit -s -m "newest commit"

## this gives the following git log:
$ git log --oneline --no-decorate next-20220718..HEAD
d229ef49d3e9 newest commit
5eca0029017f newer commit
3e53e24f818e oldest commit
c48ebb144244 cover title for patch-ordering

## and is ordered as following with git format-patch:
$ mkdir format-patch
$ git format-patch -o format-patch next-20220718..HEAD
format-patch/0001-cover-title-for-patch-ordering.patch
format-patch/0002-oldest-commit.patch
format-patch/0003-newer-commit.patch
format-patch/0004-newest-commit.patch

## using b4 submit -o, we see that the order is not the same:
$ mkdir b4-submit
$ b4 submit --send --no-sign -o b4-submit 
Converted the branch to 3 patches
Populating the To: and Cc: fields with automatically collected addresses
  0000-cover-title-for-patch-ordering.patch
  0001-newest-commit.patch
  0002-newer-commit.patch
  0003-oldest-commit.patch

Is this the expected behaviour ?
To me, it seems that git format-patch gives the right order.

Thanks,
Mattijs

>
> The first will print out the patches to send on stdout, exactly as they would
> be fed to the SMTP server, and the second will output the patches into the
> specified directory a-la git-format-patch -- useful for running "checkpatch"
> or any other checks, as necessary.
>
> When you are satisfied that the patches are looking sane, you can fire off the
> actual --send command.
>
> NOTE: when we find a scripts/get_maintainer.pl in the repo toplevel, we will
> automatically run the most-recommended (that I could find) combination of
> flags to automatically populate the To: and Cc: list in addition to whatever
> we get out of the cover letter and commit trailers. You can turn this off with
> --no-auto-to-cc.
>
> NOTE: at this time, a working SMTP server and a valid [sendemail] section are
> required for sending patches. The web submission endpoint work is not yet
> completed.
>
> 5. b4 submit --reroll
>
> After you send your series and when you're ready to start working on a new
> revision, you'll want to --reroll to increment your tracked version. This will
> automatically update the cover letter commit to add a "Changes in v[N]"
> section that will also contain a link to the v[N-1] series, creating a
> historical breadcrumb of revisions.
>
> 6. b4 submit --update-trailers [--signoff]
>
> As you receive feedback on your patches, you can retrieve any received
> trailers from lore.kernel.org and apply them directly to your branch. This is
> accomplished by retrieving any threads matching the unique "change-id"
> submitted as part of the cover letter.
>
> One thing to keep in mind here is that actual trailer matching is done by
> comparing the patch-id (see git-patch-id for details). This has the following
> important bonuses and caveats:
>
>   - You can rearrange patches or modify summaries without it affecting the
>     match.
>   - If you modify actual code (even as much as fixing a typo in a string), any
>     received trailers will no longer match. This is as designed, because if
>     you change the code, it's no longer reviewed (but we won't remove any
>     existing trailers that were already applied).
>   - when you use --signoff, this will move your own "Signed-off-by" trailer
>     below any other trailers in all series commits, which is commonly
>     recommended for the chain-of-custody purposes.
>
> 7. b4 submit --send
>
> GOTO 4.
>
> In addition to the above, please see "b4 submit --help" for other flags you
> can use with these operations.
>
> ### Editing the cover letter commit manually
>
> If you ever need to edit the cover letter commit manually (e.g. to fix
> something about the tracking section json), you can use regular "git rebase
> -i" with the "reword" action. When rebase bails complaining about the empty
> commit, just run "git commit --amend --allow-empty" to edit the cover letter
> and then "git rebase --continue".
>
> Again, this is not recommended and it's best to always use "b4 submit
> --edit-cover".
>
> ## Getting the b4 dev version
>
> To install the b4 development version, you will need to run it from the git
> checkout:
>
>   git clone https://git.kernel.org/pub/scm/utils/b4/b4.git
>   cd b4
>   git submodule update --init
>   alias b4="$PWD/b4.sh"
>
> You can find out more details in the README file. You will also want to make
> sure that git-filter-repo is installed (either from your distro packages or
> from pip).
>
> If you're already running b4 from the git checkout, make sure you run "git
> submodule update" to pull in the latest unreleased patatt version.
>
> ## Feedback
>
> I hope you have a positive experience using b4 submit. I will be happy to
> receive any feedback, as I have to make a lot of assumptions and they aren't
> always the right ones. :) The best place for this feedback is on the
> tools@linux.kernel.org list.
>
> Best regards,
> Konstantin

Re: b4 submit ready for beta testing

[Geert Uytterhoeven]

Hi Mark,

On Tue, Jul 19, 2022 at 2:09 PM Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jul 19, 2022 at 09:02:40AM +0200, Geert Uytterhoeven wrote:
> > On Tue, Jul 19, 2022 at 1:10 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > See 'git-tag(1)' "On Re-tagging" for a discussion on why changing tags
> > > that have been pushed is a bad idea. There are many troublesome
> > > behaviors here.
>
> > > Tags are not a solution to store the cover letter during development..
>
> > Oh, I always write my cover letters _after_ development, just before
> > sending out my series.
>
> I tend to find that's less true for followup versions - when revising a
> series I tend to add to the inter version changelog as I go through
> making changes.

You don't add per-patch inter-version changelogs below the "---",
and collect them all in the cover letter (plus optional rewriting)
just before sending the new version?

Gr{oetje,eeting}s,

                        Geert

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

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

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Tue, Jul 19, 2022 at 09:02:40AM +0200, Geert Uytterhoeven wrote:

> > If I use a tag I have to remember to update the tag after every
> > rebase. It is much more likely to break down.
> 
> Why would you have to update the tag? The tag would represent what
> was sent out, i.e. the tag name would include the revision number.
> After rebase, you have to recreate the cover letter (with an
> increased revision number), using the previous tag description as
> a template.

Using a tag as 'what was sent out' might make sense - but that is a
different problem.

In many cases the cover letter is part of the series, and complex
series often have very big cover letters explaining what is going on
that take alot of effort to make and review in a "team based" flow.

Fundamentally, the cover letter needs to be source controlled while it
is being authored just like anything else.

> It would be great if this could auto-insert a lore link to the previous
> revision ("Changes since v${n-1} [https://lore...]").  Git can store
> the commit hash into the Message-Id, but it also contains a timestamp,
> so you can't go easily from tag to Message-Id.

What I've found useful is a tool that automatically generates a git
range-diff between the current branch and the last posted and also
displays the lore link. This helps tremendously producing the change
log.

Jason

Re: b4 submit ready for beta testing

[James Bottomley]

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

On Tue, 2022-07-19 at 13:09 +0100, Mark Brown wrote:
> On Tue, Jul 19, 2022 at 09:02:40AM +0200, Geert Uytterhoeven wrote:
> > On Tue, Jul 19, 2022 at 1:10 AM Jason Gunthorpe <jgg@ziepe.ca>
> > wrote:
> > > See 'git-tag(1)' "On Re-tagging" for a discussion on why changing
> > > tags that have been pushed is a bad idea. There are many
> > > troublesome behaviors here.
> > > Tags are not a solution to store the cover letter during
> > > development..
> > Oh, I always write my cover letters _after_ development, just
> > before sending out my series.
> 
> I tend to find that's less true for followup versions - when revising
> a series I tend to add to the inter version changelog as I go through
> making changes.

I tend to put it history into all commits, so the v<n> version after '-
--' in all patch trailers as well.  This means my patch series has to
go via email to get rid of that information in the final tree.

> > > If I use a tag I have to remember to update the tag after every
> > > rebase. It is much more likely to break down.
> > Why would you have to update the tag? The tag would represent what
> > was sent out, i.e. the tag name would include the revision number.
> 
> Yes, tagging versions as they go out can be very useful (I do as
> several people have suggested here and tag things as I post them with
> the cover letter in the tag).

I've always liked the idea of cover letter in tag, but the tag only
goes into the merge commit if there's a direct pull.  Pretty much all
of the patch series I work on with git go over email without a direct
pull, so the easiest worklflow for me is 

git format-patch --cover-from-description=message

It's just really annoying none of our tools supports all workflows: 
git-format-patch can't work nicely from a tag; the pull workflow seems
nicest but has no tooling at all around cover letter maintenance and
the empty commit will be b4 specific.  I think this means there's no
general agreement on a preferred workflow, which possibly means none is
deal, but now your tool decides your workflow.

James


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: b4 submit ready for beta testing

[Geert Uytterhoeven]

Hi Jason,

On Tue, Jul 19, 2022 at 2:34 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Tue, Jul 19, 2022 at 09:02:40AM +0200, Geert Uytterhoeven wrote:
> > > If I use a tag I have to remember to update the tag after every
> > > rebase. It is much more likely to break down.
> >
> > Why would you have to update the tag? The tag would represent what
> > was sent out, i.e. the tag name would include the revision number.
> > After rebase, you have to recreate the cover letter (with an
> > increased revision number), using the previous tag description as
> > a template.
>
> Using a tag as 'what was sent out' might make sense - but that is a
> different problem.
>
> In many cases the cover letter is part of the series, and complex
> series often have very big cover letters explaining what is going on
> that take alot of effort to make and review in a "team based" flow.
>
> Fundamentally, the cover letter needs to be source controlled while it
> is being authored just like anything else.

Makes sense.

So during development, you want the cover letter to be a real file,
using a specific file naming convention.
When sending out the series, you need some machinery to detect the
cover letter file, and turn it into the 0000-cover-letter.txt file instead?

Gr{oetje,eeting}s,

                        Geert

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

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

Re: b4 submit ready for beta testing

[Geert Uytterhoeven]

Hi James,

On Tue, Jul 19, 2022 at 2:44 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Tue, 2022-07-19 at 13:09 +0100, Mark Brown wrote:
> > On Tue, Jul 19, 2022 at 09:02:40AM +0200, Geert Uytterhoeven wrote:
> > > On Tue, Jul 19, 2022 at 1:10 AM Jason Gunthorpe <jgg@ziepe.ca>
> > > wrote:
> > > > See 'git-tag(1)' "On Re-tagging" for a discussion on why changing
> > > > tags that have been pushed is a bad idea. There are many
> > > > troublesome behaviors here.
> > > > Tags are not a solution to store the cover letter during
> > > > development..
> > > Oh, I always write my cover letters _after_ development, just
> > > before sending out my series.
> >
> > I tend to find that's less true for followup versions - when revising
> > a series I tend to add to the inter version changelog as I go through
> > making changes.
>
> I tend to put it history into all commits, so the v<n> version after '-
> --' in all patch trailers as well.  This means my patch series has to
> go via email to get rid of that information in the final tree.

Exactly.  Patches go out through git format-patch/send-email,
patches (even my own) go in through b4/git am.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Tue, Jul 19, 2022 at 02:47:09PM +0200, Geert Uytterhoeven wrote:
> Hi Jason,
> 
> On Tue, Jul 19, 2022 at 2:34 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > On Tue, Jul 19, 2022 at 09:02:40AM +0200, Geert Uytterhoeven wrote:
> > > > If I use a tag I have to remember to update the tag after every
> > > > rebase. It is much more likely to break down.
> > >
> > > Why would you have to update the tag? The tag would represent what
> > > was sent out, i.e. the tag name would include the revision number.
> > > After rebase, you have to recreate the cover letter (with an
> > > increased revision number), using the previous tag description as
> > > a template.
> >
> > Using a tag as 'what was sent out' might make sense - but that is a
> > different problem.
> >
> > In many cases the cover letter is part of the series, and complex
> > series often have very big cover letters explaining what is going on
> > that take alot of effort to make and review in a "team based" flow.
> >
> > Fundamentally, the cover letter needs to be source controlled while it
> > is being authored just like anything else.
> 
> Makes sense.
> 
> So during development, you want the cover letter to be a real file,
> using a specific file naming convention.
> When sending out the series, you need some machinery to detect the
> cover letter file, and turn it into the 0000-cover-letter.txt file instead?

Here we've always used empty commits for this, for years and years
now. 

I'm puzzled why people are saying this is non-standard, it is well
supported by git and works well with all the review and web tools. The
only missing bit in git is a git format-patch flag to process it
automatically into to the cover letter email.

Adding dummy files to store the cover commit instead of using the
commit blob just seems worse. You get needless rebase conflicts for
instance when working on multi-series projects.

Jason

Re: b4 submit ready for beta testing

[James Bottomley]

On Mon, 2022-07-18 at 15:17 -0300, Jason Gunthorpe wrote:
> On Sun, Jul 17, 2022 at 12:02:18PM -0400, Konstantin Ryabitsev wrote:
> 
> > Perhaps, but I also have other reasons to like using an empty
> > commit for this. For example, it makes it very easy to mark where
> > exactly our series starts.
> 
> It is a good point, but it is backwards to how alot of people have
> been doing things already for a long time..
> 
> Putting the commit last makes it work a lot more like the usual
> merge-commit approach to preserve the cover letter. Particularly if
> you open the branch in any of the web viewers for git, or gitk you
> get a very nice view of the cover letter explaining the branch
> followed by the usual code in reverse patch order.
> 
> It would be nice to use merge commits to mark the series boundary,
> but IMHO, the tooling is poor for this.

The problem with empty commit is that a lot of the projects I
contribute to can be pains about only taking one or two commits in a
series, which means I use rebase to figure out the exactness of what
they've done and drop included commits.  I can't keep this behaviour
and work with empty commits.

James

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Tue, Jul 19, 2022 at 02:23:52PM +0200, Mattijs Korpershoek wrote:
> ## using b4 submit -o, we see that the order is not the same:
> $ mkdir b4-submit
> $ b4 submit --send --no-sign -o b4-submit 
> Converted the branch to 3 patches
> Populating the To: and Cc: fields with automatically collected addresses
>   0000-cover-title-for-patch-ordering.patch
>   0001-newest-commit.patch
>   0002-newer-commit.patch
>   0003-oldest-commit.patch
> 
> Is this the expected behaviour ?

Hah, no. I messed up. :) I'll fix this in the next commit. Thanks for pointing
it out.

-K

Re: b4 submit ready for beta testing

[Michael S. Tsirkin]

On Tue, Jul 19, 2022 at 08:44:31AM -0400, James Bottomley wrote:
> It's just really annoying none of our tools supports all workflows: 
> git-format-patch can't work nicely from a tag; the pull workflow seems
> nicest but has no tooling at all around cover letter maintenance and
> the empty commit will be b4 specific.  I think this means there's no
> general agreement on a preferred workflow, which possibly means none is
> deal, but now your tool decides your workflow.

Well the empty commit possibly also with something
special in the subject (e.g. cover! prefix?) sounds like
something that git could learn to support well, no?

Maybe patches in the patchset could gain a prefix too (set! ?)
to make it possible to detect a situation where an unrelated patch
appears in the history after the patchset.

With that I can see some projects actually pushing history like this,
keeping the cover in the tree.

This doesn't require too much work to integrate into git - for starters
just rebase and commit to handle cover letter specially and then git
format-patch to strip the prefix.

-- 
MST

Re: b4 submit ready for beta testing

[Geert Uytterhoeven]

Hi Jason,

On Tue, Jul 19, 2022 at 3:00 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Tue, Jul 19, 2022 at 02:47:09PM +0200, Geert Uytterhoeven wrote:
> > On Tue, Jul 19, 2022 at 2:34 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > On Tue, Jul 19, 2022 at 09:02:40AM +0200, Geert Uytterhoeven wrote:
> > > > > If I use a tag I have to remember to update the tag after every
> > > > > rebase. It is much more likely to break down.
> > > >
> > > > Why would you have to update the tag? The tag would represent what
> > > > was sent out, i.e. the tag name would include the revision number.
> > > > After rebase, you have to recreate the cover letter (with an
> > > > increased revision number), using the previous tag description as
> > > > a template.
> > >
> > > Using a tag as 'what was sent out' might make sense - but that is a
> > > different problem.
> > >
> > > In many cases the cover letter is part of the series, and complex
> > > series often have very big cover letters explaining what is going on
> > > that take alot of effort to make and review in a "team based" flow.
> > >
> > > Fundamentally, the cover letter needs to be source controlled while it
> > > is being authored just like anything else.
> >
> > Makes sense.
> >
> > So during development, you want the cover letter to be a real file,
> > using a specific file naming convention.
> > When sending out the series, you need some machinery to detect the
> > cover letter file, and turn it into the 0000-cover-letter.txt file instead?
>
> Here we've always used empty commits for this, for years and years
> now.

OK, seems like I understood something different by "source
controlled"...

Gr{oetje,eeting}s,

                        Geert

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

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

Re: b4 submit ready for beta testing

[Maxime Ripard]

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

On Tue, Jul 19, 2022 at 10:00:39AM -0300, Jason Gunthorpe wrote:
> On Tue, Jul 19, 2022 at 02:47:09PM +0200, Geert Uytterhoeven wrote:
> > Hi Jason,
> > 
> > On Tue, Jul 19, 2022 at 2:34 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > On Tue, Jul 19, 2022 at 09:02:40AM +0200, Geert Uytterhoeven wrote:
> > > > > If I use a tag I have to remember to update the tag after every
> > > > > rebase. It is much more likely to break down.
> > > >
> > > > Why would you have to update the tag? The tag would represent what
> > > > was sent out, i.e. the tag name would include the revision number.
> > > > After rebase, you have to recreate the cover letter (with an
> > > > increased revision number), using the previous tag description as
> > > > a template.
> > >
> > > Using a tag as 'what was sent out' might make sense - but that is a
> > > different problem.
> > >
> > > In many cases the cover letter is part of the series, and complex
> > > series often have very big cover letters explaining what is going on
> > > that take alot of effort to make and review in a "team based" flow.
> > >
> > > Fundamentally, the cover letter needs to be source controlled while it
> > > is being authored just like anything else.
> > 
> > Makes sense.
> > 
> > So during development, you want the cover letter to be a real file,
> > using a specific file naming convention.
> > When sending out the series, you need some machinery to detect the
> > cover letter file, and turn it into the 0000-cover-letter.txt file instead?
> 
> Here we've always used empty commits for this, for years and years
> now. 
> 
> I'm puzzled why people are saying this is non-standard, it is well
> supported by git and works well with all the review and web tools. The
> only missing bit in git is a git format-patch flag to process it
> automatically into to the cover letter email.

So, I definitely haven't been around for as long as you, but I still
have been using git daily for the last 15 years. It's literally the
first time I'm hearing about someone using that workflow.

The only reference I could find remotely describing how that would work
are those two:
https://lkml.org/lkml/2021/11/14/37
https://public-inbox.org/git/CA+P7+xpHDGY5RTR8ntrABdxqM6b4V9dndS68=kV1+1Ym1N6YKw@mail.gmail.com/

I'm sure that it works great for you and others using that workflow, but
it's far from widely used still. I wouldn't call it non-standard
because, well, there's pretty much no standard when it comes to how you
should use git, but if we're targeting outsiders we shouldn't go too far
off the typical workflow.

Maxime

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

Re: b4 submit ready for beta testing

[Mark Brown]

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

On Tue, Jul 19, 2022 at 08:44:31AM -0400, James Bottomley wrote:
> On Tue, 2022-07-19 at 13:09 +0100, Mark Brown wrote:

> > > Oh, I always write my cover letters _after_ development, just
> > > before sending out my series.

> > I tend to find that's less true for followup versions - when revising
> > a series I tend to add to the inter version changelog as I go through
> > making changes.

> I tend to put it history into all commits, so the v<n> version after '-
> --' in all patch trailers as well.  This means my patch series has to
> go via email to get rid of that information in the final tree.

I tend to only track the versioning over the whole series for a bunch of
workflow reasons.

> > Yes, tagging versions as they go out can be very useful (I do as
> > several people have suggested here and tag things as I post them with
> > the cover letter in the tag).

> I've always liked the idea of cover letter in tag, but the tag only
> goes into the merge commit if there's a direct pull.  Pretty much all
> of the patch series I work on with git go over email without a direct
> pull, so the easiest worklflow for me is 

> git format-patch --cover-from-description=message

Yeah, what's actually happening with my scripting is that both the cover
letter and a tag are being assembled by the script and then the mails
are sent out, I don't actually tell anyone about the tags.  I keep
thinking I should perhaps just include a pull request style reference to
them in the generated cover latter (everything else that'd go into a
pull request is there already) but I worry that might annoy some people
so I don't.

> It's just really annoying none of our tools supports all workflows: 
> git-format-patch can't work nicely from a tag; the pull workflow seems
> nicest but has no tooling at all around cover letter maintenance and
> the empty commit will be b4 specific.  I think this means there's no
> general agreement on a preferred workflow, which possibly means none is
> deal, but now your tool decides your workflow.

Indeed.

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

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Tue, Jul 19, 2022 at 03:59:49PM +0200, Maxime Ripard wrote:

> > I'm puzzled why people are saying this is non-standard, it is well
> > supported by git and works well with all the review and web tools. The
> > only missing bit in git is a git format-patch flag to process it
> > automatically into to the cover letter email.
> 
> So, I definitely haven't been around for as long as you, but I still
> have been using git daily for the last 15 years. It's literally the
> first time I'm hearing about someone using that workflow.

To be fair, I didn't encounter this until I started to have to work
with gerrit, maybe 5 years ago.

It seems common enough, eg I can find other communities using gerrit
documenting this methodology:

https://fabric-documentations.readthedocs.io/en/latest/Gerrit/best-practices.html#creating-a-cover-letter-for-a-topic

If you are working alone you can get by with tags or the branch
description, but as we discussed it really doesn't scale into a team
environment because the data falls outside the natural source control.

> I'm sure that it works great for you and others using that workflow, but
> it's far from widely used still. I wouldn't call it non-standard
> because, well, there's pretty much no standard when it comes to how you
> should use git, but if we're targeting outsiders we shouldn't go too far
> off the typical workflow.

I think this thread shows exactly that there is no typical workflow
here.

IMHO Konstantin shoul decide if the goal if this b4 feature is to
support everyone's unique idea of a work flow, or to implement a
reasonable broadly useful workflow that is simple to use and
understand.

And I think it is a legitimate point that it would be nice if 'git
format-email' could be taught to natively handle whatever cover-letter
storage b4 wants to use.

Jason

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Tue, Jul 19, 2022 at 09:01:36AM -0400, James Bottomley wrote:
> On Mon, 2022-07-18 at 15:17 -0300, Jason Gunthorpe wrote:
> > On Sun, Jul 17, 2022 at 12:02:18PM -0400, Konstantin Ryabitsev wrote:
> > 
> > > Perhaps, but I also have other reasons to like using an empty
> > > commit for this. For example, it makes it very easy to mark where
> > > exactly our series starts.
> > 
> > It is a good point, but it is backwards to how alot of people have
> > been doing things already for a long time..
> > 
> > Putting the commit last makes it work a lot more like the usual
> > merge-commit approach to preserve the cover letter. Particularly if
> > you open the branch in any of the web viewers for git, or gitk you
> > get a very nice view of the cover letter explaining the branch
> > followed by the usual code in reverse patch order.
> > 
> > It would be nice to use merge commits to mark the series boundary,
> > but IMHO, the tooling is poor for this.
> 
> The problem with empty commit is that a lot of the projects I
> contribute to can be pains about only taking one or two commits in a
> series, which means I use rebase to figure out the exactness of what
> they've done and drop included commits.  I can't keep this behaviour
> and work with empty commits.

Why not? rebase works fine with empty commits.

Jason

Re: b4 submit ready for beta testing

[James Bottomley]

On Tue, 2022-07-19 at 12:34 -0300, Jason Gunthorpe wrote:
> On Tue, Jul 19, 2022 at 09:01:36AM -0400, James Bottomley wrote:
> > On Mon, 2022-07-18 at 15:17 -0300, Jason Gunthorpe wrote:
> > > On Sun, Jul 17, 2022 at 12:02:18PM -0400, Konstantin Ryabitsev
> > > wrote:
> > > 
> > > > Perhaps, but I also have other reasons to like using an empty
> > > > commit for this. For example, it makes it very easy to mark
> > > > where exactly our series starts.
> > > 
> > > It is a good point, but it is backwards to how alot of people
> > > have been doing things already for a long time..
> > > 
> > > Putting the commit last makes it work a lot more like the usual
> > > merge-commit approach to preserve the cover letter. Particularly
> > > if you open the branch in any of the web viewers for git, or gitk
> > > you get a very nice view of the cover letter explaining the
> > > branch followed by the usual code in reverse patch order.
> > > 
> > > It would be nice to use merge commits to mark the series
> > > boundary,
> > > but IMHO, the tooling is poor for this.
> > 
> > The problem with empty commit is that a lot of the projects I
> > contribute to can be pains about only taking one or two commits in
> > a series, which means I use rebase to figure out the exactness of
> > what they've done and drop included commits.  I can't keep this
> > behaviour and work with empty commits.
> 
> Why not? rebase works fine with empty commits.

Because I don't want a load of non-cover letter empty commits in my
repo.  The problem is the current behaviour is all or nothing and I
want elimination of commits that did have code but now don't but not
elimination of the empty commit for the cover letter.

James

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Tue, Jul 19, 2022 at 11:38:25AM -0400, James Bottomley wrote:
> On Tue, 2022-07-19 at 12:34 -0300, Jason Gunthorpe wrote:
> > On Tue, Jul 19, 2022 at 09:01:36AM -0400, James Bottomley wrote:
> > > On Mon, 2022-07-18 at 15:17 -0300, Jason Gunthorpe wrote:
> > > > On Sun, Jul 17, 2022 at 12:02:18PM -0400, Konstantin Ryabitsev
> > > > wrote:
> > > > 
> > > > > Perhaps, but I also have other reasons to like using an empty
> > > > > commit for this. For example, it makes it very easy to mark
> > > > > where exactly our series starts.
> > > > 
> > > > It is a good point, but it is backwards to how alot of people
> > > > have been doing things already for a long time..
> > > > 
> > > > Putting the commit last makes it work a lot more like the usual
> > > > merge-commit approach to preserve the cover letter. Particularly
> > > > if you open the branch in any of the web viewers for git, or gitk
> > > > you get a very nice view of the cover letter explaining the
> > > > branch followed by the usual code in reverse patch order.
> > > > 
> > > > It would be nice to use merge commits to mark the series
> > > > boundary,
> > > > but IMHO, the tooling is poor for this.
> > > 
> > > The problem with empty commit is that a lot of the projects I
> > > contribute to can be pains about only taking one or two commits in
> > > a series, which means I use rebase to figure out the exactness of
> > > what they've done and drop included commits.  I can't keep this
> > > behaviour and work with empty commits.
> > 
> > Why not? rebase works fine with empty commits.
> 
> Because I don't want a load of non-cover letter empty commits in my
> repo.  The problem is the current behaviour is all or nothing and I
> want elimination of commits that did have code but now don't but not
> elimination of the empty commit for the cover letter.

I still don't get it. Today git rebase will preserve a empty commit
cover letter with no special option. So are you unhappy with how git
rebase is working?

I've done the workflow you describe and have not ended up with empty
commits. In the first pass git rebase will remove commits that are
already applied by using the patch-id, these commits don't even show
up in the todo you can see with 'git rebase -i'

If rebase does apply a commit and the merge resolution leaves it
empty, I'm pretty sure it goes ahead and discards it.

Jason

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Tue, Jul 19, 2022 at 12:32:47PM -0300, Jason Gunthorpe wrote:
> > I'm sure that it works great for you and others using that workflow, but
> > it's far from widely used still. I wouldn't call it non-standard
> > because, well, there's pretty much no standard when it comes to how you
> > should use git, but if we're targeting outsiders we shouldn't go too far
> > off the typical workflow.
> 
> I think this thread shows exactly that there is no typical workflow
> here.
> 
> IMHO Konstantin shoul decide if the goal if this b4 feature is to
> support everyone's unique idea of a work flow, or to implement a
> reasonable broadly useful workflow that is simple to use and
> understand.

I'm implementing two different cover letter strategies:

- the default one, using the cover letter commit
- the tag-based one, a-la git-publish

Newbies and occasional contributors will almost always want to stick with the
"cover letter in an empty commit" strategy, but those who really hate the idea
can pick the tag strategy.

-K

Re: b4 submit ready for beta testing

[Rob Herring]

On Tue, Jul 19, 2022 at 9:33 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Tue, Jul 19, 2022 at 03:59:49PM +0200, Maxime Ripard wrote:
>
> > > I'm puzzled why people are saying this is non-standard, it is well
> > > supported by git and works well with all the review and web tools. The
> > > only missing bit in git is a git format-patch flag to process it
> > > automatically into to the cover letter email.
> >
> > So, I definitely haven't been around for as long as you, but I still
> > have been using git daily for the last 15 years. It's literally the
> > first time I'm hearing about someone using that workflow.
>
> To be fair, I didn't encounter this until I started to have to work
> with gerrit, maybe 5 years ago.
>
> It seems common enough, eg I can find other communities using gerrit
> documenting this methodology:
>
> https://fabric-documentations.readthedocs.io/en/latest/Gerrit/best-practices.html#creating-a-cover-letter-for-a-topic
>
> If you are working alone you can get by with tags or the branch
> description, but as we discussed it really doesn't scale into a team
> environment because the data falls outside the natural source control.
>
> > I'm sure that it works great for you and others using that workflow, but
> > it's far from widely used still. I wouldn't call it non-standard
> > because, well, there's pretty much no standard when it comes to how you
> > should use git, but if we're targeting outsiders we shouldn't go too far
> > off the typical workflow.
>
> I think this thread shows exactly that there is no typical workflow
> here.
>
> IMHO Konstantin shoul decide if the goal if this b4 feature is to
> support everyone's unique idea of a work flow, or to implement a
> reasonable broadly useful workflow that is simple to use and
> understand.
>
> And I think it is a legitimate point that it would be nice if 'git
> format-email' could be taught to natively handle whatever cover-letter
> storage b4 wants to use.

git-format-patch already handles using the branch description with
--cover-from-description. Not sure if that means the git community
thinks that is the 'right' workflow, or just that empty commits for
cover letters hasn't been discussed. If git did learn to use empty
commits or tags for cover letters, then we are back to the question of
which one does the kernel community (or at least the subset that cares
about 'b4 submit') use.

Rob

Re: b4 submit ready for beta testing

[Sudeep Holla]

On Tue, Jul 19, 2022 at 02:28:02PM +0200, Geert Uytterhoeven wrote:
> Hi Mark,
> 
> On Tue, Jul 19, 2022 at 2:09 PM Mark Brown <broonie@kernel.org> wrote:
> > On Tue, Jul 19, 2022 at 09:02:40AM +0200, Geert Uytterhoeven wrote:
> > > On Tue, Jul 19, 2022 at 1:10 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > > See 'git-tag(1)' "On Re-tagging" for a discussion on why changing tags
> > > > that have been pushed is a bad idea. There are many troublesome
> > > > behaviors here.
> >
> > > > Tags are not a solution to store the cover letter during development..
> >
> > > Oh, I always write my cover letters _after_ development, just before
> > > sending out my series.
> >
> > I tend to find that's less true for followup versions - when revising a
> > series I tend to add to the inter version changelog as I go through
> > making changes.
> 
> You don't add per-patch inter-version changelogs below the "---",
> and collect them all in the cover letter (plus optional rewriting)
> just before sending the new version?
>

I just noticed the same after sending out few fixes[1] using b4 summit.
I didn't use --send directly though as I wanted to add a note in one of the
patch. Also noticed the Subject line broke at the very beginning in 1/3
which git send-email complained and I fixed, not sure if that is an issue
if b4 is used to send email as well.

Sorry for replying here with other comments/feedback as well but the diff
stat missing is main thing and hence I used this thread.

-- 
Regards,
Sudeep

[1] https://lore.kernel.org/all/20220720-arch_topo_fixes-v3-0-43d696288e84@arm.com/

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

Hello, all:

Based on your feedback, I've refactored the submit functionality and it's
ready for some more poking and testing. Here are the implemented changes:

1. Pluggable cover letter strategies
   The goal is to support 3 different cover-letter strategies:

   "commit": same as first implementation with the cover letter stored in an
   empty commit at the beginning of the series.

   "branch-description": stores the cover letter in the branch description
   (which really just means the git config file). The cover letter can be
   edited using the same b4 --cover-letter command, or just with "git branch
   --edit-description".

   "tag" (not yet implemented): will store the cover letter in tags a-la
   git-publish.

   The default cover strategy remains "commit", but you can switch using
   b4.ez-cover-strategy, e.g. in your ~/.gitconfig:

       [b4]
         ez-cover-strategy = branch-description

  Note, that switching from one to the other for existing tracked series is
  not supported (and I'm not sure will be). It's also not currently possible
  to enroll an existing branch into ez-series with the "commit" strategy
  (because I haven't yet figured out how to insert a random commit into
  history with git-filter-repo, but I'm sure it's possible).

2. "b4 submit" command got split into 3 different commands:

  - "b4 ez-series" to create/enroll branches and manage cover letters

  - "b4 ez-trailers" for retrieving trailer updates and updating your local
    branch. Note, that with -F this can run on any branch, not necessarily the
    branch managed by ez-series. E.g.:

        git checkout arbitrary-branch
        b4 ez-trailers -us -F [arbitrary-msgid]

  - "b4 ez-send" for sending the series to the mailing list. It has all the
    same flags as formerly for "b4 submit", e.g.:

        b4 ez-send --no-auto-to-cc --dry-run

    There is no more --reroll command, as using ez-send will automatically
    increment the tracked revision number (unless RESEND was in the prefixes).

    There's still no support on the web submission endpoint, but it's coming
    in the near future.

Thank you for everyone's feedback, and please continue testing it out. If you
want to test the dev version of b4, use the same instructions:

> ## Getting the b4 dev version
> 
> To install the b4 development version, you will need to run it from the git
> checkout:
> 
>   git clone https://git.kernel.org/pub/scm/utils/b4/b4.git
>   cd b4
>   git submodule update --init
>   alias b4="$PWD/b4.sh"
> 
> You can find out more details in the README file. You will also want to make
> sure that git-filter-repo is installed (either from your distro packages or
> from pip).
> 
> If you're already running b4 from the git checkout, make sure you run "git
> submodule update" to pull in the latest unreleased patatt version.

Best regards,
Konstantin

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Wed, Jul 20, 2022 at 02:48:35PM -0400, Konstantin Ryabitsev wrote:
> Hello, all:
> 
> Based on your feedback, I've refactored the submit functionality and it's
> ready for some more poking and testing. Here are the implemented changes:
> 
> 1. Pluggable cover letter strategies
>    The goal is to support 3 different cover-letter strategies:
> 
>    "commit": same as first implementation with the cover letter stored in an
>    empty commit at the beginning of the series.

How about an 'end of the series' option if we are doing lots of
choices?

I've never actually seen 'start of the series' in the wild before..

Jason

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Wed, Jul 20, 2022 at 04:24:56PM -0300, Jason Gunthorpe wrote:
> > 1. Pluggable cover letter strategies
> >    The goal is to support 3 different cover-letter strategies:
> > 
> >    "commit": same as first implementation with the cover letter stored in an
> >    empty commit at the beginning of the series.
> 
> How about an 'end of the series' option if we are doing lots of
> choices?

Like, in a tip commit? Wouldn't that make it super awkward to add more
commits?

With the "empty commit at start" approach I solve two important problems:

1. I clearly mark where the series starts, which is super non-trivial with
   git. With other strategies like branch-description we use the "base branch"
   to find the fork-point, but even there are common situations that can mess
   things up, e.g.:

   git remote update origin
   git rebase origin/master

   If we have the local "master" branch as our tracked base branch, and we
   didn't actually fast-forward it to match origin/master, then what we
   consider "our series" has suddenly grown by a bunch of commits.

2. I have a place to store tracking information (the json bits) from the
   moment the branch is created, which is handy.

I guess we can add a "tip-commit" cover letter strategy that we can use when
enrolling existing branches, but then I'd have to consider the situation where
there are commits past the cover letter tip commit and what to do with those.
Overall, it's doable, but only as an additional strategy to the "commit at the
start of the series."

-K

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Wed, Jul 20, 2022 at 03:40:14PM -0400, Konstantin Ryabitsev wrote:
> On Wed, Jul 20, 2022 at 04:24:56PM -0300, Jason Gunthorpe wrote:
> > > 1. Pluggable cover letter strategies
> > >    The goal is to support 3 different cover-letter strategies:
> > > 
> > >    "commit": same as first implementation with the cover letter stored in an
> > >    empty commit at the beginning of the series.
> > 
> > How about an 'end of the series' option if we are doing lots of
> > choices?
> 
> Like, in a tip commit? Wouldn't that make it super awkward to add more
> commits?

No more difficult than amending a commit lower down in the stack. Just
use rebase and fix all sins. If you are using a git commit based
workflow then you are going to be rebasing *constantly* before the
series is ready to send.
 
> 1. I clearly mark where the series starts, which is super non-trivial with
>    git. With other strategies like branch-description we use the "base branch"
>    to find the fork-point, but even there are common situations that can mess
>    things up, e.g.:

So you intend to keep the first commit in the other strategies too?
Seems odd.

> 2. I have a place to store tracking information (the json bits) from the
>    moment the branch is created, which is handy.

Hum, I admit I don't know what that is about, the scripting I use
doesn't rely on json bits. It only relies on the user telling what the
fork point is for the cases it is ambiguous

Jason

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Wed, Jul 20, 2022 at 04:55:31PM -0300, Jason Gunthorpe wrote:
> > Like, in a tip commit? Wouldn't that make it super awkward to add more
> > commits?
> 
> No more difficult than amending a commit lower down in the stack. Just
> use rebase and fix all sins. If you are using a git commit based
> workflow then you are going to be rebasing *constantly* before the
> series is ready to send.
>  
> > 1. I clearly mark where the series starts, which is super non-trivial with
> >    git. With other strategies like branch-description we use the "base branch"
> >    to find the fork-point, but even there are common situations that can mess
> >    things up, e.g.:
> 
> So you intend to keep the first commit in the other strategies too?

No-no, just the default "commit" strategy that I reserve for newbies. I expect
everyone who opts to use any other strategy to know how to recognize and deal
with odd rebase situations so they don't actually end up sending unintended
500-patch series. :)

I will consider adding "tip-commit" as another strategy, since it's fairly
straighforward.

-K

Re: b4 submit ready for beta testing

[Junio C Hamano]

Jason Gunthorpe <jgg@ziepe.ca> writes:

> On Wed, Jul 20, 2022 at 02:48:35PM -0400, Konstantin Ryabitsev wrote:
>> Hello, all:
>> 
>> Based on your feedback, I've refactored the submit functionality and it's
>> ready for some more poking and testing. Here are the implemented changes:
>> 
>> 1. Pluggable cover letter strategies
>>    The goal is to support 3 different cover-letter strategies:
>> 
>>    "commit": same as first implementation with the cover letter stored in an
>>    empty commit at the beginning of the series.
>
> How about an 'end of the series' option if we are doing lots of
> choices?
>
> I've never actually seen 'start of the series' in the wild before..

FWIW, I was planning to do a new feature for 

 - "git am", where the cover letter becomes an empty commit at the
   tip of the series; and

 - "git merge", where such a branch with an empty commit that stores
   the overall topic description at the tip of it would lose the tip
   commit when the topic is merged, and instead the resulting merge
   message would inherit the message from the empty commit at the tip.

 - "git format-patch", where such an empty commit at the tip is
   recognised and is used to pre-populate the cover letter.

And having cover letter consistently at the top of the series would
make the life easier, I would imagine.

Thanks.

Re: b4 submit ready for beta testing

[Linus Torvalds]

On Wed, Jul 20, 2022 at 4:16 PM Junio C Hamano <gitster@pobox.com> wrote:
>
>  - "git am", where the cover letter becomes an empty commit at the
>    tip of the series; and
>
>  - "git merge", where such a branch with an empty commit that stores
>    the overall topic description at the tip of it would lose the tip
>    commit when the topic is merged, and instead the resulting merge
>    message would inherit the message from the empty commit at the tip.
>
>  - "git format-patch", where such an empty commit at the tip is
>    recognised and is used to pre-populate the cover letter.
>
> And having cover letter consistently at the top of the series would
> make the life easier, I would imagine.

Honestly, I think the *really* convenient place for the cover letter
information would be in the branch description.

I don't think branch descriptions are used very much, but they have
the advantage that they stick around with the branch as you add or
edit commits in the branch.

There's no "empty commit at the top or bottom of the series" that you
have to worry about, or any tag that needs to be updated as the branch
contents change.

The argument against branch descriptions was always that they didn't
move around with the branch when you use git pull/push, but for the
stated use of "branch for sending emailed patches with a cover
letter", the notion of just having it in the branch description seems
to be exactly the rigth thing to do.

It also has the advantage that you can edit it at any point, so people
who don't want to write the cover letter until the very end (as
somebody piped up in this thread) can continue to do that, and people
who want to just update the description as they go about their branch
work can _also_ do that.

Hmm?

                Linus

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Wed, Jul 20, 2022 at 04:23:57PM -0700, Linus Torvalds wrote:

> The argument against branch descriptions was always that they didn't
> move around with the branch when you use git pull/push, but for the
> stated use of "branch for sending emailed patches with a cover
> letter", the notion of just having it in the branch description seems
> to be exactly the rigth thing to do.

The thing is not all workflows are "single developer working on their
local machine and then emailing patches". We have "team" workflows
where patches are being shuffled around with push/pull, not email, so
those push/pull problems quickly become relevant. 

Quite often the person who writes the patches and the cover letter is
not the person in the team who is going to git send-email them.

> It also has the advantage that you can edit it at any point, so people
> who don't want to write the cover letter until the very end (as
> somebody piped up in this thread) can continue to do that

If the cover letter is the last commit then 'git commit --amend'
allows editing it as easily as 'git branch --edit description'.

Generally though I think git could use a good built-in tool to edit
commit messages quickly without having to rebase or touch the working
tree.

Jason

Re: b4 submit ready for beta testing

[Linus Torvalds]

On Wed, Jul 20, 2022 at 4:39 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> If the cover letter is the last commit then 'git commit --amend'
> allows editing it as easily as 'git branch --edit description'.

.. and what about adding new commits during development?

That sounds oh-so-*very* inconvenient.

               Linus

Re: b4 submit ready for beta testing

[Junio C Hamano]

"git rebase -i" is your friend ;-)

On Wed, Jul 20, 2022 at 4:41 PM Linus Torvalds
<torvalds@linuxfoundation.org> wrote:
>
> On Wed, Jul 20, 2022 at 4:39 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > If the cover letter is the last commit then 'git commit --amend'
> > allows editing it as easily as 'git branch --edit description'.
>
> .. and what about adding new commits during development?
>
> That sounds oh-so-*very* inconvenient.
>
>                Linus

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Wed, Jul 20, 2022 at 04:42:44PM -0700, Junio C Hamano wrote:
 
> On Wed, Jul 20, 2022 at 4:41 PM Linus Torvalds <torvalds@linuxfoundation.org> wrote:
> >
> > On Wed, Jul 20, 2022 at 4:39 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >
> > > If the cover letter is the last commit then 'git commit --amend'
> > > allows editing it as easily as 'git branch --edit description'.
> >
> > .. and what about adding new commits during development?
> >
> > That sounds oh-so-*very* inconvenient.
>
> "git rebase -i" is your friend ;-)

Yeah, exactly.

New commits are not even that likely to be added at the end of the
series anyhow. A new commit needs to be added at the logical place it
belongs in the sequence, so someone should already be proficient using
rebase to shuffle commit order around.

Either write the commit at the end and 'rebase -i' it into place, or
'rebase -i' with "edit" to directly author it where it belongs.

An empty commit is trivial to reorder with 'rebase -i' - guarenteed no
conflicts :)

And when there are issues in the commits I generally end up with
fixup! commits and repeated "rebase -i --autosquash"'s until the
series is done.

I'm getting the impression that people are building series in git
without using a lot of 'rebase -i' ? How is this done ?

Jason

Re: b4 submit ready for beta testing

[Theodore Ts'o]

On Wed, Jul 20, 2022 at 09:02:56PM -0300, Jason Gunthorpe wrote:
> 
> I'm getting the impression that people are building series in git
> without using a lot of 'rebase -i' ? How is this done ?

There are lots of different mechanisms.  Some of them are syntactic
sugar on top of git rebase and/or git cherry-pick.  For example, there
is "Stacked Git"[1].

I used to use "guilt"[2], which I liked because it stored the patch
series as a quilt patch series, but when you used commands like "guilt
push" it would apply the patch to the current git branch, and "guilt
pop" would remove the patch from git branch (i.e., the moral
equivalent of "git reset --hard HEAD^").  You could then edit the
current using "git commit --amend" and then "guilt refresh" would
update the quilt series to reflect the changes made with "git commit
--amend".

The reason why I liked guilt is that since the source of truth was a
quilt patch series directory, I could keep the quilt patch series
under checked into a git repo, and so I could easily look at a
previous version of the patch series if I screwed up.  Rebasing to a
newer baseline was also pretty easy --- "guilt pop -a; git pull .... ;
guilt push -a".

Alas, "guilt" is now unmaintained, and I've gotten a lot matter at
using "git rebase -i" and tagging different versions of the patch
series.  But there are certainly a large number tools for different
developer's work flow.

[1] https://stacked-git.github.io/
[2] https://github.com/jeffpc/guilt

					- Ted

Re: b4 submit ready for beta testing

[Dave Chinner]

On Wed, Jul 20, 2022 at 08:54:07PM -0400, Theodore Ts'o wrote:
> On Wed, Jul 20, 2022 at 09:02:56PM -0300, Jason Gunthorpe wrote:
> > 
> > I'm getting the impression that people are building series in git
> > without using a lot of 'rebase -i' ? How is this done ?
> 
> There are lots of different mechanisms.  Some of them are syntactic
> sugar on top of git rebase and/or git cherry-pick.  For example, there
> is "Stacked Git"[1].
> 
> I used to use "guilt"[2],

I still use guilt.

I have a hacked version of it that can store the commit description
in an empty commit as the first patch in the series, and 'guilt
patchbomb' has an option to take that first patch as the cover
letter for the series being patchbombed.

However, I don't use that anymore, because updating the cover letter
as I work on the series turned out to be a PITA. These days I have
a series of untracked files in the tree root that are named
"changelog-<branch-name>.txt" and I edit them as I need as I work on
the series.

But then there's the problems of using those branches for pull
requests: it's not really possible because we want an editted
version of the cover letter in the signed tag for the pull request
so it ends up in the merge commit, not as an empty, unrelated commit
in the tree.

Hence when I sign the tag for the branch, I pull up the branch
description file and chop all the bits that aren't relevant out of
it (e.g. change logs, last testing status, what it was last rebased
on, where to pull it from for testing/review, etc) so it only
contains things directly relevant to the code that is being merged.

I'm only mentioning this because it's relevant to the discussion:
I've moved directly from using the the "empty commit holds cover
letter" model to the "branch description holds cover letter" model
because the branch description model has much lower overhead to
maintain and allows for far more flexibility in my local
workflows...

> which I liked because it stored the patch
> series as a quilt patch series, but when you used commands like "guilt
> push" it would apply the patch to the current git branch, and "guilt
> pop" would remove the patch from git branch (i.e., the moral
> equivalent of "git reset --hard HEAD^").  You could then edit the
> current using "git commit --amend" and then "guilt refresh" would
> update the quilt series to reflect the changes made with "git commit
> --amend".
> 
> The reason why I liked guilt is that since the source of truth was a
> quilt patch series directory, I could keep the quilt patch series
> under checked into a git repo, and so I could easily look at a
> previous version of the patch series if I screwed up.  Rebasing to a
> newer baseline was also pretty easy --- "guilt pop -a; git pull .... ;
> guilt push -a".

Yup. if I cared enough, I would have hacked guilt to store a
"description" file alongside the series file added a command like
'guilt description --edit' and then plumbed that into commands like
patchbomb. If it was still maintained, I'd probably do that work.
But it's not, and keeping the description in guilt does not save any
time or operations in my workflow, so I haven't.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: b4 submit ready for beta testing

[Geert Uytterhoeven]

Hi Jason,

On Thu, Jul 21, 2022 at 2:03 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Wed, Jul 20, 2022 at 04:42:44PM -0700, Junio C Hamano wrote:
> > On Wed, Jul 20, 2022 at 4:41 PM Linus Torvalds <torvalds@linuxfoundation.org> wrote:
> > > On Wed, Jul 20, 2022 at 4:39 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > >
> > > > If the cover letter is the last commit then 'git commit --amend'
> > > > allows editing it as easily as 'git branch --edit description'.
> > >
> > > .. and what about adding new commits during development?
> > >
> > > That sounds oh-so-*very* inconvenient.
> >
> > "git rebase -i" is your friend ;-)
>
> Yeah, exactly.
>
> New commits are not even that likely to be added at the end of the
> series anyhow. A new commit needs to be added at the logical place it
> belongs in the sequence, so someone should already be proficient using
> rebase to shuffle commit order around.
>
> Either write the commit at the end and 'rebase -i' it into place, or
> 'rebase -i' with "edit" to directly author it where it belongs.
>
> An empty commit is trivial to reorder with 'rebase -i' - guarenteed no
> conflicts :)
>
> And when there are issues in the commits I generally end up with
> fixup! commits and repeated "rebase -i --autosquash"'s until the
> series is done.

I'm wondering how this rebasing is working well with the "team"
workflows where patches are being shuffled around with push/pull?

> I'm getting the impression that people are building series in git
> without using a lot of 'rebase -i' ? How is this done ?

FTR, I use "rebase -i" all the time.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Thu, Jul 21, 2022 at 12:31:13PM +1000, Dave Chinner wrote:

> However, I don't use that anymore, because updating the cover letter
> as I work on the series turned out to be a PITA. 

Can you detail how it was painful?

Jason

Re: b4 submit ready for beta testing

[Jason Gunthorpe]

On Thu, Jul 21, 2022 at 10:48:14AM +0200, Geert Uytterhoeven wrote:

> I'm wondering how this rebasing is working well with the "team"
> workflows where patches are being shuffled around with push/pull?

It is very much like the email workflow, except instead of doing
send-email to share with a reviewer it is done with push

So it isn't being used in a flow where people are building on top of
the work and are hurt by the rebase, the flow is the rebase is part of
the re-review cycle

Jason

Re: b4 submit ready for beta testing

[Dave Chinner]

On Thu, Jul 21, 2022 at 10:07:18AM -0300, Jason Gunthorpe wrote:
> On Thu, Jul 21, 2022 at 12:31:13PM +1000, Dave Chinner wrote:
> 
> > However, I don't use that anymore, because updating the cover letter
> > as I work on the series turned out to be a PITA. 
> 
> Can you detail how it was painful?

Take a 20 patch series in a topic branch and work through the review
comments one patch at a time, updating the changelog in the
description commit as you go. Change first patch, commit. Update
changelog, rebase series. Change second patch, commit. Update
changelog, rebase series...

Now do that a couple of times a week for a dozen topic branches that
contain 100+ patches across different lines of development with
various inter-dependencies. Upstream tree changes trigger this
update pain, as does testing finding bugs in random patches
somewhere in the middle of the combined stack of topic branches.

It's far simpler to have the topic branch description and changelog
in a file/context that is completely independent of the commit chain
that represents the series - it can remain open in an editor in a
separate terminal as the rebase/rework/bug fix is performed and only
needs to be saved once when the update/rework of the patch series is
completed.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: b4 submit ready for beta testing

[Geert Uytterhoeven]

Hi Dave,

On Fri, Jul 22, 2022 at 1:19 AM Dave Chinner <david@fromorbit.com> wrote:
> On Thu, Jul 21, 2022 at 10:07:18AM -0300, Jason Gunthorpe wrote:
> > On Thu, Jul 21, 2022 at 12:31:13PM +1000, Dave Chinner wrote:
> >
> > > However, I don't use that anymore, because updating the cover letter
> > > as I work on the series turned out to be a PITA.
> >
> > Can you detail how it was painful?
>
> Take a 20 patch series in a topic branch and work through the review
> comments one patch at a time, updating the changelog in the
> description commit as you go. Change first patch, commit. Update
> changelog, rebase series. Change second patch, commit. Update
> changelog, rebase series...

I use "git commit --squash" for each change I make, adding the
vN+1 changelog to the description.

When ready to submit, I run "rebase -i" to fold all updates into
the original commits, and update their descriptions.

> Now do that a couple of times a week for a dozen topic branches that
> contain 100+ patches across different lines of development with
> various inter-dependencies. Upstream tree changes trigger this
> update pain, as does testing finding bugs in random patches
> somewhere in the middle of the combined stack of topic branches.

Unless you start sharing topic branches, it might be easier to have
just a single branch to rebase, containing all WIP series.

> It's far simpler to have the topic branch description and changelog
> in a file/context that is completely independent of the commit chain
> that represents the series - it can remain open in an editor in a
> separate terminal as the rebase/rework/bug fix is performed and only
> needs to be saved once when the update/rework of the patch series is
> completed.

I only update the cover letter just before resubmission, based on the
"vN1+" descriptions in the individual commits.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: b4 submit ready for beta testing

[Michael S. Tsirkin]

On Sun, Jul 17, 2022 at 12:02:18PM -0400, Konstantin Ryabitsev wrote:
> On Sat, Jul 16, 2022 at 12:55:26PM -0400, James Bottomley wrote:
> > >  So, if you work on a series, send it off, and then pick it up again
> > > two weeks later from a different workstation, that's no good for most
> > > of what b4 submit needs to do. :(
> > 
> > Are you so sure that's the common case.  For me, to connect to an SMTP
> > server, I need to be on a system running my VPN, which is only my
> > laptop.  Thus however much I transfer the series around for testing
> > etc., it always comes back to the same system for submission, so
> > keeping the cover letter in the branch description of that system works
> > just fine.
> 
> Perhaps, but I also have other reasons to like using an empty commit for this.
> For example, it makes it very easy to mark where exactly our series starts.
> It's possible to do this with other tricks like tracking merge-base, using
> tags, etc, but they are a lot more complicated and have other caveats (e.g.
> when using merge-base tricks you have to also track the branch you forked
> from, and when using tags you have to also consider that someone may push the
> branch but not push the tags).

Very much this. Further, I hope down the road it will become possible
to keep serveral series stacked on top of each other.

-- 
MST

Re: b4 submit ready for beta testing

[Mattijs Korpershoek]

Hi Konstantin,

Thank you for the new version. I will use it for v2 of these series:
https://lore.kernel.org/all/20220720-mt8183-keypad-v1-0-ef9fc29dbff4@baylibre.com/

On Wed, Jul 20, 2022 at 14:48, Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:

> Hello, all:
>
> Based on your feedback, I've refactored the submit functionality and it's
> ready for some more poking and testing. Here are the implemented changes:
>
> 1. Pluggable cover letter strategies
>    The goal is to support 3 different cover-letter strategies:
>
>    "commit": same as first implementation with the cover letter stored in an
>    empty commit at the beginning of the series.

I'm still using the "commit" strategy.

>
>    "branch-description": stores the cover letter in the branch description
>    (which really just means the git config file). The cover letter can be
>    edited using the same b4 --cover-letter command, or just with "git branch
>    --edit-description".
>
>    "tag" (not yet implemented): will store the cover letter in tags a-la
>    git-publish.
>
>    The default cover strategy remains "commit", but you can switch using
>    b4.ez-cover-strategy, e.g. in your ~/.gitconfig:
>
>        [b4]
>          ez-cover-strategy = branch-description
>
>   Note, that switching from one to the other for existing tracked series is
>   not supported (and I'm not sure will be). It's also not currently possible
>   to enroll an existing branch into ez-series with the "commit" strategy
>   (because I haven't yet figured out how to insert a random commit into
>   history with git-filter-repo, but I'm sure it's possible).
>
> 2. "b4 submit" command got split into 3 different commands:

While I do agree that these commands make life easier, I don't think
that we should "ez-" as a prefix.

The "ez-" part has, in my opinion, no added value for users. It
assumes that using these commands is so easy that "you can't use it,
you're really incompetent". I think we should use more "neutral" langage
for commands (without judging if it's easy or difficult).

How about:
- b4 series (for creating/handling series. Also includes "send"/"submit")
- b4 trailers (for handling trailer updates)

>
>   - "b4 ez-series" to create/enroll branches and manage cover letters
>
>   - "b4 ez-trailers" for retrieving trailer updates and updating your local
>     branch. Note, that with -F this can run on any branch, not necessarily the
>     branch managed by ez-series. E.g.:
>
>         git checkout arbitrary-branch
>         b4 ez-trailers -us -F [arbitrary-msgid]

Using "b4 trailers" versus "b4 ez-trailers" seems also to document
better that this commands applies on all branches. (not only ez-series branches)

>
>   - "b4 ez-send" for sending the series to the mailing list. It has all the
>     same flags as formerly for "b4 submit", e.g.:
>
>         b4 ez-send --no-auto-to-cc --dry-run

In my opinion, "b4 send" should be part of series because we cannot send
series that were not created with "b4 series".

>
>     There is no more --reroll command, as using ez-send will automatically
>     increment the tracked revision number (unless RESEND was in the prefixes).

This is very useful, especially the cover letter boilerplate update.

>
>     There's still no support on the web submission endpoint, but it's coming
>     in the near future.
>
> Thank you for everyone's feedback, and please continue testing it out. If you
> want to test the dev version of b4, use the same instructions:
>
>> ## Getting the b4 dev version
>> 
>> To install the b4 development version, you will need to run it from the git
>> checkout:
>> 
>>   git clone https://git.kernel.org/pub/scm/utils/b4/b4.git
>>   cd b4
>>   git submodule update --init
>>   alias b4="$PWD/b4.sh"
>> 
>> You can find out more details in the README file. You will also want to make
>> sure that git-filter-repo is installed (either from your distro packages or
>> from pip).
>> 
>> If you're already running b4 from the git checkout, make sure you run "git
>> submodule update" to pull in the latest unreleased patatt version.
>
> Best regards,
> Konstantin

Re: b4 submit ready for beta testing

[Mattijs Korpershoek]

Hi Konstantin,

On Wed, Jul 20, 2022 at 14:48, Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:

> Hello, all:
>
> Based on your feedback, I've refactored the submit functionality and it's
> ready for some more poking and testing. Here are the implemented changes:
>
> 1. Pluggable cover letter strategies
>    The goal is to support 3 different cover-letter strategies:
>
>    "commit": same as first implementation with the cover letter stored in an
>    empty commit at the beginning of the series.
>
>    "branch-description": stores the cover letter in the branch description
>    (which really just means the git config file). The cover letter can be
>    edited using the same b4 --cover-letter command, or just with "git branch
>    --edit-description".
>
>    "tag" (not yet implemented): will store the cover letter in tags a-la
>    git-publish.
>
>    The default cover strategy remains "commit", but you can switch using
>    b4.ez-cover-strategy, e.g. in your ~/.gitconfig:
>
>        [b4]
>          ez-cover-strategy = branch-description
>
>   Note, that switching from one to the other for existing tracked series is
>   not supported (and I'm not sure will be). It's also not currently possible
>   to enroll an existing branch into ez-series with the "commit" strategy
>   (because I haven't yet figured out how to insert a random commit into
>   history with git-filter-repo, but I'm sure it's possible).
>
> 2. "b4 submit" command got split into 3 different commands:
>
>   - "b4 ez-series" to create/enroll branches and manage cover letters
>
>   - "b4 ez-trailers" for retrieving trailer updates and updating your local
>     branch. Note, that with -F this can run on any branch, not necessarily the
>     branch managed by ez-series. E.g.:
>
>         git checkout arbitrary-branch
>         b4 ez-trailers -us -F [arbitrary-msgid]
>
>   - "b4 ez-send" for sending the series to the mailing list. It has all the
>     same flags as formerly for "b4 submit", e.g.:
>
>         b4 ez-send --no-auto-to-cc --dry-run
>
>     There is no more --reroll command, as using ez-send will automatically
>     increment the tracked revision number (unless RESEND was in the prefixes).
>
>     There's still no support on the web submission endpoint, but it's coming
>     in the near future.
>
> Thank you for everyone's feedback, and please continue testing it out. If you

I would like to file another bug around b4 ez-send --dry-run.
When using the --dry-run option, the To: section is not shown for all
patches. In fact, we only see the To: section on the cover letter email.

This is not reproduced when using b4 ez-send -o <folder>.

revision of b4:
  78725ba63b2f ("ez-send: initial support for tagging sent revisions")

git submodule status:
  0eb41be65707a1e156a59fd25ea9824c1a9e95ce patatt (v0.5.0-1-g0eb41be65707)

Steps to reproduce:

## prepare branch/work
$ b4 ez-series -n dry-run -f next-20220725
$ b4 ez-series --edit-cover # to remove EDITME and be able to use --dry-run/-o

## make an edit on a file which is maintained by Matthias Brugger
$ vim drivers/soc/mediatek/mt8167-mmsys.h # make some change
$ git add drivers/soc/mediatek/mt8167-mmsys.h && git commit -s -m "Example change for mt8167-mmsys.h"
$ ./scripts/get_maintainer.pl --m drivers/soc/mediatek/mt8167-mmsys.h
Matthias Brugger <matthias.bgg@gmail.com> (maintainer:ARM/Mediatek SoC support)
[...]

## show dry run
$ b4 ez-send --dry-run --no-sign

## show output in .patch files
$ mkdir dry-run
$ b4 ez-send --no-sign -o dry-run
$ vim dry-run/0001-example-change-for-mt8167-mmsys-h.patch

Is this the expected behaviour?
To me, -o and --dry-run should produce as close as possible results.

Thanks,
Mattijs


> want to test the dev version of b4, use the same instructions:
>
>> ## Getting the b4 dev version
>> 
>> To install the b4 development version, you will need to run it from the git
>> checkout:
>> 
>>   git clone https://git.kernel.org/pub/scm/utils/b4/b4.git
>>   cd b4
>>   git submodule update --init
>>   alias b4="$PWD/b4.sh"
>> 
>> You can find out more details in the README file. You will also want to make
>> sure that git-filter-repo is installed (either from your distro packages or
>> from pip).
>> 
>> If you're already running b4 from the git checkout, make sure you run "git
>> submodule update" to pull in the latest unreleased patatt version.
>
> Best regards,
> Konstantin

Re: b4 submit ready for beta testing

[Paolo Bonzini]

On 7/26/22 10:37, Mattijs Korpershoek wrote:
> The "ez-" part has, in my opinion, no added value for users. It
> assumes that using these commands is so easy that "you can't use it,
> you're really incompetent". I think we should use more "neutral" langage
> for commands (without judging if it's easy or difficult).

I agree, and in fact I'm wondering if the series/send part really 
belongs in b4 (as opposed to a separate git-publish-like tool).  b4 
right now is a lore-specific tool, while series and send apply generally 
to mail-based patch processing.

Paolo

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Tue, Jul 26, 2022 at 03:55:25PM +0200, Paolo Bonzini wrote:
> On 7/26/22 10:37, Mattijs Korpershoek wrote:
> > The "ez-" part has, in my opinion, no added value for users. It
> > assumes that using these commands is so easy that "you can't use it,
> > you're really incompetent". I think we should use more "neutral" langage
> > for commands (without judging if it's easy or difficult).
> 
> I agree, and in fact I'm wondering if the series/send part really belongs in
> b4 (as opposed to a separate git-publish-like tool).  b4 right now is a
> lore-specific tool, while series and send apply generally to mail-based
> patch processing.

No, they really are very much intertwined:

- we rely on lore to retrieve trailer updates
- we use the change-id for series-tracking (via lore lookups)
- the web submission endpoint will be submitting to the lore backend system
  and written to public-inbox

This is why I kept this functionality as part of b4 and not a separate tool.

-K

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Tue, Jul 26, 2022 at 10:37:18AM +0200, Mattijs Korpershoek wrote:
> > 2. "b4 submit" command got split into 3 different commands:
> 
> While I do agree that these commands make life easier, I don't think
> that we should "ez-" as a prefix.
> 
> The "ez-" part has, in my opinion, no added value for users. It
> assumes that using these commands is so easy that "you can't use it,
> you're really incompetent". I think we should use more "neutral" langage
> for commands (without judging if it's easy or difficult).

Yeah, ez- is a gimmick and may not stay. It's easy to type and tab-complete,
though, which is why I went with that (not that tab completion bits are done
for b4 yet).

> How about:
> - b4 series (for creating/handling series. Also includes "send"/"submit")

I think that makes it hard to distinguish from "b4 am/shazam" that also
operate on series. In my mind, the command should indicate that it's
specifically for working on series for submitting them upstream as opposed to
for retrieving series for inclusion into your repos.

Perhaps "b4 prep"?

> - b4 trailers (for handling trailer updates)

Okay, since one of the plans is to add a way for maintainers to quickly send
their own follow-up trailers, e.g.:

    b4 trailers --ack [--trailer 'My-Trailer: Foo Bar <eml@addr.foo>'] <msgid>

this would allow defining keybindings in mutt to quickly submit an "Acked-by"
or any other trailer.

> >         b4 ez-send --no-auto-to-cc --dry-run
> 
> In my opinion, "b4 send" should be part of series because we cannot send
> series that were not created with "b4 series".

It won't be limited to just series, though -- you'll soon be able to send pull
requests as well, and that will accept arbitrary tags. So, I intend to keep it
separate from "prep".

Thanks for your feedback!

-K

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Tue, Jul 26, 2022 at 03:11:07PM +0200, Mattijs Korpershoek wrote:
> I would like to file another bug around b4 ez-send --dry-run.
> When using the --dry-run option, the To: section is not shown for all
> patches. In fact, we only see the To: section on the cover letter email.

Yeah, this is a bug I added recently.
I'll fix it this afternoon after I'm done rebooting the world.

> ## prepare branch/work
> $ b4 ez-series -n dry-run -f next-20220725
> $ b4 ez-series --edit-cover # to remove EDITME and be able to use --dry-run/-o
> 
> ## make an edit on a file which is maintained by Matthias Brugger
> $ vim drivers/soc/mediatek/mt8167-mmsys.h # make some change
> $ git add drivers/soc/mediatek/mt8167-mmsys.h && git commit -s -m "Example change for mt8167-mmsys.h"
> $ ./scripts/get_maintainer.pl --m drivers/soc/mediatek/mt8167-mmsys.h
> Matthias Brugger <matthias.bgg@gmail.com> (maintainer:ARM/Mediatek SoC support)
> [...]
> 
> ## show dry run
> $ b4 ez-send --dry-run --no-sign
> 
> ## show output in .patch files
> $ mkdir dry-run
> $ b4 ez-send --no-sign -o dry-run
> $ vim dry-run/0001-example-change-for-mt8167-mmsys-h.patch
> 
> Is this the expected behaviour?
> To me, -o and --dry-run should produce as close as possible results.

When we use "-o" we operate in the git-format-patch compatibility mode, which
doesn't collect to/cc addresses (this is done at the git-send-email stage).
Perhaps this should be split into two different commands

instead of:

    b4 send --output-dir OUTPUT_DIR

do:

    b4 prep --format-patch OUTPUT_DIR
    b4 send --dry-run [-o OUTPUT_DIR]

and then fix "b4 send -o" to output the same thing as --dry-run, but into
files as opposed to stdout, complete with all the headers as they will be
sent.

I'll implement this change this afternoon.

-K

Re: b4 submit ready for beta testing

[Mattijs Korpershoek]

On Tue, Jul 26, 2022 at 10:27, Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:

> On Tue, Jul 26, 2022 at 10:37:18AM +0200, Mattijs Korpershoek wrote:
>> > 2. "b4 submit" command got split into 3 different commands:
>> 
>> While I do agree that these commands make life easier, I don't think
>> that we should "ez-" as a prefix.
>> 
>> The "ez-" part has, in my opinion, no added value for users. It
>> assumes that using these commands is so easy that "you can't use it,
>> you're really incompetent". I think we should use more "neutral" langage
>> for commands (without judging if it's easy or difficult).
>
> Yeah, ez- is a gimmick and may not stay. It's easy to type and tab-complete,
> though, which is why I went with that (not that tab completion bits are done
> for b4 yet).

Ok. I really think it should be removed because it gives a judgment on
the user. For some folks it's also "easy" to "just use git send-email".

>
>> How about:
>> - b4 series (for creating/handling series. Also includes "send"/"submit")
>
> I think that makes it hard to distinguish from "b4 am/shazam" that also
> operate on series. In my mind, the command should indicate that it's
> specifically for working on series for submitting them upstream as opposed to
> for retrieving series for inclusion into your repos.
>
> Perhaps "b4 prep"?

Understood. series is indeed ambiguous and "prep" or "prepare" seems
more appropriate.

>
>> - b4 trailers (for handling trailer updates)
>
> Okay, since one of the plans is to add a way for maintainers to quickly send
> their own follow-up trailers, e.g.:
>
>     b4 trailers --ack [--trailer 'My-Trailer: Foo Bar <eml@addr.foo>'] <msgid>
>
> this would allow defining keybindings in mutt to quickly submit an "Acked-by"
> or any other trailer.

That looks awesome :)

>
>> >         b4 ez-send --no-auto-to-cc --dry-run
>> 
>> In my opinion, "b4 send" should be part of series because we cannot send
>> series that were not created with "b4 series".
>
> It won't be limited to just series, though -- you'll soon be able to send pull
> requests as well, and that will accept arbitrary tags. So, I intend to keep it
> separate from "prep".

Ok. I did not think about the pull requests part (I'm an
occasional contributor). It makes sense to keep them separated.

>
> Thanks for your feedback!
>
> -K

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Tue, Jul 26, 2022 at 04:54:09PM +0200, Mattijs Korpershoek wrote:
> > I think that makes it hard to distinguish from "b4 am/shazam" that also
> > operate on series. In my mind, the command should indicate that it's
> > specifically for working on series for submitting them upstream as opposed to
> > for retrieving series for inclusion into your repos.
> >
> > Perhaps "b4 prep"?
> 
> Understood. series is indeed ambiguous and "prep" or "prepare" seems
> more appropriate.

Okay, the latest master should have all this work in place:

- b4 prep
- b4 trailers
- b4 send

See --help for each of these subcommands. The only notable change other than
that:

1. `b4 prep --format-patch somedir` will output the current series as patches
   in a format very similar to format-patch (no To/Cc auto-population and
   minimal headers)
2. `b4 send --dry-run -o somedir` will output the messages exactly as they
   would be fed to the SMTP server

You should be able to git pull and continue with your existing series. Thank
you for your feedback -- please let me know if something else can be improved.

-Konstantin

Re: b4 submit ready for beta testing

[Maxime Ripard]

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

Hi,

On Wed, Jul 20, 2022 at 02:48:35PM -0400, Konstantin Ryabitsev wrote:
> Hello, all:
> 
> Based on your feedback, I've refactored the submit functionality and it's
> ready for some more poking and testing. Here are the implemented changes:
> 
> 1. Pluggable cover letter strategies
>    The goal is to support 3 different cover-letter strategies:
> 
>    "commit": same as first implementation with the cover letter stored in an
>    empty commit at the beginning of the series.
> 
>    "branch-description": stores the cover letter in the branch description
>    (which really just means the git config file). The cover letter can be
>    edited using the same b4 --cover-letter command, or just with "git branch
>    --edit-description".
> 
>    "tag" (not yet implemented): will store the cover letter in tags a-la
>    git-publish.
> 
>    The default cover strategy remains "commit", but you can switch using
>    b4.ez-cover-strategy, e.g. in your ~/.gitconfig:
> 
>        [b4]
>          ez-cover-strategy = branch-description
> 
>   Note, that switching from one to the other for existing tracked series is
>   not supported (and I'm not sure will be). It's also not currently possible
>   to enroll an existing branch into ez-series with the "commit" strategy
>   (because I haven't yet figured out how to insert a random commit into
>   history with git-filter-repo, but I'm sure it's possible).
> 
> 2. "b4 submit" command got split into 3 different commands:
> 
>   - "b4 ez-series" to create/enroll branches and manage cover letters
> 
>   - "b4 ez-trailers" for retrieving trailer updates and updating your local
>     branch. Note, that with -F this can run on any branch, not necessarily the
>     branch managed by ez-series. E.g.:
> 
>         git checkout arbitrary-branch
>         b4 ez-trailers -us -F [arbitrary-msgid]
> 
>   - "b4 ez-send" for sending the series to the mailing list. It has all the
>     same flags as formerly for "b4 submit", e.g.:
> 
>         b4 ez-send --no-auto-to-cc --dry-run
> 
>     There is no more --reroll command, as using ez-send will automatically
>     increment the tracked revision number (unless RESEND was in the prefixes).
> 
>     There's still no support on the web submission endpoint, but it's coming
>     in the near future.
> 
> Thank you for everyone's feedback, and please continue testing it out. If you
> want to test the dev version of b4, use the same instructions:

I've started to test the current master (9e95d523c9b9), and there's a
typo in b4/ez.py:

sed -i 's/crtitical/critical/' b4/ez.py

I also have a branch based on an linux-next tag that I'd want to enroll,
but it looks like we can't enroll branches based on arbitrary commits?

Neither the commit ID or the tag seems to work:

$ b4 prep -e 37b355fdaf31
CRITICAL: Could not find common ancestor with 37b355fdaf31

$ b4 prep -e next-20220714
CRITICAL: Could not find common ancestor with next-20220714

Maxime

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

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Thu, Jul 28, 2022 at 06:04:18PM +0200, Maxime Ripard wrote:
> I've started to test the current master (9e95d523c9b9), and there's a
> typo in b4/ez.py:
> 
> sed -i 's/crtitical/critical/' b4/ez.py

Thank you, fixed in the latest master. I'm surprised pycharm didn't complain
about it.

> I also have a branch based on an linux-next tag that I'd want to enroll,
> but it looks like we can't enroll branches based on arbitrary commits?
> 
> Neither the commit ID or the tag seems to work:
> 
> $ b4 prep -e 37b355fdaf31
> CRITICAL: Could not find common ancestor with 37b355fdaf31
> 
> $ b4 prep -e next-20220714
> CRITICAL: Could not find common ancestor with next-20220714

You're right, we should support this using -f. 
I'll work on getting that in.

-K

Re: b4 submit ready for beta testing

[Maxime Ripard]

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

On Thu, Jul 28, 2022 at 12:24:34PM -0400, Konstantin Ryabitsev wrote:
> On Thu, Jul 28, 2022 at 06:04:18PM +0200, Maxime Ripard wrote:
> > I've started to test the current master (9e95d523c9b9), and there's a
> > typo in b4/ez.py:
> > 
> > sed -i 's/crtitical/critical/' b4/ez.py
> 
> Thank you, fixed in the latest master. I'm surprised pycharm didn't complain
> about it.
> 
> > I also have a branch based on an linux-next tag that I'd want to enroll,
> > but it looks like we can't enroll branches based on arbitrary commits?
> > 
> > Neither the commit ID or the tag seems to work:
> > 
> > $ b4 prep -e 37b355fdaf31
> > CRITICAL: Could not find common ancestor with 37b355fdaf31
> > 
> > $ b4 prep -e next-20220714
> > CRITICAL: Could not find common ancestor with next-20220714
> 
> You're right, we should support this using -f. 
> I'll work on getting that in.

I've used b4 prep a bit more, and a few more rough edges:

  * If the repo has untracked files, b4 trailers errors out. Adding
    --untracked-files=no to the args list of git_get_repo_status
    makes it work fine.

  * b4 prep -n will create a branch with the tracking commit last, but
    b4 prep -e will add the commit first.

  * If we have a branch tracked by b4 (and using a tracking commit), and
    we want to enroll another branch based on that first branch, b4 prep
    -e will refuse to enroll it stating that it's already tracked.

    It seems like this is due to is_prep_branch() returning True because
    find_cover_commit() will consider the entire history and not only
    the base..HEAD range.

Thanks again for working on b4
Maxime

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

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Mon, Aug 15, 2022 at 06:17:57PM +0200, Maxime Ripard wrote:
> I've used b4 prep a bit more, and a few more rough edges:

Thank you for reporting your findings!

>   * If the repo has untracked files, b4 trailers errors out. Adding
>     --untracked-files=no to the args list of git_get_repo_status
>     makes it work fine.

Yes, most of the time we'll not care about untracked files for this work. I
added the flag as default setting.

>   * b4 prep -n will create a branch with the tracking commit last, but
>     b4 prep -e will add the commit first.

I assume you set your strategy as tip-commit, right?

Can you give me some further details here? "b4 prep -n" will create a brand
new branch and create the cover letter commit right at the tip, so it's the
only commit past the fork point. Is that not what you're expecting?

>   * If we have a branch tracked by b4 (and using a tracking commit), and
>     we want to enroll another branch based on that first branch, b4 prep
>     -e will refuse to enroll it stating that it's already tracked.

Yes, I have not yet implemented a way to have more complicated topologies than
that for all strategies. I.e. this works:

-A-B-C
    \-B4A0-B4A1-B4A2
    |-B4B0-B4B1-B4B2

But I haven't really tested various gotchas of this yet:

-A-B-C
   \-B4A0-B4A1-B4A2-B4A3
                      \-B4B0-B4B1-B4B2

Which basically means that multi-series are not currently supported -- but
they will be once I get most of the basic kinks out.

Best regards,
Konstantin

Re: b4 submit ready for beta testing

[Maxime Ripard]

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

On Mon, Aug 15, 2022 at 01:05:18PM -0400, Konstantin Ryabitsev wrote:
> On Mon, Aug 15, 2022 at 06:17:57PM +0200, Maxime Ripard wrote:
> >   * b4 prep -n will create a branch with the tracking commit last, but
> >     b4 prep -e will add the commit first.
> 
> I assume you set your strategy as tip-commit, right?
> 
> Can you give me some further details here? "b4 prep -n" will create a brand
> new branch and create the cover letter commit right at the tip, so it's the
> only commit past the fork point. Is that not what you're expecting?

Yeah, I got confused by -f

I was expecting it to create a new b4 branch reusing the current branch
I was on, and adding the tracking commit at the fork point. But I
realized after sending that mail that it wasn't really the point of -f
anyway, so it's more a PEBKAC than anything :)

> >   * If we have a branch tracked by b4 (and using a tracking commit), and
> >     we want to enroll another branch based on that first branch, b4 prep
> >     -e will refuse to enroll it stating that it's already tracked.
> 
> Yes, I have not yet implemented a way to have more complicated topologies than
> that for all strategies. I.e. this works:
> 
> -A-B-C
>     \-B4A0-B4A1-B4A2
>     |-B4B0-B4B1-B4B2
> 
> But I haven't really tested various gotchas of this yet:
> 
> -A-B-C
>    \-B4A0-B4A1-B4A2-B4A3
>                       \-B4B0-B4B1-B4B2
> 
> Which basically means that multi-series are not currently supported -- but
> they will be once I get most of the basic kinks out.

Ack :)

Thanks!
Maxime

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

Re: b4 submit ready for beta testing

[Conor Dooley]

Hey Konstantin,

On 26/07/2022 21:56, Konstantin Ryabitsev wrote:
> On Tue, Jul 26, 2022 at 04:54:09PM +0200, Mattijs Korpershoek wrote:
>>> I think that makes it hard to distinguish from "b4 am/shazam" that also
>>> operate on series. In my mind, the command should indicate that it's
>>> specifically for working on series for submitting them upstream as opposed to
>>> for retrieving series for inclusion into your repos.
>>>
>>> Perhaps "b4 prep"?
>>
>> Understood. series is indeed ambiguous and "prep" or "prepare" seems
>> more appropriate.
> 
> Okay, the latest master should have all this work in place:
> 
> - b4 prep
> - b4 trailers

Apologies if I am missing something that (should) be obvious here.
I am trying to use "b4 trailers" to update a local branch that was
not created using b4. I ran:

b4 trailers -u --trailers-from 20220817201212.990712-1-mail@conchuod.ie

It took 5 minutes before I got any output from b4, so I actually
began writing this email in the meantime as a bug report...
It eventually worked, outputting:
Calculating patch-ids from 16806 commits

Would it be possible to add some sort of "working, this may take some
time" message so that others don't think it is hung like I did?

Delay aside, it worked "perfectly", will use it going forwards, thanks!

Conor.

FWIW:
b4 --version
0.10.0-dev-c53d8

> - b4 send
> 
> See --help for each of these subcommands. The only notable change other than
> that:
> 
> 1. `b4 prep --format-patch somedir` will output the current series as patches
>    in a format very similar to format-patch (no To/Cc auto-population and
>    minimal headers)
> 2. `b4 send --dry-run -o somedir` will output the messages exactly as they
>    would be fed to the SMTP server
> 
> You should be able to git pull and continue with your existing series. Thank
> you for your feedback -- please let me know if something else can be improved.
> 
> -Konstantin
> 

Re: b4 submit ready for beta testing

[Conor Dooley]

On 18/08/2022 20:30, Conor Dooley wrote:
> Hey Konstantin,
> 
> On 26/07/2022 21:56, Konstantin Ryabitsev wrote:
>> On Tue, Jul 26, 2022 at 04:54:09PM +0200, Mattijs Korpershoek wrote:
>>>> I think that makes it hard to distinguish from "b4 am/shazam" that also
>>>> operate on series. In my mind, the command should indicate that it's
>>>> specifically for working on series for submitting them upstream as opposed to
>>>> for retrieving series for inclusion into your repos.
>>>>
>>>> Perhaps "b4 prep"?
>>>
>>> Understood. series is indeed ambiguous and "prep" or "prepare" seems
>>> more appropriate.
>>
>> Okay, the latest master should have all this work in place:
>>
>> - b4 prep
>> - b4 trailers
> 
> Apologies if I am missing something that (should) be obvious here.
> I am trying to use "b4 trailers" to update a local branch that was
> not created using b4. I ran:
> 
> b4 trailers -u --trailers-from 20220817201212.990712-1-mail@conchuod.ie
> 
> It took 5 minutes before I got any output from b4, so I actually
> began writing this email in the meantime as a bug report...
> It eventually worked, outputting:
> Calculating patch-ids from 16806 commits

(realised this may be misleading, it gave an normal output after that)
> 
> Would it be possible to add some sort of "working, this may take some
> time" message so that others don't think it is hung like I did?
> 
> Delay aside, it worked "perfectly", will use it going forwards, thanks!

Hmm maybe less than perfectly. I think I found a bug & have a few things
that I noticed.

Firstly, the option that am has for cover-letter trailers seems like a
it could be a good idea here too. The other thing is that the trailers
got added after my SoB, when I *think* that before is more correct.

I decided to re-run even though I had no updates with to see if that
might reorder my sob with:
b4 trailers -u -s --trailers-from 20220817201212.990712-1-mail@conchuod.ie

I think b4 then decided that all 16806 commits needed to have my SoB
added to them & proceeded to do that even though my series is only a
couple of patches, like so:

  wifi: cfg80211: use strscpy to replace strlcpy
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  wifi: mac80211: make 4addr null frames using min_rate for WDS
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  wifi: mac80211: fix mesh airtime link metric estimating
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  wifi: mac80211_hwsim: Support link channel matching on rx
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  wifi: mac80211: consistently use sdata_dereference()
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  wifi: mac80211: rx: accept link-addressed frames
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  wifi: mac80211: Consider MLO links in offchannel logic
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  wifi: cfg80211: Allow MLO TX with link source address
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  wifi: mac80211: Remove AP SMPS leftovers
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  wifi: mac80211: add an ieee80211_get_link_sband
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  wifi: cfg80211: add API to add/modify/remove a link station
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
^CTraceback (most recent call last):

If there's anything I can do to help, please let me know.
I pushed the branch to:
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ b4-trailers

Thanks again Konstantin,
Conor.

> 
> Conor.
> 
> FWIW:
> b4 --version
> 0.10.0-dev-c53d8
> 
>> - b4 send
>>
>> See --help for each of these subcommands. The only notable change other than
>> that:
>>
>> 1. `b4 prep --format-patch somedir` will output the current series as patches
>>    in a format very similar to format-patch (no To/Cc auto-population and
>>    minimal headers)
>> 2. `b4 send --dry-run -o somedir` will output the messages exactly as they
>>    would be fed to the SMTP server
>>
>> You should be able to git pull and continue with your existing series. Thank
>> you for your feedback -- please let me know if something else can be improved.
>>
>> -Konstantin
>>

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Thu, Aug 18, 2022 at 09:12:54PM +0100, Conor Dooley wrote:
> > Apologies if I am missing something that (should) be obvious here.
> > I am trying to use "b4 trailers" to update a local branch that was
> > not created using b4. I ran:

Thank you for testing it out -- I've only lightly tested this code in the
context of not using a fully-managed branch, so you're probably hitting some
very nice bugs.

> > b4 trailers -u --trailers-from 20220817201212.990712-1-mail@conchuod.ie
> > 
> > It took 5 minutes before I got any output from b4, so I actually
> > began writing this email in the meantime as a bug report...
> > It eventually worked, outputting:
> > Calculating patch-ids from 16806 commits

Ew, this is going horribly wrong already! It's only supposed to find recent
commits where you're the committer (git log --committer=your@email),  so I'm
wondering how it's matching 16806 commits in the first place, unless you've
done some really crazy rebases.

I'll take a look at it first thing tomorrow.

-K

Re: b4 submit ready for beta testing

[Conor Dooley]

On 18/08/2022 22:04, Konstantin Ryabitsev wrote:
> On Thu, Aug 18, 2022 at 09:12:54PM +0100, Conor Dooley wrote:
>>> Apologies if I am missing something that (should) be obvious here.
>>> I am trying to use "b4 trailers" to update a local branch that was
>>> not created using b4. I ran:
> 
> Thank you for testing it out -- I've only lightly tested this code in the
> context of not using a fully-managed branch, so you're probably hitting some
> very nice bugs.

It was fine, a rebase fixed it up again nicely.

> 
>>> b4 trailers -u --trailers-from 20220817201212.990712-1-mail@conchuod.ie
>>>
>>> It took 5 minutes before I got any output from b4, so I actually
>>> began writing this email in the meantime as a bug report...
>>> It eventually worked, outputting:
>>> Calculating patch-ids from 16806 commits
> 
> Ew, this is going horribly wrong already! It's only supposed to find recent
> commits where you're the committer (git log --committer=your@email),  so I'm
> wondering how it's matching 16806 commits in the first place, unless you've
> done some really crazy rebases.

Possibly I rebased the original branch on -rc1, but I wouldn't count
that as crazy. Committer should be Linus. I supplied --base=v6.0-rc1 to
format-patch.

Hmm I wonder if it might be because I am sending mail from mail@conchuod.ie
but my committer/author email is conor.dooley@microchip.com?

> 
> I'll take a look at it first thing tomorrow.


Whenever you get a chance :)
Thanks,
Conor.

Re: b4 submit ready for beta testing

[Konstantin Ryabitsev]

On Thu, Aug 18, 2022 at 08:30:03PM +0100, Conor Dooley wrote:
> Apologies if I am missing something that (should) be obvious here.
> I am trying to use "b4 trailers" to update a local branch that was
> not created using b4. I ran:
> 
> b4 trailers -u --trailers-from 20220817201212.990712-1-mail@conchuod.ie
> 
> It took 5 minutes before I got any output from b4, so I actually
> began writing this email in the meantime as a bug report...
> It eventually worked, outputting:
> Calculating patch-ids from 16806 commits

I found the problem that was causing this to happen -- there was an off-by-one
logic bug when we were looking at the non-contiguous ranges. In your
particular case, we were running:

	$ git --no-pager log -F --committer=conor.dooley@microchip.com --since=3.months --format="%H %P" --no-merges
	2e80b31e5fda2503b96cf810c827fd22eef71a3c 31b809d2e455925a3d13ef4bfed62dac13ba45be
	31b809d2e455925a3d13ef4bfed62dac13ba45be adbeff44dcd5989a6508dfc3aa1c7f549bdea9de
	adbeff44dcd5989a6508dfc3aa1c7f549bdea9de e620e1db41c2d5a9024b12d24db1e86fa55e3fe6
	e620e1db41c2d5a9024b12d24db1e86fa55e3fe6 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
	88d319c6abaeb37f0e2323275eaf57a8388e0265 3f8ccf5f1a8c349364055b73caf1bb0314229976
	efa310ba00716d7a872bdc5fa1f5545edc9efd69 5e757deddd918edb8cb2fdb56eb79656ffc6dade
	3f8ccf5f1a8c349364055b73caf1bb0314229976 294b2111ff1027a942c362d523b847d73d91b70c
	5e757deddd918edb8cb2fdb56eb79656ffc6dade f2906aa863381afb0015a9eb7fefad885d4e5a56

The goal is to find the most recent contiguous set of your own commits and use
that as the base for lookups. In the above output, the break was here:


	2e80b31e5fda2503b96cf810c827fd22eef71a3c 31b809d2e455925a3d13ef4bfed62dac13ba45be
	31b809d2e455925a3d13ef4bfed62dac13ba45be adbeff44dcd5989a6508dfc3aa1c7f549bdea9de
	adbeff44dcd5989a6508dfc3aa1c7f549bdea9de e620e1db41c2d5a9024b12d24db1e86fa55e3fe6
	e620e1db41c2d5a9024b12d24db1e86fa55e3fe6 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
    -- 8< --
	88d319c6abaeb37f0e2323275eaf57a8388e0265 3f8ccf5f1a8c349364055b73caf1bb0314229976
	efa310ba00716d7a872bdc5fa1f5545edc9efd69 5e757deddd918edb8cb2fdb56eb79656ffc6dade
	3f8ccf5f1a8c349364055b73caf1bb0314229976 294b2111ff1027a942c362d523b847d73d91b70c
	5e757deddd918edb8cb2fdb56eb79656ffc6dade f2906aa863381afb0015a9eb7fefad885d4e5a56

When I tested, there was only the single range of commits, which is why it
worked in my tests. However, in your case there are two sets of contiguous
commits, and we were using the wrong one as the starting commit (the next in
line). So, instead of a nice selection of just 4 commits, which we wanted:

    $ git rev-list e620e1db41c2d5a9024b12d24db1e86fa55e3fe6~1..2e80b31e5fda2503b96cf810c827fd22eef71a3c | wc -l
    4

We ended up doing this:

	$ git rev-list 88d319c6abaeb37f0e2323275eaf57a8388e0265~1..2e80b31e5fda2503b96cf810c827fd22eef71a3c | wc -l
	16806

And then calculating patch-ids on each one of those. I'm shocked it only took 5
minutes.

The fix is in master.

-K

Re: b4 submit ready for beta testing

[Conor Dooley]

On 19/08/2022 21:43, Konstantin Ryabitsev wrote:
> On Thu, Aug 18, 2022 at 08:30:03PM +0100, Conor Dooley wrote:
>> Apologies if I am missing something that (should) be obvious here.
>> I am trying to use "b4 trailers" to update a local branch that was
>> not created using b4. I ran:
>>
>> b4 trailers -u --trailers-from 20220817201212.990712-1-mail@conchuod.ie
>>
>> It took 5 minutes before I got any output from b4, so I actually
>> began writing this email in the meantime as a bug report...
>> It eventually worked, outputting:
>> Calculating patch-ids from 16806 commits
> 
> I found the problem that was causing this to happen -- there was an off-by-one
> logic bug when we were looking at the non-contiguous ranges. In your
> particular case, we were running:
> 
> 	$ git --no-pager log -F --committer=conor.dooley@microchip.com --since=3.months --format="%H %P" --no-merges
> 	2e80b31e5fda2503b96cf810c827fd22eef71a3c 31b809d2e455925a3d13ef4bfed62dac13ba45be
> 	31b809d2e455925a3d13ef4bfed62dac13ba45be adbeff44dcd5989a6508dfc3aa1c7f549bdea9de
> 	adbeff44dcd5989a6508dfc3aa1c7f549bdea9de e620e1db41c2d5a9024b12d24db1e86fa55e3fe6
> 	e620e1db41c2d5a9024b12d24db1e86fa55e3fe6 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
> 	88d319c6abaeb37f0e2323275eaf57a8388e0265 3f8ccf5f1a8c349364055b73caf1bb0314229976
> 	efa310ba00716d7a872bdc5fa1f5545edc9efd69 5e757deddd918edb8cb2fdb56eb79656ffc6dade
> 	3f8ccf5f1a8c349364055b73caf1bb0314229976 294b2111ff1027a942c362d523b847d73d91b70c
> 	5e757deddd918edb8cb2fdb56eb79656ffc6dade f2906aa863381afb0015a9eb7fefad885d4e5a56
> 
> The goal is to find the most recent contiguous set of your own commits and use
> that as the base for lookups. In the above output, the break was here:
> 
> 
> 	2e80b31e5fda2503b96cf810c827fd22eef71a3c 31b809d2e455925a3d13ef4bfed62dac13ba45be
> 	31b809d2e455925a3d13ef4bfed62dac13ba45be adbeff44dcd5989a6508dfc3aa1c7f549bdea9de
> 	adbeff44dcd5989a6508dfc3aa1c7f549bdea9de e620e1db41c2d5a9024b12d24db1e86fa55e3fe6
> 	e620e1db41c2d5a9024b12d24db1e86fa55e3fe6 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
>     -- 8< --
> 	88d319c6abaeb37f0e2323275eaf57a8388e0265 3f8ccf5f1a8c349364055b73caf1bb0314229976
> 	efa310ba00716d7a872bdc5fa1f5545edc9efd69 5e757deddd918edb8cb2fdb56eb79656ffc6dade
> 	3f8ccf5f1a8c349364055b73caf1bb0314229976 294b2111ff1027a942c362d523b847d73d91b70c
> 	5e757deddd918edb8cb2fdb56eb79656ffc6dade f2906aa863381afb0015a9eb7fefad885d4e5a56

Right, these are all committed by me & included via PRs.

> 
> When I tested, there was only the single range of commits, which is why it
> worked in my tests. However, in your case there are two sets of contiguous
> commits,
> and we were using the wrong one as the starting commit (the next in
> line). So, instead of a nice selection of just 4 commits, which we wanted:
> 
>     $ git rev-list e620e1db41c2d5a9024b12d24db1e86fa55e3fe6~1..2e80b31e5fda2503b96cf810c827fd22eef71a3c | wc -l
>     4
> 
> We ended up doing this:
> 
> 	$ git rev-list 88d319c6abaeb37f0e2323275eaf57a8388e0265~1..2e80b31e5fda2503b96cf810c827fd22eef71a3c | wc -l
> 	16806
> 
> And then calculating patch-ids on each one of those. I'm shocked it only took 5
> minutes.

This is not even an especially great machine either, just my home server with a
couple generations old i5 etc.

> 
> The fix is in master.

Sweet, thanks!
I'll give it a whirl & let you know if I run into anything else.
Conor.