{"thread":{"id":"30517","subject":"[PATCH] pack-objects: use streaming interface for reading large loose blobs","startedAt":"2012-05-12T10:26:15Z","lastAt":"2012-05-19T02:43:59Z","messageCount":16,"participants":["Nguyễn Thái Ngọc Duy","Nicolas Pitre","Junio C Hamano","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"191440","messageId":"1336818375-16895-1-git-send-email-pclouds@gmail.com","threadId":"30517","inReplyTo":null,"subject":"[PATCH] pack-objects: use streaming interface for reading large loose blobs","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-12T10:26:15Z","receivedAt":"2012-05-12T10:26:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"git usually streams large blobs directly to packs. But there are cases\nwhere git can create large loose blobs (unpack-objects or hash-object\nover pipe). Or they can come from other git implementations.\ncore.bigfilethreshold can also be lowered down and introduce a new\nwave of large loose blobs.\n\nUse streaming interface to read these blobs and compress/write at the\nsame time.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n index-pack's streaming support is on the way. unpack-objects is\n another story because I'm thinking of merging it back to index-pack\n first, which may take more than one release cycle.\n\n builtin/pack-objects.c |   73 ++++++++++++++++++++++++++++++++++++++++++++----\n t/t1050-large.sh       |   16 ++++++++++\n 2 files changed, 83 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1861093..98b51c1 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -16,6 +16,7 @@\n #include \"list-objects.h\"\n #include \"progress.h\"\n #include \"refs.h\"\n+#include \"streaming.h\"\n #include \"thread-utils.h\"\n \n static const char *pack_usage[] = {\n@@ -150,6 +151,55 @@ static unsigned long do_compress(void **pptr, unsigned long size)\n \treturn stream.total_out;\n }\n \n+static void write_large_blob_data(struct sha1file *f, const unsigned char *sha1)\n+{\n+\tgit_zstream stream;\n+\tunsigned char ibuf[1024 * 16];\n+\tunsigned char obuf[1024 * 16];\n+\tint zret;\n+\n+\tstruct git_istream *st;\n+\tenum object_type type;\n+\tunsigned long sz;\n+\n+\tst = open_istream(sha1, &type, &sz, NULL);\n+\tif (!st)\n+\t\tdie(_(\"failed to read %s\"), sha1_to_hex(sha1));\n+\n+\tmemset(&stream, 0, sizeof(stream));\n+\tgit_deflate_init(&stream, pack_compression_level);\n+\n+\tif (type != OBJ_BLOB)\n+\t\tdie(\"BUG: %s is not a blob\", sha1_to_hex(sha1));\n+\n+\tfor (;;) {\n+\t\tssize_t readlen;\n+\t\treadlen = read_istream(st, ibuf, sizeof(ibuf));\n+\t\tif (readlen == -1)\n+\t\t\tdie(_(\"failed to read %s\"), sha1_to_hex(sha1));\n+\n+\t\tstream.next_in = ibuf;\n+\t\tstream.avail_in = readlen;\n+\t\tzret = Z_OK;\n+\t\twhile ((stream.avail_in || readlen == 0) &&\n+\t\t       (zret == Z_OK || zret == Z_BUF_ERROR)) {\n+\t\t\tstream.next_out = obuf;\n+\t\t\tstream.avail_out = sizeof(obuf);\n+\t\t\tzret = git_deflate(&stream, readlen ? 0 : Z_FINISH);\n+\t\t\tsha1write(f, obuf, stream.next_out - obuf);\n+\t\t}\n+\t\tif (stream.avail_in)\n+\t\t\tdie(_(\"deflate error (%d)\"), zret);\n+\t\tif (readlen == 0) {\n+\t\t\tif (zret != Z_STREAM_END)\n+\t\t\t\tdie(_(\"deflate error (%d)\"), zret);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\tclose_istream(st);\n+\tgit_deflate_end(&stream);\n+}\n+\n /*\n  * we are going to reuse the existing object data as is.  make\n  * sure it is not corrupt.\n@@ -259,9 +309,14 @@ static unsigned long write_object(struct sha1file *f,\n \tif (!to_reuse) {\n \t\tno_reuse:\n \t\tif (!usable_delta) {\n-\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n-\t\t\tif (!buf)\n-\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n+\t\t\ttype = sha1_object_info(entry->idx.sha1, &size);\n+\t\t\tif (type == OBJ_BLOB && size > big_file_threshold)\n+\t\t\t\tbuf = NULL;\n+\t\t\telse {\n+\t\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n+\t\t\t\tif (!buf)\n+\t\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n+\t\t\t}\n \t\t\t/*\n \t\t\t * make sure no cached delta data remains from a\n \t\t\t * previous attempt before a pack split occurred.\n@@ -284,8 +339,11 @@ static unsigned long write_object(struct sha1file *f,\n \n \t\tif (entry->z_delta_size)\n \t\t\tdatalen = entry->z_delta_size;\n-\t\telse\n+\t\telse if (buf)\n \t\t\tdatalen = do_compress(&buf, size);\n+\t\telse\n+\t\t\t/* large blob case, just assume we don't compress well */\n+\t\t\tdatalen = size;\n \n \t\t/*\n \t\t * The object header is a byte of 'type' followed by zero or\n@@ -330,8 +388,11 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t}\n \t\t\tsha1write(f, header, hdrlen);\n \t\t}\n-\t\tsha1write(f, buf, datalen);\n-\t\tfree(buf);\n+\t\tif (buf) {\n+\t\t\tsha1write(f, buf, datalen);\n+\t\t\tfree(buf);\n+\t\t} else\n+\t\t\twrite_large_blob_data(f, entry->idx.sha1);\n \t}\n \telse {\n \t\tstruct packed_git *p = entry->in_pack;\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 55ed955..7fbd2e1 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -134,6 +134,22 @@ test_expect_success 'repack' '\n \tgit repack -ad\n '\n \n+test_expect_success 'pack-objects with large loose object' '\n+\techo Z | dd of=large4 bs=1k seek=2000 &&\n+\tOBJ=9f36d94e145816ec642592c09cc8e601d83af157 &&\n+\tP=.git/objects/9f/36d94e145816ec642592c09cc8e601d83af157 &&\n+\t(\n+\tunset GIT_ALLOC_LIMIT &&\n+\tcat large4 | git hash-object -w --stdin &&\n+\tgit cat-file blob $OBJ >actual &&\n+\tcmp large4 actual\n+\t) &&\n+\techo $OBJ | git pack-objects .git/objects/pack/pack &&\n+\trm $P &&\n+\tgit cat-file blob $OBJ >actual &&\n+\tcmp large4 actual\n+'\n+\n test_expect_success 'tar achiving' '\n \tgit archive --format=tar HEAD >/dev/null\n '\n-- \n1.7.8.36.g69ee2\n"},{"id":"191459","messageId":"alpine.LFD.2.02.1205121220070.21030@xanadu.home","threadId":"30517","inReplyTo":"1336818375-16895-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] pack-objects: use streaming interface for reading large loose blobs","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2012-05-12T16:51:05Z","receivedAt":"2012-05-12T16:51:05Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sat, 12 May 2012, Nguyễn Thái Ngọc Duy wrote:\n\n> git usually streams large blobs directly to packs. But there are cases\n> where git can create large loose blobs (unpack-objects or hash-object\n> over pipe). Or they can come from other git implementations.\n> core.bigfilethreshold can also be lowered down and introduce a new\n> wave of large loose blobs.\n> \n> Use streaming interface to read these blobs and compress/write at the\n> same time.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n\nComments below.\n\n> ---\n>  index-pack's streaming support is on the way. unpack-objects is\n>  another story because I'm thinking of merging it back to index-pack\n>  first, which may take more than one release cycle.\n> \n>  builtin/pack-objects.c |   73 ++++++++++++++++++++++++++++++++++++++++++++----\n>  t/t1050-large.sh       |   16 ++++++++++\n>  2 files changed, 83 insertions(+), 6 deletions(-)\n> \n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index 1861093..98b51c1 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -259,9 +309,14 @@ static unsigned long write_object(struct sha1file *f,\n>  \tif (!to_reuse) {\n>  \t\tno_reuse:\n>  \t\tif (!usable_delta) {\n> -\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n> -\t\t\tif (!buf)\n> -\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n> +\t\t\ttype = sha1_object_info(entry->idx.sha1, &size);\n\nPlease don't use sha1_object_info() lightly.  This is a potentially \nexpensive operation, and you really don't want to do it on each objects.\n\nAnd as a matter of fact, the information you are looking for has already \nbeen determined earlier.  See the code in check_object() which tries \nhard to avoid sha1_object_info() as much as possible.\n\nTherefore you should have entry->type and entry->size already set for \nyou to use.\n\n\nNicolas\n"},{"id":"191464","messageId":"1336883862-9013-1-git-send-email-pclouds@gmail.com","threadId":"30517","inReplyTo":"alpine.LFD.2.02.1205121220070.21030@xanadu.home","subject":"[PATCH v2] pack-objects: use streaming interface for reading large loose blobs","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-13T04:37:42Z","receivedAt":"2012-05-13T04:37:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"git usually streams large blobs directly to packs. But there are cases\nwhere git can create large loose blobs (unpack-objects or hash-object\nover pipe). Or they can come from other git implementations.\ncore.bigfilethreshold can also be lowered down and introduce a new\nwave of large loose blobs.\n\nUse streaming interface to read/compress/write these blobs in one go.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n On Sat, May 12, 2012 at 11:51 PM, Nicolas Pitre <nico@fluxnic.net> wrote:\n >> @@ -259,9 +309,14 @@ static unsigned long write_object(struct sha1file *f,\n >>       if (!to_reuse) {\n >>               no_reuse:\n >>               if (!usable_delta) {\n >> -                     buf = read_sha1_file(entry->idx.sha1, &type, &size);\n >> -                     if (!buf)\n >> -                             die(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n >> +                     type = sha1_object_info(entry->idx.sha1, &size);\n >\n > Please don't use sha1_object_info() lightly.  This is a potentially\n > expensive operation, and you really don't want to do it on each objects.\n >\n > And as a matter of fact, the information you are looking for has already\n > been determined earlier.  See the code in check_object() which tries\n > hard to avoid sha1_object_info() as much as possible.\n >\n > Therefore you should have entry->type and entry->size already set for\n > you to use.\n\n Thanks. sha1_object_info() removed in favor of entry->type and entry->size.\n\n builtin/pack-objects.c |   72 ++++++++++++++++++++++++++++++++++++++++++++----\n t/t1050-large.sh       |   16 ++++++++++\n 2 files changed, 82 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1861093..ab5438a 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -16,6 +16,7 @@\n #include \"list-objects.h\"\n #include \"progress.h\"\n #include \"refs.h\"\n+#include \"streaming.h\"\n #include \"thread-utils.h\"\n \n static const char *pack_usage[] = {\n@@ -150,6 +151,55 @@ static unsigned long do_compress(void **pptr, unsigned long size)\n \treturn stream.total_out;\n }\n \n+static void write_large_blob_data(struct sha1file *f, const unsigned char *sha1)\n+{\n+\tgit_zstream stream;\n+\tunsigned char ibuf[1024 * 16];\n+\tunsigned char obuf[1024 * 16];\n+\tint zret;\n+\n+\tstruct git_istream *st;\n+\tenum object_type type;\n+\tunsigned long sz;\n+\n+\tst = open_istream(sha1, &type, &sz, NULL);\n+\tif (!st)\n+\t\tdie(_(\"unable to read %s\"), sha1_to_hex(sha1));\n+\n+\tmemset(&stream, 0, sizeof(stream));\n+\tgit_deflate_init(&stream, pack_compression_level);\n+\n+\tif (type != OBJ_BLOB)\n+\t\tdie(\"BUG: %s is not a blob\", sha1_to_hex(sha1));\n+\n+\tfor (;;) {\n+\t\tssize_t readlen;\n+\t\treadlen = read_istream(st, ibuf, sizeof(ibuf));\n+\t\tif (readlen == -1)\n+\t\t\tdie(_(\"unable to read %s\"), sha1_to_hex(sha1));\n+\n+\t\tstream.next_in = ibuf;\n+\t\tstream.avail_in = readlen;\n+\t\tzret = Z_OK;\n+\t\twhile ((stream.avail_in || readlen == 0) &&\n+\t\t       (zret == Z_OK || zret == Z_BUF_ERROR)) {\n+\t\t\tstream.next_out = obuf;\n+\t\t\tstream.avail_out = sizeof(obuf);\n+\t\t\tzret = git_deflate(&stream, readlen ? 0 : Z_FINISH);\n+\t\t\tsha1write(f, obuf, stream.next_out - obuf);\n+\t\t}\n+\t\tif (stream.avail_in)\n+\t\t\tdie(_(\"deflate error (%d)\"), zret);\n+\t\tif (readlen == 0) {\n+\t\t\tif (zret != Z_STREAM_END)\n+\t\t\t\tdie(_(\"deflate error (%d)\"), zret);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\tclose_istream(st);\n+\tgit_deflate_end(&stream);\n+}\n+\n /*\n  * we are going to reuse the existing object data as is.  make\n  * sure it is not corrupt.\n@@ -259,9 +309,13 @@ static unsigned long write_object(struct sha1file *f,\n \tif (!to_reuse) {\n \t\tno_reuse:\n \t\tif (!usable_delta) {\n-\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n-\t\t\tif (!buf)\n-\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n+\t\t\tif (entry->type == OBJ_BLOB && entry->size > big_file_threshold)\n+\t\t\t\tbuf = NULL;\n+\t\t\telse {\n+\t\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n+\t\t\t\tif (!buf)\n+\t\t\t\t\tdie(_(\"unable to read %s\"), sha1_to_hex(entry->idx.sha1));\n+\t\t\t}\n \t\t\t/*\n \t\t\t * make sure no cached delta data remains from a\n \t\t\t * previous attempt before a pack split occurred.\n@@ -284,8 +338,11 @@ static unsigned long write_object(struct sha1file *f,\n \n \t\tif (entry->z_delta_size)\n \t\t\tdatalen = entry->z_delta_size;\n-\t\telse\n+\t\telse if (buf)\n \t\t\tdatalen = do_compress(&buf, size);\n+\t\telse\n+\t\t\t/* large blob case, just assume we don't compress well */\n+\t\t\tdatalen = size;\n \n \t\t/*\n \t\t * The object header is a byte of 'type' followed by zero or\n@@ -330,8 +387,11 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t}\n \t\t\tsha1write(f, header, hdrlen);\n \t\t}\n-\t\tsha1write(f, buf, datalen);\n-\t\tfree(buf);\n+\t\tif (buf) {\n+\t\t\tsha1write(f, buf, datalen);\n+\t\t\tfree(buf);\n+\t\t} else\n+\t\t\twrite_large_blob_data(f, entry->idx.sha1);\n \t}\n \telse {\n \t\tstruct packed_git *p = entry->in_pack;\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 55ed955..7fbd2e1 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -134,6 +134,22 @@ test_expect_success 'repack' '\n \tgit repack -ad\n '\n \n+test_expect_success 'pack-objects with large loose object' '\n+\techo Z | dd of=large4 bs=1k seek=2000 &&\n+\tOBJ=9f36d94e145816ec642592c09cc8e601d83af157 &&\n+\tP=.git/objects/9f/36d94e145816ec642592c09cc8e601d83af157 &&\n+\t(\n+\tunset GIT_ALLOC_LIMIT &&\n+\tcat large4 | git hash-object -w --stdin &&\n+\tgit cat-file blob $OBJ >actual &&\n+\tcmp large4 actual\n+\t) &&\n+\techo $OBJ | git pack-objects .git/objects/pack/pack &&\n+\trm $P &&\n+\tgit cat-file blob $OBJ >actual &&\n+\tcmp large4 actual\n+'\n+\n test_expect_success 'tar achiving' '\n \tgit archive --format=tar HEAD >/dev/null\n '\n-- \n1.7.8.36.g69ee2\n"},{"id":"191496","messageId":"7vvcjyiwn4.fsf@alter.siamese.dyndns.org","threadId":"30517","inReplyTo":"1336883862-9013-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2] pack-objects: use streaming interface for reading large loose blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-14T15:56:47Z","receivedAt":"2012-05-14T15:56:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:\n\n> +\tst = open_istream(sha1, &type, &sz, NULL);\n> +\tif (!st)\n> +\t\tdie(_(\"unable to read %s\"), sha1_to_hex(sha1));\n> +\n> +\tmemset(&stream, 0, sizeof(stream));\n> +\tgit_deflate_init(&stream, pack_compression_level);\n> +\n> +\tif (type != OBJ_BLOB)\n> +\t\tdie(\"BUG: %s is not a blob\", sha1_to_hex(sha1));\n\nJust a comment, not an expression of preference, but if we are treating\nistream interface as an add-on, I wonder it might make sense not to die in\nthese places, and instead fall back to the usual \"read and then write\"\ncodepath.\n\nTo put it another way, I am wondering if it makes the logic more clear if\nyou restructure the calling side a bit more, perhaps like this:\n\n        if (!to_reuse) {\n                if (!usable_delta) {\n                        /* we know we will write as base object */\n                        ... do *NOT* do read_sha1_file() here !!!\n                } else if ... delta cases {\n                        decide delta_data and z_delta_size as before\n                }\n\n                if (entry->z_delta_size) {\n                        datalen = entry->z_delta_size;\n                        hdrlen = encode_in_pack_object_header(type, size, header);\n                        ... write either OFS_DELTA or REF_DELTA here\n                } else {\n                        /* base object case */\n                        if (write_object_in_base_representation(f, sha1)) {\n                                /* stream interface punted */\n                                read_sha1_file();\n                                do_compress();\n                                write it;\n                        }\n                }\n        } else { /* reuse case ... */\n\n> @@ -259,9 +309,13 @@ static unsigned long write_object(struct sha1file *f,\n>  \tif (!to_reuse) {\n>  \t\tno_reuse:\n>  \t\tif (!usable_delta) {\n> -\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n> -\t\t\tif (!buf)\n> -\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n> +\t\t\tif (entry->type == OBJ_BLOB && entry->size > big_file_threshold)\n> +\t\t\t\tbuf = NULL;\n\nTwo comments:\n\n - In get_object_details(), we do this:\n\n\t\tif (big_file_threshold <= entry->size)\n\t\t\tentry->no_try_delta = 1;\n\n   The new code is inconsistent with respect to the boundary conditions,\n   when the size is exactly at the threshold.\n\n - Magic condition \"buf has NULL means we will read via streaming\n   interface\" feels somewhat hacky and tasteless.  I am borderline OK with\n   such a hack whose scope is clearly limited to a short function like\n   this one, but at least it needs to be documented.  I'd rather see the\n   conditionals that look at !buf/buf are rewritten to use a new bool\n   variable similar to to_reuse and usable_delta whose name tells the\n   reader more explicitly what is going on, though.\n"},{"id":"191523","messageId":"7v62byh7l4.fsf@alter.siamese.dyndns.org","threadId":"30517","inReplyTo":"1336883862-9013-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2] pack-objects: use streaming interface for reading large loose blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-14T19:43:19Z","receivedAt":"2012-05-14T19:43:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> git usually streams large blobs directly to packs. But there are cases\n> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n> index 55ed955..7fbd2e1 100755\n> --- a/t/t1050-large.sh\n> +++ b/t/t1050-large.sh\n> @@ -134,6 +134,22 @@ test_expect_success 'repack' '\n>  \tgit repack -ad\n>  '\n>  \n> +test_expect_success 'pack-objects with large loose object' '\n> +\techo Z | dd of=large4 bs=1k seek=2000 &&\n> +\tOBJ=9f36d94e145816ec642592c09cc8e601d83af157 &&\n> +\tP=.git/objects/9f/36d94e145816ec642592c09cc8e601d83af157 &&\n\nI do not think you need these hardcoded constants; you will run\nhash-object later, no?\n\nAlso, relying on $P to exist after hash-object -w returns is somewhat\nflaky, no?\n\n> +\t(\n> +\tunset GIT_ALLOC_LIMIT &&\n\nsane_unset?\n\n> +\tcat large4 | git hash-object -w --stdin &&\n> +\tgit cat-file blob $OBJ >actual &&\n> +\tcmp large4 actual\n> +\t) &&\n> +\techo $OBJ | git pack-objects .git/objects/pack/pack &&\n\nIf you do not write directly into this directory, and then\ncreate another _empty_ repository and deposit the packfile and its\nassociated .idx file there, then you do not have to care how the earlier\nhash-object wrote its object.  You are interested only in pack-objects\nproducing a usable pack, and testing it like so will be the most direct\nway to test it, no?\n\n> +\trm $P &&\n> +\tgit cat-file blob $OBJ >actual &&\n> +\tcmp large4 actual\n> +'\n\nIn any case, the patch when applied on top of cd07cc5 (Update draft\nrelease notes to 1.7.11 (11th batch), 2012-05-11) does not pass this part\nof the test on my box.\n\n>  test_expect_success 'tar achiving' '\n>  \tgit archive --format=tar HEAD >/dev/null\n>  '\n"},{"id":"191543","messageId":"CACsJy8Bk5=X8hQAHKXCArfq57o2tLBv0RN641N3-ws69S3GghQ@mail.gmail.com","threadId":"30517","inReplyTo":"7v62byh7l4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] pack-objects: use streaming interface for reading large loose blobs","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-15T11:18:42Z","receivedAt":"2012-05-15T11:18:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, May 15, 2012 at 2:43 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> git usually streams large blobs directly to packs. But there are cases\n>> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n>> index 55ed955..7fbd2e1 100755\n>> --- a/t/t1050-large.sh\n>> +++ b/t/t1050-large.sh\n>> @@ -134,6 +134,22 @@ test_expect_success 'repack' '\n>>       git repack -ad\n>>  '\n>>\n>> +test_expect_success 'pack-objects with large loose object' '\n>> +     echo Z | dd of=large4 bs=1k seek=2000 &&\n>> +     OBJ=9f36d94e145816ec642592c09cc8e601d83af157 &&\n>> +     P=.git/objects/9f/36d94e145816ec642592c09cc8e601d83af157 &&\n>\n> I do not think you need these hardcoded constants; you will run\n> hash-object later, no?\n>\n> Also, relying on $P to exist after hash-object -w returns is somewhat\n> flaky, no?\n\nI need it to be a loose object to test this code path. Maybe this instead?\n\ntest_expect_success 'pack-objects with large loose object' '\n\tSHA1=`git hash-object huge` &&\n\ttest_create_repo loose &&\n\techo $SHA1 | git pack-objects --stdout |\n\t\tGIT_ALLOC_LIMIT=0 GIT_DIR=loose/.git git unpack-objects &&\n\techo $SHA1 | GIT_DIR=loose/.git git pack-objects pack  &&\n\ttest_create_repo packed &&\n\tmv pack-* packed/.git/objects/pack &&\n\tGIT_DIR=packed/.git git cat-file blob $SHA1 >actual &&\n\tcmp huge actual\n'\n\n>> +     rm $P &&\n>> +     git cat-file blob $OBJ >actual &&\n>> +     cmp large4 actual\n>> +'\n>\n> In any case, the patch when applied on top of cd07cc5 (Update draft\n> release notes to 1.7.11 (11th batch), 2012-05-11) does not pass this part\n> of the test on my box.\n\nInteresting. It passes for me (same base). I assume rm failed?\n-- \nDuy\n"},{"id":"191553","messageId":"7vhavhforl.fsf@alter.siamese.dyndns.org","threadId":"30517","inReplyTo":"CACsJy8Bk5=X8hQAHKXCArfq57o2tLBv0RN641N3-ws69S3GghQ@mail.gmail.com","subject":"Re: [PATCH v2] pack-objects: use streaming interface for reading large loose blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-15T15:27:26Z","receivedAt":"2012-05-15T15:27:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n> On Tue, May 15, 2012 at 2:43 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>>\n>>> git usually streams large blobs directly to packs. But there are cases\n>>> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n>>> index 55ed955..7fbd2e1 100755\n>>> --- a/t/t1050-large.sh\n>>> +++ b/t/t1050-large.sh\n>>> @@ -134,6 +134,22 @@ test_expect_success 'repack' '\n>>>       git repack -ad\n>>>  '\n>>>\n>>> +test_expect_success 'pack-objects with large loose object' '\n>>> +     echo Z | dd of=large4 bs=1k seek=2000 &&\n>>> +     OBJ=9f36d94e145816ec642592c09cc8e601d83af157 &&\n>>> +     P=.git/objects/9f/36d94e145816ec642592c09cc8e601d83af157 &&\n>>\n>> I do not think you need these hardcoded constants; you will run\n>> hash-object later, no?\n>>\n>> Also, relying on $P to exist after hash-object -w returns is somewhat\n>> flaky, no?\n>\n> I need it to be a loose object to test this code path.\n\nNo you don't.  You only need it to be something istream_read() will read\nfrom, iow, it could come from a base representation in a packfile.\n\nIn short, what I was hinting at was to do something like this instead:\n\n\tcreate a large file \"large4\"\n        BLOB=$(git hash-objects -w \"large4\")\n        PACK=$(echo \"$BLOB\" | git pack-objects pack)\n        create a new empty repository \"check\"\n        move pack-$PACK.{pack,idx} to \"check\" repository\n        in \"check\" repository, try to see if you can read $BLOB correctly\n\n>> In any case, the patch when applied on top of cd07cc5 (Update draft\n>> release notes to 1.7.11 (11th batch), 2012-05-11) does not pass this part\n>> of the test on my box.\n>\n> Interesting. It passes for me (same base). I assume rm failed?\n\nNo, reading the resulting pack dies with an error message that says the\nobject could not be read at offset 12, implying that the pack writer wrote\nsomething bogus.\n"},{"id":"191577","messageId":"CACsJy8DzdFORUMy7p_eVotr=HdkMX10uXy25H=05TBDjOmi4yw@mail.gmail.com","threadId":"30517","inReplyTo":"7vhavhforl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] pack-objects: use streaming interface for reading large loose blobs","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T07:09:32Z","receivedAt":"2012-05-16T07:09:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, May 15, 2012 at 10:27 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n>\n>> On Tue, May 15, 2012 at 2:43 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>>>\n>>>> git usually streams large blobs directly to packs. But there are cases\n>>>> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n>>>> index 55ed955..7fbd2e1 100755\n>>>> --- a/t/t1050-large.sh\n>>>> +++ b/t/t1050-large.sh\n>>>> @@ -134,6 +134,22 @@ test_expect_success 'repack' '\n>>>>       git repack -ad\n>>>>  '\n>>>>\n>>>> +test_expect_success 'pack-objects with large loose object' '\n>>>> +     echo Z | dd of=large4 bs=1k seek=2000 &&\n>>>> +     OBJ=9f36d94e145816ec642592c09cc8e601d83af157 &&\n>>>> +     P=.git/objects/9f/36d94e145816ec642592c09cc8e601d83af157 &&\n>>>\n>>> I do not think you need these hardcoded constants; you will run\n>>> hash-object later, no?\n>>>\n>>> Also, relying on $P to exist after hash-object -w returns is somewhat\n>>> flaky, no?\n>>\n>> I need it to be a loose object to test this code path.\n>\n> No you don't.  You only need it to be something istream_read() will read\n> from, iow, it could come from a base representation in a packfile.\n\nNo, an in-pack object will set to_reuse to 1, which goes a completely\ndifferent code path. write_large_blob_data() is only called when\nto_reuse == 0.\n\n>>> In any case, the patch when applied on top of cd07cc5 (Update draft\n>>> release notes to 1.7.11 (11th batch), 2012-05-11) does not pass this part\n>>> of the test on my box.\n>>\n>> Interesting. It passes for me (same base). I assume rm failed?\n>\n> No, reading the resulting pack dies with an error message that says the\n> object could not be read at offset 12, implying that the pack writer wrote\n> something bogus.\n\nI'm still unable to reproduce that. But I think I found the problem.\nIn streaming code path, I set datalen = <uncompressed size> but\nwrite_object() returns \"hdrlen + (wrong) datalen\". Patches will come a\ncouple of hours from now.\n-- \nDuy\n"},{"id":"191578","messageId":"1337169731-23416-1-git-send-email-pclouds@gmail.com","threadId":"30517","inReplyTo":"1336818375-16895-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 1/4] streaming: allow to call close_istream(NULL);","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T12:02:08Z","receivedAt":"2012-05-16T12:02:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This makes it more convenient in cleanup code, like free(NULL).\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n streaming.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/streaming.c b/streaming.c\nindex 3a3cd12..38b39cd 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -94,7 +94,7 @@ struct git_istream {\n \n int close_istream(struct git_istream *st)\n {\n-\tint r = st->vtbl->close(st);\n+\tint r = st ? st->vtbl->close(st) : 0;\n \tfree(st);\n \treturn r;\n }\n-- \n1.7.8.36.g69ee2\n"},{"id":"191579","messageId":"1337169731-23416-2-git-send-email-pclouds@gmail.com","threadId":"30517","inReplyTo":"1337169731-23416-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 2/4] pack-objects, streaming: turn \"xx >= big_file_threshold\" to \".. > ..\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T12:02:09Z","receivedAt":"2012-05-16T12:02:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is because all other places do \"xx > big_file_threshold\"\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Without two comparisons I added recently in archive-*.c, it's still\n inconsistent (two xx >= big_file and one xx > big_file).  I have no\n preference between > and >=. If some of you vote >=, I'll update the\n patch.\n\n builtin/pack-objects.c |    2 +-\n streaming.c            |    2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1861093..b2e0940 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1327,7 +1327,7 @@ static void get_object_details(void)\n \tfor (i = 0; i < nr_objects; i++) {\n \t\tstruct object_entry *entry = sorted_by_offset[i];\n \t\tcheck_object(entry);\n-\t\tif (big_file_threshold <= entry->size)\n+\t\tif (big_file_threshold < entry->size)\n \t\t\tentry->no_try_delta = 1;\n \t}\n \ndiff --git a/streaming.c b/streaming.c\nindex 38b39cd..829ce7d 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -121,7 +121,7 @@ static enum input_source istream_source(const unsigned char *sha1,\n \tcase OI_LOOSE:\n \t\treturn loose;\n \tcase OI_PACKED:\n-\t\tif (!oi->u.packed.is_delta && big_file_threshold <= size)\n+\t\tif (!oi->u.packed.is_delta && big_file_threshold < size)\n \t\t\treturn pack_non_delta;\n \t\t/* fallthru */\n \tdefault:\n-- \n1.7.8.36.g69ee2\n"},{"id":"191580","messageId":"1337169731-23416-3-git-send-email-pclouds@gmail.com","threadId":"30517","inReplyTo":"1337169731-23416-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 3/4] pack-objects: refactor write_object()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T12:02:10Z","receivedAt":"2012-05-16T12:02:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Move !to_reuse and to_reuse write code out into two separate functions\nand remove \"goto no_reuse;\" hack\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/pack-objects.c |  322 ++++++++++++++++++++++++++----------------------\n 1 files changed, 172 insertions(+), 150 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex b2e0940..ccfcbad 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -200,22 +200,178 @@ static void copy_pack_data(struct sha1file *f,\n }\n \n /* Return 0 if we will bust the pack-size limit */\n-static unsigned long write_object(struct sha1file *f,\n-\t\t\t\t  struct object_entry *entry,\n-\t\t\t\t  off_t write_offset)\n+static unsigned long write_no_reuse_object(struct sha1file *f, struct object_entry *entry,\n+\t\t\t\t\t   unsigned long limit, int usable_delta)\n {\n-\tunsigned long size, limit, datalen;\n-\tvoid *buf;\n+\tunsigned long size, datalen;\n \tunsigned char header[10], dheader[10];\n \tunsigned hdrlen;\n \tenum object_type type;\n+\tvoid *buf;\n+\n+\tif (!usable_delta) {\n+\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n+\t\tif (!buf)\n+\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n+\t\t/*\n+\t\t * make sure no cached delta data remains from a\n+\t\t * previous attempt before a pack split occurred.\n+\t\t */\n+\t\tfree(entry->delta_data);\n+\t\tentry->delta_data = NULL;\n+\t\tentry->z_delta_size = 0;\n+\t} else if (entry->delta_data) {\n+\t\tsize = entry->delta_size;\n+\t\tbuf = entry->delta_data;\n+\t\tentry->delta_data = NULL;\n+\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n+\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n+\t} else {\n+\t\tbuf = get_delta(entry);\n+\t\tsize = entry->delta_size;\n+\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n+\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n+\t}\n+\n+\tif (entry->z_delta_size)\n+\t\tdatalen = entry->z_delta_size;\n+\telse\n+\t\tdatalen = do_compress(&buf, size);\n+\n+\t/*\n+\t * The object header is a byte of 'type' followed by zero or\n+\t * more bytes of length.\n+\t */\n+\thdrlen = encode_in_pack_object_header(type, size, header);\n+\n+\tif (type == OBJ_OFS_DELTA) {\n+\t\t/*\n+\t\t * Deltas with relative base contain an additional\n+\t\t * encoding of the relative offset for the delta\n+\t\t * base from this object's position in the pack.\n+\t\t */\n+\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n+\t\tunsigned pos = sizeof(dheader) - 1;\n+\t\tdheader[pos] = ofs & 127;\n+\t\twhile (ofs >>= 7)\n+\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n+\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n+\t\t\tfree(buf);\n+\t\t\treturn 0;\n+\t\t}\n+\t\tsha1write(f, header, hdrlen);\n+\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n+\t\thdrlen += sizeof(dheader) - pos;\n+\t} else if (type == OBJ_REF_DELTA) {\n+\t\t/*\n+\t\t * Deltas with a base reference contain\n+\t\t * an additional 20 bytes for the base sha1.\n+\t\t */\n+\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n+\t\t\tfree(buf);\n+\t\t\treturn 0;\n+\t\t}\n+\t\tsha1write(f, header, hdrlen);\n+\t\tsha1write(f, entry->delta->idx.sha1, 20);\n+\t\thdrlen += 20;\n+\t} else {\n+\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n+\t\t\tfree(buf);\n+\t\t\treturn 0;\n+\t\t}\n+\t\tsha1write(f, header, hdrlen);\n+\t}\n+\tsha1write(f, buf, datalen);\n+\tfree(buf);\n+\n+\treturn hdrlen + datalen;\n+}\n+\n+/* Return 0 if we will bust the pack-size limit */\n+static unsigned long write_reuse_object(struct sha1file *f, struct object_entry *entry,\n+\t\t\t\t\tunsigned long limit, int usable_delta)\n+{\n+\tstruct packed_git *p = entry->in_pack;\n+\tstruct pack_window *w_curs = NULL;\n+\tstruct revindex_entry *revidx;\n+\toff_t offset;\n+\tenum object_type type = entry->type;\n+\tunsigned long datalen;\n+\tunsigned char header[10], dheader[10];\n+\tunsigned hdrlen;\n+\n+\tif (entry->delta)\n+\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n+\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n+\thdrlen = encode_in_pack_object_header(type, entry->size, header);\n+\n+\toffset = entry->in_pack_offset;\n+\trevidx = find_pack_revindex(p, offset);\n+\tdatalen = revidx[1].offset - offset;\n+\tif (!pack_to_stdout && p->index_version > 1 &&\n+\t    check_pack_crc(p, &w_curs, offset, datalen, revidx->nr)) {\n+\t\terror(\"bad packed object CRC for %s\", sha1_to_hex(entry->idx.sha1));\n+\t\tunuse_pack(&w_curs);\n+\t\treturn write_no_reuse_object(f, entry, limit, usable_delta);\n+\t}\n+\n+\toffset += entry->in_pack_header_size;\n+\tdatalen -= entry->in_pack_header_size;\n+\n+\tif (!pack_to_stdout && p->index_version == 1 &&\n+\t    check_pack_inflate(p, &w_curs, offset, datalen, entry->size)) {\n+\t\terror(\"corrupt packed object for %s\", sha1_to_hex(entry->idx.sha1));\n+\t\tunuse_pack(&w_curs);\n+\t\treturn write_no_reuse_object(f, entry, limit, usable_delta);\n+\t}\n+\n+\tif (type == OBJ_OFS_DELTA) {\n+\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n+\t\tunsigned pos = sizeof(dheader) - 1;\n+\t\tdheader[pos] = ofs & 127;\n+\t\twhile (ofs >>= 7)\n+\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n+\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n+\t\t\tunuse_pack(&w_curs);\n+\t\t\treturn 0;\n+\t\t}\n+\t\tsha1write(f, header, hdrlen);\n+\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n+\t\thdrlen += sizeof(dheader) - pos;\n+\t\treused_delta++;\n+\t} else if (type == OBJ_REF_DELTA) {\n+\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n+\t\t\tunuse_pack(&w_curs);\n+\t\t\treturn 0;\n+\t\t}\n+\t\tsha1write(f, header, hdrlen);\n+\t\tsha1write(f, entry->delta->idx.sha1, 20);\n+\t\thdrlen += 20;\n+\t\treused_delta++;\n+\t} else {\n+\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n+\t\t\tunuse_pack(&w_curs);\n+\t\t\treturn 0;\n+\t\t}\n+\t\tsha1write(f, header, hdrlen);\n+\t}\n+\tcopy_pack_data(f, p, &w_curs, offset, datalen);\n+\tunuse_pack(&w_curs);\n+\treused++;\n+\treturn hdrlen + datalen;\n+}\n+\n+/* Return 0 if we will bust the pack-size limit */\n+static unsigned long write_object(struct sha1file *f,\n+\t\t\t\t  struct object_entry *entry,\n+\t\t\t\t  off_t write_offset)\n+{\n+\tunsigned long limit, len;\n \tint usable_delta, to_reuse;\n \n \tif (!pack_to_stdout)\n \t\tcrc32_begin(f);\n \n-\ttype = entry->type;\n-\n \t/* apply size limit if limited packsize and not first object */\n \tif (!pack_size_limit || !nr_written)\n \t\tlimit = 0;\n@@ -243,11 +399,11 @@ static unsigned long write_object(struct sha1file *f,\n \t\tto_reuse = 0;\t/* explicit */\n \telse if (!entry->in_pack)\n \t\tto_reuse = 0;\t/* can't reuse what we don't have */\n-\telse if (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA)\n+\telse if (entry->type == OBJ_REF_DELTA || entry->type == OBJ_OFS_DELTA)\n \t\t\t\t/* check_object() decided it for us ... */\n \t\tto_reuse = usable_delta;\n \t\t\t\t/* ... but pack split may override that */\n-\telse if (type != entry->in_pack_type)\n+\telse if (entry->type != entry->in_pack_type)\n \t\tto_reuse = 0;\t/* pack has delta which is unusable */\n \telse if (entry->delta)\n \t\tto_reuse = 0;\t/* we want to pack afresh */\n@@ -256,153 +412,19 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\t * and we do not need to deltify it.\n \t\t\t\t */\n \n-\tif (!to_reuse) {\n-\t\tno_reuse:\n-\t\tif (!usable_delta) {\n-\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n-\t\t\tif (!buf)\n-\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n-\t\t\t/*\n-\t\t\t * make sure no cached delta data remains from a\n-\t\t\t * previous attempt before a pack split occurred.\n-\t\t\t */\n-\t\t\tfree(entry->delta_data);\n-\t\t\tentry->delta_data = NULL;\n-\t\t\tentry->z_delta_size = 0;\n-\t\t} else if (entry->delta_data) {\n-\t\t\tsize = entry->delta_size;\n-\t\t\tbuf = entry->delta_data;\n-\t\t\tentry->delta_data = NULL;\n-\t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n-\t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n-\t\t} else {\n-\t\t\tbuf = get_delta(entry);\n-\t\t\tsize = entry->delta_size;\n-\t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n-\t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n-\t\t}\n-\n-\t\tif (entry->z_delta_size)\n-\t\t\tdatalen = entry->z_delta_size;\n-\t\telse\n-\t\t\tdatalen = do_compress(&buf, size);\n-\n-\t\t/*\n-\t\t * The object header is a byte of 'type' followed by zero or\n-\t\t * more bytes of length.\n-\t\t */\n-\t\thdrlen = encode_in_pack_object_header(type, size, header);\n-\n-\t\tif (type == OBJ_OFS_DELTA) {\n-\t\t\t/*\n-\t\t\t * Deltas with relative base contain an additional\n-\t\t\t * encoding of the relative offset for the delta\n-\t\t\t * base from this object's position in the pack.\n-\t\t\t */\n-\t\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n-\t\t\tunsigned pos = sizeof(dheader) - 1;\n-\t\t\tdheader[pos] = ofs & 127;\n-\t\t\twhile (ofs >>= 7)\n-\t\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n-\t\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n-\t\t\t\tfree(buf);\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t\tsha1write(f, header, hdrlen);\n-\t\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n-\t\t\thdrlen += sizeof(dheader) - pos;\n-\t\t} else if (type == OBJ_REF_DELTA) {\n-\t\t\t/*\n-\t\t\t * Deltas with a base reference contain\n-\t\t\t * an additional 20 bytes for the base sha1.\n-\t\t\t */\n-\t\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n-\t\t\t\tfree(buf);\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t\tsha1write(f, header, hdrlen);\n-\t\t\tsha1write(f, entry->delta->idx.sha1, 20);\n-\t\t\thdrlen += 20;\n-\t\t} else {\n-\t\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n-\t\t\t\tfree(buf);\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t\tsha1write(f, header, hdrlen);\n-\t\t}\n-\t\tsha1write(f, buf, datalen);\n-\t\tfree(buf);\n-\t}\n-\telse {\n-\t\tstruct packed_git *p = entry->in_pack;\n-\t\tstruct pack_window *w_curs = NULL;\n-\t\tstruct revindex_entry *revidx;\n-\t\toff_t offset;\n-\n-\t\tif (entry->delta)\n-\t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n-\t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n-\t\thdrlen = encode_in_pack_object_header(type, entry->size, header);\n-\n-\t\toffset = entry->in_pack_offset;\n-\t\trevidx = find_pack_revindex(p, offset);\n-\t\tdatalen = revidx[1].offset - offset;\n-\t\tif (!pack_to_stdout && p->index_version > 1 &&\n-\t\t    check_pack_crc(p, &w_curs, offset, datalen, revidx->nr)) {\n-\t\t\terror(\"bad packed object CRC for %s\", sha1_to_hex(entry->idx.sha1));\n-\t\t\tunuse_pack(&w_curs);\n-\t\t\tgoto no_reuse;\n-\t\t}\n-\n-\t\toffset += entry->in_pack_header_size;\n-\t\tdatalen -= entry->in_pack_header_size;\n-\t\tif (!pack_to_stdout && p->index_version == 1 &&\n-\t\t    check_pack_inflate(p, &w_curs, offset, datalen, entry->size)) {\n-\t\t\terror(\"corrupt packed object for %s\", sha1_to_hex(entry->idx.sha1));\n-\t\t\tunuse_pack(&w_curs);\n-\t\t\tgoto no_reuse;\n-\t\t}\n+\tif (!to_reuse)\n+\t\tlen = write_no_reuse_object(f, entry, limit, usable_delta);\n+\telse\n+\t\tlen = write_reuse_object(f, entry, limit, usable_delta);\n+\tif (!len)\n+\t\treturn 0;\n \n-\t\tif (type == OBJ_OFS_DELTA) {\n-\t\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n-\t\t\tunsigned pos = sizeof(dheader) - 1;\n-\t\t\tdheader[pos] = ofs & 127;\n-\t\t\twhile (ofs >>= 7)\n-\t\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n-\t\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n-\t\t\t\tunuse_pack(&w_curs);\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t\tsha1write(f, header, hdrlen);\n-\t\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n-\t\t\thdrlen += sizeof(dheader) - pos;\n-\t\t\treused_delta++;\n-\t\t} else if (type == OBJ_REF_DELTA) {\n-\t\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n-\t\t\t\tunuse_pack(&w_curs);\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t\tsha1write(f, header, hdrlen);\n-\t\t\tsha1write(f, entry->delta->idx.sha1, 20);\n-\t\t\thdrlen += 20;\n-\t\t\treused_delta++;\n-\t\t} else {\n-\t\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n-\t\t\t\tunuse_pack(&w_curs);\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t\tsha1write(f, header, hdrlen);\n-\t\t}\n-\t\tcopy_pack_data(f, p, &w_curs, offset, datalen);\n-\t\tunuse_pack(&w_curs);\n-\t\treused++;\n-\t}\n \tif (usable_delta)\n \t\twritten_delta++;\n \twritten++;\n \tif (!pack_to_stdout)\n \t\tentry->idx.crc32 = crc32_end(f);\n-\treturn hdrlen + datalen;\n+\treturn len;\n }\n \n enum write_one_status {\n-- \n1.7.8.36.g69ee2\n"},{"id":"191581","messageId":"1337169731-23416-4-git-send-email-pclouds@gmail.com","threadId":"30517","inReplyTo":"1337169731-23416-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 4/4] pack-objects: use streaming interface for reading large loose blobs","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T12:02:11Z","receivedAt":"2012-05-16T12:02:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"git usually streams large blobs directly to packs. But there are cases\nwhere git can create large loose blobs (unpack-objects or hash-object\nover pipe). Or they can come from other git implementations.\ncore.bigfilethreshold can also be lowered down and introduce a new\nwave of large loose blobs.\n\nUse streaming interface to read/compress/write these blobs in one go.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Changes since the first version:\n\n  - use \"struct git_istream *\" as streaming indicator, instead of \"buf\n    != NULL\"\n  - fall back to read_sha1_file() if open_istream() fails\n  - write_object() returns correct written number of bytes (previously\n    datalen was miscalculated, leading to wrong pack size calculation)\n\n builtin/pack-objects.c |   63 ++++++++++++++++++++++++++++++++++++++++++++---\n t/t1050-large.sh       |   12 +++++++++\n 2 files changed, 71 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex ccfcbad..b2679cb 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -16,6 +16,7 @@\n #include \"list-objects.h\"\n #include \"progress.h\"\n #include \"refs.h\"\n+#include \"streaming.h\"\n #include \"thread-utils.h\"\n \n static const char *pack_usage[] = {\n@@ -150,6 +151,46 @@ static unsigned long do_compress(void **pptr, unsigned long size)\n \treturn stream.total_out;\n }\n \n+static unsigned long write_large_blob_data(struct git_istream *st, struct sha1file *f,\n+\t\t\t\t\t   const unsigned char *sha1)\n+{\n+\tgit_zstream stream;\n+\tunsigned char ibuf[1024 * 16];\n+\tunsigned char obuf[1024 * 16];\n+\tunsigned long olen = 0;\n+\n+\tmemset(&stream, 0, sizeof(stream));\n+\tgit_deflate_init(&stream, pack_compression_level);\n+\n+\tfor (;;) {\n+\t\tssize_t readlen;\n+\t\tint zret = Z_OK;\n+\t\treadlen = read_istream(st, ibuf, sizeof(ibuf));\n+\t\tif (readlen == -1)\n+\t\t\tdie(_(\"unable to read %s\"), sha1_to_hex(sha1));\n+\n+\t\tstream.next_in = ibuf;\n+\t\tstream.avail_in = readlen;\n+\t\twhile ((stream.avail_in || readlen == 0) &&\n+\t\t       (zret == Z_OK || zret == Z_BUF_ERROR)) {\n+\t\t\tstream.next_out = obuf;\n+\t\t\tstream.avail_out = sizeof(obuf);\n+\t\t\tzret = git_deflate(&stream, readlen ? 0 : Z_FINISH);\n+\t\t\tsha1write(f, obuf, stream.next_out - obuf);\n+\t\t\tolen += stream.next_out - obuf;\n+\t\t}\n+\t\tif (stream.avail_in)\n+\t\t\tdie(_(\"deflate error (%d)\"), zret);\n+\t\tif (readlen == 0) {\n+\t\t\tif (zret != Z_STREAM_END)\n+\t\t\t\tdie(_(\"deflate error (%d)\"), zret);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\tgit_deflate_end(&stream);\n+\treturn olen;\n+}\n+\n /*\n  * we are going to reuse the existing object data as is.  make\n  * sure it is not corrupt.\n@@ -208,11 +249,18 @@ static unsigned long write_no_reuse_object(struct sha1file *f, struct object_ent\n \tunsigned hdrlen;\n \tenum object_type type;\n \tvoid *buf;\n+\tstruct git_istream *st = NULL;\n \n \tif (!usable_delta) {\n-\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n-\t\tif (!buf)\n-\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n+\t\tif (entry->type == OBJ_BLOB &&\n+\t\t    entry->size > big_file_threshold &&\n+\t\t    (st = open_istream(entry->idx.sha1, &type, &size, NULL)) != NULL)\n+\t\t\tbuf = NULL;\n+\t\telse {\n+\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n+\t\t\tif (!buf)\n+\t\t\t\tdie(_(\"unable to read %s\"), sha1_to_hex(entry->idx.sha1));\n+\t\t}\n \t\t/*\n \t\t * make sure no cached delta data remains from a\n \t\t * previous attempt before a pack split occurred.\n@@ -235,6 +283,9 @@ static unsigned long write_no_reuse_object(struct sha1file *f, struct object_ent\n \n \tif (entry->z_delta_size)\n \t\tdatalen = entry->z_delta_size;\n+\telse if (st)\n+\t\t/* large blob case, just assume we don't compress well */\n+\t\tdatalen = size;\n \telse\n \t\tdatalen = do_compress(&buf, size);\n \n@@ -281,7 +332,11 @@ static unsigned long write_no_reuse_object(struct sha1file *f, struct object_ent\n \t\t}\n \t\tsha1write(f, header, hdrlen);\n \t}\n-\tsha1write(f, buf, datalen);\n+\tif (st)\n+\t\tdatalen = write_large_blob_data(st, f, entry->idx.sha1);\n+\telse\n+\t\tsha1write(f, buf, datalen);\n+\tclose_istream(st);\n \tfree(buf);\n \n \treturn hdrlen + datalen;\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 55ed955..313889b 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -134,6 +134,18 @@ test_expect_success 'repack' '\n \tgit repack -ad\n '\n \n+test_expect_success 'pack-objects with large loose object' '\n+\tSHA1=`git hash-object huge` &&\n+\ttest_create_repo loose &&\n+\techo $SHA1 | git pack-objects --stdout |\n+\t\tGIT_ALLOC_LIMIT=0 GIT_DIR=loose/.git git unpack-objects &&\n+\techo $SHA1 | GIT_DIR=loose/.git git pack-objects pack &&\n+\ttest_create_repo packed &&\n+\tmv pack-* packed/.git/objects/pack &&\n+\tGIT_DIR=packed/.git git cat-file blob $SHA1 >actual &&\n+\tcmp huge actual\n+'\n+\n test_expect_success 'tar achiving' '\n \tgit archive --format=tar HEAD >/dev/null\n '\n-- \n1.7.8.36.g69ee2\n"},{"id":"191677","messageId":"xmqqobpl2oep.fsf@junio.mtv.corp.google.com","threadId":"30517","inReplyTo":"1337169731-23416-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 1/4] streaming: allow to call close_istream(NULL);","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-18T21:02:38Z","receivedAt":"2012-05-18T21:02:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> This makes it more convenient in cleanup code, like free(NULL).\n\nIf the justification were \"like fclose(NULL)\", it have made more sense,\nbut fclose(NULL) does not silently turn itself into a successful no-op.\nYou are expected to check the returned value from the matching function\n(i.e. fopen()) and do not call it.  So taking this patch alone, I smell\na bad design taste.\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  streaming.c |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/streaming.c b/streaming.c\n> index 3a3cd12..38b39cd 100644\n> --- a/streaming.c\n> +++ b/streaming.c\n> @@ -94,7 +94,7 @@ struct git_istream {\n>  \n>  int close_istream(struct git_istream *st)\n>  {\n> -\tint r = st->vtbl->close(st);\n> +\tint r = st ? st->vtbl->close(st) : 0;\n>  \tfree(st);\n>  \treturn r;\n>  }\n"},{"id":"191679","messageId":"xmqqk4092o9g.fsf@junio.mtv.corp.google.com","threadId":"30517","inReplyTo":"1337169731-23416-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 2/4] pack-objects, streaming: turn \"xx >= big_file_threshold\" to \".. > ..\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-18T21:05:47Z","receivedAt":"2012-05-18T21:05:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> This is because all other places do \"xx > big_file_threshold\"\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  Without two comparisons I added recently in archive-*.c, it's still\n>  inconsistent (two xx >= big_file and one xx > big_file).  I have no\n>  preference between > and >=. If some of you vote >=, I'll update the\n>  patch.\n\nThanks; I do not have huge preference between the two, but \"I want\nspecial treatment for anything bigger than 2MiB\" is probably more\nnatural than \"I want special treatment for things that are exactly 2MiB\nand also things bigger than that threashold\".\n\n>  builtin/pack-objects.c |    2 +-\n>  streaming.c            |    2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index 1861093..b2e0940 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -1327,7 +1327,7 @@ static void get_object_details(void)\n>  \tfor (i = 0; i < nr_objects; i++) {\n>  \t\tstruct object_entry *entry = sorted_by_offset[i];\n>  \t\tcheck_object(entry);\n> -\t\tif (big_file_threshold <= entry->size)\n> +\t\tif (big_file_threshold < entry->size)\n>  \t\t\tentry->no_try_delta = 1;\n>  \t}\n>  \n> diff --git a/streaming.c b/streaming.c\n> index 38b39cd..829ce7d 100644\n> --- a/streaming.c\n> +++ b/streaming.c\n> @@ -121,7 +121,7 @@ static enum input_source istream_source(const unsigned char *sha1,\n>  \tcase OI_LOOSE:\n>  \t\treturn loose;\n>  \tcase OI_PACKED:\n> -\t\tif (!oi->u.packed.is_delta && big_file_threshold <= size)\n> +\t\tif (!oi->u.packed.is_delta && big_file_threshold < size)\n>  \t\t\treturn pack_non_delta;\n>  \t\t/* fallthru */\n>  \tdefault:\n"},{"id":"191680","messageId":"xmqqfwax2nqw.fsf@junio.mtv.corp.google.com","threadId":"30517","inReplyTo":"1337169731-23416-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 3/4] pack-objects: refactor write_object()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-18T21:16:55Z","receivedAt":"2012-05-18T21:16:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> Move !to_reuse and to_reuse write code out into two separate functions\n> and remove \"goto no_reuse;\" hack\n\nI do not necessarily think goto is a harmful hack, but the original code\nis sufficiently large and its flow clearly ought to be separable into\ntwo phases (phase I: decide if we want to copy from existing pack or\nbuild data to be sent to the pack afresh; phase II: carry out the\ndecision, doing one of the two things we decided to do), so I think it\nmakes tons of sense to separate out the second phase into two helper\nfunctions.\n\nLooks good from a cursory reading.\n\nThanks.\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  builtin/pack-objects.c |  322 ++++++++++++++++++++++++++----------------------\n>  1 files changed, 172 insertions(+), 150 deletions(-)\n>\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index b2e0940..ccfcbad 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -200,22 +200,178 @@ static void copy_pack_data(struct sha1file *f,\n>  }\n>  \n>  /* Return 0 if we will bust the pack-size limit */\n> +static unsigned long write_no_reuse_object(struct sha1file *f, struct object_entry *entry,\n> +\t\t\t\t\t   unsigned long limit, int usable_delta)\n>  {\n> +\tunsigned long size, datalen;\n>  \tunsigned char header[10], dheader[10];\n>  \tunsigned hdrlen;\n>  \tenum object_type type;\n> +\tvoid *buf;\n> +\n> +\tif (!usable_delta) {\n> +\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n> +\t\tif (!buf)\n> +\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n> +\t\t/*\n> +\t\t * make sure no cached delta data remains from a\n> +\t\t * previous attempt before a pack split occurred.\n> +\t\t */\n> +\t\tfree(entry->delta_data);\n> +\t\tentry->delta_data = NULL;\n> +\t\tentry->z_delta_size = 0;\n> +\t} else if (entry->delta_data) {\n> +\t\tsize = entry->delta_size;\n> +\t\tbuf = entry->delta_data;\n> +\t\tentry->delta_data = NULL;\n> +\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n> +\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n> +\t} else {\n> +\t\tbuf = get_delta(entry);\n> +\t\tsize = entry->delta_size;\n> +\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n> +\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n> +\t}\n> +\n> +\tif (entry->z_delta_size)\n> +\t\tdatalen = entry->z_delta_size;\n> +\telse\n> +\t\tdatalen = do_compress(&buf, size);\n> +\n> +\t/*\n> +\t * The object header is a byte of 'type' followed by zero or\n> +\t * more bytes of length.\n> +\t */\n> +\thdrlen = encode_in_pack_object_header(type, size, header);\n> +\n> +\tif (type == OBJ_OFS_DELTA) {\n> +\t\t/*\n> +\t\t * Deltas with relative base contain an additional\n> +\t\t * encoding of the relative offset for the delta\n> +\t\t * base from this object's position in the pack.\n> +\t\t */\n> +\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n> +\t\tunsigned pos = sizeof(dheader) - 1;\n> +\t\tdheader[pos] = ofs & 127;\n> +\t\twhile (ofs >>= 7)\n> +\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n> +\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n> +\t\t\tfree(buf);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n> +\t\thdrlen += sizeof(dheader) - pos;\n> +\t} else if (type == OBJ_REF_DELTA) {\n> +\t\t/*\n> +\t\t * Deltas with a base reference contain\n> +\t\t * an additional 20 bytes for the base sha1.\n> +\t\t */\n> +\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n> +\t\t\tfree(buf);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t\tsha1write(f, entry->delta->idx.sha1, 20);\n> +\t\thdrlen += 20;\n> +\t} else {\n> +\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n> +\t\t\tfree(buf);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t}\n> +\tsha1write(f, buf, datalen);\n> +\tfree(buf);\n> +\n> +\treturn hdrlen + datalen;\n> +}\n> +\n> +/* Return 0 if we will bust the pack-size limit */\n> +static unsigned long write_reuse_object(struct sha1file *f, struct object_entry *entry,\n> +\t\t\t\t\tunsigned long limit, int usable_delta)\n> +{\n> +\tstruct packed_git *p = entry->in_pack;\n> +\tstruct pack_window *w_curs = NULL;\n> +\tstruct revindex_entry *revidx;\n> +\toff_t offset;\n> +\tenum object_type type = entry->type;\n> +\tunsigned long datalen;\n> +\tunsigned char header[10], dheader[10];\n> +\tunsigned hdrlen;\n> +\n> +\tif (entry->delta)\n> +\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n> +\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n> +\thdrlen = encode_in_pack_object_header(type, entry->size, header);\n> +\n> +\toffset = entry->in_pack_offset;\n> +\trevidx = find_pack_revindex(p, offset);\n> +\tdatalen = revidx[1].offset - offset;\n> +\tif (!pack_to_stdout && p->index_version > 1 &&\n> +\t    check_pack_crc(p, &w_curs, offset, datalen, revidx->nr)) {\n> +\t\terror(\"bad packed object CRC for %s\", sha1_to_hex(entry->idx.sha1));\n> +\t\tunuse_pack(&w_curs);\n> +\t\treturn write_no_reuse_object(f, entry, limit, usable_delta);\n> +\t}\n> +\n> +\toffset += entry->in_pack_header_size;\n> +\tdatalen -= entry->in_pack_header_size;\n> +\n> +\tif (!pack_to_stdout && p->index_version == 1 &&\n> +\t    check_pack_inflate(p, &w_curs, offset, datalen, entry->size)) {\n> +\t\terror(\"corrupt packed object for %s\", sha1_to_hex(entry->idx.sha1));\n> +\t\tunuse_pack(&w_curs);\n> +\t\treturn write_no_reuse_object(f, entry, limit, usable_delta);\n> +\t}\n> +\n> +\tif (type == OBJ_OFS_DELTA) {\n> +\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n> +\t\tunsigned pos = sizeof(dheader) - 1;\n> +\t\tdheader[pos] = ofs & 127;\n> +\t\twhile (ofs >>= 7)\n> +\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n> +\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n> +\t\t\tunuse_pack(&w_curs);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n> +\t\thdrlen += sizeof(dheader) - pos;\n> +\t\treused_delta++;\n> +\t} else if (type == OBJ_REF_DELTA) {\n> +\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n> +\t\t\tunuse_pack(&w_curs);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t\tsha1write(f, entry->delta->idx.sha1, 20);\n> +\t\thdrlen += 20;\n> +\t\treused_delta++;\n> +\t} else {\n> +\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n> +\t\t\tunuse_pack(&w_curs);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t}\n> +\tcopy_pack_data(f, p, &w_curs, offset, datalen);\n> +\tunuse_pack(&w_curs);\n> +\treused++;\n> +\treturn hdrlen + datalen;\n> +}\n> +\n> +/* Return 0 if we will bust the pack-size limit */\n> +static unsigned long write_object(struct sha1file *f,\n> +\t\t\t\t  struct object_entry *entry,\n> +\t\t\t\t  off_t write_offset)\n> +{\n> +\tunsigned long limit, len;\n>  \tint usable_delta, to_reuse;\n>  \n>  \tif (!pack_to_stdout)\n>  \t\tcrc32_begin(f);\n>  \n>  \t/* apply size limit if limited packsize and not first object */\n>  \tif (!pack_size_limit || !nr_written)\n>  \t\tlimit = 0;\n> @@ -243,11 +399,11 @@ static unsigned long write_object(struct sha1file *f,\n>  \t\tto_reuse = 0;\t/* explicit */\n>  \telse if (!entry->in_pack)\n>  \t\tto_reuse = 0;\t/* can't reuse what we don't have */\n> +\telse if (entry->type == OBJ_REF_DELTA || entry->type == OBJ_OFS_DELTA)\n>  \t\t\t\t/* check_object() decided it for us ... */\n>  \t\tto_reuse = usable_delta;\n>  \t\t\t\t/* ... but pack split may override that */\n> +\telse if (entry->type != entry->in_pack_type)\n>  \t\tto_reuse = 0;\t/* pack has delta which is unusable */\n>  \telse if (entry->delta)\n>  \t\tto_reuse = 0;\t/* we want to pack afresh */\n> @@ -256,153 +412,19 @@ static unsigned long write_object(struct sha1file *f,\n>  \t\t\t\t * and we do not need to deltify it.\n>  \t\t\t\t */\n>  \n> +\tif (!to_reuse)\n> +\t\tlen = write_no_reuse_object(f, entry, limit, usable_delta);\n> +\telse\n> +\t\tlen = write_reuse_object(f, entry, limit, usable_delta);\n> +\tif (!len)\n> +\t\treturn 0;\n>  \n>  \tif (usable_delta)\n>  \t\twritten_delta++;\n>  \twritten++;\n>  \tif (!pack_to_stdout)\n>  \t\tentry->idx.crc32 = crc32_end(f);\n> -\treturn hdrlen + datalen;\n> +\treturn len;\n>  }\n>  \n>  enum write_one_status {\n"},{"id":"191714","messageId":"alpine.LFD.2.02.1205182243180.13185@xanadu.home","threadId":"30517","inReplyTo":"1337169731-23416-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 3/4] pack-objects: refactor write_object()","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2012-05-19T02:43:59Z","receivedAt":"2012-05-19T02:43:59Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 16 May 2012, Nguyễn Thái Ngọc Duy wrote:\n\n> Move !to_reuse and to_reuse write code out into two separate functions\n> and remove \"goto no_reuse;\" hack\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n\nI like this very much.\n\nAcked-by: Nicolas Pitre <nico@fluxnic.net>\n\n\n> ---\n>  builtin/pack-objects.c |  322 ++++++++++++++++++++++++++----------------------\n>  1 files changed, 172 insertions(+), 150 deletions(-)\n> \n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index b2e0940..ccfcbad 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -200,22 +200,178 @@ static void copy_pack_data(struct sha1file *f,\n>  }\n>  \n>  /* Return 0 if we will bust the pack-size limit */\n> -static unsigned long write_object(struct sha1file *f,\n> -\t\t\t\t  struct object_entry *entry,\n> -\t\t\t\t  off_t write_offset)\n> +static unsigned long write_no_reuse_object(struct sha1file *f, struct object_entry *entry,\n> +\t\t\t\t\t   unsigned long limit, int usable_delta)\n>  {\n> -\tunsigned long size, limit, datalen;\n> -\tvoid *buf;\n> +\tunsigned long size, datalen;\n>  \tunsigned char header[10], dheader[10];\n>  \tunsigned hdrlen;\n>  \tenum object_type type;\n> +\tvoid *buf;\n> +\n> +\tif (!usable_delta) {\n> +\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n> +\t\tif (!buf)\n> +\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n> +\t\t/*\n> +\t\t * make sure no cached delta data remains from a\n> +\t\t * previous attempt before a pack split occurred.\n> +\t\t */\n> +\t\tfree(entry->delta_data);\n> +\t\tentry->delta_data = NULL;\n> +\t\tentry->z_delta_size = 0;\n> +\t} else if (entry->delta_data) {\n> +\t\tsize = entry->delta_size;\n> +\t\tbuf = entry->delta_data;\n> +\t\tentry->delta_data = NULL;\n> +\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n> +\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n> +\t} else {\n> +\t\tbuf = get_delta(entry);\n> +\t\tsize = entry->delta_size;\n> +\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n> +\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n> +\t}\n> +\n> +\tif (entry->z_delta_size)\n> +\t\tdatalen = entry->z_delta_size;\n> +\telse\n> +\t\tdatalen = do_compress(&buf, size);\n> +\n> +\t/*\n> +\t * The object header is a byte of 'type' followed by zero or\n> +\t * more bytes of length.\n> +\t */\n> +\thdrlen = encode_in_pack_object_header(type, size, header);\n> +\n> +\tif (type == OBJ_OFS_DELTA) {\n> +\t\t/*\n> +\t\t * Deltas with relative base contain an additional\n> +\t\t * encoding of the relative offset for the delta\n> +\t\t * base from this object's position in the pack.\n> +\t\t */\n> +\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n> +\t\tunsigned pos = sizeof(dheader) - 1;\n> +\t\tdheader[pos] = ofs & 127;\n> +\t\twhile (ofs >>= 7)\n> +\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n> +\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n> +\t\t\tfree(buf);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n> +\t\thdrlen += sizeof(dheader) - pos;\n> +\t} else if (type == OBJ_REF_DELTA) {\n> +\t\t/*\n> +\t\t * Deltas with a base reference contain\n> +\t\t * an additional 20 bytes for the base sha1.\n> +\t\t */\n> +\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n> +\t\t\tfree(buf);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t\tsha1write(f, entry->delta->idx.sha1, 20);\n> +\t\thdrlen += 20;\n> +\t} else {\n> +\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n> +\t\t\tfree(buf);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t}\n> +\tsha1write(f, buf, datalen);\n> +\tfree(buf);\n> +\n> +\treturn hdrlen + datalen;\n> +}\n> +\n> +/* Return 0 if we will bust the pack-size limit */\n> +static unsigned long write_reuse_object(struct sha1file *f, struct object_entry *entry,\n> +\t\t\t\t\tunsigned long limit, int usable_delta)\n> +{\n> +\tstruct packed_git *p = entry->in_pack;\n> +\tstruct pack_window *w_curs = NULL;\n> +\tstruct revindex_entry *revidx;\n> +\toff_t offset;\n> +\tenum object_type type = entry->type;\n> +\tunsigned long datalen;\n> +\tunsigned char header[10], dheader[10];\n> +\tunsigned hdrlen;\n> +\n> +\tif (entry->delta)\n> +\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n> +\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n> +\thdrlen = encode_in_pack_object_header(type, entry->size, header);\n> +\n> +\toffset = entry->in_pack_offset;\n> +\trevidx = find_pack_revindex(p, offset);\n> +\tdatalen = revidx[1].offset - offset;\n> +\tif (!pack_to_stdout && p->index_version > 1 &&\n> +\t    check_pack_crc(p, &w_curs, offset, datalen, revidx->nr)) {\n> +\t\terror(\"bad packed object CRC for %s\", sha1_to_hex(entry->idx.sha1));\n> +\t\tunuse_pack(&w_curs);\n> +\t\treturn write_no_reuse_object(f, entry, limit, usable_delta);\n> +\t}\n> +\n> +\toffset += entry->in_pack_header_size;\n> +\tdatalen -= entry->in_pack_header_size;\n> +\n> +\tif (!pack_to_stdout && p->index_version == 1 &&\n> +\t    check_pack_inflate(p, &w_curs, offset, datalen, entry->size)) {\n> +\t\terror(\"corrupt packed object for %s\", sha1_to_hex(entry->idx.sha1));\n> +\t\tunuse_pack(&w_curs);\n> +\t\treturn write_no_reuse_object(f, entry, limit, usable_delta);\n> +\t}\n> +\n> +\tif (type == OBJ_OFS_DELTA) {\n> +\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n> +\t\tunsigned pos = sizeof(dheader) - 1;\n> +\t\tdheader[pos] = ofs & 127;\n> +\t\twhile (ofs >>= 7)\n> +\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n> +\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n> +\t\t\tunuse_pack(&w_curs);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n> +\t\thdrlen += sizeof(dheader) - pos;\n> +\t\treused_delta++;\n> +\t} else if (type == OBJ_REF_DELTA) {\n> +\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n> +\t\t\tunuse_pack(&w_curs);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t\tsha1write(f, entry->delta->idx.sha1, 20);\n> +\t\thdrlen += 20;\n> +\t\treused_delta++;\n> +\t} else {\n> +\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n> +\t\t\tunuse_pack(&w_curs);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tsha1write(f, header, hdrlen);\n> +\t}\n> +\tcopy_pack_data(f, p, &w_curs, offset, datalen);\n> +\tunuse_pack(&w_curs);\n> +\treused++;\n> +\treturn hdrlen + datalen;\n> +}\n> +\n> +/* Return 0 if we will bust the pack-size limit */\n> +static unsigned long write_object(struct sha1file *f,\n> +\t\t\t\t  struct object_entry *entry,\n> +\t\t\t\t  off_t write_offset)\n> +{\n> +\tunsigned long limit, len;\n>  \tint usable_delta, to_reuse;\n>  \n>  \tif (!pack_to_stdout)\n>  \t\tcrc32_begin(f);\n>  \n> -\ttype = entry->type;\n> -\n>  \t/* apply size limit if limited packsize and not first object */\n>  \tif (!pack_size_limit || !nr_written)\n>  \t\tlimit = 0;\n> @@ -243,11 +399,11 @@ static unsigned long write_object(struct sha1file *f,\n>  \t\tto_reuse = 0;\t/* explicit */\n>  \telse if (!entry->in_pack)\n>  \t\tto_reuse = 0;\t/* can't reuse what we don't have */\n> -\telse if (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA)\n> +\telse if (entry->type == OBJ_REF_DELTA || entry->type == OBJ_OFS_DELTA)\n>  \t\t\t\t/* check_object() decided it for us ... */\n>  \t\tto_reuse = usable_delta;\n>  \t\t\t\t/* ... but pack split may override that */\n> -\telse if (type != entry->in_pack_type)\n> +\telse if (entry->type != entry->in_pack_type)\n>  \t\tto_reuse = 0;\t/* pack has delta which is unusable */\n>  \telse if (entry->delta)\n>  \t\tto_reuse = 0;\t/* we want to pack afresh */\n> @@ -256,153 +412,19 @@ static unsigned long write_object(struct sha1file *f,\n>  \t\t\t\t * and we do not need to deltify it.\n>  \t\t\t\t */\n>  \n> -\tif (!to_reuse) {\n> -\t\tno_reuse:\n> -\t\tif (!usable_delta) {\n> -\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n> -\t\t\tif (!buf)\n> -\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n> -\t\t\t/*\n> -\t\t\t * make sure no cached delta data remains from a\n> -\t\t\t * previous attempt before a pack split occurred.\n> -\t\t\t */\n> -\t\t\tfree(entry->delta_data);\n> -\t\t\tentry->delta_data = NULL;\n> -\t\t\tentry->z_delta_size = 0;\n> -\t\t} else if (entry->delta_data) {\n> -\t\t\tsize = entry->delta_size;\n> -\t\t\tbuf = entry->delta_data;\n> -\t\t\tentry->delta_data = NULL;\n> -\t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n> -\t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n> -\t\t} else {\n> -\t\t\tbuf = get_delta(entry);\n> -\t\t\tsize = entry->delta_size;\n> -\t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n> -\t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n> -\t\t}\n> -\n> -\t\tif (entry->z_delta_size)\n> -\t\t\tdatalen = entry->z_delta_size;\n> -\t\telse\n> -\t\t\tdatalen = do_compress(&buf, size);\n> -\n> -\t\t/*\n> -\t\t * The object header is a byte of 'type' followed by zero or\n> -\t\t * more bytes of length.\n> -\t\t */\n> -\t\thdrlen = encode_in_pack_object_header(type, size, header);\n> -\n> -\t\tif (type == OBJ_OFS_DELTA) {\n> -\t\t\t/*\n> -\t\t\t * Deltas with relative base contain an additional\n> -\t\t\t * encoding of the relative offset for the delta\n> -\t\t\t * base from this object's position in the pack.\n> -\t\t\t */\n> -\t\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n> -\t\t\tunsigned pos = sizeof(dheader) - 1;\n> -\t\t\tdheader[pos] = ofs & 127;\n> -\t\t\twhile (ofs >>= 7)\n> -\t\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n> -\t\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n> -\t\t\t\tfree(buf);\n> -\t\t\t\treturn 0;\n> -\t\t\t}\n> -\t\t\tsha1write(f, header, hdrlen);\n> -\t\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n> -\t\t\thdrlen += sizeof(dheader) - pos;\n> -\t\t} else if (type == OBJ_REF_DELTA) {\n> -\t\t\t/*\n> -\t\t\t * Deltas with a base reference contain\n> -\t\t\t * an additional 20 bytes for the base sha1.\n> -\t\t\t */\n> -\t\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n> -\t\t\t\tfree(buf);\n> -\t\t\t\treturn 0;\n> -\t\t\t}\n> -\t\t\tsha1write(f, header, hdrlen);\n> -\t\t\tsha1write(f, entry->delta->idx.sha1, 20);\n> -\t\t\thdrlen += 20;\n> -\t\t} else {\n> -\t\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n> -\t\t\t\tfree(buf);\n> -\t\t\t\treturn 0;\n> -\t\t\t}\n> -\t\t\tsha1write(f, header, hdrlen);\n> -\t\t}\n> -\t\tsha1write(f, buf, datalen);\n> -\t\tfree(buf);\n> -\t}\n> -\telse {\n> -\t\tstruct packed_git *p = entry->in_pack;\n> -\t\tstruct pack_window *w_curs = NULL;\n> -\t\tstruct revindex_entry *revidx;\n> -\t\toff_t offset;\n> -\n> -\t\tif (entry->delta)\n> -\t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n> -\t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n> -\t\thdrlen = encode_in_pack_object_header(type, entry->size, header);\n> -\n> -\t\toffset = entry->in_pack_offset;\n> -\t\trevidx = find_pack_revindex(p, offset);\n> -\t\tdatalen = revidx[1].offset - offset;\n> -\t\tif (!pack_to_stdout && p->index_version > 1 &&\n> -\t\t    check_pack_crc(p, &w_curs, offset, datalen, revidx->nr)) {\n> -\t\t\terror(\"bad packed object CRC for %s\", sha1_to_hex(entry->idx.sha1));\n> -\t\t\tunuse_pack(&w_curs);\n> -\t\t\tgoto no_reuse;\n> -\t\t}\n> -\n> -\t\toffset += entry->in_pack_header_size;\n> -\t\tdatalen -= entry->in_pack_header_size;\n> -\t\tif (!pack_to_stdout && p->index_version == 1 &&\n> -\t\t    check_pack_inflate(p, &w_curs, offset, datalen, entry->size)) {\n> -\t\t\terror(\"corrupt packed object for %s\", sha1_to_hex(entry->idx.sha1));\n> -\t\t\tunuse_pack(&w_curs);\n> -\t\t\tgoto no_reuse;\n> -\t\t}\n> +\tif (!to_reuse)\n> +\t\tlen = write_no_reuse_object(f, entry, limit, usable_delta);\n> +\telse\n> +\t\tlen = write_reuse_object(f, entry, limit, usable_delta);\n> +\tif (!len)\n> +\t\treturn 0;\n>  \n> -\t\tif (type == OBJ_OFS_DELTA) {\n> -\t\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n> -\t\t\tunsigned pos = sizeof(dheader) - 1;\n> -\t\t\tdheader[pos] = ofs & 127;\n> -\t\t\twhile (ofs >>= 7)\n> -\t\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n> -\t\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n> -\t\t\t\tunuse_pack(&w_curs);\n> -\t\t\t\treturn 0;\n> -\t\t\t}\n> -\t\t\tsha1write(f, header, hdrlen);\n> -\t\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n> -\t\t\thdrlen += sizeof(dheader) - pos;\n> -\t\t\treused_delta++;\n> -\t\t} else if (type == OBJ_REF_DELTA) {\n> -\t\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n> -\t\t\t\tunuse_pack(&w_curs);\n> -\t\t\t\treturn 0;\n> -\t\t\t}\n> -\t\t\tsha1write(f, header, hdrlen);\n> -\t\t\tsha1write(f, entry->delta->idx.sha1, 20);\n> -\t\t\thdrlen += 20;\n> -\t\t\treused_delta++;\n> -\t\t} else {\n> -\t\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n> -\t\t\t\tunuse_pack(&w_curs);\n> -\t\t\t\treturn 0;\n> -\t\t\t}\n> -\t\t\tsha1write(f, header, hdrlen);\n> -\t\t}\n> -\t\tcopy_pack_data(f, p, &w_curs, offset, datalen);\n> -\t\tunuse_pack(&w_curs);\n> -\t\treused++;\n> -\t}\n>  \tif (usable_delta)\n>  \t\twritten_delta++;\n>  \twritten++;\n>  \tif (!pack_to_stdout)\n>  \t\tentry->idx.crc32 = crc32_end(f);\n> -\treturn hdrlen + datalen;\n> +\treturn len;\n>  }\n>  \n>  enum write_one_status {\n> -- \n> 1.7.8.36.g69ee2\n> \n"}]}