send: generate patches with --no-mailmap
PATCH v2 1/1 send: generate patches with --no-mailmap
b4 uses 'git show' to generate patches. By default, this command will use the mailmap file to convert author and committer names and email addresses if there is a match.
That's good to display the last identity when looking at old commits in the history, but it doesn't sound like a good idea when sending patches. 'git format-patch' doesn't use the mailmap file, it currently doesn't even support it, and it is not planned to support it apparently [1]. Note that 'git send-email' might support a new '--mailmap' option, but it looks like this option will not be enabled by default [2].
In my case, I had to send some patches, including one from someone else who has to use the email address from his company, but has an entry in the mailmap file to his @kernel.org email address. When I sent his patch, b4 seamlessly converted his email address, and caused checkpatch to complain, because the From and the SoB entries were different.
Here, we ensure the conversion is not done, to avoid any surprise once the emails are sent.
---
Changes in v2:
- Rebased and resolved conflicts.
- Link to v1: https://patch.msgid.link/[email protected]
--- src/b4/__init__.py | 1 + 1 file changed, 1 insertion(+)
src/b4/__init__.py +1 -0
@@ -3463,6 +3463,7 @@ def git_range_to_patches(gitdir: Optional[str], start: str, end: str, '--binary', '--patch-with-stat', '--encoding=utf-8', + '--no-mailmap', commit, ], decode=False,
---
base-commit: dedf88cb947bab87c418b49d975df11f83621692
change-id: 20240820-send-mailmap-ef8ba398450f
Best regards,
Signature · 1 line
Matthieu Baerts (NGI0) <[email protected]>
Quoted 11 lines · On Mon, Sep 23, 2024 at 06:04:31PM +0200, Matthieu Baerts (NGI0) wrote:
b4 uses 'git show' to generate patches. By default, this command will use the mailmap file to convert author and committer names and email addresses if there is a match.
That's good to display the last identity when looking at old commits in the history, but it doesn't sound like a good idea when sending patches. 'git format-patch' doesn't use the mailmap file, it currently doesn't even support it, and it is not planned to support it apparently [1]. Note that 'git send-email' might support a new '--mailmap' option, but it looks like this option will not be enabled by default [2].
Quoted 5 lines
In my case, I had to send some patches, including one from someone else who has to use the email address from his company, but has an entry in the mailmap file to his @kernel.org email address. When I sent his patch, b4 seamlessly converted his email address, and caused checkpatch to complain, because the From and the SoB entries were different.
I've been sitting on this for a bit, simply because I'm 50/50 on whether this is the right thing to do. On the one hand, I see your point, but on the other it seems that if there is a mailmap entry, then the person would probably like to prefer to receive mail at their new address, not at their old one.
What does everyone else think?
-K
Hi Konstantin,
Quoted 18 lines · Konstantin Ryabitsev
Quoted 11 lines · On Mon, Sep 23, 2024 at 06:04:31PM +0200, Matthieu Baerts (NGI0) wrote:
b4 uses 'git show' to generate patches. By default, this command will use the mailmap file to convert author and committer names and email addresses if there is a match.
That's good to display the last identity when looking at old commits in the history, but it doesn't sound like a good idea when sending patches. 'git format-patch' doesn't use the mailmap file, it currently doesn't even support it, and it is not planned to support it apparently [1]. Note that 'git send-email' might support a new '--mailmap' option, but it looks like this option will not be enabled by default [2].
Quoted 5 lines
In my case, I had to send some patches, including one from someone else who has to use the email address from his company, but has an entry in the mailmap file to his @kernel.org email address. When I sent his patch, b4 seamlessly converted his email address, and caused checkpatch to complain, because the From and the SoB entries were different.
Quoted 4 lines
I've been sitting on this for a bit, simply because I'm 50/50 on whether this is the right thing to do. On the one hand, I see your point, but on the other it seems that if there is a mailmap entry, then the person would probably like to prefer to receive mail at their new address, not at their old one.
Thank you for having looked at this patch!
To me, most of the time, it doesn't make sense to change the 'From' field when sending a patch: it might remove attributions, create differences when sending patches for stable, incoherences between the From and the Signed-off-by, etc.
(In my case, it was not an "old" address, but the owner is not able to use it to send emails on the mailing list. Still, his employer asked him to keep his email address in the patches he sends. That's why his work email is in the mailmap file, even if it is still used in patches, but not in the mailing lists.)
Quoted 1 line
What does everyone else think?
I guess that's a particular case, because I happened to send series including such email address :)
Please note that since then, a new "sendemail.mailmap" option has been added, set to False by default:
https://git-scm.com/docs/git-config#Documentation/git-config.txt-sendemailmailmap
Maybe this could be read before adding '--no-mailmap'?
Cheers,
Matt
Signature · 1 line
Sponsored by the NGI0 Core fund.
Quoted 39 lines · Matthieu Baerts
Hi Konstantin,
Quoted 18 lines · Konstantin Ryabitsev
Quoted 11 lines · On Mon, Sep 23, 2024 at 06:04:31PM +0200, Matthieu Baerts (NGI0) wrote:
b4 uses 'git show' to generate patches. By default, this command will use the mailmap file to convert author and committer names and email addresses if there is a match.
That's good to display the last identity when looking at old commits in the history, but it doesn't sound like a good idea when sending patches. 'git format-patch' doesn't use the mailmap file, it currently doesn't even support it, and it is not planned to support it apparently [1]. Note that 'git send-email' might support a new '--mailmap' option, but it looks like this option will not be enabled by default [2].
Quoted 5 lines
In my case, I had to send some patches, including one from someone else who has to use the email address from his company, but has an entry in the mailmap file to his @kernel.org email address. When I sent his patch, b4 seamlessly converted his email address, and caused checkpatch to complain, because the From and the SoB entries were different.
Quoted 4 lines
I've been sitting on this for a bit, simply because I'm 50/50 on whether this is the right thing to do. On the one hand, I see your point, but on the other it seems that if there is a mailmap entry, then the person would probably like to prefer to receive mail at their new address, not at their old one.
Thank you for having looked at this patch!
To me, most of the time, it doesn't make sense to change the 'From' field when sending a patch: it might remove attributions, create differences when sending patches for stable, incoherences between the From and the Signed-off-by, etc.
(In my case, it was not an "old" address, but the owner is not able to use it to send emails on the mailing list. Still, his employer asked him to keep his email address in the patches he sends. That's why his work email is in the mailmap file, even if it is still used in patches, but not in the mailing lists.)
Quoted 4 lines
Quoted 1 line
What does everyone else think?
I guess that's a particular case, because I happened to send series including such email address :)
I would expect b4 working here exactly like git format-patch, where you have commit with old identity (mailmapped or not, does not matter) and resulting git format-patch (with mailmap entry and default arguments) produces result as:
From: old me
Signed-off-by: old me
Signed-off-by: new me
(with or without final SoB, discussion is not about DCO)
And above is necessary for all cases of changing employers.
So if b4 does not work like this, then this should be either fixed or +1 for this approach.
Best regards,
Krzysztof
Quoted 11 lines · Matthieu Baerts
Quoted 1 line
What does everyone else think?
I guess that's a particular case, because I happened to send series including such email address :)
Please note that since then, a new "sendemail.mailmap" option has been added, set to False by default:
https://git-scm.com/docs/git-config#Documentation/git-config.txt-sendemailmailmap
Maybe this could be read before adding '--no-mailmap'?
Ah, that sounds like the best course of action. We already parse sendemail configuration options, so looking up if sendemail.mailmap.* is set is the right way to go.
Thanks for the suggestion! I won't take the patch, but I'll work this in and give credit.
-K
Hi Konstantin,
Quoted 11 lines · Konstantin Ryabitsev
Quoted 10 lines · Matthieu Baerts
Quoted 1 line
What does everyone else think?
I guess that's a particular case, because I happened to send series including such email address :)
Please note that since then, a new "sendemail.mailmap" option has been added, set to False by default:
https://git-scm.com/docs/git-config#Documentation/git-config.txt-sendemailmailmap
Quoted 8 lines
Quoted 1 line
Maybe this could be read before adding '--no-mailmap'?
Ah, that sounds like the best course of action. We already parse sendemail configuration options, so looking up if sendemail.mailmap.* is set is the right way to go.
Thanks for the suggestion! I won't take the patch, but I'll work this in and give credit.
Thank you for having applied my suggestion [1].
However, there was maybe a small confusion: I think it is important no to use mailmap for the equivalent of 'git format-patch', but it is still interesting to continue using it for the equivalent of 'git send-email'.
My issue was that the author's 'From' tag was modified: I wanted to send a patch from someone else using an address having an entry in the mailmap file, and b4 changed the 'From' tag, no longer corresponding to the SoB.
In other words, instead of:
From: Me
Subject: Foo
From: Dev <Original>
(...)
b4 prepared this patch:
From: Me
Subject: Foo
From: Dev <MailMap> <== Modified :(
(...)
=> I don't think b4 should modify the From (due to "git show --mailmap") by default. This can also happen when sending an "old" patch to stable for example. Your recent modification fixes that in __init__.py:git_range_to_patches() by using "git show --no-mailmap" by default, thank you!
Now there is a second part in your patch: the modifications of the To/Cc fields. For me, it is OK and maybe better to send the patch to "Dev <MailMap>" instead of / and "Dev <Original>", because that's what "Dev" wants according to the mailmap file. This behaviour has been changed with your modifications in ez.py. Maybe the emails should continue to be updated to use the mailmap version by default, if sendemail.mailmap is not set?
So doing this in cmd_send():
- mailmap_replace = b4.get_git_bool(sconfig.get('mailmap', 'false'))
+ mailmap_replace = b4.get_git_bool(sconfig.get('mailmap', 'true'))
WDYT?
[1] https://git.kernel.org/pub/scm/utils/b4/b4.git/commit/?id=8438e306
Cheers,
Matt
Signature · 1 line
Sponsored by the NGI0 Core fund.
Quoted 10 lines · Matthieu Baerts
b4 prepared this patch:
From: Me
Subject: FooFrom: Dev <MailMap> <== Modified :(
(...)
Signed-off-by Dev <Original> <== Unmodified (good)
Thank you for explaining that -- I wasn't actually fully understanding the problem. I thought it would only affect the to/cc entries, therefore my confusion why would anyone *not* want that. :)
Quoted 14 lines
=> I don't think b4 should modify the From (due to "git show --mailmap") by default. This can also happen when sending an "old" patch to stable for example. Your recent modification fixes that in __init__.py:git_range_to_patches() by using "git show --no-mailmap" by default, thank you!
Now there is a second part in your patch: the modifications of the To/Cc fields. For me, it is OK and maybe better to send the patch to "Dev <MailMap>" instead of / and "Dev <Original>", because that's what "Dev" wants according to the mailmap file. This behaviour has been changed with your modifications in
ez.py. Maybe the emails should continue to be updated to use the mailmap version by default, if sendemail.mailmap is not set?
Quoted 6 lines
So doing this in
cmd_send():-
mailmap_replace=b4.get_git_bool(sconfig.get('mailmap', 'false'))
+mailmap_replace=b4.get_git_bool(sconfig.get('mailmap', 'true'))WDYT?
I'll just roll back that part and simplify things back down to just pass --no-mailmap to the git-show command.
Thanks for the clarification!
-K
Quoted 1 line · Konstantin Ryabitsev
On Wed, Feb 05, 2025 at 04:46:16PM +0100, Matthieu Baerts wrote:
(...)
Quoted 9 lines
Quoted 6 lines
So doing this in
cmd_send():-
mailmap_replace=b4.get_git_bool(sconfig.get('mailmap', 'false'))
+mailmap_replace=b4.get_git_bool(sconfig.get('mailmap', 'true'))WDYT?
I'll just roll back that part and simplify things back down to just pass --no-mailmap to the git-show command.
Thank you, looks good to me!
Cheers,
Matt
Signature · 1 line
Sponsored by the NGI0 Core fund.
Quoted 11 lines · Matthieu Baerts
Quoted 9 lines
Quoted 6 lines
So doing this in
cmd_send():-
mailmap_replace=b4.get_git_bool(sconfig.get('mailmap', 'false'))
+mailmap_replace=b4.get_git_bool(sconfig.get('mailmap', 'true'))WDYT?
I'll just roll back that part and simplify things back down to just pass --no-mailmap to the git-show command.
Thank you, looks good to me!
The changes are in place in the latest master. Please follow up if there's more work that is required.
Thank you,
-K
Hi Konstantin,
Quoted 6 lines · Konstantin Ryabitsev
Quoted 5 lines · Matthieu Baerts
Quoted 5 lines
Quoted 5 lines
So doing this in
cmd_send():-
mailmap_replace=b4.get_git_bool(sconfig.get('mailmap', 'false'))
+mailmap_replace=b4.get_git_bool(sconfig.get('mailmap', 'true'))
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
WDYT?
I'll just roll back that part and simplify things back down to just pass --no-mailmap to the git-show command.
Thank you, looks good to me!
The changes are in place in the latest master.
Great, thanks!
Quoted 1 line
Please follow up if there's more work that is required.
Sorry, I have one last question about this. When reading this part of your commit message ...
Quoted 3 lines
The consensus is that we *do* want to apply mailmap updates to the actual addresses going into to/cc, just not to the in-body From:
addresses generated during the "git show" stage.
... I'm wondering if there is still a need to look at the sendemail.mailmap config: it is now only used to modify the behaviour of patches generation, and not how/to who they are going to be sent. In Git, this config option is supposed to act only on the second part.
Don't get me wrong, the current version fixes my issue, and I'm happy with that! Plus I know I suggested looking at sendemail.mailmap as a "workaround". But now that the confusion around my issue has been cleared up, maybe all we need here is to always use "git show --no-mailmap" and forget about sendemail.mailmap? :)
Cheers,
Matt
Signature · 1 line
Sponsored by the NGI0 Core fund.
Quoted 12 lines · Matthieu Baerts
Sorry, I have one last question about this. When reading this part of your commit message ...
Quoted 3 lines
The consensus is that we *do* want to apply mailmap updates to the actual addresses going into to/cc, just not to the in-body From:
addresses generated during the "git show" stage.... I'm wondering if there is still a need to look at the sendemail.mailmap config: it is now only used to modify the behaviour of patches generation, and not how/to who they are going to be sent. In Git, this config option is supposed to act only on the second part.
Quoted 5 lines
Don't get me wrong, the current version fixes my issue, and I'm happy with that! Plus I know I suggested looking at sendemail.mailmap as a "workaround". But now that the confusion around my issue has been cleared up, maybe all we need here is to always use "git show --no-mailmap" and forget about sendemail.mailmap? :)
I'm okay if we keep it as-is right now, but we'll keep this in mind for later, should this come up again.
-K
Quoted 13 lines · Konstantin Ryabitsev
Quoted 12 lines · Matthieu Baerts
Sorry, I have one last question about this. When reading this part of your commit message ...
Quoted 3 lines
The consensus is that we *do* want to apply mailmap updates to the actual addresses going into to/cc, just not to the in-body From:
addresses generated during the "git show" stage.... I'm wondering if there is still a need to look at the sendemail.mailmap config: it is now only used to modify the behaviour of patches generation, and not how/to who they are going to be sent. In Git, this config option is supposed to act only on the second part.
Quoted 8 lines
Quoted 5 lines
Don't get me wrong, the current version fixes my issue, and I'm happy with that! Plus I know I suggested looking at sendemail.mailmap as a "workaround". But now that the confusion around my issue has been cleared up, maybe all we need here is to always use "git show --no-mailmap" and forget about sendemail.mailmap? :)
I'm okay if we keep it as-is right now, but we'll keep this in mind for later, should this come up again.
Works for me, thank you!
Cheers,
Matt
Signature · 1 line
Sponsored by the NGI0 Core fund.