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

3326 diff mbox series

Add a pre-push commit hooks to prevent unintentional push of patches
containing local changelogs to the master branch.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 utils/hooks/pre-push | 59 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100755 utils/hooks/pre-push

Hi Laurent,

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 ...


> Signed-off-by: Laurent Pinchart <laurent.pinchart@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.


> +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
>

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@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