Re: [PATCH] Add a sample hook which saves push certs as notes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Shikher,

I'm not familiar with push certs, but I did notice some general issues
in the sample hook.  I hope they're helpful.

Shikher Verma wrote:
index 000000000..b4366e43f
--- /dev/null
+++ b/templates/hooks--post-receive.sample
+#!/bin/sh
...
+if test -z GIT_PUSH_CERT ; then
+    exit 0
+fi

The $ is missing from GIT_PUSH_CERT.  test -z GIT_PUSH_CERT will
always be false. :)

The variable should also be quoted.  Not all sh implementations accept
a missing argument to test -z, as bash does.

More minor, Documentation/CodingGuidelines suggests placing 'then' on
a new line:

   if test -z "$GIT_PUSH_CERT"
   then
       exit 0
   fi

(There is plenty of code that doesn't follow that, so I don't know how
strong that preference is.)

This could also be written as:

   test -z "$GIT_PUSH_CERT" && exit 0

I don't know if there's any general preference to shorten it in git's
code or not.

+push_cert=$(git cat-file -p  $GIT_PUSH_CERT)

Very minor: there's an extra space before the variable here.

(I also noticed the tests which use $GIT_PUSH_CERT, like t5534, use
'cat-file blob ...' rather than 'cat-file -p ...'.  I don't know if
that's much safer/better than letting cat-file guess the object type
in the hook.  I have no idea if there's a chance that "$GIT_PUSH_CERT"
has some unexpected, non-blob object type.)

+while read oval nval ref
+do
+	# Verify that the ref update matches that in push certificate.
+	if [[ $push_cert == *$oval" "$nval" "$ref* ]]; then

[[ isn't portable across all the sh implementations git strives to
support, as far as I know.

The minor point about 'then' on new line is applicable here too.  It
would also better match the outer 'while' loop.

+		# add the push cert as note (namespaced pushcerts) to nval.
+		git notes --ref=pushcerts add -m "$push_cert" $nval -f
+	fi
+done

--
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Learn from the mistakes of others--you can never live long enough to
make them all yourself.
   -- John Luther




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux