[v3] devtools: ensure proper tag sequence
Checks
Commit Message
From: Jakub Palider <jpalider@marvell.com>
This change to log checking procedure ensures that certain
tags are in proper order.
The order of tags is as follows:
* Coverity issue
* Bugzilla ID
* Fixes
* Cc
* <BLANK LINE>
* Suggested-by
* Reported-by
+ Signed-off-by
* Acked-by
* Reviewed-by
* Tested-by
where:
* => 0 or more than one instance possible
+ => more than once instance possible
In order to satisfy the above requirements an extra check
is performed for obligatory tags.
v3:
* restored some tags under check
* defined chronological order after Signed-off
v2:
* narrowed down the tags under check
Signed-off-by: Jakub Palider <jpalider@marvell.com>
---
devtools/check-git-log.sh | 56 +++++++++++++++++++++++++++++
doc/guides/contributing/patches.rst | 28 +++++++++++++++
2 files changed, 84 insertions(+)
Comments
> -----Original Message-----
> From: jpalider@marvell.com <jpalider@marvell.com>
> Sent: Tuesday, July 19, 2022 2:01 PM
> To: Jakub Palider <jpalider@marvell.com>
> Cc: david.marchand@redhat.com; dev@dpdk.org; thomas@monjalon.net
> Subject: [PATCH v3] devtools: ensure proper tag sequence
>
> From: Jakub Palider <jpalider@marvell.com>
>
> This change to log checking procedure ensures that certain tags are in proper
> order.
> The order of tags is as follows:
> * Coverity issue
> * Bugzilla ID
> * Fixes
> * Cc
> * <BLANK LINE>
> * Suggested-by
> * Reported-by
> + Signed-off-by
> * Acked-by
> * Reviewed-by
> * Tested-by
> where:
> * => 0 or more than one instance possible + => more than once instance
> possible In order to satisfy the above requirements an extra check is
> performed for obligatory tags.
>
> v3:
> * restored some tags under check
> * defined chronological order after Signed-off
>
> v2:
> * narrowed down the tags under check
>
> Signed-off-by: Jakub Palider <jpalider@marvell.com>
> ---
> devtools/check-git-log.sh | 56 +++++++++++++++++++++++++++++
> doc/guides/contributing/patches.rst | 28 +++++++++++++++
> 2 files changed, 84 insertions(+)
>
> diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh index
> 23c6a7d9bb..9b98207f3a 100755
> --- a/devtools/check-git-log.sh
> +++ b/devtools/check-git-log.sh
> @@ -54,6 +54,7 @@ fixes=$(git log --format='%h %s' --reverse $range | grep
> -i ': *fix' | cut -d' '
> stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d' | cut -d' ' -
> f2) tags=$(git log --format='%b' --reverse $range | grep -i -e 'by *:' -e 'fix.*:')
> bytag='\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-
> by:'
> +exttag='Coverity issue:\|Bugzilla ID:\|Fixes:\|Cc:'
>
> failure=false
>
> @@ -203,6 +204,61 @@ done)
> [ -z "$bad" ] || { printf "Is it candidate for Cc: stable@dpdk.org
> backport?\n$bad\n"\
> && failure=true;}
>
> +# check tag sequence
> +bad=$(for commit in $commits; do
> + body=$(git log --format='%b' -1 $commit)
> + echo "$body" |\
> + grep -o -e "$exttag\|^[[:blank:]]*$\|$bytag" | \
> + # retrieve tags only
> + cut -f1 -d":" |\
> + # it is okay to have several tags of the same type but for processing
> + # we need to squash them
> + uniq |\
> + # make sure the tags are in the proper order as presented in SEQ
> + awk -v cmt="$commit" 'BEGIN{
> + SEQ[0] = "Coverity issue";
> + SEQ[1] = "Bugzilla ID";
> + SEQ[2] = "Fixes";
> + SEQ[3] = "Cc";
> + SEQ[4] = "^$";
> + SEQ[5] = "Suggested-by";
> + SEQ[6] = "Reported-by";
> + SEQ[7] = "Signed-off-by";
> + SEQ[8] = "Acked-by";
> + SEQ[9] = "Reviewed-by";
> + SEQ[10] = "Tested-by";
> + latest = 0;
> + chronological = 0;
> + }
> + {
> + for (seq = 0; seq < length(SEQ); seq++) {
> + if (chronological == 1)
> + continue;
> + if (match($0, SEQ[seq])) {
> + if (seq < latest) {
> + print "\tCommit " cmt " (" $0 ":)";
> + break;
> + } else {
> + latest = seq;
> + }
> + }
> + }
> + if (match($0, "Signed-off-by"))
> + chronological = 1;
> + }'
> +done)
> +[ -z "$bad" ] || { printf "Wrong tag order: \n$bad\n"\
> + && failure=true;}
> +
> +# check required tag
> +bad=$(for commit in $commits; do
> + body=$(git log --format='%b' -1 $commit)
> + echo $body | grep -q "Signed-off-by:" \
> + || echo "\tCommit" $commit "(Signed-off-by:)"
> + done)
> +[ -z "$bad" ] || { printf "Missing obligatory tag: \n$bad\n"\
> + && failure=true;}
> +
> total=$(echo "$commits" | wc -l)
> if $failure ; then
> printf "\nInvalid patch(es) found - checked $total patch"
> diff --git a/doc/guides/contributing/patches.rst
> b/doc/guides/contributing/patches.rst
> index bebcaf3925..996089f2eb 100644
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -360,6 +360,34 @@ Where ``NNNNN`` is patchwork ID for patch or
> series::
> ---
> Depends-on: series-10000 ("Title of the series")
>
> +Tag order
> +~~~~~~~~~
> +
> +There is a pattern indicating how certain tags should relate to each other.
> +
> +Example of proper tag sequence::
> +
> + Coverity issue:
> + Bugzilla ID:
> + Fixes:
> + Cc:
> +
> + Suggested-by:
> + Reported-by:
> + Signed-off-by:
> + Acked-by:
> + Reviewed-by:
> + Tested-by:
> +
> +Between first and second tag section there is and empty line.
> +
> +While ``Signed-off-by:`` is an obligatory tag and must exists in each
> +commit, all other tags are optional. Any tag, as long as it is in
> +proper location to other adjacent tags (if present), may occur multiple
> times.
> +
> +Other tags after the first occurrence of ``Signed-off-by:`` shall be
> +laid out in a chronological order.
> +
> Creating Patches
> ----------------
>
> --
> 2.25.1
Hello,
as the automatic tests reported performance only degradation on patch analysis change would it be feasible to integrate this patch?
I think all concerns from previous versions are addressed.
Regards,
Jakub
19/07/2022 14:01, jpalider@marvell.com:
> From: Jakub Palider <jpalider@marvell.com>
>
> This change to log checking procedure ensures that certain
> tags are in proper order.
> The order of tags is as follows:
> * Coverity issue
> * Bugzilla ID
> * Fixes
> * Cc
> * <BLANK LINE>
> * Suggested-by
> * Reported-by
> + Signed-off-by
> * Acked-by
> * Reviewed-by
> * Tested-by
> where:
> * => 0 or more than one instance possible
> + => more than once instance possible
> In order to satisfy the above requirements an extra check
> is performed for obligatory tags.
>
> v3:
> * restored some tags under check
> * defined chronological order after Signed-off
>
> v2:
> * narrowed down the tags under check
>
> Signed-off-by: Jakub Palider <jpalider@marvell.com>
I've moved "Reported-by" before "Suggested-by" and
made the output more consistent with other checks.
Applied, thanks.
@@ -54,6 +54,7 @@ fixes=$(git log --format='%h %s' --reverse $range | grep -i ': *fix' | cut -d' '
stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d' | cut -d' ' -f2)
tags=$(git log --format='%b' --reverse $range | grep -i -e 'by *:' -e 'fix.*:')
bytag='\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-by:'
+exttag='Coverity issue:\|Bugzilla ID:\|Fixes:\|Cc:'
failure=false
@@ -203,6 +204,61 @@ done)
[ -z "$bad" ] || { printf "Is it candidate for Cc: stable@dpdk.org backport?\n$bad\n"\
&& failure=true;}
+# check tag sequence
+bad=$(for commit in $commits; do
+ body=$(git log --format='%b' -1 $commit)
+ echo "$body" |\
+ grep -o -e "$exttag\|^[[:blank:]]*$\|$bytag" | \
+ # retrieve tags only
+ cut -f1 -d":" |\
+ # it is okay to have several tags of the same type but for processing
+ # we need to squash them
+ uniq |\
+ # make sure the tags are in the proper order as presented in SEQ
+ awk -v cmt="$commit" 'BEGIN{
+ SEQ[0] = "Coverity issue";
+ SEQ[1] = "Bugzilla ID";
+ SEQ[2] = "Fixes";
+ SEQ[3] = "Cc";
+ SEQ[4] = "^$";
+ SEQ[5] = "Suggested-by";
+ SEQ[6] = "Reported-by";
+ SEQ[7] = "Signed-off-by";
+ SEQ[8] = "Acked-by";
+ SEQ[9] = "Reviewed-by";
+ SEQ[10] = "Tested-by";
+ latest = 0;
+ chronological = 0;
+ }
+ {
+ for (seq = 0; seq < length(SEQ); seq++) {
+ if (chronological == 1)
+ continue;
+ if (match($0, SEQ[seq])) {
+ if (seq < latest) {
+ print "\tCommit " cmt " (" $0 ":)";
+ break;
+ } else {
+ latest = seq;
+ }
+ }
+ }
+ if (match($0, "Signed-off-by"))
+ chronological = 1;
+ }'
+done)
+[ -z "$bad" ] || { printf "Wrong tag order: \n$bad\n"\
+ && failure=true;}
+
+# check required tag
+bad=$(for commit in $commits; do
+ body=$(git log --format='%b' -1 $commit)
+ echo $body | grep -q "Signed-off-by:" \
+ || echo "\tCommit" $commit "(Signed-off-by:)"
+ done)
+[ -z "$bad" ] || { printf "Missing obligatory tag: \n$bad\n"\
+ && failure=true;}
+
total=$(echo "$commits" | wc -l)
if $failure ; then
printf "\nInvalid patch(es) found - checked $total patch"
@@ -360,6 +360,34 @@ Where ``NNNNN`` is patchwork ID for patch or series::
---
Depends-on: series-10000 ("Title of the series")
+Tag order
+~~~~~~~~~
+
+There is a pattern indicating how certain tags should relate to each other.
+
+Example of proper tag sequence::
+
+ Coverity issue:
+ Bugzilla ID:
+ Fixes:
+ Cc:
+
+ Suggested-by:
+ Reported-by:
+ Signed-off-by:
+ Acked-by:
+ Reviewed-by:
+ Tested-by:
+
+Between first and second tag section there is and empty line.
+
+While ``Signed-off-by:`` is an obligatory tag and must exists in each commit,
+all other tags are optional. Any tag, as long as it is in proper location
+to other adjacent tags (if present), may occur multiple times.
+
+Other tags after the first occurrence of ``Signed-off-by:`` shall be laid out in
+a chronological order.
+
Creating Patches
----------------