[libcamera-devel] [PATCH] utils: hooks: pre-push: Catch commits without committer's SoB

[Niklas Söderlund]

Hi Laurent,

On 2020-04-29 03:56:23 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> On Tue, Apr 28, 2020 at 12:36:27AM +0200, Niklas Söderlund wrote:
> > On 2020-04-27 01:23:23 +0300, Laurent Pinchart wrote:
> > > Improve the pre-push git hook script to reject commits without the
> > > committer's Signed-off-by line.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  utils/hooks/pre-push | 31 +++++++++++++++++++++++++++----
> > >  1 file changed, 27 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/utils/hooks/pre-push b/utils/hooks/pre-push
> > > index 34906ddee5ed..0eb8f5ce193d 100755
> > > --- a/utils/hooks/pre-push
> > > +++ b/utils/hooks/pre-push
> > > @@ -32,12 +32,35 @@ do
> > >  		range="$remote_sha..$local_sha"
> > >  	fi
> > >  
> > > +	#
> > >  	# Find invalid commits.
> > > -	commit=$(git rev-list -n 1 --grep '^---' "$range")
> > > -	if [ -n "$commit" ]
> > > +	#
> > > +	errors=0
> > > +	for commit in $(git rev-list "$range")
> > > +	do
> > > +		msg=$(git cat-file commit "$commit")
> > > +
> > > +		# 1. The commit message shall not contain a local changelog.
> > > +		if echo "$msg" | grep -q '^--- *$'
> > 
> > This sends cold down my spine, too many times have I been bitten by 
> > trying to check the 'sum' of piped return codes in sh if statements.
> > 
> > $ if true | false; then echo "1"; else echo "0"; fi
> > 0
> > $ if false | true; then echo "1"; else echo "0"; fi
> > 1
> > 
> > If we really wish to check the status piped commands in shell scripts 
> > please switch this to #!/bin/bash and use 'set -o pipefail'.
> > 
> > $ set -o pipefail
> > $ if true | false; then echo "1"; else echo "0"; fi
> > 0
> > $ if false | true; then echo "1"; else echo "0"; fi
> > 0
> 
> But in this case I want to check the return value of grep, not "summing" the two.

As discussed on IRC the problem with dash not supporting pipefail and 
the unlikely event of echo failing I think the pragmatic thing to do is

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> 
> > > +		then
> > > +			echo >&2 "Found local changelog in commit $commit"
> > > +			errors=$((errors+1))
> > > +		fi
> > > +
> > > +		# 2. The commit message shall have a Signed-off-by line
> > > +		# corresponding the committer.
> > > +		committer=$(echo "$msg" | grep '^committer ' | head -1 | \
> > > +				cut -d ' ' -f 2- | rev | cut -d ' ' -f 3- | rev)
> > > +		if ! echo "$msg" | grep -F -q "Signed-off-by: ${committer}"
> > > +		then
> > > +			echo >&2 "Missing committer Signed-off-by in commit $commit"
> > > +			errors=$((errors+1))
> > > +		fi
> > > +	done
> > > +
> > > +	if [ $errors != 0 ]
> > >  	then
> > > -		echo >&2 "Found local changelog in $local_ref, not pushing"
> > > -		echo >&2 "Check commit $commit"
> > > +		echo >&2 "Found $errors errors in $local_ref, not pushing"
> > >  		exit 1
> > >  	fi
> > >  done
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund