All of lore.kernel.org
 help / color / mirror / Atom feed
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
To: Andy Whitcroft <apw@canonical.com>, Joe Perches <joe@perches.com>,
	Dwaipayan Ray <dwaipayanray1@gmail.com>,
	Lukas Bulwahn <lukas.bulwahn@gmail.com>,
	linux-kernel@vger.kernel.org
Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	Jacob Keller <jacob.e.keller@intel.com>
Subject: [PATCH] checkpatch: allow tags between co-developed-by and their sign-off
Date: Fri, 20 Oct 2023 15:21:56 +0200	[thread overview]
Message-ID: <20231020132156.37882-1-przemyslaw.kitszel@intel.com> (raw)

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>
---
 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();
-- 
2.38.1


             reply	other threads:[~2023-10-20 13:22 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-20 13:21 Przemek Kitszel [this message]
2023-10-20 22:34 ` [PATCH] checkpatch: allow tags between co-developed-by and their sign-off Joe Perches
2023-10-23  9:24   ` Przemek Kitszel
2023-10-24 20:07     ` Joe Perches
2023-10-23 10:28 Przemek Kitszel
2023-10-23 14:02 ` Sean Christopherson
2023-10-24  9:15   ` Przemek Kitszel
2023-10-29  9:35     ` Krzysztof Kozlowski
2023-10-30  9:05       ` Przemek Kitszel
2023-10-23 14:16 ` Lukas Bulwahn
2023-10-23 14:25   ` Joe Perches
2023-10-24  9:15   ` Przemek Kitszel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231020132156.37882-1-przemyslaw.kitszel@intel.com \
    --to=przemyslaw.kitszel@intel.com \
    --cc=apw@canonical.com \
    --cc=dwaipayanray1@gmail.com \
    --cc=jacob.e.keller@intel.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.