Re: [PATCH] show_one_mergetag: print non-parent in hex form.

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

Hi Harald,

On Sat, 29 Feb 2020 at 14:11, Harald van Dijk via GitGitGadget <gitgitgadget@gmail.com> wrote:
> When a mergetag names a non-parent, which can occur after a shallow
> clone, its hash was previously printed as raw data. Print it in hex form
> instead.

Minor nit: "..., its hash is printed as raw data. ...". (It *is* indeed
being printed as raw data as you set out to write this patch.)

>         else if ((nth = which_parent(&tag->tagged->oid, commit)) < 0)
>                 strbuf_addf(&verify_message, "tag %s names a non-parent %s\n",
> -                                   tag->tag, tag->tagged->oid.hash);
> +                                   tag->tag, oid_to_hex(&tag->tagged->oid));

Looks obviously correct.

> +test_expect_success GPG 'log --graph --show-signature for merged tag in shallow clone' '
> +       test_when_finished "git reset --hard && git checkout master" &&
> +       git checkout -b plain-shallow master &&
> +       echo aaa >bar &&
> +       git add bar &&
> +       git commit -m bar_commit &&

These last three lines could be "test_commit bar".

> +       git checkout --detach master &&
> +       echo bbb >baz &&
> +       git add baz &&
> +       git commit -m baz_commit &&

Similarly here.

> +       git tag -s -m signed_tag_msg signed_tag_shallow &&
> +       hash=$(git rev-parse HEAD) &&
> +       git checkout plain-shallow &&
> +       git merge --no-ff -m msg signed_tag_shallow &&
> +       git clone --depth 1 --no-local . shallow &&
> +       test_when_finished "rm -rf shallow" &&
> +       git -C shallow log --graph --show-signature -n1 plain-shallow >actual &&
> +       grep "tag signed_tag_shallow names a non-parent $hash" actual
> +'

But I also wonder if we even need the "bar" commit. Similarly,
"--detach"-ing when checking out master seems unnecessary, unless we're
afraid of "messing up" master, by modifying the expectations of later
tests? Was that something you were concerned about?

I realize the test you add is similar to the ones surrounding it. But it
does look tempting to squash in the diff below. The test still fails
before and passes after. What do you think about this? Does this
correctly capture your scenario?

(I suppose even the "test_commit baz" could be dropped, but then this
test needs to assume that "master" already contains a commit.)

Martin

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 20cb436c43..7441485e73 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1636,17 +1636,11 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' '
 
 test_expect_success GPG 'log --graph --show-signature for merged tag in shallow clone' '
 	test_when_finished "git reset --hard && git checkout master" &&
-	git checkout -b plain-shallow master &&
-	echo aaa >bar &&
-	git add bar &&
-	git commit -m bar_commit &&
-	git checkout --detach master &&
-	echo bbb >baz &&
-	git add baz &&
-	git commit -m baz_commit &&
+	git checkout master &&
+	test_commit baz &&
 	git tag -s -m signed_tag_msg signed_tag_shallow &&
 	hash=$(git rev-parse HEAD) &&
-	git checkout plain-shallow &&
+	git checkout -b plain-shallow HEAD~ &&
 	git merge --no-ff -m msg signed_tag_shallow &&
 	git clone --depth 1 --no-local . shallow &&
 	test_when_finished "rm -rf shallow" &&
-- 
2.25.1