Re: [PATCH 3/4] hooks--pre-push.sample: use hash-agnostic null OID

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 18, 2020, 17:06 UTC
Message-ID: <CAPig+cRKrgOhu3bH=TjdWnPaDfKmcM0OcL2ajktc-itGyyi1Ww@mail.gmail.com>
In-Reply-To: <9d6c2951ab0a2c2e92353c238b008538fe6a6327.1600427894.git.liu.denton@gmail.com>

On Fri, Sep 18, 2020 at 7:19 AM Denton Liu <liu.denton@gmail.com> wrote:

Show 15 quoted lines

> The pre-push sample hook has the null OID hardcoded as 40 zeros.
> However, with the introduction of SHA-256 support, this assumption no
> longer holds true. Replace the hardcoded $z40 with a call to
> `git rev-parse --null-oid` so the sample hook becomes hash-agnostic.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample
> @@ -22,16 +22,16 @@
> +null_oid="$(git rev-parse --null-oid)"
>
>  while read local_ref local_sha remote_ref remote_sha
>  do
> -       if [ "$local_sha" = $z40 ]
> +       if [ "$local_sha" = "$null_oid" ]

Seeing this made me wonder if, rather than introducing a --null-oid option (or --zero-oid per Taylor's suggestion), it would make more sense to answer the question more directly. For instance:

    if test rev-parse --is-null-oid "$local_sha"
    then
        ...

Or, if following Taylor's suggestion, you would add --is-zero-oid, --is-empty-blob, --is-empty-tree.

On the other hand, as this is used in a loop, being able to ask for the null (or zero) OID just once before the loop does make sense, so maybe the --is-*-oid forms are less practical.

By the way, if you're cleaning up the sample scripts to make them hash-agnostic, then it would also make sense to s/sha/oid/.