diff mbox series

[v3,3/5] t4301: verify that merge-tree fails on missing blob objects

Message ID e82fdf7fbcbf12fffdf4a720927c2f4f006068f8.1708612605.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series merge-tree: handle missing objects correctly | expand

Commit Message

Johannes Schindelin Feb. 22, 2024, 2:36 p.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

We just fixed a problem where `merge-tree` would not fail on missing
tree objects. Let's ensure that that problem does not occur with blob
objects (and won't, in the future, either).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t4301-merge-tree-write-tree.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Junio C Hamano Feb. 22, 2024, 5:27 p.m. UTC | #1
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> We just fixed a problem where `merge-tree` would not fail on missing
> tree objects. Let's ensure that that problem does not occur with blob
> objects (and won't, in the future, either).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/t4301-merge-tree-write-tree.sh | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
> index 908c9b540c8..d4463a45706 100755
> --- a/t/t4301-merge-tree-write-tree.sh
> +++ b/t/t4301-merge-tree-write-tree.sh
> @@ -962,4 +962,20 @@ test_expect_success 'error out on missing tree objects' '
>  	test_must_be_empty actual
>  '
>  
> +test_expect_success 'error out on missing blob objects' '
> +	echo 1 | git hash-object -w --stdin >blob1 &&
> +	echo 2 | git hash-object -w --stdin >blob2 &&
> +	echo 3 | git hash-object -w --stdin >blob3 &&
> +	printf "100644 blob $(cat blob1)\tblob\n" | git mktree >tree1 &&
> +	printf "100644 blob $(cat blob2)\tblob\n" | git mktree >tree2 &&
> +	printf "100644 blob $(cat blob3)\tblob\n" | git mktree >tree3 &&
> +	git init --bare missing-blob.git &&
> +	cat blob1 blob3 tree1 tree2 tree3 |
> +	git pack-objects missing-blob.git/objects/pack/side1-whatever-is-missing &&
> +	test_must_fail git --git-dir=missing-blob.git >actual 2>err \
> +		merge-tree --merge-base=$(cat tree1) $(cat tree2) $(cat tree3) &&
> +	test_grep "unable to read blob object $(cat blob2)" err &&
> +	test_must_be_empty actual
> +'

It would have been even easier to see that blob2 is what we expect
to be complained about, if you listed all objects and filtered blob2
out, but the number of objects involved here is so small, a "cat" of
all objects we want to keep is OK here.

Again, I very much love the way the test repository/object store
that lack certain objects are constructed without making our hands
dirty.

I see no need for further comments.  Looking very good.
Junio C Hamano Feb. 22, 2024, 6:23 p.m. UTC | #2
Junio C Hamano <gitster@pobox.com> writes:

>> +test_expect_success 'error out on missing blob objects' '
>> +	echo 1 | git hash-object -w --stdin >blob1 &&
>> +	echo 2 | git hash-object -w --stdin >blob2 &&
>> +	echo 3 | git hash-object -w --stdin >blob3 &&
>> +	printf "100644 blob $(cat blob1)\tblob\n" | git mktree >tree1 &&
>> +	printf "100644 blob $(cat blob2)\tblob\n" | git mktree >tree2 &&
>> +	printf "100644 blob $(cat blob3)\tblob\n" | git mktree >tree3 &&
>> +	git init --bare missing-blob.git &&
>> +	cat blob1 blob3 tree1 tree2 tree3 |
>> +	git pack-objects missing-blob.git/objects/pack/side1-whatever-is-missing &&
>> +	test_must_fail git --git-dir=missing-blob.git >actual 2>err \
>> +		merge-tree --merge-base=$(cat tree1) $(cat tree2) $(cat tree3) &&
>> +	test_grep "unable to read blob object $(cat blob2)" err &&
>> +	test_must_be_empty actual
>> +'
>
> It would have been even easier to see that blob2 is what we expect
> to be complained about, if you listed all objects and filtered blob2
> out, but the number of objects involved here is so small, a "cat" of
> all objects we want to keep is OK here.

Just FYI for anybody reading from the sideline.

	git cat-file --unordered \
		--batch-check="%(objectname)" --batch-all-objects |
	grep -v $(cat blob2) |
	git pack-objects ...

would be a compact way to say "Give me a pack that lets me easily
simulate what happens in a repository identical to the current one,
if it lacked object "blob2", without having to enumerate what object
we want to include in the test repository.

Again, I think in this particular case, it is easy enough to see in
the "blob1 blob3 tree1 tree2 tree3" enumeration what is missing, so
the way the patch is written is fine.

Thanks.
diff mbox series

Patch

diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index 908c9b540c8..d4463a45706 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -962,4 +962,20 @@  test_expect_success 'error out on missing tree objects' '
 	test_must_be_empty actual
 '
 
+test_expect_success 'error out on missing blob objects' '
+	echo 1 | git hash-object -w --stdin >blob1 &&
+	echo 2 | git hash-object -w --stdin >blob2 &&
+	echo 3 | git hash-object -w --stdin >blob3 &&
+	printf "100644 blob $(cat blob1)\tblob\n" | git mktree >tree1 &&
+	printf "100644 blob $(cat blob2)\tblob\n" | git mktree >tree2 &&
+	printf "100644 blob $(cat blob3)\tblob\n" | git mktree >tree3 &&
+	git init --bare missing-blob.git &&
+	cat blob1 blob3 tree1 tree2 tree3 |
+	git pack-objects missing-blob.git/objects/pack/side1-whatever-is-missing &&
+	test_must_fail git --git-dir=missing-blob.git >actual 2>err \
+		merge-tree --merge-base=$(cat tree1) $(cat tree2) $(cat tree3) &&
+	test_grep "unable to read blob object $(cat blob2)" err &&
+	test_must_be_empty actual
+'
+
 test_done