checkpatch: extend same_email_address check to load mailmap entries
RFC PATCH 1/1 checkpatch: extend same_email_address check to load mailmap entries
Checkpatch throws a NO_AUTHOR_SIGN_OFF warning whenever either the name or email address of author doesn't match any signed-off-by entry. But it has no mechanism to check whether different email addresses belong to the same author.
As a result, there have been multiple commits where the author has signed off with a different email address which caused checkpatch to product that warning.
An example is commit dc5bdb68b5b3 ("drm/fb-helper: Fix vt restore"), where the author uses two different mail addresses, and checkpatch generates a NO_AUTHOR_SIGN_OFF warning.
This was fixed by adding support for loading .mailmap entries. If a mismatch in name or email address then occurs, the entries in mailmap can be looked up to deterime the actual author name and email address.
--- scripts/checkpatch.pl | 113 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 111 insertions(+), 2 deletions(-)
scripts/checkpatch.pl +111 -2
@@ -1128,6 +1128,78 @@ sub top_of_kernel_tree { return 1; } +my $mailmap; + +sub read_mailmap { + $mailmap = { + names => {}, + addresses => {} + }; + + return if (!defined($root) || !(-f "${root}/.mailmap")); + + open(my $mailmap_file, '<', "${root}/.mailmap") + or warn "$P: Can't open .mailmap: $!\n"; + + while (<$mailmap_file>) { + s/#.*$//; #strip comments + s/^\s+|\s+$//g; #trim + + next if (/^\s*$/); #skip empty lines + #entries have one of the following formats: + # name1 <mail1> + # <mail1> <mail2> + # name1 <mail1> <mail2> + # name1 <mail1> name2 <mail2> + # (see man git-shortlog) + + if (/^([^<]+)<([^>]+)>$/) { + my $real_name = $1; + my $address = $2; + + $real_name =~ s/\s+$//; + ($real_name, $address) = parse_email("$real_name <$address>"); + $mailmap->{names}->{$address} = $real_name; + + } elsif (/^<([^>]+)>\s*<([^>]+)>$/) { + my $real_address = $1; + my $wrong_address = $2; + + $mailmap->{addresses}->{$wrong_address} = $real_address; + + } elsif (/^(.+)<([^>]+)>\s*<([^>]+)>$/) { + my $real_name = $1; + my $real_address = $2; + my $wrong_address = $3; + + $real_name =~ s/\s+$//; + ($real_name, $real_address) = + parse_email("$real_name <$real_address>"); + $mailmap->{names}->{$wrong_address} = $real_name; + $mailmap->{addresses}->{$wrong_address} = $real_address; + + } elsif (/^(.+)<([^>]+)>\s*(.+)\s*<([^>]+)>$/) { + my $real_name = $1; + my $real_address = $2; + my $wrong_name = $3; + my $wrong_address = $4; + + $real_name =~ s/\s+$//; + ($real_name, $real_address) = + parse_email("$real_name <$real_address>"); + + $wrong_name =~ s/\s+$//; + ($wrong_name, $wrong_address) = + parse_email("$wrong_name <$wrong_address>"); + + my $wrong_email = format_email($wrong_name, $wrong_address); + $mailmap->{names}->{$wrong_email} = $real_name; + $mailmap->{addresses}->{$wrong_email} = $real_address; + } + } + close($mailmap_file); +} + sub parse_email { my ($formatted_email) = @_;
@@ -1210,14 +1282,50 @@ sub reformat_email { return format_email($email_name, $email_address); } +sub mailmap_email { + my ($name, $address) = @_; + + my $email = format_email($name, $address); + my $real_name = $name; + my $real_address = $address; + + if (exists $mailmap->{names}->{$email} || + exists $mailmap->{addresses}->{$email}) { + if (exists $mailmap->{names}->{$email}) { + $real_name = $mailmap->{names}->{$email}; + } + if (exists $mailmap->{addresses}->{$email}) { + $real_address = $mailmap->{addresses}->{$email}; + } + } else { + if (exists $mailmap->{names}->{$address}) { + $real_name = $mailmap->{names}->{$address}; + } + if (exists $mailmap->{addresses}->{$address}) { + $real_address = $mailmap->{addresses}->{$address}; + } + } + return ($real_name, $real_address); +} + sub same_email_addresses { my ($email1, $email2) = @_; my ($email1_name, $name1_comment, $email1_address, $comment1) = parse_email($email1); my ($email2_name, $name2_comment, $email2_address, $comment2) = parse_email($email2); - return $email1_name eq $email2_name && - $email1_address eq $email2_address; + my $name_match = $email1_name eq $email2_name; + my $address_match = $email1_address eq $email2_address; + + if(!$name_match || !$address_match) { + my ($real_name1, $real_address1) = mailmap_email($email1_name, $email1_address); + my ($real_name2, $real_address2) = mailmap_email($email2_name, $email2_address); + + $name_match |= ($real_name1 eq $real_name2); + $address_match |= ($real_address1 eq $real_address2); + } + + return $name_match && $address_match; } sub which {
@@ -2400,6 +2508,7 @@ sub process { my $checklicenseline = 1; + read_mailmap(); sanitise_line_reset(); my $line; foreach my $rawline (@rawlines) {
Signature · 6 lines
2.27.0 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Joe Perches
maintainerQuoted 5 lines · Dwaipayan Ray
Checkpatch throws a
NO_AUTHOR_SIGN_OFFwarning whenever either the name or email address of author doesn't match any signed-off-by entry. But it has no mechanism to check whether different email addresses belong to the same author.
Quoted 7 lines
As a result, there have been multiple commits where the author has signed off with a different email address which caused checkpatch to product that warning.
An example is commit
dc5bdb68b5b3("drm/fb-helper: Fix vt restore"), where the author uses two different mail addresses, and checkpatch generates aNO_AUTHOR_SIGN_OFFwarning.
.mailmap entries are used when a person no longer has access to a particular email and a mapping is created to allow these old/invalid email addresses to be converted to a current/valid email address.
The idea here is that a person may have a .gitconfig or equivalent that still uses an old/invalid email address but uses a Signed-off-by: for a current/valid one, so a warning _should_ be emitted for this case.
At least for checkpatch, mismatches in email addresses should not be papered over by assuming equivalence between multiple email addresses.
So this should be a separate test and not used in a function named
"same_email_address". These are specifically _not_ the same email
addresses.
_______________________________________________
Linux-kernel-mentees mailing list [email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 14 lines
.mailmap entries are used when a person no longer has access to a particular email and a mapping is created to allow these old/invalid email addresses to be converted to a current/valid email address.
The idea here is that a person may have a .gitconfig or equivalent that still uses an old/invalid email address but uses a Signed-off-by: for a current/valid one, so a warning _should_ be emitted for this case.
At least for checkpatch, mismatches in email addresses should not be papered over by assuming equivalence between multiple email addresses.
Quoted 7 lines
So this should be a separate test and not used in a function named
"same_email_address". These are specifically _not_ the same email
addresses.
Hi,
Thanks for the clarification.
So instead of eliminating the warning completely will it be a better alternative to display a more descriptive warning on the lines of:
"WARNING: NO_AUTHOR_SIGN_OFF:
invalid/obsolete email address used in signoff by author $author"
Or is it better to ignore the mailmap extension in checkpatch and just display warnings for either mismatch in name/ mismatch in email.
Like, you had mentioned one in:
https://lore.kernel.org/linux-kernel-mentees/
[email protected]/
In that case, same name, but different mail address was reported as a different warning.
So, I believe two cases can be handled better:
1) Same name, different email
2) Same email, different name
For case 2, it might also be possible to ignore the warning completely.
What do you think is better?
Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Joe Perches
maintainerQuoted 44 lines · Dwaipayan Ray
Quoted 14 lines
.mailmap entries are used when a person no longer has access to a particular email and a mapping is created to allow these old/invalid email addresses to be converted to a current/valid email address.
The idea here is that a person may have a .gitconfig or equivalent that still uses an old/invalid email address but uses a Signed-off-by: for a current/valid one, so a warning _should_ be emitted for this case.
At least for checkpatch, mismatches in email addresses should not be papered over by assuming equivalence between multiple email addresses.
Quoted 7 lines
So this should be a separate test and not used in a function named
"same_email_address". These are specifically _not_ the same email
addresses.Hi,
Thanks for the clarification.So instead of eliminating the warning completely will it be a better alternative to display a more descriptive warning on the lines of:
"WARNING:
NO_AUTHOR_SIGN_OFF:
invalid/obsolete email address used in signoff by author $author"Or is it better to ignore the mailmap extension in checkpatch and just display warnings for either mismatch in name/ mismatch in email.
Like, you had mentioned one in:
https://lore.kernel.org/linux-kernel-mentees/
[email protected]/In that case, same name, but different mail address was reported as a different warning.
Quoted 6 lines
So, I believe two cases can be handled better:
1) Same name, different email
2) Same email, different nameFor case 2, it might also be possible to ignore the warning completely.
Maybe both.
I suggest seeing if extensions to email addresses like:
<[email protected]>
could be compared to:
<[email protected]>
and comment blocks like:
"First Last" (via mailing list) <[email protected]>
could be compared to:
"First Last" <[email protected]> (MAINTAINER)
Still, I think all of these should emit warnings or --strict checks messages.
The idea is to notify that some oddity has occurred rather than silently accept it.
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees