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

Nguyễn Thái Ngọc Duy  [off-list ref] writes:

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
 '

On Tue, May 15, 2012 at 2:43 AM, Junio C Hamano [off-list ref] wrote:

Nguyễn Thái Ngọc Duy  [off-list ref] writes:

quoted

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?

I need it to be a loose object to test this code path. Maybe this instead?

test_expect_success 'pack-objects with large loose object' '
	SHA1=`git hash-object huge` &&
	test_create_repo loose &&
	echo $SHA1 | git pack-objects --stdout |
		GIT_ALLOC_LIMIT=0 GIT_DIR=loose/.git git unpack-objects &&
	echo $SHA1 | GIT_DIR=loose/.git git pack-objects pack  &&
	test_create_repo packed &&
	mv pack-* packed/.git/objects/pack &&
	GIT_DIR=packed/.git git cat-file blob $SHA1 >actual &&
	cmp huge actual
'

quoted

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

Interesting. It passes for me (same base). I assume rm failed?
-- 
Duy