[b4] add notes to patch for commit with git-notes?

[b4] add notes to patch for commit with git-notes?

[Quentin Schulz]

Hi all,

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes 
says that

 > If one patch depends on another patch in order for a change to be 
complete, that is OK. Simply note “this patch depends on patch X” in 
your patch description.

We forgot to add this note (we weren't aware of the rule at the time) 
and made some people unhappy due to some partial merges (one part going 
through one tree, its dependency in the same patch series still in review).

I asked some maintainer whether we should have this note in the cover 
letter or in the patch and the answer was "ideally both". I know how to 
do the former with b4, but not the latter. I would like to avoid having 
to use multiple tools to do the same job.

I think it should be possible to use git-notes for the latter?

We did a quick test and it seems it works (with a diff I'm pasting later 
in this mail), c.f. 
https://lore.kernel.org/linux-rockchip/20240902-dev-mule-i2c-mux-v7-3-bf7b8f5385ed@cherry.de/

For that we had to set notes.rewrite.rebase config to true so that b4 
prep --edit-cover wouldn't "erase" the notes (and same with 
notes.rewrite.amend just in case one does git commit --amend or reword 
with git rebase).

The diff we had for b4 to make this work is the following:

"""
diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index ec230e7..72621b8 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -3424,7 +3424,7 @@ def git_range_to_patches(gitdir: Optional[str], 
start: str, end: str,
          if commit in ignore_commits:
              logger.debug('Ignoring commit %s', commit)
              continue
-        ecode, out = git_run_command(gitdir, ['show', '--format=email', 
'--patch-with-stat', '--encoding=utf-8',
+        ecode, out = git_run_command(gitdir, ['show', '--format=email', 
'--patch-with-stat', '--encoding=utf-8', '--notes',
                                                commit], decode=False)
          if ecode > 0:
              raise RuntimeError(f'Could not get a patch out of {commit}')

"""

Now I'm wondering if we should have this always on or not, also if/how 
we should document that one needs to have notes.rewrite.rebase/amend set 
for this to work. Should we also add a knob to NOT add the notes (e.g. 
someone wants to keep notes for their use but only locally and not send 
those).

Let me know if git-notes would be fine and the direction to go for 
patches. Thanks!

Cheers,
Quentin

Re: [b4] add notes to patch for commit with git-notes?

[Matthieu Baerts]

Hi Quentin,

On 02/09/2024 18:51, Quentin Schulz wrote:
> I think it should be possible to use git-notes for the latter?

Please note :) that these notes can be easily dropped by b4 itself when
editing commits because it uses 'git filter-repo' to do so [1]. In other
words, these notes will be dropped when using 'b4 trailers', or when
using some 'b4 prep' commands when the cover-letter is stored in a
dedicated commit.

> The diff we had for b4 to make this work is the following:

The diff you attached has already proposed before in [2], but not
applied because the notes can be dropped.

> Let me know if git-notes would be fine and the direction to go for
> patches. Thanks!

Unfortunately, I don't think that's the right direction. The alternative
is to add the notes in the commit message, at the end, under '---' [3].
Maybe the direction to take is to document this alternative :)

[1]
https://lore.kernel.org/tools/20220929074717.u2xfvfwq4ki732tw@wittgenstein/T/
[2]
https://lore.kernel.org/tools/20240131223054.371692-1-Frank.Li@nxp.com/T/
[3]
https://lore.kernel.org/tools/jx7vyqdj3qt2zwgsxh6hjac6osr52n2vcmkj2pg3elapcth62m@jtl27si2pa3g/

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [b4] add notes to patch for commit with git-notes?

[Quentin Schulz]

Hi Matthieu,

On 9/2/24 7:10 PM, Matthieu Baerts wrote:
> [Some people who received this message don't often get email from matttbe@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Hi Quentin,
> 
> On 02/09/2024 18:51, Quentin Schulz wrote:
>> I think it should be possible to use git-notes for the latter?
> 
> Please note :) that these notes can be easily dropped by b4 itself when
> editing commits because it uses 'git filter-repo' to do so [1]. In other
> words, these notes will be dropped when using 'b4 trailers', or when
> using some 'b4 prep' commands when the cover-letter is stored in a
> dedicated commit.
>  >> The diff we had for b4 to make this work is the following:
> 
> The diff you attached has already proposed before in [2], but not
> applied because the notes can be dropped.
> 
>> Let me know if git-notes would be fine and the direction to go for
>> patches. Thanks!
> 
> Unfortunately, I don't think that's the right direction. The alternative
> is to add the notes in the commit message, at the end, under '---' [3].
> Maybe the direction to take is to document this alternative :)
> 
> [1]
> https://lore.kernel.org/tools/20220929074717.u2xfvfwq4ki732tw@wittgenstein/T/
> [2]
> https://lore.kernel.org/tools/20240131223054.371692-1-Frank.Li@nxp.com/T/
> [3]
> https://lore.kernel.org/tools/jx7vyqdj3qt2zwgsxh6hjac6osr52n2vcmkj2pg3elapcth62m@jtl27si2pa3g/
> 

Darn it, I didn't look far enough in lore :( Thanks for doing the 
research work I should have done myself!

I didn't know git would keep the --- part in the commit logs, should 
have tried the obvious (though I've been bit by #-starting lines 
disappearing so I assumed similar behavior :) ).

Thanks for the info and links!

Cheers,
Quentin