[v2,1/3] object tests: add test for unexpected objects in tags

Commit Message

Ævar Arnfjörð Bjarmason Dec. 30, 2022, 1:52 a.m. UTC

Fix a blind spot in the tests added in 0616617c7e1 (t: introduce tests
for unexpected object types, 2019-04-09), there were no meaningful
tests for checking how we reported on finding the incorrect object
type in a tag, i.e. one that broke the "type" promise in the tag
header.

We'll report the wrong object type in these cases, and thus fail on
the "test_cmp", e.g. for the first "error: " output being tested here
we should say "$commit is a tag, not a commit", instead we say
"$commit is a commit, not a tag". This will be fixed in a subsequent
commit.

See the discussion & notes in [1] and downthread of there for test
snippets that are adapted here.

In the case of "fsck" which objects we visit in what order, and if we
report errors on them depends on their OIDs. So the test uses the
technique of extracting the OID/type combinations that fsck does
report, and asserting that those are correct (currently, it's far from
correct).

As these tests happen to run into a memory leak skip them under
SANITIZE=leak, as the test file was previously marked leak-free in
[3]. There is a concurrent fix for the leak in question[4].

1. https://lore.kernel.org/git/[e-mail]/
2. https://lore.kernel.org/git/[e-mail]/
3. dd9cede9136 (leak tests: mark some rev-list tests as passing with
   SANITIZE=leak, 2021-10-31)
4. https://lore.kernel.org/git/[e-mail]/

Helped-by: Jeff King <[e-mail]>
Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
---
 t/t6102-rev-list-unexpected-objects.sh | 146 +++++++++++++++++++++++++
 1 file changed, 146 insertions(+)

Comments

Junio C Hamano Dec. 30, 2022, 4:25 a.m. UTC | #1

Ævar Arnfjörð Bjarmason  <[e-mail]> writes:

> +test_expect_success !SANITIZE_LEAK 'setup unexpected non-tag tag' '
> +	test_when_finished "git tag -d tag-commit tag-tag" &&
> +
> +	git tag -a -m"my tagged commit" tag-commit $commit &&
> +	tag_commit=$(git rev-parse tag-commit) &&
> +	git tag -a -m"my tagged tag" tag-tag tag-commit &&
> +	tag_tag=$(git rev-parse tag-tag) &&
> +
> +	git cat-file tag tag-tag >good-tag-tag &&
> +	git cat-file tag tag-commit >good-commit-tag &&
> +
> +	sed -e "s/$tag_commit/$commit/" <good-tag-tag >broken-tag-tag-commit &&
> +	sed -e "s/$tag_commit/$tree/" <good-tag-tag >broken-tag-tag-tree &&
> +	sed -e "s/$tag_commit/$blob/" <good-tag-tag >broken-tag-tag-blob &&
> +
> +	sed -e "s/$commit/$tag_commit/" <good-commit-tag >broken-commit-tag-tag &&
> +	sed -e "s/$commit/$tree/" <good-commit-tag >broken-commit-tag-tree &&
> +	sed -e "s/$commit/$blob/" <good-commit-tag >broken-commit-tag-blob &&
> +
> +	tag_tag_commit=$(git hash-object -w -t tag broken-tag-tag-commit) &&
> +	tag_tag_tree=$(git hash-object -w -t tag broken-tag-tag-tree) &&
> +	tag_tag_blob=$(git hash-object -w -t tag broken-tag-tag-blob) &&

If the second block of 3 sed commands to prepare data for tags that
incorrectly claim to point at a commit are moved a bit, i.e. make
"sed's && hash-object's && update-ref's" as a logical group, the
above would become slightly easier to read, but in any case the
set-up step looks quite repetitive and boring to read ;-)

There is no strong reason to use broken-tag-* temporary files,
though.  Each of them is used exactly once, but you can just
pipe the output from "sed" to "git hash-object --stdin" without
losing any exit status, e.g.

	tag_tag_commit=$(sed -e '...' good-commit-tag |
			git hash-object -w -t tag --stdin)

> +	git update-ref refs/tags/tag_tag_commit $tag_tag_commit &&
> +	git update-ref refs/tags/tag_tag_tree $tag_tag_tree &&
> +	git update-ref refs/tags/tag_tag_blob $tag_tag_blob &&
> +
> +	commit_tag_tag=$(git hash-object -w -t tag broken-commit-tag-tag) &&
> +	commit_tag_tree=$(git hash-object -w -t tag broken-commit-tag-tree) &&
> +	commit_tag_blob=$(git hash-object -w -t tag broken-commit-tag-blob) &&
> +
> +	git update-ref refs/tags/commit_tag_tag $commit_tag_tag &&
> +	git update-ref refs/tags/commit_tag_tree $commit_tag_tree &&
> +	git update-ref refs/tags/commit_tag_blob $commit_tag_blob
> +'
> +
> +test_expect_failure !SANITIZE_LEAK 'traverse unexpected incorrectly typed tag (to commit & tag)' '
> +	test_must_fail git rev-list --objects $tag_tag_commit 2>err &&

Does this have to be "rev-list --objects" or would something like
"cat-file -t $tag_tag_commit^0" do?

Especially because "rev-list --objects" is a rather heavy-weight
command that does a lot of checking, I am wondering if it is
sensible to rely on the assumption that the errors expected below
will stay to be the only errors we get before the command exits
(hence a wish to replace it with a more narrowly focused comamnd).

> +	cat >expect <<-EOF &&
> +	error: object $commit is a commit, not a tag
> +	fatal: bad object $commit
> +	EOF
> +	test_cmp expect err &&



> +test_expect_failure !SANITIZE_LEAK 'traverse unexpected objects with for-each-ref' '
> +	cat >expect <<-EOF &&
> +	error: bad tag pointer to $tree in $tag_tag_tree
> +	fatal: parse_object_buffer failed on $tag_tag_tree for refs/tags/tag_tag_tree
> +	EOF

This depends on the fact that among the broken ones tag_tag_tree
sorts the earliest (and the command stops after barfing on a single
bad object), doesn't it?  I wonder if it makes the test more robust
by feeding refs/tags/tag_tag_tree from the command line to limit the
tips the command needs to inspect.

> +>fsck-object-isa

Move it inside the setup as the first command in case "git fsck"
succeeds?

> +test_expect_success 'setup: unexpected objects with fsck' '
> +	test_must_fail git fsck 2>err &&
> +	sed -n -e "/^error: object .* is a .*, not a .*$/ {
> +		s/^error: object \([0-9a-f]*\) is a \([a-z]*\), not a [a-z]*$/\\1 \\2/;
> +		p;
> +	}" <err >fsck-object-isa
> +'

Patch

diff --git a/t/t6102-rev-list-unexpected-objects.sh b/t/t6102-rev-list-unexpected-objects.sh
index 9350b5fd2c2..f1c30db2654 100755
--- a/t/t6102-rev-list-unexpected-objects.sh
+++ b/t/t6102-rev-list-unexpected-objects.sh
@@ -130,4 +130,150 @@  test_expect_success 'traverse unexpected non-blob tag (seen)' '
 	test_i18ngrep "not a blob" output
 '
 
+test_expect_success !SANITIZE_LEAK 'setup unexpected non-tag tag' '
+	test_when_finished "git tag -d tag-commit tag-tag" &&
+
+	git tag -a -m"my tagged commit" tag-commit $commit &&
+	tag_commit=$(git rev-parse tag-commit) &&
+	git tag -a -m"my tagged tag" tag-tag tag-commit &&
+	tag_tag=$(git rev-parse tag-tag) &&
+
+	git cat-file tag tag-tag >good-tag-tag &&
+	git cat-file tag tag-commit >good-commit-tag &&
+
+	sed -e "s/$tag_commit/$commit/" <good-tag-tag >broken-tag-tag-commit &&
+	sed -e "s/$tag_commit/$tree/" <good-tag-tag >broken-tag-tag-tree &&
+	sed -e "s/$tag_commit/$blob/" <good-tag-tag >broken-tag-tag-blob &&
+
+	sed -e "s/$commit/$tag_commit/" <good-commit-tag >broken-commit-tag-tag &&
+	sed -e "s/$commit/$tree/" <good-commit-tag >broken-commit-tag-tree &&
+	sed -e "s/$commit/$blob/" <good-commit-tag >broken-commit-tag-blob &&
+
+	tag_tag_commit=$(git hash-object -w -t tag broken-tag-tag-commit) &&
+	tag_tag_tree=$(git hash-object -w -t tag broken-tag-tag-tree) &&
+	tag_tag_blob=$(git hash-object -w -t tag broken-tag-tag-blob) &&
+
+	git update-ref refs/tags/tag_tag_commit $tag_tag_commit &&
+	git update-ref refs/tags/tag_tag_tree $tag_tag_tree &&
+	git update-ref refs/tags/tag_tag_blob $tag_tag_blob &&
+
+	commit_tag_tag=$(git hash-object -w -t tag broken-commit-tag-tag) &&
+	commit_tag_tree=$(git hash-object -w -t tag broken-commit-tag-tree) &&
+	commit_tag_blob=$(git hash-object -w -t tag broken-commit-tag-blob) &&
+
+	git update-ref refs/tags/commit_tag_tag $commit_tag_tag &&
+	git update-ref refs/tags/commit_tag_tree $commit_tag_tree &&
+	git update-ref refs/tags/commit_tag_blob $commit_tag_blob
+'
+
+test_expect_failure !SANITIZE_LEAK 'traverse unexpected incorrectly typed tag (to commit & tag)' '
+	test_must_fail git rev-list --objects $tag_tag_commit 2>err &&
+	cat >expect <<-EOF &&
+	error: object $commit is a commit, not a tag
+	fatal: bad object $commit
+	EOF
+	test_cmp expect err &&
+
+	test_must_fail git rev-list --objects $commit_tag_tag 2>err &&
+	cat >expect <<-EOF &&
+	error: object $tag_commit is a tag, not a commit
+	fatal: bad object $tag_commit
+	EOF
+	test_cmp expect err
+'
+
+test_expect_failure !SANITIZE_LEAK 'traverse unexpected incorrectly typed tag (to tree)' '
+	test_must_fail git rev-list --objects $tag_tag_tree 2>err &&
+	cat >expect <<-EOF &&
+	error: object $tree is a tree, not a tag
+	fatal: bad object $tree
+	EOF
+	test_cmp expect err &&
+
+	test_must_fail git rev-list --objects $commit_tag_tree 2>err &&
+	cat >expect <<-EOF &&
+	error: object $tree is a tree, not a commit
+	fatal: bad object $tree
+	EOF
+	test_cmp expect err
+'
+
+test_expect_failure !SANITIZE_LEAK 'traverse unexpected incorrectly typed tag (to blob)' '
+	test_must_fail git rev-list --objects $tag_tag_blob 2>err &&
+	cat >expect <<-EOF &&
+	error: object $blob is a blob, not a tag
+	fatal: bad object $blob
+	EOF
+	test_cmp expect err &&
+
+	test_must_fail git rev-list --objects $commit_tag_blob 2>err &&
+	cat >expect <<-EOF &&
+	error: object $blob is a blob, not a commit
+	fatal: bad object $blob
+	EOF
+	test_cmp expect err
+'
+
+test_expect_failure !SANITIZE_LEAK 'traverse unexpected non-tag tag (tree seen to blob)' '
+	test_must_fail git rev-list --objects $tree $commit_tag_blob 2>err &&
+	cat >expect <<-EOF &&
+	error: object $blob is a blob, not a commit
+	fatal: bad object $blob
+	EOF
+	test_cmp expect err &&
+
+	test_must_fail git rev-list --objects $tree $tag_tag_blob 2>err &&
+	cat >expect <<-EOF &&
+	error: object $blob is a blob, not a tag
+	fatal: bad object $blob
+	EOF
+	test_cmp expect err
+'
+
+
+test_expect_failure !SANITIZE_LEAK 'traverse unexpected objects with for-each-ref' '
+	cat >expect <<-EOF &&
+	error: bad tag pointer to $tree in $tag_tag_tree
+	fatal: parse_object_buffer failed on $tag_tag_tree for refs/tags/tag_tag_tree
+	EOF
+	test_must_fail git for-each-ref --format="%(*objectname)" 2>actual &&
+	test_cmp expect actual
+'
+
+>fsck-object-isa
+test_expect_success 'setup: unexpected objects with fsck' '
+	test_must_fail git fsck 2>err &&
+	sed -n -e "/^error: object .* is a .*, not a .*$/ {
+		s/^error: object \([0-9a-f]*\) is a \([a-z]*\), not a [a-z]*$/\\1 \\2/;
+		p;
+	}" <err >fsck-object-isa
+'
+
+while read oid type
+do
+	test_expect_failure "fsck knows unexpected object $oid is $type" '
+		git cat-file -t $oid >expect &&
+		echo $type >actual &&
+		test_cmp expect actual
+	'
+done <fsck-object-isa
+
+test_expect_success !SANITIZE_LEAK 'traverse unexpected non-tag tag (blob seen to blob)' '
+	test_must_fail git rev-list --objects $blob $commit_tag_blob 2>err &&
+	cat >expected <<-EOF &&
+	error: object $blob is a blob, not a commit
+	error: bad tag pointer to $blob in $commit_tag_blob
+	fatal: bad object $commit_tag_blob
+	EOF
+	test_cmp expected err &&
+
+	test_must_fail git rev-list --objects $blob $tag_tag_blob 2>err &&
+	cat >expected <<-EOF &&
+	error: object $blob is a blob, not a tag
+	error: bad tag pointer to $blob in $tag_tag_blob
+	fatal: bad object $tag_tag_blob
+	EOF
+	test_cmp expected err
+'
+
 test_done