Re: [PATCH v2] pack-objects: use streaming interface for reading large loose blobs

From: Junio C Hamano <gitster@pobox.com>
Date: May 14, 2012, 19:43 UTC
Message-ID: <7v62byh7l4.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1336883862-9013-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 13 quoted lines

> git usually streams large blobs directly to packs. But there are cases
> diff --git a/t/t1050-large.sh b/t/t1050-large.sh
> index 55ed955..7fbd2e1 100755
> --- a/t/t1050-large.sh
> +++ b/t/t1050-large.sh
> @@ -134,6 +134,22 @@ test_expect_success 'repack' '
>  	git repack -ad
>  '
>  
> +test_expect_success 'pack-objects with large loose object' '
> +	echo Z | dd of=large4 bs=1k seek=2000 &&
> +	OBJ=9f36d94e145816ec642592c09cc8e601d83af157 &&
> +	P=.git/objects/9f/36d94e145816ec642592c09cc8e601d83af157 &&

I do not think you need these hardcoded constants; you will run hash-object later, no?

Also, relying on $P to exist after hash-object -w returns is somewhat flaky, no?

> +	(
> +	unset GIT_ALLOC_LIMIT &&

sane_unset?

Show 5 quoted lines

> +	cat large4 | git hash-object -w --stdin &&
> +	git cat-file blob $OBJ >actual &&
> +	cmp large4 actual
> +	) &&
> +	echo $OBJ | git pack-objects .git/objects/pack/pack &&

If you do not write directly into this directory, and then create another _empty_ repository and deposit the packfile and its associated .idx file there, then you do not have to care how the earlier hash-object wrote its object. You are interested only in pack-objects producing a usable pack, and testing it like so will be the most direct way to test it, no?

> +	rm $P &&
> +	git cat-file blob $OBJ >actual &&
> +	cmp large4 actual
> +'

In any case, the patch when applied on top of cd07cc5 (Update draft release notes to 1.7.11 (11th batch), 2012-05-11) does not pass this part of the test on my box.

>  test_expect_success 'tar achiving' '
>  	git archive --format=tar HEAD >/dev/null
>  '