[PATCH] devtools: ensure proper tag sequence

[PATCH] devtools: ensure proper tag sequence

[jpalider]

From: Jakub Palider <jpalider@marvell.com>

This change to log checking procedure ensures that all
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.

Signed-off-by: Jakub Palider <jpalider@marvell.com>
Change-Id: I58d94fecddd5f978e0567b04f72bd2dc1c0b8a08
---
 devtools/check-git-log.sh           | 51 +++++++++++++++++++++++++++++
 doc/guides/contributing/patches.rst | 25 ++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index 23c6a7d9bb..0d8fff94ee 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,56 @@ 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;
+	}
+	{
+		for (seq = 0; seq < length(SEQ); seq++) {
+			if (match($0, SEQ[seq])) {
+				if (seq < latest) {
+					print "\tCommit " cmt " (" $0 ":)";
+					break;
+				} else {
+					latest = seq;
+				}
+			}
+		}
+	 }'
+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..868a9a7bf2 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -360,6 +360,31 @@ 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 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.
+
 Creating Patches
 ----------------
 
-- 
2.25.1

Re: [PATCH] devtools: ensure proper tag sequence

[Thomas Monjalon]

12/06/2022 16:23, jpalider@marvell.com:
> From: Jakub Palider <jpalider@marvell.com>
> 
> This change to log checking procedure ensures that all
> 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

For the *-by sequence, the order is more chronological.
Ack/Review/Test can be in any order I think.

RE: [EXT] Re: [PATCH] devtools: ensure proper tag sequence

[Jakub Palider]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Sunday, June 12, 2022 10:29 PM
> To: Jakub Palider <jpalider@marvell.com>
> Cc: dev@dpdk.org; david.marchand@redhat.com
> Subject: [EXT] Re: [PATCH] devtools: ensure proper tag sequence
> 
> External Email
> 
> ----------------------------------------------------------------------
> 12/06/2022 16:23, jpalider@marvell.com:
> > From: Jakub Palider <jpalider@marvell.com>
> >
> > This change to log checking procedure ensures that all
> > 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
> 
> For the *-by sequence, the order is more chronological.
> Ack/Review/Test can be in any order I think.
> 
> 
Hi Thomas,
Do I get it right that the lines after <BLANK LINE> are fine in
current state and the patch could take care of the first section
only?

Re: [EXT] Re: [PATCH] devtools: ensure proper tag sequence

[Thomas Monjalon]

13/06/2022 12:35, Jakub Palider:
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Sunday, June 12, 2022 10:29 PM
> > To: Jakub Palider <jpalider@marvell.com>
> > Cc: dev@dpdk.org; david.marchand@redhat.com
> > Subject: [EXT] Re: [PATCH] devtools: ensure proper tag sequence
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > 12/06/2022 16:23, jpalider@marvell.com:
> > > From: Jakub Palider <jpalider@marvell.com>
> > >
> > > This change to log checking procedure ensures that all
> > > 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
> > 
> > For the *-by sequence, the order is more chronological.
> > Ack/Review/Test can be in any order I think.
> > 
> > 
> Hi Thomas,
> Do I get it right that the lines after <BLANK LINE> are fine in
> current state and the patch could take care of the first section
> only?

I'm not sure myself.
We can check at least that Suggested/Reported is before Signed-off.