[libcamera-devel] [PATCH] utils: hooks: Add pre-push commit hook

[Laurent Pinchart]

Hi Kieran,

On Thu, Mar 26, 2020 at 12:13:10PM +0000, Kieran Bingham wrote:
> On 26/03/2020 11:49, Laurent Pinchart wrote:
> > Add a pre-push commit hooks to prevent unintentional push of patches
> > containing local changelogs to the master branch.
> 
> Thanks this sounds like a useful check/addition.
> 
> Unfortunately due to lacking a .sh extension, checkstyle.py did not run
> shell-check on this for you ...

Oops :-/ I could add a .sh extension, but that would be a bit less
user-friendly.

> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  utils/hooks/pre-push | 59 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> >  create mode 100755 utils/hooks/pre-push
> > 
> > diff --git a/utils/hooks/pre-push b/utils/hooks/pre-push
> > new file mode 100755
> > index 000000000000..dc054b2553ff
> > --- /dev/null
> > +++ b/utils/hooks/pre-push
> > @@ -0,0 +1,59 @@
> > +#!/bin/sh
> > +
> > +# A hook script to verify what is about to be pushed.  Called by "git
> > +# push" after it has checked the remote status, but before anything has been
> > +# pushed.  If this script exits with a non-zero status nothing will be pushed.
> > +#
> > +# This hook is called with the following parameters:
> > +#
> > +# $1 -- Name of the remote to which the push is being done
> > +# $2 -- URL to which the push is being done
> > +#
> > +# If pushing without using a named remote those arguments will be equal.
> > +#
> > +# Information about the commits which are being pushed is supplied as lines to
> > +# the standard input in the form:
> > +#
> > +#   <local ref> <local sha1> <remote ref> <remote sha1>
> > +#
> > +# This script prevents push of commits than contain a local changelog to the
> > +# master branch.
> > +
> > +remote="$1"
> > +url="$2"
> 
> In ./utils/hooks/pre-push line 22:
> remote="$1"
> ^-- SC2034: remote appears unused. Verify use (or export if used
> externally).
> 
> In ./utils/hooks/pre-push line 23:
> url="$2"
> ^-- SC2034: url appears unused. Verify use (or export if used externally).
> 
> 
> But these are indeed 'unused' - but probably not needed, though useful
> reference.
> 
> > +
> > +z40=0000000000000000000000000000000000000000
> > +
> > +while read local_ref local_sha remote_ref remote_sha
> 
> 
> In ./utils/hooks/pre-push line 27:
> while read local_ref local_sha remote_ref remote_sha
>       ^-- SC2162: read without -r will mangle backslashes.

All these come straight from the example pre-push hook from git. I'll
still fix them, but if we depart from the original, I'll take that as an
opportunity to remove more unneeded code.

> > +do
> > +	if [ "$remote_ref" != refs/heads/master ]
> > +	then
> > +		continue
> > +	fi
> > +
> > +	if [ "$local_sha" = $z40 ]
> > +	then
> > +		# Handle delete
> > +		:
> > +	else
> > +		if [ "$remote_sha" = $z40 ]
> > +		then
> > +			# New branch, examine all commits
> > +			range="$local_sha"
> > +		else
> > +			# Update to existing branch, examine new commits
> > +			range="$remote_sha..$local_sha"
> > +		fi
> > +
> > +		# Check for WIP commit
> > +		commit=`git rev-list -n 1 --grep '^---' "$range"`
> 
> In ./utils/hooks/pre-push line 49:
> 		commit=`git rev-list -n 1 --grep '^---' "$range"`
>                        ^-- SC2006: Use $(..) instead of legacy `..`.
> 
> > +		if [ -n "$commit" ]
> > +		then
> > +			echo >&2 "Found local changelog in $local_ref, not pushing"
> > +			echo >&2 "Check commit $commit"
> > +			exit 1
> > +		fi
> 
> It might be interesting to do some last minute call to checkstyle.py
> here too ... (with an optional manual accept if it flags anything).
> 
> But that is feature creep and can be outside of this patch.
> 
> > +	fi
> > +done
> > +
> > +exit 0

-- 
Regards,

Laurent Pinchart