[dpdk-dev] [PATCH 0/2] standardize devtools check scripts

[dpdk-dev] [PATCH 0/2] standardize devtools check scripts

[Ciara Power]

This patchset standardizes the checkpatches and check-git-log scripts
to accept the same syntax commandline arguments, to make them easier
to use.

The output is also standardized, check-git-log previously showed no
output to the user when no errors were found, but now prints a similar
output to that of the checkpatches script.

Ciara Power (2):
  devtools: standardize script arguments
  devtools: added stats print

 devtools/check-git-log.sh | 76 +++++++++++++++++++++++++++------------
 devtools/checkpatches.sh  |  2 +-
 2 files changed, 54 insertions(+), 24 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH 1/2] devtools: standardize script arguments

[Ciara Power]

This patch modifies the arguments expected by the check-git-log script,
to match the format of arguments for the checkpatches script. Both
scripts now take certain argument options in the same format, making
them easier to use.
e.g. Both now take a commit ID range by "-r <range>"

The checkpatches help print is also updated to include the "-h" option.

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 devtools/check-git-log.sh | 31 +++++++++++++++++++++++--------
 devtools/checkpatches.sh  |  2 +-
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index f9d055039..22b2a6d84 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -7,23 +7,38 @@
 # If any doubt about the formatting, please check in the most recent history:
 #	git log --format='%>|(15)%cr   %s' --reverse | grep -i <pattern>
 
-if [ "$1" = '-h' -o "$1" = '--help' ] ; then
+print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [-h] [range]
+	usage: $(basename $0) [-h] [-nX|-r range]
 
 	Check commit log formatting.
-	The git range can be specified as a "git log" option,
-	e.g. -1 to check only the latest commit.
-	The default range starts from origin/master to HEAD.
+	The git commits to be checked can be specified as a "git log" option,
+	by latest git commits limited with -n option, or commits in the git
+	range specified with -r option.
+	e.g. -n1 to check only the latest commit.
+	The default starts from origin/master to HEAD.
 	END_OF_HELP
 	exit
-fi
+}
 
 selfdir=$(dirname $(readlink -f $0))
 range=${1:-origin/master..}
+
+if [ "$range" = '--help' ] ; then
+	print_usage
 # convert -N to HEAD~N.. in order to comply with git-log-fixes.sh getopts
-if printf -- $range | grep -q '^-[0-9]\+' ; then
-	range="HEAD$(printf -- $range | sed 's,^-,~,').."
+elif printf -- "$range" | grep -q '^-[0-9]\+' ; then
+	range="HEAD$(printf -- "$range" | sed 's,^-,~,').."
+else
+	while getopts hr:n: ARG ; do
+		case $ARG in
+			n ) range="HEAD~$OPTARG.." ;;
+			r ) range=$OPTARG ;;
+			h ) print_usage ; exit 0 ;;
+			? ) print_usage ; exit 1 ;;
+		esac
+	done
+	shift $(($OPTIND - 1))
 fi
 
 commits=$(git log --format='%h' --reverse $range)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index b16bace92..084191984 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -38,7 +38,7 @@ options="$options $DPDK_CHECKPATCH_OPTIONS"
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [-q] [-v] [-nX|-r range|patch1 [patch2] ...]]
+	usage: $(basename $0) [-h] [-q] [-v] [-nX|-r range|patch1 [patch2] ...]
 
 	Run Linux kernel checkpatch.pl with DPDK options.
 	The environment variable DPDK_CHECKPATCH_PATH must be set.
-- 
2.17.1

Re: [dpdk-dev] [PATCH 1/2] devtools: standardize script arguments

[Ferruh Yigit]

On 1/28/2020 3:02 PM, Ciara Power wrote:
> This patch modifies the arguments expected by the check-git-log script,
> to match the format of arguments for the checkpatches script. Both
> scripts now take certain argument options in the same format, making
> them easier to use.
> e.g. Both now take a commit ID range by "-r <range>"
> 
> The checkpatches help print is also updated to include the "-h" option.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH 1/2] devtools: standardize script arguments

[Thomas Monjalon]

Hi,

Thanks for improving tooling.

28/01/2020 16:02, Ciara Power:
> range=${1:-origin/master..}

If doing a real option management, range should be the remaining argument
after option parsing.

> +if [ "$range" = '--help' ] ; then
> +	print_usage

Missing "exit 0" after usage.

>  # convert -N to HEAD~N.. in order to comply with git-log-fixes.sh getopts
> -if printf -- $range | grep -q '^-[0-9]\+' ; then
> -	range="HEAD$(printf -- $range | sed 's,^-,~,').."
> +elif printf -- "$range" | grep -q '^-[0-9]\+' ; then
> +	range="HEAD$(printf -- "$range" | sed 's,^-,~,').."

getopts won't be called if $1 starts with -N.
I think it would be cleaner to handle this in "?" case below.

> +else
> +	while getopts hr:n: ARG ; do
> +		case $ARG in
> +			n ) range="HEAD~$OPTARG.." ;;
> +			r ) range=$OPTARG ;;

-r is not a git-log option.
Please handle it without the need for -r.

> +			h ) print_usage ; exit 0 ;;
> +			? ) print_usage ; exit 1 ;;
> +		esac
> +	done
> +	shift $(($OPTIND - 1))

Re: [dpdk-dev] [PATCH 1/2] devtools: standardize script arguments

[Power, Ciara]

Hi Thomas,

Thanks for the review,


>Subject: Re: [dpdk-dev] [PATCH 1/2] devtools: standardize script arguments
>
>Hi,
>
>Thanks for improving tooling.
>
>28/01/2020 16:02, Ciara Power:
>> range=${1:-origin/master..}
>
>If doing a real option management, range should be the remaining argument
>after option parsing.


The goal of this patch is to make the check-git-log and checkpatches scripts more
consistent, while maintaining backward compatibility.
I think the range value here should remain unchanged, so users can use the script
as they have been using it before this patch, passing range as $1.


>
>> +if [ "$range" = '--help' ] ; then
>> +	print_usage
>
>Missing "exit 0" after usage.
>
>>  # convert -N to HEAD~N.. in order to comply with git-log-fixes.sh
>> getopts -if printf -- $range | grep -q '^-[0-9]\+' ; then
>> -	range="HEAD$(printf -- $range | sed 's,^-,~,').."
>> +elif printf -- "$range" | grep -q '^-[0-9]\+' ; then
>> +	range="HEAD$(printf -- "$range" | sed 's,^-,~,').."
>
>getopts won't be called if $1 starts with -N.
>I think it would be cleaner to handle this in "?" case below.
>


Does the getopts need to be called if $1 is -N? 
I am not sure handling this in the "?" case below would work - as far as I 
know if the N value is more than one character, it would be treated as 
multiple separate characters (e.g. -123 would be treated as -1 2 3)


>> +else
>> +	while getopts hr:n: ARG ; do
>> +		case $ARG in
>> +			n ) range="HEAD~$OPTARG.." ;;
>> +			r ) range=$OPTARG ;;
>
>-r is not a git-log option.
>Please handle it without the need for -r.
>


The -r option is added here to standardise the scripts - this is currently being
used in the checkpatches getops.


>> +			h ) print_usage ; exit 0 ;;
>> +			? ) print_usage ; exit 1 ;;
>> +		esac
>> +	done
>> +	shift $(($OPTIND - 1))
>
>


Thanks,
Ciara

[dpdk-dev] [PATCH 2/2] devtools: added stats print

[Ciara Power]

When all checks are completed on the specified commit logs, the script
indicates if all are valid, or if there were some failures.

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 devtools/check-git-log.sh | 45 ++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index 22b2a6d84..2f7a9f344 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -23,6 +23,7 @@ print_usage () {
 
 selfdir=$(dirname $(readlink -f $0))
 range=${1:-origin/master..}
+failure=false
 
 if [ "$range" = '--help' ] ; then
 	print_usage
@@ -61,7 +62,7 @@ bad=$(echo "$headlines" | grep --color=always \
 	-e ':[^ ]' \
 	-e ' :' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline format:\n$bad\n" && failure=true;}
 
 # check headline prefix when touching only drivers, e.g. net/<driver name>
 bad=$(for commit in $commits ; do
@@ -79,7 +80,7 @@ bad=$(for commit in $commits ; do
 		echo "$headline" | grep -v "^$drv"
 	fi
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline prefix:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline prefix:\n$bad\n" && failure=true;}
 
 # check headline label for common typos
 bad=$(echo "$headlines" | grep --color=always \
@@ -89,14 +90,14 @@ bad=$(echo "$headlines" | grep --color=always \
 	-e 'test-pmd' \
 	-e '^bond:' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline label:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline label:\n$bad\n" && failure=true;}
 
 # check headline lowercase for first words
 bad=$(echo "$headlines" | grep --color=always \
 	-e '^.*[[:upper:]].*:' \
 	-e ': *[[:upper:]]' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline uppercase:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline uppercase:\n$bad\n" && failure=true;}
 
 # check headline uppercase (Rx/Tx, VF, L2, MAC, Linux, ARM...)
 bad=$(echo "$headlines" | grep -E --color=always \
@@ -142,39 +143,41 @@ bad=$(echo "$headlines" | grep -E --color=always \
 	| grep \
 	-v ':.*\<OCTEON\ TX\>' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline lowercase:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline lowercase:\n$bad\n" && failure=true;}
 
 # special case check for VMDq to give good error message
 bad=$(echo "$headlines" | grep -E --color=always \
 	-e '\<(vmdq|VMDQ)\>' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline capitalization, use 'VMDq':\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline capitalization, use 'VMDq':\n$bad\n"\
+	&& failure=true;}
 
 # check headline length (60 max)
 bad=$(echo "$headlines" |
 	awk 'length>60 {print}' |
 	sed 's,^,\t,')
-[ -z "$bad" ] || printf "Headline too long:\n$bad\n"
+[ -z "$bad" ] || { printf "Headline too long:\n$bad\n" && failure=true;}
 
 # check body lines length (75 max)
 bad=$(echo "$bodylines" | grep -v '^Fixes:' |
 	awk 'length>75 {print}' |
 	sed 's,^,\t,')
-[ -z "$bad" ] || printf "Line too long:\n$bad\n"
+[ -z "$bad" ] || { printf "Line too long:\n$bad\n" && failure=true;}
 
 # check starting commit message with "It"
 bad=$(for commit in $commits ; do
 	firstbodyline=$(git log --format='%b' -1 $commit | head -n1)
 	echo "$firstbodyline" | grep --color=always -ie '^It '
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong beginning of commit message:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong beginning of commit message:\n$bad\n"\
+	&& failure=true;}
 
 # check tags spelling
 bad=$(echo "$tags" |
 	grep -v "^$bytag [^,]* <.*@.*>$" |
 	grep -v '^Fixes: [0-9a-f]\{7\}[0-9a-f]* (".*")$' |
 	sed 's,^.,\t&,')
-[ -z "$bad" ] || printf "Wrong tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong tag:\n$bad\n" && failure=true;}
 
 # check missing Coverity issue: tag
 bad=$(for commit in $commits; do
@@ -183,7 +186,8 @@ bad=$(for commit in $commits; do
 	echo "$body" | grep -q '^Coverity issue:' && continue
 	git log --format='\t%s' -1 $commit
 done)
-[ -z "$bad" ] || printf "Missing 'Coverity issue:' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Coverity issue:' tag:\n$bad\n"\
+	&& failure=true;}
 
 # check missing Bugzilla ID: tag
 bad=$(for commit in $commits; do
@@ -192,14 +196,15 @@ bad=$(for commit in $commits; do
 	echo "$body" | grep -q '^Bugzilla ID:' && continue
 	git log --format='\t%s' -1 $commit
 done)
-[ -z "$bad" ] || printf "Missing 'Bugzilla ID:' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Bugzilla ID:' tag:\n$bad\n"\
+	&& failure=true;}
 
 # check missing Fixes: tag
 bad=$(for fix in $fixes ; do
 	git log --format='%b' -1 $fix | grep -q '^Fixes: ' ||
 		git log --format='\t%s' -1 $fix
 done)
-[ -z "$bad" ] || printf "Missing 'Fixes' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Fixes' tag:\n$bad\n" && failure=true;}
 
 # check Fixes: reference
 IFS='
@@ -214,11 +219,21 @@ bad=$(for fixtag in $fixtags ; do
 	fi
 	printf "$fixtag" | grep -v "^$good$"
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong 'Fixes' reference:\n$bad\n" && failure=true;}
 
 # check Cc: stable@dpdk.org for fixes
 bad=$(for fix in $stablefixes ; do
 	git log --format='%b' -1 $fix | grep -qi '^Cc: *stable@dpdk.org' ||
 		git log --format='\t%s' -1 $fix
 done)
-[ -z "$bad" ] || printf "Is it candidate for Cc: stable@dpdk.org backport?\n$bad\n"
+[ -z "$bad" ] || { printf "Is it candidate for Cc: stable@dpdk.org backport?\n$bad\n"\
+	&& failure=true;}
+
+total=$(echo "$commits" | wc -l)
+if $failure ; then
+	printf "\nInvalid patch(es) found - checked $total patch"
+else
+	printf "\n$total/$total valid patch"
+fi
+[ $total -le 1 ] || printf 'es'
+printf '\n'
-- 
2.17.1

Re: [dpdk-dev] [PATCH 2/2] devtools: added stats print

[Ferruh Yigit]

On 1/28/2020 3:02 PM, Ciara Power wrote:
> When all checks are completed on the specified commit logs, the script
> indicates if all are valid, or if there were some failures.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH 2/2] devtools: added stats print

[Thomas Monjalon]

28/01/2020 16:02, Ciara Power:
> When all checks are completed on the specified commit logs, the script
> indicates if all are valid, or if there were some failures.

I think the most important would be to have an error code.
Please could you exit with 0 only if no issue is found?
[...]
> +total=$(echo "$commits" | wc -l)
> +if $failure ; then
> +	printf "\nInvalid patch(es) found - checked $total patch"
> +else
> +	printf "\n$total/$total valid patch"
> +fi
> +[ $total -le 1 ] || printf 'es'
> +printf '\n'

[dpdk-dev] [PATCH v2 0/2] standardize devtools check scripts

[Ciara Power]

v2: fix comments on v1.

This patchset standardizes the checkpatches and check-git-log scripts
to accept the same syntax commandline arguments, to make them easier
to use.

The output is also standardized, check-git-log previously showed no
output to the user when no errors were found, but now prints a similar
output to that of the checkpatches script.

Ciara Power (2):
  devtools: standardize script arguments
  devtools: added stats print

 devtools/check-git-log.sh | 78 ++++++++++++++++++++++++++-------------
 devtools/checkpatches.sh  |  2 +-
 2 files changed, 54 insertions(+), 26 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v2 1/2] devtools: standardize script arguments

[Ciara Power]

This patch modifies the arguments expected by the check-git-log script,
to match the format of arguments for the checkpatches script. Both
scripts now take certain argument options in the same format, making
them easier to use.
e.g. Both now take a commit ID range by "-r <range>"

The checkpatches help print is also updated to include the "-h" option.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
v2: Added exit 0 after print usage.
---
 devtools/check-git-log.sh | 33 ++++++++++++++++++++++++---------
 devtools/checkpatches.sh  |  2 +-
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index 4e65be0e4b..efdfca27cc 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -7,23 +7,38 @@
 # If any doubt about the formatting, please check in the most recent history:
 #	git log --format='%>|(15)%cr   %s' --reverse | grep -i <pattern>
 
-if [ "$1" = '-h' -o "$1" = '--help' ] ; then
+print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [-h] [range]
+	usage: $(basename $0) [-h] [-nX|-r range]
 
 	Check commit log formatting.
-	The git range can be specified as a "git log" option,
-	e.g. -1 to check only the latest commit.
-	The default range starts from origin/master to HEAD.
+	The git commits to be checked can be specified as a "git log" option,
+	by latest git commits limited with -n option, or commits in the git
+	range specified with -r option.
+	e.g. -n1 to check only the latest commit.
+	The default starts from origin/master to HEAD.
 	END_OF_HELP
-	exit
-fi
+}
 
 selfdir=$(dirname $(readlink -f $0))
 range=${1:-origin/master..}
+
+if [ "$range" = '--help' ] ; then
+	print_usage
+	exit 0
 # convert -N to HEAD~N.. in order to comply with git-log-fixes.sh getopts
-if printf -- $range | grep -q '^-[0-9]\+' ; then
-	range="HEAD$(printf -- $range | sed 's,^-,~,').."
+elif printf -- "$range" | grep -q '^-[0-9]\+' ; then
+	range="HEAD$(printf -- "$range" | sed 's,^-,~,').."
+else
+	while getopts hr:n: ARG ; do
+		case $ARG in
+			n ) range="HEAD~$OPTARG.." ;;
+			r ) range=$OPTARG ;;
+			h ) print_usage ; exit 0 ;;
+			? ) print_usage ; exit 1 ;;
+		esac
+	done
+	shift $(($OPTIND - 1))
 fi
 
 commits=$(git log --format='%h' --reverse $range)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 42b833e0d7..e111c31d7d 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -38,7 +38,7 @@ options="$options $DPDK_CHECKPATCH_OPTIONS"
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [-q] [-v] [-nX|-r range|patch1 [patch2] ...]]
+	usage: $(basename $0) [-h] [-q] [-v] [-nX|-r range|patch1 [patch2] ...]
 
 	Run Linux kernel checkpatch.pl with DPDK options.
 	The environment variable DPDK_CHECKPATCH_PATH must be set.
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2 1/2] devtools: standardize script arguments

[Thomas Monjalon]

06/05/2020 11:55, Ciara Power:
> This patch modifies the arguments expected by the check-git-log script,
> to match the format of arguments for the checkpatches script. Both
> scripts now take certain argument options in the same format, making
> them easier to use.
> e.g. Both now take a commit ID range by "-r <range>"
[...]
> -	usage: $(basename $0) [-h] [range]
> +	usage: $(basename $0) [-h] [-nX|-r range]

Why not specifying that range can be also the first argument?
It is a discrepancy with what is documented in
doc/guides/contributing/patches.rst


>  	Check commit log formatting.
> -	The git range can be specified as a "git log" option,
> -	e.g. -1 to check only the latest commit.
> -	The default range starts from origin/master to HEAD.
> +	The git commits to be checked can be specified as a "git log" option,
> +	by latest git commits limited with -n option, or commits in the git
> +	range specified with -r option.
> +	e.g. -n1 to check only the latest commit.

This line "e.g. -n1" looks disconnected from the above lines.

> +	The default starts from origin/master to HEAD.

The rest looks OK.

Re: [dpdk-dev] [PATCH v2 1/2] devtools: standardize script arguments

[Power, Ciara]

Hi Thomas,


>-----Original Message-----
>From: Thomas Monjalon <thomas@monjalon.net>
>Sent: Sunday 24 May 2020 21:58
>To: Power, Ciara <ciara.power@intel.com>
>Cc: dev@dpdk.org
>Subject: Re: [dpdk-dev] [PATCH v2 1/2] devtools: standardize script
>arguments
>
>06/05/2020 11:55, Ciara Power:
>> This patch modifies the arguments expected by the check-git-log
>> script, to match the format of arguments for the checkpatches script.
>> Both scripts now take certain argument options in the same format,
>> making them easier to use.
>> e.g. Both now take a commit ID range by "-r <range>"
>[...]
>> -	usage: $(basename $0) [-h] [range]
>> +	usage: $(basename $0) [-h] [-nX|-r range]
>
>Why not specifying that range can be also the first argument?
>It is a discrepancy with what is documented in
>doc/guides/contributing/patches.rst
>

I didn't include the old format (that takes range as first argument) here to encourage
using the standardised format moving forward, but anyone that uses the old format
via scripts etc. will not see any difference in use.
I can also update the doc to show this standardised format to avoid discrepancy.

<snip>


Thanks,
Ciara

Re: [dpdk-dev] [PATCH v2 1/2] devtools: standardize script arguments

[Thomas Monjalon]

28/05/2020 16:37, Power, Ciara:
> From: Thomas Monjalon <thomas@monjalon.net>
> >06/05/2020 11:55, Ciara Power:
> >> This patch modifies the arguments expected by the check-git-log
> >> script, to match the format of arguments for the checkpatches script.
> >> Both scripts now take certain argument options in the same format,
> >> making them easier to use.
> >> e.g. Both now take a commit ID range by "-r <range>"
> >[...]
> >> -	usage: $(basename $0) [-h] [range]
> >> +	usage: $(basename $0) [-h] [-nX|-r range]
> >
> >Why not specifying that range can be also the first argument?
> >It is a discrepancy with what is documented in
> >doc/guides/contributing/patches.rst
> >
> 
> I didn't include the old format (that takes range as first argument) here to encourage
> using the standardised format moving forward, but anyone that uses the old format
> via scripts etc. will not see any difference in use.

OK
So maybe just add a comment in the script on the old argument parsing
to mention it is the old syntax.

> I can also update the doc to show this standardised format to avoid discrepancy.

Yes please replace with the new syntax in the doc.

[dpdk-dev] [PATCH v2 2/2] devtools: added stats print

[Ciara Power]

When all checks are completed on the specified commit logs, the script
indicates if all are valid, or if there were some failures.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
v2: Added appropriate exit codes based on failure status.
---
 devtools/check-git-log.sh | 45 +++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index efdfca27cc..270fe391d7 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -61,7 +61,7 @@ bad=$(echo "$headlines" | grep --color=always \
 	-e ':[^ ]' \
 	-e ' :' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline format:\n$bad\n" && failure=true;}
 
 # check headline prefix when touching only drivers, e.g. net/<driver name>
 bad=$(for commit in $commits ; do
@@ -79,7 +79,7 @@ bad=$(for commit in $commits ; do
 		echo "$headline" | grep -v "^$drv"
 	fi
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline prefix:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline prefix:\n$bad\n" && failure=true;}
 
 # check headline label for common typos
 bad=$(echo "$headlines" | grep --color=always \
@@ -89,14 +89,14 @@ bad=$(echo "$headlines" | grep --color=always \
 	-e 'test-pmd' \
 	-e '^bond:' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline label:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline label:\n$bad\n" && failure=true;}
 
 # check headline lowercase for first words
 bad=$(echo "$headlines" | grep --color=always \
 	-e '^.*[[:upper:]].*:' \
 	-e ': *[[:upper:]]' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline uppercase:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline uppercase:\n$bad\n" && failure=true;}
 
 # check headline case (Rx/Tx, VF, L2, MAC, Linux ...)
 IFS='
@@ -109,9 +109,8 @@ for word in $(cat $words); do
 	fi
 	for bad_line in $bad; do
 		bad_word=$(echo $bad_line | cut -d":" -f2 | grep -io $word)
-		if [ -n "$bad_word" ]; then
-			printf "Wrong headline case:\n\"$bad_line\": $bad_word --> $word\n"
-		fi
+		[ -z "$bad_word" ] || { printf "Wrong headline case:\n\
+			\"$bad_line\": $bad_word --> $word\n" && failure=true;}
 	done
 done
 
@@ -119,27 +118,28 @@ done
 bad=$(echo "$headlines" |
 	awk 'length>60 {print}' |
 	sed 's,^,\t,')
-[ -z "$bad" ] || printf "Headline too long:\n$bad\n"
+[ -z "$bad" ] || { printf "Headline too long:\n$bad\n" && failure=true;}
 
 # check body lines length (75 max)
 bad=$(echo "$bodylines" | grep -v '^Fixes:' |
 	awk 'length>75 {print}' |
 	sed 's,^,\t,')
-[ -z "$bad" ] || printf "Line too long:\n$bad\n"
+[ -z "$bad" ] || { printf "Line too long:\n$bad\n" && failure=true;}
 
 # check starting commit message with "It"
 bad=$(for commit in $commits ; do
 	firstbodyline=$(git log --format='%b' -1 $commit | head -n1)
 	echo "$firstbodyline" | grep --color=always -ie '^It '
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong beginning of commit message:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong beginning of commit message:\n$bad\n"\
+	&& failure=true;}
 
 # check tags spelling
 bad=$(echo "$tags" |
 	grep -v "^$bytag [^,]* <.*@.*>$" |
 	grep -v '^Fixes: [0-9a-f]\{7\}[0-9a-f]* (".*")$' |
 	sed 's,^.,\t&,')
-[ -z "$bad" ] || printf "Wrong tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong tag:\n$bad\n" && failure=true;}
 
 # check missing Coverity issue: tag
 bad=$(for commit in $commits; do
@@ -148,7 +148,8 @@ bad=$(for commit in $commits; do
 	echo "$body" | grep -q '^Coverity issue:' && continue
 	git log --format='\t%s' -1 $commit
 done)
-[ -z "$bad" ] || printf "Missing 'Coverity issue:' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Coverity issue:' tag:\n$bad\n"\
+	&& failure=true;}
 
 # check missing Bugzilla ID: tag
 bad=$(for commit in $commits; do
@@ -157,14 +158,15 @@ bad=$(for commit in $commits; do
 	echo "$body" | grep -q '^Bugzilla ID:' && continue
 	git log --format='\t%s' -1 $commit
 done)
-[ -z "$bad" ] || printf "Missing 'Bugzilla ID:' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Bugzilla ID:' tag:\n$bad\n"\
+	&& failure=true;}
 
 # check missing Fixes: tag
 bad=$(for fix in $fixes ; do
 	git log --format='%b' -1 $fix | grep -q '^Fixes: ' ||
 		git log --format='\t%s' -1 $fix
 done)
-[ -z "$bad" ] || printf "Missing 'Fixes' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Fixes' tag:\n$bad\n" && failure=true;}
 
 # check Fixes: reference
 fixtags=$(echo "$tags" | grep '^Fixes: ')
@@ -177,11 +179,22 @@ bad=$(for fixtag in $fixtags ; do
 	fi
 	printf "$fixtag" | grep -v "^$good$"
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong 'Fixes' reference:\n$bad\n" && failure=true;}
 
 # check Cc: stable@dpdk.org for fixes
 bad=$(for fix in $stablefixes ; do
 	git log --format='%b' -1 $fix | grep -qi '^Cc: *stable@dpdk.org' ||
 		git log --format='\t%s' -1 $fix
 done)
-[ -z "$bad" ] || printf "Is it candidate for Cc: stable@dpdk.org backport?\n$bad\n"
+[ -z "$bad" ] || { printf "Is it candidate for Cc: stable@dpdk.org backport?\n$bad\n"\
+	&& failure=true;}
+
+total=$(echo "$commits" | wc -l)
+if [ -n "$failure" ] ; then
+	printf "\nInvalid patch(es) found - checked $total patch"
+else
+	printf "\n$total/$total valid patch"
+fi
+[ $total -le 1 ] || printf 'es'
+printf '\n'
+[ -n "$failure" ] && exit 1 || exit 0
-- 
2.17.1

[dpdk-dev] [PATCH v3 0/3] standardize devtools check scripts

[Ciara Power]

v3:
  - Fix comments on v2.
  - Add patch to update contributor's guide.

v2: Fix comments on v1.

This patchset standardizes the checkpatches and check-git-log scripts
to accept the same syntax commandline arguments, to make them easier
to use.

The output is also standardized, check-git-log previously showed no
output to the user when no errors were found, but now prints a similar
output to that of the checkpatches script.

Ciara Power (3):
  devtools: standardize script arguments
  devtools: added stats print
  doc/guides: updated script usage for checking patches

 devtools/check-git-log.sh           | 87 ++++++++++++++++++++---------
 devtools/checkpatches.sh            |  2 +-
 doc/guides/contributing/patches.rst | 12 +++-
 3 files changed, 72 insertions(+), 29 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v3 1/3] devtools: standardize script arguments

[Ciara Power]

This patch modifies the arguments expected by the check-git-log script,
to match the format of arguments for the checkpatches script. Both
scripts now take certain argument options in the same format, making
them easier to use.
e.g. Both now take a commit ID range by "-r <range>"

The checkpatches help print is also updated to include the "-h" option.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
v3:
  - Reworded check-git-log help text example.
  - Added comment to indicate new and old format argument parsing.
v2: Added exit 0 after print usage.
---
 devtools/check-git-log.sh | 42 ++++++++++++++++++++++++++++++---------
 devtools/checkpatches.sh  |  2 +-
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index 4e65be0e4..5220765b9 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -7,23 +7,47 @@
 # If any doubt about the formatting, please check in the most recent history:
 #	git log --format='%>|(15)%cr   %s' --reverse | grep -i <pattern>
 
-if [ "$1" = '-h' -o "$1" = '--help' ] ; then
+print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [-h] [range]
+	usage: $(basename $0) [-h] [-nX|-r range]
 
 	Check commit log formatting.
-	The git range can be specified as a "git log" option,
-	e.g. -1 to check only the latest commit.
-	The default range starts from origin/master to HEAD.
+	The git commits to be checked can be specified as a "git log" option,
+	by latest git commits limited with -n option, or commits in the git
+	range specified with -r option.
+	e.g. To check only the last commit, ‘-n1’ or ‘-r@~..’ is used.
+	When no parameter is provided, script will use range starting from
+	origin/master to HEAD.
 	END_OF_HELP
-	exit
-fi
+}
 
 selfdir=$(dirname $(readlink -f $0))
+# The script caters for two formats, the new preferred format, and the old
+# format to ensure backward compatibility.
+# The new format is aligned with the format of the checkpatches script,
+# and allows for specifying the patches to check by passing -nX or -r range.
+# e.g. To check only the last commit, ‘-n1’ or ‘-r@~..’ is used.
+# The old format allows for specifying patches by passing -X or range
+# as the first argument.
+# e.g. To check only the last commit, '-1'  or '@~..' is used as first argument.
 range=${1:-origin/master..}
+
+if [ "$range" = '--help' ] ; then
+	print_usage
+	exit 0
 # convert -N to HEAD~N.. in order to comply with git-log-fixes.sh getopts
-if printf -- $range | grep -q '^-[0-9]\+' ; then
-	range="HEAD$(printf -- $range | sed 's,^-,~,').."
+elif printf -- "$range" | grep -q '^-[0-9]\+' ; then
+	range="HEAD$(printf -- "$range" | sed 's,^-,~,').."
+else
+	while getopts hr:n: ARG ; do
+		case $ARG in
+			n ) range="HEAD~$OPTARG.." ;;
+			r ) range=$OPTARG ;;
+			h ) print_usage ; exit 0 ;;
+			? ) print_usage ; exit 1 ;;
+		esac
+	done
+	shift $(($OPTIND - 1))
 fi
 
 commits=$(git log --format='%h' --reverse $range)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 42b833e0d..e111c31d7 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -38,7 +38,7 @@ options="$options $DPDK_CHECKPATCH_OPTIONS"
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [-q] [-v] [-nX|-r range|patch1 [patch2] ...]]
+	usage: $(basename $0) [-h] [-q] [-v] [-nX|-r range|patch1 [patch2] ...]
 
 	Run Linux kernel checkpatch.pl with DPDK options.
 	The environment variable DPDK_CHECKPATCH_PATH must be set.
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3 1/3] devtools: standardize script arguments

[Thomas Monjalon]

02/06/2020 15:53, Ciara Power:
> This patch modifies the arguments expected by the check-git-log script,
> to match the format of arguments for the checkpatches script. Both
> scripts now take certain argument options in the same format, making
> them easier to use.
> e.g. Both now take a commit ID range by "-r <range>"
> 
> The checkpatches help print is also updated to include the "-h" option.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> ---
> v3:
>   - Reworded check-git-log help text example.
>   - Added comment to indicate new and old format argument parsing.
> v2: Added exit 0 after print usage.
> ---
> --- a/devtools/check-git-log.sh
> +++ b/devtools/check-git-log.sh
> +	The git commits to be checked can be specified as a "git log" option,
> +	by latest git commits limited with -n option, or commits in the git
> +	range specified with -r option.
> +	e.g. To check only the last commit, ‘-n1’ or ‘-r@~..’ is used.
> +	When no parameter is provided, script will use range starting from
> +	origin/master to HEAD.

I suggest this shorter sentence:
If no range provided, default is origin/master..HEAD.

> +# The script caters for two formats, the new preferred format, and the old
> +# format to ensure backward compatibility.
> +# The new format is aligned with the format of the checkpatches script,
> +# and allows for specifying the patches to check by passing -nX or -r range.
> +# e.g. To check only the last commit, ‘-n1’ or ‘-r@~..’ is used.

This example line is useless here.

> +# The old format allows for specifying patches by passing -X or range
> +# as the first argument.
> +# e.g. To check only the last commit, '-1'  or '@~..' is used as first argument.

This example line is useless here.

[dpdk-dev] [PATCH v3 2/3] devtools: added stats print

[Ciara Power]

When all checks are completed on the specified commit logs, the script
indicates if all are valid, or if there were some failures.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
v2: Added appropriate exit codes based on failure status.
---
 devtools/check-git-log.sh | 45 +++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index 5220765b9..eca8506d9 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -70,7 +70,7 @@ bad=$(echo "$headlines" | grep --color=always \
 	-e ':[^ ]' \
 	-e ' :' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline format:\n$bad\n" && failure=true;}
 
 # check headline prefix when touching only drivers, e.g. net/<driver name>
 bad=$(for commit in $commits ; do
@@ -88,7 +88,7 @@ bad=$(for commit in $commits ; do
 		echo "$headline" | grep -v "^$drv"
 	fi
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline prefix:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline prefix:\n$bad\n" && failure=true;}
 
 # check headline label for common typos
 bad=$(echo "$headlines" | grep --color=always \
@@ -98,14 +98,14 @@ bad=$(echo "$headlines" | grep --color=always \
 	-e 'test-pmd' \
 	-e '^bond:' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline label:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline label:\n$bad\n" && failure=true;}
 
 # check headline lowercase for first words
 bad=$(echo "$headlines" | grep --color=always \
 	-e '^.*[[:upper:]].*:' \
 	-e ': *[[:upper:]]' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline uppercase:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline uppercase:\n$bad\n" && failure=true;}
 
 # check headline case (Rx/Tx, VF, L2, MAC, Linux ...)
 IFS='
@@ -118,9 +118,8 @@ for word in $(cat $words); do
 	fi
 	for bad_line in $bad; do
 		bad_word=$(echo $bad_line | cut -d":" -f2 | grep -io $word)
-		if [ -n "$bad_word" ]; then
-			printf "Wrong headline case:\n\"$bad_line\": $bad_word --> $word\n"
-		fi
+		[ -z "$bad_word" ] || { printf "Wrong headline case:\n\
+			\"$bad_line\": $bad_word --> $word\n" && failure=true;}
 	done
 done
 
@@ -128,27 +127,28 @@ done
 bad=$(echo "$headlines" |
 	awk 'length>60 {print}' |
 	sed 's,^,\t,')
-[ -z "$bad" ] || printf "Headline too long:\n$bad\n"
+[ -z "$bad" ] || { printf "Headline too long:\n$bad\n" && failure=true;}
 
 # check body lines length (75 max)
 bad=$(echo "$bodylines" | grep -v '^Fixes:' |
 	awk 'length>75 {print}' |
 	sed 's,^,\t,')
-[ -z "$bad" ] || printf "Line too long:\n$bad\n"
+[ -z "$bad" ] || { printf "Line too long:\n$bad\n" && failure=true;}
 
 # check starting commit message with "It"
 bad=$(for commit in $commits ; do
 	firstbodyline=$(git log --format='%b' -1 $commit | head -n1)
 	echo "$firstbodyline" | grep --color=always -ie '^It '
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong beginning of commit message:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong beginning of commit message:\n$bad\n"\
+	&& failure=true;}
 
 # check tags spelling
 bad=$(echo "$tags" |
 	grep -v "^$bytag [^,]* <.*@.*>$" |
 	grep -v '^Fixes: [0-9a-f]\{7\}[0-9a-f]* (".*")$' |
 	sed 's,^.,\t&,')
-[ -z "$bad" ] || printf "Wrong tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong tag:\n$bad\n" && failure=true;}
 
 # check missing Coverity issue: tag
 bad=$(for commit in $commits; do
@@ -157,7 +157,8 @@ bad=$(for commit in $commits; do
 	echo "$body" | grep -q '^Coverity issue:' && continue
 	git log --format='\t%s' -1 $commit
 done)
-[ -z "$bad" ] || printf "Missing 'Coverity issue:' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Coverity issue:' tag:\n$bad\n"\
+	&& failure=true;}
 
 # check missing Bugzilla ID: tag
 bad=$(for commit in $commits; do
@@ -166,14 +167,15 @@ bad=$(for commit in $commits; do
 	echo "$body" | grep -q '^Bugzilla ID:' && continue
 	git log --format='\t%s' -1 $commit
 done)
-[ -z "$bad" ] || printf "Missing 'Bugzilla ID:' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Bugzilla ID:' tag:\n$bad\n"\
+	&& failure=true;}
 
 # check missing Fixes: tag
 bad=$(for fix in $fixes ; do
 	git log --format='%b' -1 $fix | grep -q '^Fixes: ' ||
 		git log --format='\t%s' -1 $fix
 done)
-[ -z "$bad" ] || printf "Missing 'Fixes' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Fixes' tag:\n$bad\n" && failure=true;}
 
 # check Fixes: reference
 fixtags=$(echo "$tags" | grep '^Fixes: ')
@@ -186,11 +188,22 @@ bad=$(for fixtag in $fixtags ; do
 	fi
 	printf "$fixtag" | grep -v "^$good$"
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong 'Fixes' reference:\n$bad\n" && failure=true;}
 
 # check Cc: stable@dpdk.org for fixes
 bad=$(for fix in $stablefixes ; do
 	git log --format='%b' -1 $fix | grep -qi '^Cc: *stable@dpdk.org' ||
 		git log --format='\t%s' -1 $fix
 done)
-[ -z "$bad" ] || printf "Is it candidate for Cc: stable@dpdk.org backport?\n$bad\n"
+[ -z "$bad" ] || { printf "Is it candidate for Cc: stable@dpdk.org backport?\n$bad\n"\
+	&& failure=true;}
+
+total=$(echo "$commits" | wc -l)
+if [ -n "$failure" ] ; then
+	printf "\nInvalid patch(es) found - checked $total patch"
+else
+	printf "\n$total/$total valid patch"
+fi
+[ $total -le 1 ] || printf 'es'
+printf '\n'
+[ -n "$failure" ] && exit 1 || exit 0
-- 
2.17.1

[dpdk-dev] [PATCH v3 3/3] doc/guides: updated script usage for checking patches

[Ciara Power]

The contributor's guide includes the usage of both the checkpatches and
check-git-log scripts, which needed to be updated to reflect the now
standardised format.

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 doc/guides/contributing/patches.rst | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index 59442824a..f11e4d7c7 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -429,22 +429,28 @@ Once the environment variable the script can be run as follows::
 
 The script usage is::
 
-   checkpatches.sh [-h] [-q] [-v] [patch1 [patch2] ...]]"
+   checkpatches.sh [-h] [-q] [-v] [-nX|-r range|patch1 [patch2] ...]"
 
 Where:
 
 * ``-h``: help, usage.
 * ``-q``: quiet. Don't output anything for files without issues.
 * ``-v``: verbose.
+* ``-nX``: the number of commits to check.
+* ``-r range``: the range to check, range must be a ``git log`` option.
 * ``patchX``: path to one or more patches.
 
 Then the git logs should be checked using the ``check-git-log.sh`` script.
 
 The script usage is::
 
-   check-git-log.sh [range]
+   check-git-log.sh [-h] [-nX|-r range]
 
-Where the range is a ``git log`` option.
+Where:
+
+* ``-h``: help, usage.
+* ``-nX``: the number of commits to check.
+* ``-r range``: the range to check, range must be a ``git log`` option.
 
 
 .. _contrib_check_compilation:
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3 3/3] doc/guides: updated script usage for checking patches

[Ferruh Yigit]

On 6/2/2020 2:53 PM, Ciara Power wrote:
> The contributor's guide includes the usage of both the checkpatches and
> check-git-log scripts, which needed to be updated to reflect the now
> standardised format.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v3 3/3] doc/guides: updated script usage for checking patches

[Thomas Monjalon]

02/06/2020 15:53, Ciara Power:
> The contributor's guide includes the usage of both the checkpatches and
> check-git-log scripts, which needed to be updated to reflect the now
> standardised format.

I think the doc update should be merged in the first patch.
In general, doc and code updates should come together.

[...]
>  The script usage is::
>  
> -   checkpatches.sh [-h] [-q] [-v] [patch1 [patch2] ...]]"
> +   checkpatches.sh [-h] [-q] [-v] [-nX|-r range|patch1 [patch2] ...]"
>  
>  Where:
>  
>  * ``-h``: help, usage.
>  * ``-q``: quiet. Don't output anything for files without issues.
>  * ``-v``: verbose.
> +* ``-nX``: the number of commits to check.
> +* ``-r range``: the range to check, range must be a ``git log`` option.
>  * ``patchX``: path to one or more patches.

This is repeating what we get with the -h option.
I would suggest dropping the options explanation from this guide.

>  
>  Then the git logs should be checked using the ``check-git-log.sh`` script.
>  
>  The script usage is::
>  
> -   check-git-log.sh [range]
> +   check-git-log.sh [-h] [-nX|-r range]
>  
> -Where the range is a ``git log`` option.
> +Where:
> +
> +* ``-h``: help, usage.
> +* ``-nX``: the number of commits to check.
> +* ``-r range``: the range to check, range must be a ``git log`` option.

Same here, it is repeating what is documented with using -h.

If we really want to give an indication, we can say once for both that
the -n option is a number of commits from HEAD,
and -r option is a "git log" range.
A single sentence is probably enough.

In general, if we want guides to be read, they must be short.
John, as the doc owner, do you agree?

[dpdk-dev] [PATCH v4 0/2] standardize devtools check scripts

[Ciara Power]

v4:
  - Merge doc patch into patch with code changes.
  - Simplified and reduced documentation and comments.

v3:
  - Fix comments on v2.
  - Add patch to update contributor's guide.

v2: Fix comments on v1.

This patchset standardizes the checkpatches and check-git-log scripts
to accept the same syntax commandline arguments, to make them easier
to use.

The output is also standardized, check-git-log previously showed no
output to the user when no errors were found, but now prints a similar
output to that of the checkpatches script.

Ciara Power (2):
  devtools: standardize script arguments
  devtools: added stats print

 devtools/check-git-log.sh           | 84 ++++++++++++++++++++---------
 devtools/checkpatches.sh            |  2 +-
 doc/guides/contributing/patches.rst | 15 ++----
 3 files changed, 64 insertions(+), 37 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v4 1/2] devtools: standardize script arguments

[Ciara Power]

This patch modifies the arguments expected by the check-git-log script,
to match the format of arguments for the checkpatches script. Both
scripts now take certain argument options in the same format, making
them easier to use.
e.g. Both now take a commit ID range by "-r <range>"

The checkpatches help print is also updated to include the "-h" option.

The contributor's guide includes the usage of both the checkpatches and
check-git-log scripts, which needed to be updated to reflect the now
standardised format.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
v4:
  - Merged doc changes into this patch.
  - Simplified documented script usage to reduce duplication.
  - Removed examples in comments.
v3:
  - Reworded check-git-log help text example.
  - Added comment to indicate new and old format argument parsing.
v2: Added exit 0 after print usage.
---
 devtools/check-git-log.sh           | 39 ++++++++++++++++++++++-------
 devtools/checkpatches.sh            |  2 +-
 doc/guides/contributing/patches.rst | 15 +++--------
 3 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index 4e65be0e4..e5b430268 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -7,23 +7,44 @@
 # If any doubt about the formatting, please check in the most recent history:
 #	git log --format='%>|(15)%cr   %s' --reverse | grep -i <pattern>
 
-if [ "$1" = '-h' -o "$1" = '--help' ] ; then
+print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [-h] [range]
+	usage: $(basename $0) [-h] [-nX|-r range]
 
 	Check commit log formatting.
-	The git range can be specified as a "git log" option,
-	e.g. -1 to check only the latest commit.
-	The default range starts from origin/master to HEAD.
+	The git commits to be checked can be specified as a "git log" option,
+	by latest git commits limited with -n option, or commits in the git
+	range specified with -r option.
+	e.g. To check only the last commit, ‘-n1’ or ‘-r@~..’ is used.
+	If no range provided, default is origin/master..HEAD.
 	END_OF_HELP
-	exit
-fi
+}
 
 selfdir=$(dirname $(readlink -f $0))
+# The script caters for two formats, the new preferred format, and the old
+# format to ensure backward compatibility.
+# The new format is aligned with the format of the checkpatches script,
+# and allows for specifying the patches to check by passing -nX or -r range.
+# The old format allows for specifying patches by passing -X or range
+# as the first argument.
 range=${1:-origin/master..}
+
+if [ "$range" = '--help' ] ; then
+	print_usage
+	exit 0
 # convert -N to HEAD~N.. in order to comply with git-log-fixes.sh getopts
-if printf -- $range | grep -q '^-[0-9]\+' ; then
-	range="HEAD$(printf -- $range | sed 's,^-,~,').."
+elif printf -- "$range" | grep -q '^-[0-9]\+' ; then
+	range="HEAD$(printf -- "$range" | sed 's,^-,~,').."
+else
+	while getopts hr:n: ARG ; do
+		case $ARG in
+			n ) range="HEAD~$OPTARG.." ;;
+			r ) range=$OPTARG ;;
+			h ) print_usage ; exit 0 ;;
+			? ) print_usage ; exit 1 ;;
+		esac
+	done
+	shift $(($OPTIND - 1))
 fi
 
 commits=$(git log --format='%h' --reverse $range)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 158087f1c..5fc3b7d7b 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -38,7 +38,7 @@ options="$options $DPDK_CHECKPATCH_OPTIONS"
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [-q] [-v] [-nX|-r range|patch1 [patch2] ...]]
+	usage: $(basename $0) [-h] [-q] [-v] [-nX|-r range|patch1 [patch2] ...]
 
 	Run Linux kernel checkpatch.pl with DPDK options.
 	The environment variable DPDK_CHECKPATCH_PATH must be set.
diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index 16b40225f..261c4b848 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -429,23 +429,16 @@ Once the environment variable is set, the script can be run as follows::
 
 The script usage is::
 
-   checkpatches.sh [-h] [-q] [-v] [patch1 [patch2] ...]]"
-
-Where:
-
-* ``-h``: help, usage.
-* ``-q``: quiet. Don't output anything for files without issues.
-* ``-v``: verbose.
-* ``patchX``: path to one or more patches.
+   checkpatches.sh [-h] [-q] [-v] [-nX|-r range|patch1 [patch2] ...]
 
 Then the git logs should be checked using the ``check-git-log.sh`` script.
 
 The script usage is::
 
-   check-git-log.sh [range]
-
-Where the range is a ``git log`` option.
+   check-git-log.sh [-h] [-nX|-r range]
 
+For both of the above scripts, the -n option is used to specify a number of commits from HEAD,
+and the -r option allows the user specify a ``git log`` range.
 
 .. _contrib_check_compilation:
 
-- 
2.17.1

[dpdk-dev] [PATCH v4 2/2] devtools: added stats print

[Ciara Power]

When all checks are completed on the specified commit logs, the script
indicates if all are valid, or if there were some failures.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
v2: Added appropriate exit codes based on failure status.
---
 devtools/check-git-log.sh | 45 +++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index e5b430268..86746c4ad 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -67,7 +67,7 @@ bad=$(echo "$headlines" | grep --color=always \
 	-e ':[^ ]' \
 	-e ' :' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline format:\n$bad\n" && failure=true;}
 
 # check headline prefix when touching only drivers, e.g. net/<driver name>
 bad=$(for commit in $commits ; do
@@ -85,7 +85,7 @@ bad=$(for commit in $commits ; do
 		echo "$headline" | grep -v "^$drv"
 	fi
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline prefix:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline prefix:\n$bad\n" && failure=true;}
 
 # check headline label for common typos
 bad=$(echo "$headlines" | grep --color=always \
@@ -95,14 +95,14 @@ bad=$(echo "$headlines" | grep --color=always \
 	-e 'test-pmd' \
 	-e '^bond:' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline label:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline label:\n$bad\n" && failure=true;}
 
 # check headline lowercase for first words
 bad=$(echo "$headlines" | grep --color=always \
 	-e '^.*[[:upper:]].*:' \
 	-e ': *[[:upper:]]' \
 	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline uppercase:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong headline uppercase:\n$bad\n" && failure=true;}
 
 # check headline case (Rx/Tx, VF, L2, MAC, Linux ...)
 IFS='
@@ -115,9 +115,8 @@ for word in $(cat $words); do
 	fi
 	for bad_line in $bad; do
 		bad_word=$(echo $bad_line | cut -d":" -f2 | grep -io $word)
-		if [ -n "$bad_word" ]; then
-			printf "Wrong headline case:\n\"$bad_line\": $bad_word --> $word\n"
-		fi
+		[ -z "$bad_word" ] || { printf "Wrong headline case:\n\
+			\"$bad_line\": $bad_word --> $word\n" && failure=true;}
 	done
 done
 
@@ -125,27 +124,28 @@ done
 bad=$(echo "$headlines" |
 	awk 'length>60 {print}' |
 	sed 's,^,\t,')
-[ -z "$bad" ] || printf "Headline too long:\n$bad\n"
+[ -z "$bad" ] || { printf "Headline too long:\n$bad\n" && failure=true;}
 
 # check body lines length (75 max)
 bad=$(echo "$bodylines" | grep -v '^Fixes:' |
 	awk 'length>75 {print}' |
 	sed 's,^,\t,')
-[ -z "$bad" ] || printf "Line too long:\n$bad\n"
+[ -z "$bad" ] || { printf "Line too long:\n$bad\n" && failure=true;}
 
 # check starting commit message with "It"
 bad=$(for commit in $commits ; do
 	firstbodyline=$(git log --format='%b' -1 $commit | head -n1)
 	echo "$firstbodyline" | grep --color=always -ie '^It '
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong beginning of commit message:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong beginning of commit message:\n$bad\n"\
+	&& failure=true;}
 
 # check tags spelling
 bad=$(echo "$tags" |
 	grep -v "^$bytag [^,]* <.*@.*>$" |
 	grep -v '^Fixes: [0-9a-f]\{7\}[0-9a-f]* (".*")$' |
 	sed 's,^.,\t&,')
-[ -z "$bad" ] || printf "Wrong tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong tag:\n$bad\n" && failure=true;}
 
 # check missing Coverity issue: tag
 bad=$(for commit in $commits; do
@@ -154,7 +154,8 @@ bad=$(for commit in $commits; do
 	echo "$body" | grep -q '^Coverity issue:' && continue
 	git log --format='\t%s' -1 $commit
 done)
-[ -z "$bad" ] || printf "Missing 'Coverity issue:' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Coverity issue:' tag:\n$bad\n"\
+	&& failure=true;}
 
 # check missing Bugzilla ID: tag
 bad=$(for commit in $commits; do
@@ -163,14 +164,15 @@ bad=$(for commit in $commits; do
 	echo "$body" | grep -q '^Bugzilla ID:' && continue
 	git log --format='\t%s' -1 $commit
 done)
-[ -z "$bad" ] || printf "Missing 'Bugzilla ID:' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Bugzilla ID:' tag:\n$bad\n"\
+	&& failure=true;}
 
 # check missing Fixes: tag
 bad=$(for fix in $fixes ; do
 	git log --format='%b' -1 $fix | grep -q '^Fixes: ' ||
 		git log --format='\t%s' -1 $fix
 done)
-[ -z "$bad" ] || printf "Missing 'Fixes' tag:\n$bad\n"
+[ -z "$bad" ] || { printf "Missing 'Fixes' tag:\n$bad\n" && failure=true;}
 
 # check Fixes: reference
 fixtags=$(echo "$tags" | grep '^Fixes: ')
@@ -183,11 +185,22 @@ bad=$(for fixtag in $fixtags ; do
 	fi
 	printf "$fixtag" | grep -v "^$good$"
 done | sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"
+[ -z "$bad" ] || { printf "Wrong 'Fixes' reference:\n$bad\n" && failure=true;}
 
 # check Cc: stable@dpdk.org for fixes
 bad=$(for fix in $stablefixes ; do
 	git log --format='%b' -1 $fix | grep -qi '^Cc: *stable@dpdk.org' ||
 		git log --format='\t%s' -1 $fix
 done)
-[ -z "$bad" ] || printf "Is it candidate for Cc: stable@dpdk.org backport?\n$bad\n"
+[ -z "$bad" ] || { printf "Is it candidate for Cc: stable@dpdk.org backport?\n$bad\n"\
+	&& failure=true;}
+
+total=$(echo "$commits" | wc -l)
+if [ -n "$failure" ] ; then
+	printf "\nInvalid patch(es) found - checked $total patch"
+else
+	printf "\n$total/$total valid patch"
+fi
+[ $total -le 1 ] || printf 'es'
+printf '\n'
+[ -n "$failure" ] && exit 1 || exit 0
-- 
2.17.1

Re: [dpdk-dev] [PATCH v4 2/2] devtools: added stats print

[Thomas Monjalon]

23/06/2020 11:29, Ciara Power:
> When all checks are completed on the specified commit logs, the script
> indicates if all are valid, or if there were some failures.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> ---
> v2: Added appropriate exit codes based on failure status.
> ---
>  devtools/check-git-log.sh | 45 +++++++++++++++++++++++++--------------
>  1 file changed, 29 insertions(+), 16 deletions(-)
> 
> diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
> index e5b430268..86746c4ad 100755
> --- a/devtools/check-git-log.sh
> +++ b/devtools/check-git-log.sh
> -[ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
> +[ -z "$bad" ] || { printf "Wrong headline format:\n$bad\n" && failure=true;}
[...]
> +total=$(echo "$commits" | wc -l)
> +if [ -n "$failure" ] ; then
> +	printf "\nInvalid patch(es) found - checked $total patch"
> +else
> +	printf "\n$total/$total valid patch"
> +fi
> +[ $total -le 1 ] || printf 'es'
> +printf '\n'
> +[ -n "$failure" ] && exit 1 || exit 0

If $failure is initialized to false, it can be used as a real boolean:

-if [ -n "$failure" ] ; then
+if $failure ; then
        printf "\nInvalid patch(es) found - checked $total patch"
 else
        printf "\n$total/$total valid patch"
 fi
 [ $total -le 1 ] || printf 'es'
 printf '\n'
-[ -n "$failure" ] && exit 1 || exit 0
+$failure && exit 1 || exit 0

I take the liberty of doing this small improvement
without asking for a v5.

Re: [dpdk-dev] [PATCH v4 0/2] standardize devtools check scripts

[Thomas Monjalon]

23/06/2020 11:29, Ciara Power:
> v4:
>   - Merge doc patch into patch with code changes.
>   - Simplified and reduced documentation and comments.
> 
> v3:
>   - Fix comments on v2.
>   - Add patch to update contributor's guide.
> 
> v2: Fix comments on v1.
> 
> This patchset standardizes the checkpatches and check-git-log scripts
> to accept the same syntax commandline arguments, to make them easier
> to use.
> 
> The output is also standardized, check-git-log previously showed no
> output to the user when no errors were found, but now prints a similar
> output to that of the checkpatches script.
> 
> Ciara Power (2):
>   devtools: standardize script arguments
>   devtools: added stats print

Reviewed-by: Thomas Monjalon <thomas@monjalon.net>

Applied, thanks for your patience