Messages in this thread | | | Subject | Re: [PATCH] checkpatch: allow tags between co-developed-by and their sign-off | From | Joe Perches <> | Date | Fri, 20 Oct 2023 15:34:40 -0700 |
| |
On Fri, 2023-10-20 at 15:21 +0200, Przemek Kitszel wrote: > Allow additional tags between Co-developed-by: and Signed-off-by:. > Bump severity of missing SoB to ERROR. > > Additional tags between Co-developed-by and corresponding Signed-off-by > could include Reviewed-by tags collected by Submitter, which is also > a Co-developer, but should sign-off at the very end of tags provided by > themself. > > Missing SoB is promoted to error while that piece of code is touched. > > Two sets of perl %hashes introduced to keep both (int) line numbers and > (string) messages handy for warning reporting, while keeping it correct > across 100+ line long commit messages. > > Mateusz Polchlopek <mateusz.polchlopek@intel.com> has reported this to me. > > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Unless this is accepted by various process folk, and the documentation for it updated, I think this should not be applied.
> --- > scripts/checkpatch.pl | 36 +++++++++++++++++++++++------------- > 1 file changed, 23 insertions(+), 13 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 7d16f863edf1..0400bf092bfa 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -2682,6 +2682,10 @@ sub process { > my $suppress_statement = 0; > > my %signatures = (); > + my %signoffs = (); > + my %signoffs_msg = (); > + my %codevs = (); > + my %codevs_msg = (); > > # Pre-scan the patch sanitizing the lines. > # Pre-scan the patch looking for any __setup documentation. > @@ -2967,11 +2971,13 @@ sub process { > if ($line =~ /^\s*signed-off-by:\s*(.*)/i) { > $signoff++; > $in_commit_log = 0; > + my $ctx = $1; > + $signoffs{$ctx} = $linenr; > + $signoffs_msg{$ctx} = $herecurr; > if ($author ne '' && $authorsignoff != 1) { > - if (same_email_addresses($1, $author)) { > + if (same_email_addresses($ctx, $author)) { > $authorsignoff = 1; > } else { > - my $ctx = $1; > my ($email_name, $email_comment, $email_address, $comment1) = parse_email($ctx); > my ($author_name, $author_comment, $author_address, $comment2) = parse_email($author); > > @@ -3158,22 +3164,15 @@ sub process { > $signatures{$sig_nospace} = 1; > } > > -# Check Co-developed-by: immediately followed by Signed-off-by: with same name and email > +# Collect Co-developed-by: to check if each is backed up by Signed-off-by: with > +# the same name and email. Checks are made after main loop. > if ($sign_off =~ /^co-developed-by:$/i) { > if ($email eq $author) { > WARN("BAD_SIGN_OFF", > "Co-developed-by: should not be used to attribute nominal patch author '$author'\n" . $herecurr); > } > - if (!defined $lines[$linenr]) { > - WARN("BAD_SIGN_OFF", > - "Co-developed-by: must be immediately followed by Signed-off-by:\n" . $herecurr); > - } elsif ($rawlines[$linenr] !~ /^signed-off-by:\s*(.*)/i) { > - WARN("BAD_SIGN_OFF", > - "Co-developed-by: must be immediately followed by Signed-off-by:\n" . $herecurr . $rawlines[$linenr] . "\n"); > - } elsif ($1 ne $email) { > - WARN("BAD_SIGN_OFF", > - "Co-developed-by and Signed-off-by: name/email do not match\n" . $herecurr . $rawlines[$linenr] . "\n"); > - } > + $codevs{$email} = $linenr; > + $codevs_msg{$email} = $herecurr; > } > > # check if Reported-by: is followed by a Closes: tag > @@ -7712,6 +7711,17 @@ sub process { > "From:/Signed-off-by: email subaddress mismatch: $sob_msg\n"); > } > } > + # check if each Co-developed-by tag is backed up by Sign-off, > + # warn if Co-developed-by tag was put after a Signed-off-by tag > + foreach my $codev (keys %codevs) { > + if (!$signoffs{$codev}) { > + ERROR("BAD_SIGN_OFF", > + "Co-developed-by: must be followed by Signed-off-by:\n" . $codevs_msg{$codev}); > + } elsif ($signoffs{$codev} <= $codevs{$codev}) { > + WARN("BAD_SIGN_OFF", > + "Co-developed-by: must be followed by Signed-off-by:, but was placed after it\n" . $signoffs_msg{$codev} . $codevs_msg{$codev}); > + } > + } > } > > print report_dump();
| |