diff mbox series

[v2,2/2] devtools: fix patches missing if range newer than HEAD

Message ID 20210630063416.9550-2-xuemingl@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v2,1/2] devtools: fix version pattern for fix search |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot success github build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-testing fail Testing issues
ci/iol-mellanox-Functional fail Functional Testing issues
ci/iol-intel-Performance fail Performance Testing issues
ci/Intel-compilation success Compilation OK
ci/iol-abi-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

Xueming Li June 30, 2021, 6:34 a.m. UTC 
  Current fix scan scripts scanned specified range in HEAD branch.
When users ran it in an earlier branch, few patches were scanned
due to the fixes in the range are newer and not merged to HEAD
branch.

This patch introduces optional <branch> argument, default to HEAD
if not specified. Checks the <range> specified in parameter must
being merged in <branch>.

Fixes: 752d8e097ec1 ("scripts: show fixes with release version of bug")
Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: stable@dpdk.org
Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 devtools/git-log-fixes.sh | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

Comments

Thomas Monjalon Aug. 8, 2021, 11:24 a.m. UTC | #1 
30/06/2021 08:34, Xueming Li:
> Current fix scan scripts scanned specified range in HEAD branch.

I cannot parse the above sentence.

> When users ran it in an earlier branch, few patches were scanned
> due to the fixes in the range are newer and not merged to HEAD
> branch.

You mean some patches were not scanned?

> 
> This patch introduces optional <branch> argument, default to HEAD
> if not specified. Checks the <range> specified in parameter must
> being merged in <branch>.

Cannot parse either.

> Fixes: 752d8e097ec1 ("scripts: show fixes with release version of bug")
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: stable@dpdk.org
> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> 
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
[...]
> -	echo "usage: $(basename $0) [-h] <git_range>"
> +	echo "usage: $(basename $0) [-h] <git_range> [<branch>]"
[...]
> -range="$*"
> +range="$1"

I think it breaks passing range in multiple parameters without quotes.
But it is not really a problem.

> +branch="$2"
> +
> +[ -n "$branch" ] || branch="HEAD"
> +refbranch=$(git rev-parse --abbrev-ref $branch)

Why this line is needed? A comment may help.
If $branch is not used anymore, we can overwrite it
instead of introducing one more variable $refbranch.

> +range_last=$(git log --oneline $range |head -n1|cut -d' ' -f1)

spaces missing around pipes.
You can avoid "head" and "cut" by providing the right options to git.

> +if ! git branch -a --contains $range_last |grep -q -e " $refbranch$" -e " remotes/$refbranch$"; then
> +	echo "range $range not included by branch $refbranch"
> +	exit 1
> +fi
>  
>  # get major release version of a commit
>  commit_version () # <hash>
>  {
>  	local VER="v*.*"
>  	# use current branch as history reference
> -	local refbranch=$(git rev-parse --abbrev-ref HEAD)

You move a line but not its comment above.
Xueming Li Aug. 9, 2021, 7:44 a.m. UTC | #2 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Sunday, August 8, 2021 7:25 PM
> To: Xueming(Steven) Li <xuemingl@nvidia.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Christian Ehrhardt <christian.ehrhardt@canonical.com>; bluca@debian.org;
> ktraynor@redhat.com; david.marchand@redhat.com
> Subject: Re: [dpdk-stable] [PATCH v2 2/2] devtools: fix patches missing if range newer than HEAD
> 
> 30/06/2021 08:34, Xueming Li:
> > Current fix scan scripts scanned specified range in HEAD branch.
> 
> I cannot parse the above sentence.

The scripts scans patches and try to get patch version info from current branch.

> 
> > When users ran it in an earlier branch, few patches were scanned due
> > to the fixes in the range are newer and not merged to HEAD branch.
> 
> You mean some patches were not scanned?

Scanned but failed to retrieve correct version info. 

> 
> >
> > This patch introduces optional <branch> argument, default to HEAD if
> > not specified. Checks the <range> specified in parameter must being
> > merged in <branch>.
> 
> Cannot parse either.
> 
> > Fixes: 752d8e097ec1 ("scripts: show fixes with release version of
> > bug")
> > Cc: Thomas Monjalon <thomas@monjalon.net>
> > Cc: stable@dpdk.org
> > Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> >
> > Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> [...]
> > -	echo "usage: $(basename $0) [-h] <git_range>"
> > +	echo "usage: $(basename $0) [-h] <git_range> [<branch>]"
> [...]
> > -range="$*"
> > +range="$1"
> 
> I think it breaks passing range in multiple parameters without quotes.
> But it is not really a problem.
> 
> > +branch="$2"
> > +
> > +[ -n "$branch" ] || branch="HEAD"
> > +refbranch=$(git rev-parse --abbrev-ref $branch)
> 
> Why this line is needed? A comment may help.

OK

> If $branch is not used anymore, we can overwrite it instead of introducing one more variable $refbranch.

$branch will be used in function commit_version().

>
> > +range_last=$(git log --oneline $range |head -n1|cut -d' ' -f1)
> 
> spaces missing around pipes.
> You can avoid "head" and "cut" by providing the right options to git.

Yes, rev-parse will do this efficiently.

> 
> > +if ! git branch -a --contains $range_last |grep -q -e " $refbranch$" -e " remotes/$refbranch$"; then
> > +	echo "range $range not included by branch $refbranch"
> > +	exit 1
> > +fi
> >
> >  # get major release version of a commit  commit_version () # <hash>
> > {
> >  	local VER="v*.*"
> >  	# use current branch as history reference
> > -	local refbranch=$(git rev-parse --abbrev-ref HEAD)
> 
> You move a line but not its comment above.
>
diff mbox series

Patch

diff --git a/devtools/git-log-fixes.sh b/devtools/git-log-fixes.sh
index 153ba5b438..51d8b19942 100755
--- a/devtools/git-log-fixes.sh
+++ b/devtools/git-log-fixes.sh
@@ -4,7 +4,7 @@ 
 
 print_usage ()
 {
-	echo "usage: $(basename $0) [-h] <git_range>"
+	echo "usage: $(basename $0) [-h] <git_range> [<branch>]"
 }
 
 print_help ()
@@ -15,6 +15,7 @@  print_help ()
 	Find fixes to backport on previous versions.
 	It looks for the word "fix" in the headline or a tag "Fixes" or "Reverts".
 	The oldest bug origin is printed as well as partially fixed versions.
+	It looks into current branch or the branch specified.
 	END_OF_HELP
 }
 
@@ -33,14 +34,22 @@  while getopts h ARG ; do
 done
 shift $(($OPTIND - 1))
 [ $# -ge 1 ] || usage_error 'range argument required'
-range="$*"
+range="$1"
+branch="$2"
+
+[ -n "$branch" ] || branch="HEAD"
+refbranch=$(git rev-parse --abbrev-ref $branch)
+range_last=$(git log --oneline $range |head -n1|cut -d' ' -f1)
+if ! git branch -a --contains $range_last |grep -q -e " $refbranch$" -e " remotes/$refbranch$"; then
+	echo "range $range not included by branch $refbranch"
+	exit 1
+fi
 
 # get major release version of a commit
 commit_version () # <hash>
 {
 	local VER="v*.*"
 	# use current branch as history reference
-	local refbranch=$(git rev-parse --abbrev-ref HEAD)
 	local tag=$( (git tag -l "$VER" --contains $1 --sort=creatordate --merged $refbranch 2>&- ||
 		# tag --merged option has been introduced in git 2.7.0
 		# below is a fallback in case of old git version
@@ -49,9 +58,11 @@  commit_version () # <hash>
 			sed "s,.\+,$t,"
 		done) |
 		head -n1)
-	if [ -z "$tag" ] ; then
-		# before -rc1 tag of release in progress
-		cat VERSION | cut -d'.' -f-2
+	if [ -z "$tag" ]; then
+		if [ "$branch" = 'HEAD' ]; then
+			# before -rc1 tag of release in progress
+			cat VERSION | cut -d'.' -f-2
+		fi
 	else
 		echo $tag | sed 's,^v,,' | sed 's,-rc.*,,'
 	fi