Re: [PATCH v4 2/2] commit-tree: add missing --gpg-sign flag

["Martin Ågren" <martin.agren@gmail.com>]

Hi Brandon,

On Sun, 20 Jan 2019 at 00:24, Brandon Richardson
<brandon1024.br@gmail.com> wrote:
>         # explicit -S of course must sign.
>         echo 10 | git commit-tree -S HEAD^{tree} >oid &&
>         test_line_count = 1 oid &&
> -       git tag tenth-signed $(cat oid)
> +       git tag tenth-signed $(cat oid) &&
> +
> +       # --gpg-sign[=<key-id>] must sign.
> +       echo 11 | git commit-tree --gpg-sign HEAD^{tree} >oid &&
> +       test_line_count = 1 oid &&
> +       git tag eleventh-signed $(cat oid) &&
> +       echo 12 | git commit-tree --gpg-sign=B7227189 HEAD^{tree} >oid &&
> +       test_line_count = 1 oid &&
> +       git tag twelfth-signed-alt $(cat oid)
>  '

Thank you for following through.

Let's see if there any opinions from others about this more verbose
construction, vs placing the oid in a variable and quoting it. We
obviously went several years without realizing that using $(...) as an
object id risked falling back to HEAD and that a completely broken `git
commit-tree -S` would pass the test. So being over-careful and extra
obvious might very well be the right thing.

>  test_expect_success GPG 'verify and show signatures' '
>         (
>                 for commit in initial second merge fourth-signed \
> -                       fifth-signed sixth-signed seventh-signed tenth-signed
> +                       fifth-signed sixth-signed seventh-signed tenth-signed \
> +                       eleventh-signed
>                 do
>                         git verify-commit $commit &&
>                         git show --pretty=short --show-signature $commit >actual &&
> @@ -82,7 +91,7 @@ test_expect_success GPG 'verify and show signatures' '
>                 done
>         ) &&
>         (
> -               for commit in eighth-signed-alt
> +               for commit in eighth-signed-alt twelfth-signed-alt
>                 do
>                         git show --pretty=short --show-signature $commit >actual &&
>                         grep "Good signature from" actual &&

Ah, good catch. I didn't notice that we had a separate for-loop for this
key. This comes from 4baf839fe0 ("t7510: test a commit signed by an
unknown key", 2014-06-16). What we want to test here is something
different, namely that we're using a specific, named key. But FWIW, I
think we're fine, and that we're not abusing the existing difference
between these two loops too much.

Martin