All of lore.kernel.org
 help / color / mirror / Atom feed
* b4: introducing b4 shazam (like b4 am -o- | git am)
@ 2021-09-21 20:25 Konstantin Ryabitsev
  2021-09-22 17:38 ` Jason Gunthorpe
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Konstantin Ryabitsev @ 2021-09-21 20:25 UTC (permalink / raw)
  To: tools, users

Hi, all:

The current master branch landed a new feature that implements a new
subcommand "shazam". It is the functional equivalent of piping "b4 am" to "git
am", with some extra safety padding.

See "b4 shazam --help" to see the available options. You will notice that many
of them are exactly the same as for "b4 am".

The default operation still tries to be the safest possible:

- we'll create a temporary sparse worktree that will only contain the files
  being modified
- we'll use the base-commit sha, if we find it, or we'll try to guess the base
  commit from git index info
- we'll run "git am" against that temporary worktree
- if successful, we'll fetch from the worktree into your current repo and
  delete the temporary sparse checkout
- you can then "git checkout -b foo FETCH_HEAD" or "git merge FETCH_HEAD"
  or "git rebase" as necessary

Alternatively, if you run "git shazam -A", we'll just run "git am" on the
current HEAD, which is the exact equivalent of "b4 am -o- | git am".

This feature is fairly "raw" and lightly tested, so please expect things to
potentially blow up all over the place. :)

If the default operation with getting things ready in FETCH_HEAD is not
useful, please let me know how you would prefer to see things work instead.

Best regards,
-K

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-21 20:25 b4: introducing b4 shazam (like b4 am -o- | git am) Konstantin Ryabitsev
@ 2021-09-22 17:38 ` Jason Gunthorpe
  2021-09-22 18:21   ` Konstantin Ryabitsev
  2021-09-23  8:51 ` Geert Uytterhoeven
  2021-10-19 14:49 ` Rob Herring
  2 siblings, 1 reply; 17+ messages in thread
From: Jason Gunthorpe @ 2021-09-22 17:38 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools, users

On Tue, Sep 21, 2021 at 04:25:26PM -0400, Konstantin Ryabitsev wrote:
> Hi, all:
> 
> The current master branch landed a new feature that implements a new
> subcommand "shazam". It is the functional equivalent of piping "b4 am" to "git
> am", with some extra safety padding.
> 
> See "b4 shazam --help" to see the available options. You will notice that many
> of them are exactly the same as for "b4 am".
> 
> The default operation still tries to be the safest possible:
> 
> - we'll create a temporary sparse worktree that will only contain the files
>   being modified
> - we'll use the base-commit sha, if we find it, or we'll try to guess the base
>   commit from git index info
> - we'll run "git am" against that temporary worktree
> - if successful, we'll fetch from the worktree into your current repo and
>   delete the temporary sparse checkout
> - you can then "git checkout -b foo FETCH_HEAD" or "git merge FETCH_HEAD"
>   or "git rebase" as necessary

Does it prep the merge message too? ie pull in the text from the cover
letter, make the ==== markers/etc? (ie look what davem does:
d1bf73387b5adbc12c6a59c1fbaa69e05ee265ed)

> Alternatively, if you run "git shazam -A", we'll just run "git am" on the
> current HEAD, which is the exact equivalent of "b4 am -o- | git am".

It might be interesting to detect which to do based on what the
submitter has done.. No useful base-commit = no merge?

I'm generally of the feeling that git is a VCS and it is completely
fine for a tool to do whatever to the tree because I can quickly undo
it. So I think b4 is possibly overly cautious here :)

> If the default operation with getting things ready in FETCH_HEAD is not
> useful, please let me know how you would prefer to see things work instead.

I would be nice to have some process guidance when maintainers should
be using merges and when am'd patches..

Jason

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-22 17:38 ` Jason Gunthorpe
@ 2021-09-22 18:21   ` Konstantin Ryabitsev
  2021-09-28 17:46     ` Jason Gunthorpe
  0 siblings, 1 reply; 17+ messages in thread
From: Konstantin Ryabitsev @ 2021-09-22 18:21 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: tools, users

On Wed, Sep 22, 2021 at 02:38:03PM -0300, Jason Gunthorpe wrote:
> > - you can then "git checkout -b foo FETCH_HEAD" or "git merge FETCH_HEAD"
> >   or "git rebase" as necessary
> 
> Does it prep the merge message too? ie pull in the text from the cover
> letter, make the ==== markers/etc? (ie look what davem does:
> d1bf73387b5adbc12c6a59c1fbaa69e05ee265ed)

We could, theoretically, do that, but you'd have to remember to pass the -F
--edit flags to "git merge". My concern is mostly that cover letters are very
much free-form and there isn't a sane approach we can take to automatically
generate good merge commit messages from them, short of quoting them fully,
which will be overkill 9 times out of 10.

One thing I will probably do is edit .git/FETCH_HEAD to change the unhelpful
/tmp/tmpfoobar to say something like:

    [sha]\t\tpatch series from https://lore.kernel.org/r/msgid

This way the default "git merge" message will contain a clickable link for the
person doing the merge where they can see the cover letter and maybe
copy-paste from it.

> > Alternatively, if you run "git shazam -A", we'll just run "git am" on the
> > current HEAD, which is the exact equivalent of "b4 am -o- | git am".
> 
> It might be interesting to detect which to do based on what the
> submitter has done.. No useful base-commit = no merge?

This is where I have to defer to feedback. I expect it would depend on the
size of the series -- for smaller ones people will probably want to rebase and
not merge?

> I'm generally of the feeling that git is a VCS and it is completely
> fine for a tool to do whatever to the tree because I can quickly undo
> it. So I think b4 is possibly overly cautious here :)

Well, doing it via FETCH_HEAD was also an easy way to use base-commit info
(whether real or deduced) without asking for things like "what would you like
the branch to be named" etc.

> > If the default operation with getting things ready in FETCH_HEAD is not
> > useful, please let me know how you would prefer to see things work instead.
> 
> I would be nice to have some process guidance when maintainers should
> be using merges and when am'd patches..

I'll be happy to consider any feedback here.

-K

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-21 20:25 b4: introducing b4 shazam (like b4 am -o- | git am) Konstantin Ryabitsev
  2021-09-22 17:38 ` Jason Gunthorpe
@ 2021-09-23  8:51 ` Geert Uytterhoeven
  2021-09-24 21:08   ` Konstantin Ryabitsev
  2021-10-19 14:49 ` Rob Herring
  2 siblings, 1 reply; 17+ messages in thread
From: Geert Uytterhoeven @ 2021-09-23  8:51 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools, users

Hi Konstantin,

On Tue, Sep 21, 2021 at 10:25 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
> The current master branch landed a new feature that implements a new
> subcommand "shazam". It is the functional equivalent of piping "b4 am" to "git
> am", with some extra safety padding.
>
> See "b4 shazam --help" to see the available options. You will notice that many
> of them are exactly the same as for "b4 am".

Thanks for this new tool!

> The default operation still tries to be the safest possible:
>
> - we'll create a temporary sparse worktree that will only contain the files
>   being modified
> - we'll use the base-commit sha, if we find it, or we'll try to guess the base
>   commit from git index info
> - we'll run "git am" against that temporary worktree
> - if successful, we'll fetch from the worktree into your current repo and
>   delete the temporary sparse checkout
> - you can then "git checkout -b foo FETCH_HEAD" or "git merge FETCH_HEAD"
>   or "git rebase" as necessary

[...]
Magic: Preparing a sparse worktree
Error running sparse-checkout init
fatal: Unable to create
'.git/worktrees/tmpz9m4cwww/info/sparse-checkout.lock': No such file
or directory

According to "git help sparse-checkout", my git (2.25.1, Ubuntu
20.04.3LTS) does have support for that command.

> Alternatively, if you run "git shazam -A", we'll just run "git am" on the
> current HEAD, which is the exact equivalent of "b4 am -o- | git am".

It does work with -A.

Note that with -s, the Link: tag appears below my Sob, while
b4 am + git am -s adds my Sob below the Link: tag.

Gr{oetje,eeting}s,

                        Geert

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

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

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-23  8:51 ` Geert Uytterhoeven
@ 2021-09-24 21:08   ` Konstantin Ryabitsev
  2021-09-28 18:22     ` Geert Uytterhoeven
  0 siblings, 1 reply; 17+ messages in thread
From: Konstantin Ryabitsev @ 2021-09-24 21:08 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: tools, users

On Thu, Sep 23, 2021 at 10:51:21AM +0200, Geert Uytterhoeven wrote:
> [...]
> Magic: Preparing a sparse worktree
> Error running sparse-checkout init
> fatal: Unable to create
> '.git/worktrees/tmpz9m4cwww/info/sparse-checkout.lock': No such file
> or directory

Interesting. Are you already in a detached worktree, perhaps?

> According to "git help sparse-checkout", my git (2.25.1, Ubuntu
> 20.04.3LTS) does have support for that command.

I suggest you repeat the following manually:

    git worktree add --detach --no-checkout /tmp/foofoo
    pushd /tmp/foofoo
    git sparse-checkout ini
    git checkout

then, regardless of what happens above:

    popd
    git worktree remove /tmp/foofoo

That's basically all we're doing.

> Note that with -s, the Link: tag appears below my Sob, while
> b4 am + git am -s adds my Sob below the Link: tag.

Unfortunately, I can't control this beyond reversing this for everyone. The
only other alternative is setting up b4.shazam.am-flags variable for what to
pass to "git am" -- which may be a good idea anyway.

-K

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-22 18:21   ` Konstantin Ryabitsev
@ 2021-09-28 17:46     ` Jason Gunthorpe
  2021-09-29 21:18       ` Konstantin Ryabitsev
  0 siblings, 1 reply; 17+ messages in thread
From: Jason Gunthorpe @ 2021-09-28 17:46 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools, users

On Wed, Sep 22, 2021 at 02:21:54PM -0400, Konstantin Ryabitsev wrote:
> On Wed, Sep 22, 2021 at 02:38:03PM -0300, Jason Gunthorpe wrote:
> > > - you can then "git checkout -b foo FETCH_HEAD" or "git merge FETCH_HEAD"
> > >   or "git rebase" as necessary
> > 
> > Does it prep the merge message too? ie pull in the text from the cover
> > letter, make the ==== markers/etc? (ie look what davem does:
> > d1bf73387b5adbc12c6a59c1fbaa69e05ee265ed)
> 
> We could, theoretically, do that, but you'd have to remember to pass the -F
> much free-form and there isn't a sane approach we can take to automatically
> generate good merge commit messages from them, short of quoting them fully,
> which will be overkill 9 times out of 10.

Since the user would be expected to revise it, I think that is
fine. Just make the template and push in the raw text and let people
edit. It would still save a bunch of time

> > It might be interesting to detect which to do based on what the
> > submitter has done.. No useful base-commit = no merge?
> 
> This is where I have to defer to feedback. I expect it would depend on the
> size of the series -- for smaller ones people will probably want to rebase and
> not merge?

Linus has been repeatedly against working on random git history
points, so I think the unreliable auto-base is not something people
should be using.

It should only check a set of safe commits (like rc releases and the
maintainer's work-in-progress) and everything else should be a
failure..

> > > If the default operation with getting things ready in FETCH_HEAD is not
> > > useful, please let me know how you would prefer to see things work instead.
> > 
> > I would be nice to have some process guidance when maintainers should
> > be using merges and when am'd patches..
> 
> I'll be happy to consider any feedback here.

Me too :)

I feel that if b4 could maintain with high fidelity "what the
submitter tested" then a merge would be the appropriate thing,
otherwise a rolling 'am'..

Jason

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-24 21:08   ` Konstantin Ryabitsev
@ 2021-09-28 18:22     ` Geert Uytterhoeven
  2021-09-29 13:39       ` Rob Herring
  2021-09-29 14:27       ` Konstantin Ryabitsev
  0 siblings, 2 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2021-09-28 18:22 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools, users

Hi Konstantin,

On Fri, Sep 24, 2021 at 11:08 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
> On Thu, Sep 23, 2021 at 10:51:21AM +0200, Geert Uytterhoeven wrote:
> > [...]
> > Magic: Preparing a sparse worktree
> > Error running sparse-checkout init
> > fatal: Unable to create
> > '.git/worktrees/tmpz9m4cwww/info/sparse-checkout.lock': No such file
> > or directory
>
> Interesting. Are you already in a detached worktree, perhaps?
>
> > According to "git help sparse-checkout", my git (2.25.1, Ubuntu
> > 20.04.3LTS) does have support for that command.
>
> I suggest you repeat the following manually:
>
>     git worktree add --detach --no-checkout /tmp/foofoo
>     pushd /tmp/foofoo
>     git sparse-checkout ini

s/ini/init/

Fails with:

    fatal: Unable to create
'/path/to/repo/.git/worktrees/foofoo/info/sparse-checkout.lock': No
such file or directory

The info dir is missing:

    $ ls -l /path/to/repo/.git/worktrees/foofoo/
    total 20
    -rw-rw-r-- 1 geert geert    6 sep 28 20:14 commondir
    -rw-rw-r-- 1 geert geert   30 sep 28 20:14 config.worktree
    -rw-rw-r-- 1 geert geert   17 sep 28 20:14 gitdir
    -rw-rw-r-- 1 geert geert   41 sep 28 20:14 HEAD
    drwxrwsr-x 2 geert geert 4096 sep 28 20:14 logs/

>     git checkout
>
> then, regardless of what happens above:
>
>     popd
>     git worktree remove /tmp/foofoo

needs --force, else it fails with:

    fatal: '/tmp/foofoo' contains modified or untracked files, use
--force to delete it

> That's basically all we're doing.
>
> > Note that with -s, the Link: tag appears below my Sob, while
> > b4 am + git am -s adds my Sob below the Link: tag.
>
> Unfortunately, I can't control this beyond reversing this for everyone. The
> only other alternative is setting up b4.shazam.am-flags variable for what to
> pass to "git am" -- which may be a good idea anyway.

The current behavior of b4 shazam does obfuscate the patch trail.

Thanks!

Gr{oetje,eeting}s,

                        Geert

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

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

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-28 18:22     ` Geert Uytterhoeven
@ 2021-09-29 13:39       ` Rob Herring
  2021-09-29 17:54         ` Geert Uytterhoeven
  2021-09-29 18:07         ` Jason Gunthorpe
  2021-09-29 14:27       ` Konstantin Ryabitsev
  1 sibling, 2 replies; 17+ messages in thread
From: Rob Herring @ 2021-09-29 13:39 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Konstantin Ryabitsev, tools, users

On Tue, Sep 28, 2021 at 1:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Konstantin,
>
> On Fri, Sep 24, 2021 at 11:08 PM Konstantin Ryabitsev
> <konstantin@linuxfoundation.org> wrote:
> > On Thu, Sep 23, 2021 at 10:51:21AM +0200, Geert Uytterhoeven wrote:
> > > [...]
> > > Magic: Preparing a sparse worktree
> > > Error running sparse-checkout init
> > > fatal: Unable to create
> > > '.git/worktrees/tmpz9m4cwww/info/sparse-checkout.lock': No such file
> > > or directory
> >
> > Interesting. Are you already in a detached worktree, perhaps?
> >
> > > According to "git help sparse-checkout", my git (2.25.1, Ubuntu
> > > 20.04.3LTS) does have support for that command.
> >
> > I suggest you repeat the following manually:
> >
> >     git worktree add --detach --no-checkout /tmp/foofoo
> >     pushd /tmp/foofoo
> >     git sparse-checkout ini
>
> s/ini/init/
>
> Fails with:
>
>     fatal: Unable to create
> '/path/to/repo/.git/worktrees/foofoo/info/sparse-checkout.lock': No
> such file or directory
>
> The info dir is missing:
>
>     $ ls -l /path/to/repo/.git/worktrees/foofoo/
>     total 20
>     -rw-rw-r-- 1 geert geert    6 sep 28 20:14 commondir
>     -rw-rw-r-- 1 geert geert   30 sep 28 20:14 config.worktree
>     -rw-rw-r-- 1 geert geert   17 sep 28 20:14 gitdir
>     -rw-rw-r-- 1 geert geert   41 sep 28 20:14 HEAD
>     drwxrwsr-x 2 geert geert 4096 sep 28 20:14 logs/
>
> >     git checkout
> >
> > then, regardless of what happens above:
> >
> >     popd
> >     git worktree remove /tmp/foofoo
>
> needs --force, else it fails with:
>
>     fatal: '/tmp/foofoo' contains modified or untracked files, use
> --force to delete it
>
> > That's basically all we're doing.
> >
> > > Note that with -s, the Link: tag appears below my Sob, while
> > > b4 am + git am -s adds my Sob below the Link: tag.
> >
> > Unfortunately, I can't control this beyond reversing this for everyone. The
> > only other alternative is setting up b4.shazam.am-flags variable for what to
> > pass to "git am" -- which may be a good idea anyway.
>
> The current behavior of b4 shazam does obfuscate the patch trail.

Is there a defined right order for Link and committer's Sob? Either we
have to define what that is for b4 to implement or b4 has to be
configurable. If the committer != author, then Link followed by Sob
makes sense. But what about committing your own patches? If you
strictly follow the patch trail, you'd end up with your Sob twice:
Sob, Link, Sob. In these cases, I've been putting Link last and a
single Sob.

Rob

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-28 18:22     ` Geert Uytterhoeven
  2021-09-29 13:39       ` Rob Herring
@ 2021-09-29 14:27       ` Konstantin Ryabitsev
  1 sibling, 0 replies; 17+ messages in thread
From: Konstantin Ryabitsev @ 2021-09-29 14:27 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: tools, users

On Tue, Sep 28, 2021 at 08:22:59PM +0200, Geert Uytterhoeven wrote:
> > I suggest you repeat the following manually:
> >
> >     git worktree add --detach --no-checkout /tmp/foofoo
> >     pushd /tmp/foofoo
> >     git sparse-checkout ini
> 
> s/ini/init/
> 
> Fails with:
> 
>     fatal: Unable to create
> '/path/to/repo/.git/worktrees/foofoo/info/sparse-checkout.lock': No
> such file or directory
> 
> The info dir is missing:
> 
>     $ ls -l /path/to/repo/.git/worktrees/foofoo/
>     total 20
>     -rw-rw-r-- 1 geert geert    6 sep 28 20:14 commondir
>     -rw-rw-r-- 1 geert geert   30 sep 28 20:14 config.worktree
>     -rw-rw-r-- 1 geert geert   17 sep 28 20:14 gitdir
>     -rw-rw-r-- 1 geert geert   41 sep 28 20:14 HEAD
>     drwxrwsr-x 2 geert geert 4096 sep 28 20:14 logs/

This should work, so my guess is that sparse-checkout functionality has
changed/improved in versions > 2.25. It's still marked as "EXPERIMENTAL" so it
is not entirely unexpected.

It's too bad that I can't reliably expect it to work, because it helps to
dramatically speed things up when we don't need to checkout the entire tree to
try to apply the patch.

I think I'm going to change the default shazam behaviour to do --apply-here by
default, and do FETCH_HEAD/merge magic when requested.

Would that make more sense?

-K

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-29 13:39       ` Rob Herring
@ 2021-09-29 17:54         ` Geert Uytterhoeven
  2021-09-29 18:07         ` Jason Gunthorpe
  1 sibling, 0 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2021-09-29 17:54 UTC (permalink / raw)
  To: Rob Herring; +Cc: Konstantin Ryabitsev, tools, users

Hi Rob,

On Wed, Sep 29, 2021 at 3:39 PM Rob Herring <robh@kernel.org> wrote:
> On Tue, Sep 28, 2021 at 1:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Sep 24, 2021 at 11:08 PM Konstantin Ryabitsev
> > <konstantin@linuxfoundation.org> wrote:
> > > On Thu, Sep 23, 2021 at 10:51:21AM +0200, Geert Uytterhoeven wrote:
> > > > Note that with -s, the Link: tag appears below my Sob, while
> > > > b4 am + git am -s adds my Sob below the Link: tag.
> > >
> > > Unfortunately, I can't control this beyond reversing this for everyone. The
> > > only other alternative is setting up b4.shazam.am-flags variable for what to
> > > pass to "git am" -- which may be a good idea anyway.
> >
> > The current behavior of b4 shazam does obfuscate the patch trail.
>
> Is there a defined right order for Link and committer's Sob? Either we
> have to define what that is for b4 to implement or b4 has to be
> configurable. If the committer != author, then Link followed by Sob
> makes sense. But what about committing your own patches? If you

Yes.

> strictly follow the patch trail, you'd end up with your Sob twice:
> Sob, Link, Sob. In these cases, I've been putting Link last and a
> single Sob.

Same here.  I do not add a duplicate SoB in that case, and the Link
tag will be last.

BTW, I also do not add my own Acked-by and Reviewed-by tags
to patches it commit myself.

Gr{oetje,eeting}s,

                        Geert

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

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

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-29 13:39       ` Rob Herring
  2021-09-29 17:54         ` Geert Uytterhoeven
@ 2021-09-29 18:07         ` Jason Gunthorpe
  1 sibling, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2021-09-29 18:07 UTC (permalink / raw)
  To: Rob Herring; +Cc: Geert Uytterhoeven, Konstantin Ryabitsev, tools, users

On Wed, Sep 29, 2021 at 08:39:25AM -0500, Rob Herring wrote:

> Is there a defined right order for Link and committer's Sob? Either we
> have to define what that is for b4 to implement or b4 has to be
> configurable. If the committer != author, then Link followed by Sob
> makes sense. But what about committing your own patches? If you
> strictly follow the patch trail, you'd end up with your Sob twice:
> Sob, Link, Sob. In these cases, I've been putting Link last and a
> single Sob.

I've been deliberately sorting things to be 

Cc: stable
Fixes: xx
Link: xx
<everything else in chronological order>

Ie I think the cc/fixes information is important and should be
highlighted at the top and the link is sort of a breakpoint indicating
that the following is chronological providence information.

I see lots of other things too. Personally I think b4 should push a
reasonable opinion for the colour of this bike shed..

Jason

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-28 17:46     ` Jason Gunthorpe
@ 2021-09-29 21:18       ` Konstantin Ryabitsev
  2021-09-29 23:30         ` Jason Gunthorpe
  2021-10-01 16:20         ` Mark Brown
  0 siblings, 2 replies; 17+ messages in thread
From: Konstantin Ryabitsev @ 2021-09-29 21:18 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: tools, users

On Tue, Sep 28, 2021 at 02:46:53PM -0300, Jason Gunthorpe wrote:
> > We could, theoretically, do that, but you'd have to remember to pass the -F
> > much free-form and there isn't a sane approach we can take to automatically
> > generate good merge commit messages from them, short of quoting them fully,
> > which will be overkill 9 times out of 10.
> 
> Since the user would be expected to revise it, I think that is
> fine. Just make the template and push in the raw text and let people
> edit. It would still save a bunch of time

Okay, I implemented something that should be handy. When we find a cover
letter, we will prepare it as a template merge message in .git/b4-cover, so
you can do stuff like:

git merge -F .git/b4-cover --edit FETCH_HEAD

I will eventually make the template configurable, but for now it's hardcoded
to the following format (using a sample I've been working with):

    Merge patches from https://lore.kernel.org/r/20210929143600.49379-1-david@redhat.com

    Accept 6 patches from David Hildenbrand <david@redhat.com>

    mm/memory_hotplug: Kconfig and 32 bit cleanups
    ==============================================
    Some cleanups around CONFIG_MEMORY_HOTPLUG, including removing 32 bit
    leftovers of memory hotplug support.

    Compile-tested on various architectures, quickly tested memory hotplug
    on x86-64.
    ==============================================
    Link: https://lore.kernel.org/r/20210929143600.49379-1-david@redhat.com

Note, that this is after deleting a bunch of stuff during the --edit stage, as
we reuse everything that's not a signature.

This change is in the latest master.

> Linus has been repeatedly against working on random git history
> points, so I think the unreliable auto-base is not something people
> should be using.

I see. But if they do send a base-commit: line from some place random, is that
okay to accept it?

> It should only check a set of safe commits (like rc releases and the
> maintainer's work-in-progress) and everything else should be a
> failure..

I'll see if I can have a --tags-only mode for guessing base.

-K

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-29 21:18       ` Konstantin Ryabitsev
@ 2021-09-29 23:30         ` Jason Gunthorpe
  2021-09-30 14:45           ` Konstantin Ryabitsev
  2021-10-01 16:20         ` Mark Brown
  1 sibling, 1 reply; 17+ messages in thread
From: Jason Gunthorpe @ 2021-09-29 23:30 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools, users

On Wed, Sep 29, 2021 at 05:18:27PM -0400, Konstantin Ryabitsev wrote:
> On Tue, Sep 28, 2021 at 02:46:53PM -0300, Jason Gunthorpe wrote:
> > > We could, theoretically, do that, but you'd have to remember to pass the -F
> > > much free-form and there isn't a sane approach we can take to automatically
> > > generate good merge commit messages from them, short of quoting them fully,
> > > which will be overkill 9 times out of 10.
> > 
> > Since the user would be expected to revise it, I think that is
> > fine. Just make the template and push in the raw text and let people
> > edit. It would still save a bunch of time
> 
> Okay, I implemented something that should be handy. When we find a cover
> letter, we will prepare it as a template merge message in .git/b4-cover, so
> you can do stuff like:
> 
> git merge -F .git/b4-cover --edit FETCH_HEAD
> 
> I will eventually make the template configurable, but for now it's hardcoded
> to the following format (using a sample I've been working with):
> 
>     Merge patches from https://lore.kernel.org/r/20210929143600.49379-1-david@redhat.com
> 
>     Accept 6 patches from David Hildenbrand <david@redhat.com>

I've been following the netdevy style, so a merge from a cover letter
might look like this:

    Merge branch 'mlx5-next' of git://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux
    
    Leon Romanovsky says:
    ====================
    This is short series for mlx5 from Meir that adds a DevX UID to the UAR.
    ====================
        
    * mellanox/mlx5-next:
      IB/mlx5: Enable UAR to have DevX UID
      net/mlx5: Add uid field to UAR allocation structures

Where the last three lines are produced by 'git merge --log'

I would suggest a template:

Merge patch series "<cover letter subject>"

David Hildenbrand <david@redhat.com> says:
====================
<over letter body>
====================

* mailing-list:
   [..]

Link: http..
Signed-off-by: ..

As a possible starting point?

> > Linus has been repeatedly against working on random git history
> > points, so I think the unreliable auto-base is not something people
> > should be using.
> 
> I see. But if they do send a base-commit: line from some place random, is that
> okay to accept it?

I would say if that is what the submitter did, then yes, but the
submitter should probably be scolded if the random is too random :)

Jason

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-29 23:30         ` Jason Gunthorpe
@ 2021-09-30 14:45           ` Konstantin Ryabitsev
  0 siblings, 0 replies; 17+ messages in thread
From: Konstantin Ryabitsev @ 2021-09-30 14:45 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: tools, users

On Wed, Sep 29, 2021 at 08:30:53PM -0300, Jason Gunthorpe wrote:
> I would suggest a template:
> 
> Merge patch series "<cover letter subject>"
> 
> David Hildenbrand <david@redhat.com> says:
> ====================
> <over letter body>
> ====================
> 
> * mailing-list:
>    [..]
> 
> Link: http..
> Signed-off-by: ..
> 
> As a possible starting point?

Okay, the latest master allows for some minor templating and will use the
proposed netdevy format as the default. Since Signed-off-by can be added by
the git-merge command itself, I'm not adding it into the template unless
I can think of a good reason to do so.

I'm also skipping the mailing-list suggestion, as patches are routinely sent
to any number of lists, so that's bound to be a crowded section.

-K

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-29 21:18       ` Konstantin Ryabitsev
  2021-09-29 23:30         ` Jason Gunthorpe
@ 2021-10-01 16:20         ` Mark Brown
  1 sibling, 0 replies; 17+ messages in thread
From: Mark Brown @ 2021-10-01 16:20 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: Jason Gunthorpe, tools, users

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

On Wed, Sep 29, 2021 at 05:18:27PM -0400, Konstantin Ryabitsev wrote:
> On Tue, Sep 28, 2021 at 02:46:53PM -0300, Jason Gunthorpe wrote:

> > Linus has been repeatedly against working on random git history
> > points, so I think the unreliable auto-base is not something people
> > should be using.

> I see. But if they do send a base-commit: line from some place random, is that
> okay to accept it?

What my scripts do with base-commit is look for the commit in a set of
places I consider valid for where the patch is going to be applied, and
just ignore whatever was provided if it's not found there (broadly,
looking at the history of the tree I'm applying to plus Linus' tree).
Given that it'd be useful to be able to supply a list of commit ranges
that'd be considered valid.

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

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-09-21 20:25 b4: introducing b4 shazam (like b4 am -o- | git am) Konstantin Ryabitsev
  2021-09-22 17:38 ` Jason Gunthorpe
  2021-09-23  8:51 ` Geert Uytterhoeven
@ 2021-10-19 14:49 ` Rob Herring
  2021-10-19 16:30   ` Konstantin Ryabitsev
  2 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2021-10-19 14:49 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools, users

On Tue, Sep 21, 2021 at 3:25 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> Hi, all:
>
> The current master branch landed a new feature that implements a new
> subcommand "shazam". It is the functional equivalent of piping "b4 am" to "git
> am", with some extra safety padding.
>
> See "b4 shazam --help" to see the available options. You will notice that many
> of them are exactly the same as for "b4 am".
>
> The default operation still tries to be the safest possible:
>
> - we'll create a temporary sparse worktree that will only contain the files
>   being modified
> - we'll use the base-commit sha, if we find it, or we'll try to guess the base
>   commit from git index info
> - we'll run "git am" against that temporary worktree
> - if successful, we'll fetch from the worktree into your current repo and
>   delete the temporary sparse checkout
> - you can then "git checkout -b foo FETCH_HEAD" or "git merge FETCH_HEAD"
>   or "git rebase" as necessary

I get a splat due to my multiple worktree setup:

Fetching into FETCH_HEAD
Traceback (most recent call last):
  File "/home/rob/.local/bin/b4", line 33, in <module>
    sys.exit(load_entry_point('b4', 'console_scripts', 'b4')())
  File "/home/rob/proj/git/b4/b4/command.py", line 264, in cmd
    cmdargs.func(cmdargs)
  File "/home/rob/proj/git/b4/b4/command.py", line 76, in cmd_shazam
    b4.mbox.main(cmdargs)
  File "/home/rob/proj/git/b4/b4/mbox.py", line 721, in main
    make_am(msgs, cmdargs, msgid)
  File "/home/rob/proj/git/b4/b4/mbox.py", line 312, in make_am
    with open(fhf, 'r') as fhh:
NotADirectoryError: [Errno 20] Not a directory:
'/home/rob/proj/git/linux-dt/.git/FETCH_HEAD'

I just sent a patch to fix it.

>
> Alternatively, if you run "git shazam -A", we'll just run "git am" on the
> current HEAD, which is the exact equivalent of "b4 am -o- | git am".

'-A' worked fine. Can we make this a configuration value with
enable/disable override instead? I kind of expect users will use this
mostly one way or the other rather than decide per patch series. Sure,
I could wrap that in a script, but so could I with "b4 am -o- | git
am".

Rob

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

* Re: b4: introducing b4 shazam (like b4 am -o- | git am)
  2021-10-19 14:49 ` Rob Herring
@ 2021-10-19 16:30   ` Konstantin Ryabitsev
  0 siblings, 0 replies; 17+ messages in thread
From: Konstantin Ryabitsev @ 2021-10-19 16:30 UTC (permalink / raw)
  To: Rob Herring; +Cc: tools, users

On Tue, Oct 19, 2021 at 09:49:57AM -0500, Rob Herring wrote:
> I get a splat due to my multiple worktree setup:

Yes, I was expecting this not to play quite well with pre-existing worktrees.
Thanks for the report.

> I just sent a patch to fix it.

Will apply, thanks.

> '-A' worked fine. Can we make this a configuration value with
> enable/disable override instead? I kind of expect users will use this
> mostly one way or the other rather than decide per patch series. Sure,
> I could wrap that in a script, but so could I with "b4 am -o- | git
> am".

Yes, I am leaning towards "git shazam" without any flags being the equivalent
of "apply to current tree" (i.e. -A behaviour becomes the default), and the
FETCH_HEAD magic will only work with another flag (-H|--make-fetch-head,
probably).

Thanks,
-K

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

end of thread, other threads:[~2021-10-19 16:30 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-21 20:25 b4: introducing b4 shazam (like b4 am -o- | git am) Konstantin Ryabitsev
2021-09-22 17:38 ` Jason Gunthorpe
2021-09-22 18:21   ` Konstantin Ryabitsev
2021-09-28 17:46     ` Jason Gunthorpe
2021-09-29 21:18       ` Konstantin Ryabitsev
2021-09-29 23:30         ` Jason Gunthorpe
2021-09-30 14:45           ` Konstantin Ryabitsev
2021-10-01 16:20         ` Mark Brown
2021-09-23  8:51 ` Geert Uytterhoeven
2021-09-24 21:08   ` Konstantin Ryabitsev
2021-09-28 18:22     ` Geert Uytterhoeven
2021-09-29 13:39       ` Rob Herring
2021-09-29 17:54         ` Geert Uytterhoeven
2021-09-29 18:07         ` Jason Gunthorpe
2021-09-29 14:27       ` Konstantin Ryabitsev
2021-10-19 14:49 ` Rob Herring
2021-10-19 16:30   ` Konstantin Ryabitsev

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