DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] devtools: fix checkpatches subject retrieval from stdin
@ 2022-09-29 12:04 Olivier Matz
  2022-11-23 16:33 ` Thomas Monjalon
  0 siblings, 1 reply; 2+ messages in thread
From: Olivier Matz @ 2022-09-29 12:04 UTC (permalink / raw)
  To: dev; +Cc: Thomas Monjalon, stable

When passing the patch to checkpatches.sh through stdin, the subject is
retrieved by reading the input until a "Subject:" entry is found. The
rest of the input is piped to checkpatch.pl.

Since the "From:" line is before the "Subject:" line, it won't be sent to
checkpatch.pl, which won't be able to get the author of the commit.
The following error will appear:

> ERROR:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author ''

Do the subject lookup on the temporary file instead of stdin, and
send the whole lines to checkpatch.pl.

The problem is visible since the introduction of this check in linux
checkpatch.pl in version 4.19 (see link below).

Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cd2614967d8b
Fixes: 8005feef421d ("scripts: add standard input to checkpatch")
CC: stable@dpdk.org

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 devtools/checkpatches.sh | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 0e82c01947..2b7005dfac 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -278,12 +278,12 @@ print_headline() { # <title>
 total=0
 status=0
 
-check () { # <patch> <commit> <title>
+check () { # <patch-file> <commit>
 	local ret=0
+	local subject=''
 	headline_printed=false
 
 	total=$(($total + 1))
-	! $verbose || print_headline "$3"
 	if [ -n "$1" ] ; then
 		tmpinput=$1
 	else
@@ -298,10 +298,14 @@ check () { # <patch> <commit> <title>
 		fi
 	fi
 
+	# Subject can be on 2 lines
+	subject=$(sed '/^Subject: */!d;s///;N;s,\n[[:space:]]\+, ,;s,\n.*,,;q' "$tmpinput")
+	! $verbose || print_headline "$subject"
+
 	! $verbose || printf 'Running checkpatch.pl:\n'
 	report=$($DPDK_CHECKPATCH_PATH $options "$tmpinput" 2>/dev/null)
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report" | sed -n '1,/^total:.*lines checked$/p'
 		ret=1
 	fi
@@ -309,7 +313,7 @@ check () { # <patch> <commit> <title>
 	! $verbose || printf '\nChecking API additions/removals:\n'
 	report=$($VALIDATE_NEW_API "$tmpinput")
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report"
 		ret=1
 	fi
@@ -317,7 +321,7 @@ check () { # <patch> <commit> <title>
 	! $verbose || printf '\nChecking forbidden tokens additions:\n'
 	report=$(check_forbidden_additions "$tmpinput")
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report"
 		ret=1
 	fi
@@ -325,7 +329,7 @@ check () { # <patch> <commit> <title>
 	! $verbose || printf '\nChecking __rte_experimental tags:\n'
 	report=$(check_experimental_tags "$tmpinput")
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report"
 		ret=1
 	fi
@@ -333,7 +337,7 @@ check () { # <patch> <commit> <title>
 	! $verbose || printf '\nChecking __rte_internal tags:\n'
 	report=$(check_internal_tags "$tmpinput")
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report"
 		ret=1
 	fi
@@ -341,7 +345,7 @@ check () { # <patch> <commit> <title>
 	! $verbose || printf '\nChecking release notes updates:\n'
 	report=$(check_release_notes "$tmpinput")
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report"
 		ret=1
 	fi
@@ -357,20 +361,10 @@ check () { # <patch> <commit> <title>
 
 if [ -n "$1" ] ; then
 	for patch in "$@" ; do
-		# Subject can be on 2 lines
-		subject=$(sed '/^Subject: */!d;s///;N;s,\n[[:space:]]\+, ,;s,\n.*,,;q' "$patch")
-		check "$patch" '' "$subject"
+		check "$patch" ''
 	done
 elif [ ! -t 0 ] ; then # stdin
-	subject=$(while read header value ; do
-		if [ "$header" = 'Subject:' ] ; then
-			IFS= read next
-			continuation=$(echo "$next" | sed -n 's,^[[:space:]]\+, ,p')
-			echo $value$continuation
-			break
-		fi
-	done)
-	check '' '' "$subject"
+	check '' ''
 else
 	if [ $number -eq 0 ] ; then
 		commits=$(git rev-list --reverse $range)
@@ -378,8 +372,7 @@ else
 		commits=$(git rev-list --reverse --max-count=$number HEAD)
 	fi
 	for commit in $commits ; do
-		subject=$(git log --format='%s' -1 $commit)
-		check '' $commit "$subject"
+		check '' $commit
 	done
 fi
 pass=$(($total - $status))
-- 
2.30.2


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] devtools: fix checkpatches subject retrieval from stdin
  2022-09-29 12:04 [PATCH] devtools: fix checkpatches subject retrieval from stdin Olivier Matz
@ 2022-11-23 16:33 ` Thomas Monjalon
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Monjalon @ 2022-11-23 16:33 UTC (permalink / raw)
  To: Olivier Matz; +Cc: dev, stable

29/09/2022 14:04, Olivier Matz:
> When passing the patch to checkpatches.sh through stdin, the subject is
> retrieved by reading the input until a "Subject:" entry is found. The
> rest of the input is piped to checkpatch.pl.
> 
> Since the "From:" line is before the "Subject:" line, it won't be sent to
> checkpatch.pl, which won't be able to get the author of the commit.
> The following error will appear:
> 
> > ERROR:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author ''
> 
> Do the subject lookup on the temporary file instead of stdin, and
> send the whole lines to checkpatch.pl.
> 
> The problem is visible since the introduction of this check in linux
> checkpatch.pl in version 4.19 (see link below).
> 
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cd2614967d8b
> Fixes: 8005feef421d ("scripts: add standard input to checkpatch")
> CC: stable@dpdk.org
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Thomas Monjalon <thomas@monjalon.net>

Applied, thanks.



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-11-23 16:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29 12:04 [PATCH] devtools: fix checkpatches subject retrieval from stdin Olivier Matz
2022-11-23 16:33 ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).