From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 14 May 2012 19:43:19 GMT
Subject: Re: [PATCH v2] pack-objects: use streaming interface for reading large loose blobs
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:

> 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?

> +	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
>  '