{"thread":{"id":"30361","subject":"[PATCH 0/5] Large file support for git-archive","startedAt":"2012-04-30T04:57:12Z","lastAt":"2012-05-01T10:19:01Z","messageCount":23,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","René Scharfe","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"190273","messageId":"1335761837-12482-1-git-send-email-pclouds@gmail.com","threadId":"30361","inReplyTo":null,"subject":"[PATCH 0/5] Large file support for git-archive","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-30T04:57:12Z","receivedAt":"2012-04-30T04:57:12Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is a spin-off from the large file support series I posted some\ntime ago [1]. Both tar and zip format now support streaming large\nblobs. Although zip will store uncompressed large blobs.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/191605\n\nNguyễn Thái Ngọc Duy (5):\n archive-tar: turn write_tar_entry into blob-writing only\n archive-tar: unindent write_tar_entry by one level\n archive: delegate blob reading to backend\n archive-tar: stream large blobs to tar file\n archive-zip: stream large blobs into zip file\n\n Documentation/git-archive.txt | 3 +\n archive-tar.c | 184 ++++++++++++++++++++++++++++-------------\n archive-zip.c | 56 ++++++++++++-\n archive.c | 28 +++----\n archive.h | 10 ++-\n t/t1050-large.sh | 8 ++\n 6 files changed, 209 insertions(+), 80 deletions(-)\n\n-- \n1.7.8.36.g69ee2\n"},{"id":"190274","messageId":"1335761837-12482-2-git-send-email-pclouds@gmail.com","threadId":"30361","inReplyTo":"1335761837-12482-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/5] archive-tar: turn write_tar_entry into blob-writing only","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-30T04:57:13Z","receivedAt":"2012-04-30T04:57:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Before this patch write_tar_entry() can:\n\n - write global header\n by write_global_extended_header() calling write_tar_entry with\n with both sha1 and path == NULL\n\n - write extended header for symlinks, by write_tar_entry() calling\n itself with sha1 != NULL and path == NULL\n\n - write a normal blob. In this case both sha1 and path are valid.\n\nAfter this patch, the first two call sites are modified to write the\nheader without calling write_tar_entry(). The function is now for\nwriting blobs only. This simplifies handling when write_tar_entry()\nlearns about large blobs.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n archive-tar.c | 78 ++++++++++++++++++++++++++++++++++++++-------------------\n 1 files changed, 52 insertions(+), 26 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 20af005..1727ab9 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -123,6 +123,43 @@ static size_t get_path_prefix(const char *path, size_t pathlen, size_t maxlen)\n \treturn i;\n }\n \n+static void prepare_header(struct archiver_args *args,\n+\t\t\t struct ustar_header *header,\n+\t\t\t unsigned int mode, unsigned long size)\n+{\n+\tsprintf(header->mode, \"%07o\", mode & 07777);\n+\tsprintf(header->size, \"%011lo\", S_ISREG(mode) ? size : 0);\n+\tsprintf(header->mtime, \"%011lo\", (unsigned long) args->time);\n+\n+\tsprintf(header->uid, \"%07o\", 0);\n+\tsprintf(header->gid, \"%07o\", 0);\n+\tstrlcpy(header->uname, \"root\", sizeof(header->uname));\n+\tstrlcpy(header->gname, \"root\", sizeof(header->gname));\n+\tsprintf(header->devmajor, \"%07o\", 0);\n+\tsprintf(header->devminor, \"%07o\", 0);\n+\n+\tmemcpy(header->magic, \"ustar\", 6);\n+\tmemcpy(header->version, \"00\", 2);\n+\n+\tsprintf(header->chksum, \"%07o\", ustar_header_chksum(header));\n+}\n+\n+static int write_extended_header(struct archiver_args *args,\n+\t\t\t\t const unsigned char *sha1,\n+\t\t\t\t const void *buffer, unsigned long size)\n+{\n+\tstruct ustar_header header;\n+\tunsigned int mode;\n+\tmemset(&header, 0, sizeof(header));\n+\t*header.typeflag = TYPEFLAG_EXT_HEADER;\n+\tmode = 0100666;\n+\tsprintf(header.name, \"%s.paxheader\", sha1_to_hex(sha1));\n+\tprepare_header(args, &header, mode, size);\n+\twrite_blocked(&header, sizeof(header));\n+\twrite_blocked(buffer, size);\n+\treturn 0;\n+}\n+\n static int write_tar_entry(struct archiver_args *args,\n \t\tconst unsigned char *sha1, const char *path, size_t pathlen,\n \t\tunsigned int mode, void *buffer, unsigned long size)\n@@ -134,13 +171,9 @@ static int write_tar_entry(struct archiver_args *args,\n \tmemset(&header, 0, sizeof(header));\n \n \tif (!sha1) {\n-\t\t*header.typeflag = TYPEFLAG_GLOBAL_HEADER;\n-\t\tmode = 0100666;\n-\t\tstrcpy(header.name, \"pax_global_header\");\n+\t\tdie(\"BUG: sha1 == NULL is not supported\");\n \t} else if (!path) {\n-\t\t*header.typeflag = TYPEFLAG_EXT_HEADER;\n-\t\tmode = 0100666;\n-\t\tsprintf(header.name, \"%s.paxheader\", sha1_to_hex(sha1));\n+\t\tdie(\"BUG: path == NULL is not supported\");\n \t} else {\n \t\tif (S_ISDIR(mode) || S_ISGITLINK(mode)) {\n \t\t\t*header.typeflag = TYPEFLAG_DIR;\n@@ -182,25 +215,11 @@ static int write_tar_entry(struct archiver_args *args,\n \t\t\tmemcpy(header.linkname, buffer, size);\n \t}\n \n-\tsprintf(header.mode, \"%07o\", mode & 07777);\n-\tsprintf(header.size, \"%011lo\", S_ISREG(mode) ? size : 0);\n-\tsprintf(header.mtime, \"%011lo\", (unsigned long) args->time);\n-\n-\tsprintf(header.uid, \"%07o\", 0);\n-\tsprintf(header.gid, \"%07o\", 0);\n-\tstrlcpy(header.uname, \"root\", sizeof(header.uname));\n-\tstrlcpy(header.gname, \"root\", sizeof(header.gname));\n-\tsprintf(header.devmajor, \"%07o\", 0);\n-\tsprintf(header.devminor, \"%07o\", 0);\n-\n-\tmemcpy(header.magic, \"ustar\", 6);\n-\tmemcpy(header.version, \"00\", 2);\n-\n-\tsprintf(header.chksum, \"%07o\", ustar_header_chksum(&header));\n+\tprepare_header(args, &header, mode, size);\n \n \tif (ext_header.len > 0) {\n-\t\terr = write_tar_entry(args, sha1, NULL, 0, 0, ext_header.buf,\n-\t\t\t\text_header.len);\n+\t\terr = write_extended_header(args, sha1, ext_header.buf,\n+\t\t\t\t\t ext_header.len);\n \t\tif (err)\n \t\t\treturn err;\n \t}\n@@ -215,11 +234,18 @@ static int write_global_extended_header(struct archiver_args *args)\n {\n \tconst unsigned char *sha1 = args->commit_sha1;\n \tstruct strbuf ext_header = STRBUF_INIT;\n-\tint err;\n+\tstruct ustar_header header;\n+\tunsigned int mode;\n+\tint err = 0;\n \n \tstrbuf_append_ext_header(&ext_header, \"comment\", sha1_to_hex(sha1), 40);\n-\terr = write_tar_entry(args, NULL, NULL, 0, 0, ext_header.buf,\n-\t\t\text_header.len);\n+\tmemset(&header, 0, sizeof(header));\n+\t*header.typeflag = TYPEFLAG_GLOBAL_HEADER;\n+\tmode = 0100666;\n+\tstrcpy(header.name, \"pax_global_header\");\n+\tprepare_header(args, &header, mode, ext_header.len);\n+\twrite_blocked(&header, sizeof(header));\n+\twrite_blocked(ext_header.buf, ext_header.len);\n \tstrbuf_release(&ext_header);\n \treturn err;\n }\n-- \n1.7.8.36.g69ee2\n"},{"id":"190275","messageId":"1335761837-12482-3-git-send-email-pclouds@gmail.com","threadId":"30361","inReplyTo":"1335761837-12482-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/5] archive-tar: unindent write_tar_entry by one level","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-30T04:57:14Z","receivedAt":"2012-04-30T04:57:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It's used to be\n\nif (!sha1) {\n ...\n} else if (!path) {\n ...\n} else {\n ...\n}\n\nNow that the first two blocks are no-op. We can remove the if/else\nskeleton and put the else block back by one indent level.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n archive-tar.c | 56 +++++++++++++++++++++++++-------------------------------\n 1 files changed, 25 insertions(+), 31 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 1727ab9..6c8a0bd 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -170,40 +170,34 @@ static int write_tar_entry(struct archiver_args *args,\n \n \tmemset(&header, 0, sizeof(header));\n \n-\tif (!sha1) {\n-\t\tdie(\"BUG: sha1 == NULL is not supported\");\n-\t} else if (!path) {\n-\t\tdie(\"BUG: path == NULL is not supported\");\n+\tif (S_ISDIR(mode) || S_ISGITLINK(mode)) {\n+\t\t*header.typeflag = TYPEFLAG_DIR;\n+\t\tmode = (mode | 0777) & ~tar_umask;\n+\t} else if (S_ISLNK(mode)) {\n+\t\t*header.typeflag = TYPEFLAG_LNK;\n+\t\tmode |= 0777;\n+\t} else if (S_ISREG(mode)) {\n+\t\t*header.typeflag = TYPEFLAG_REG;\n+\t\tmode = (mode | ((mode & 0100) ? 0777 : 0666)) & ~tar_umask;\n \t} else {\n-\t\tif (S_ISDIR(mode) || S_ISGITLINK(mode)) {\n-\t\t\t*header.typeflag = TYPEFLAG_DIR;\n-\t\t\tmode = (mode | 0777) & ~tar_umask;\n-\t\t} else if (S_ISLNK(mode)) {\n-\t\t\t*header.typeflag = TYPEFLAG_LNK;\n-\t\t\tmode |= 0777;\n-\t\t} else if (S_ISREG(mode)) {\n-\t\t\t*header.typeflag = TYPEFLAG_REG;\n-\t\t\tmode = (mode | ((mode & 0100) ? 0777 : 0666)) & ~tar_umask;\n+\t\treturn error(\"unsupported file mode: 0%o (SHA1: %s)\",\n+\t\t\t mode, sha1_to_hex(sha1));\n+\t}\n+\tif (pathlen > sizeof(header.name)) {\n+\t\tsize_t plen = get_path_prefix(path, pathlen,\n+\t\t\t\t\t sizeof(header.prefix));\n+\t\tsize_t rest = pathlen - plen - 1;\n+\t\tif (plen > 0 && rest <= sizeof(header.name)) {\n+\t\t\tmemcpy(header.prefix, path, plen);\n+\t\t\t\tmemcpy(header.name, path + plen + 1, rest);\n \t\t} else {\n-\t\t\treturn error(\"unsupported file mode: 0%o (SHA1: %s)\",\n-\t\t\t\t\tmode, sha1_to_hex(sha1));\n+\t\t\tsprintf(header.name, \"%s.data\",\n+\t\t\t\tsha1_to_hex(sha1));\n+\t\t\tstrbuf_append_ext_header(&ext_header, \"path\",\n+\t\t\t\t\t\t path, pathlen);\n \t\t}\n-\t\tif (pathlen > sizeof(header.name)) {\n-\t\t\tsize_t plen = get_path_prefix(path, pathlen,\n-\t\t\t\t\tsizeof(header.prefix));\n-\t\t\tsize_t rest = pathlen - plen - 1;\n-\t\t\tif (plen > 0 && rest <= sizeof(header.name)) {\n-\t\t\t\tmemcpy(header.prefix, path, plen);\n-\t\t\t\tmemcpy(header.name, path + plen + 1, rest);\n-\t\t\t} else {\n-\t\t\t\tsprintf(header.name, \"%s.data\",\n-\t\t\t\t sha1_to_hex(sha1));\n-\t\t\t\tstrbuf_append_ext_header(&ext_header, \"path\",\n-\t\t\t\t\t\tpath, pathlen);\n-\t\t\t}\n-\t\t} else\n-\t\t\tmemcpy(header.name, path, pathlen);\n-\t}\n+\t} else\n+\t\tmemcpy(header.name, path, pathlen);\n \n \tif (S_ISLNK(mode) && buffer) {\n \t\tif (size > sizeof(header.linkname)) {\n-- \n1.7.8.36.g69ee2\n"},{"id":"190276","messageId":"1335761837-12482-4-git-send-email-pclouds@gmail.com","threadId":"30361","inReplyTo":"1335761837-12482-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/5] archive: delegate blob reading to backend","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-30T04:57:15Z","receivedAt":"2012-04-30T04:57:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"archive-tar.c and archive-zip.c now perform conversion check, with\nhelp of sha1_file_to_archive() from archive.c\n\nThis gives backends more freedom in dealing with (streaming) large\nblobs.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n archive-tar.c | 20 +++++++++++++++++---\n archive-zip.c | 14 ++++++++++++--\n archive.c | 28 +++++++++++-----------------\n archive.h | 10 +++++++++-\n 4 files changed, 49 insertions(+), 23 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 6c8a0bd..61821f4 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -161,11 +161,15 @@ static int write_extended_header(struct archiver_args *args,\n }\n \n static int write_tar_entry(struct archiver_args *args,\n-\t\tconst unsigned char *sha1, const char *path, size_t pathlen,\n-\t\tunsigned int mode, void *buffer, unsigned long size)\n+\t\t\t const unsigned char *sha1,\n+\t\t\t const char *path, size_t pathlen,\n+\t\t\t unsigned int mode)\n {\n \tstruct ustar_header header;\n \tstruct strbuf ext_header = STRBUF_INIT;\n+\tunsigned int old_mode = mode;\n+\tunsigned long size;\n+\tvoid *buffer;\n \tint err = 0;\n \n \tmemset(&header, 0, sizeof(header));\n@@ -199,7 +203,17 @@ static int write_tar_entry(struct archiver_args *args,\n \t} else\n \t\tmemcpy(header.name, path, pathlen);\n \n-\tif (S_ISLNK(mode) && buffer) {\n+\tif (S_ISLNK(mode) || S_ISREG(mode)) {\n+\t\tenum object_type type;\n+\t\tbuffer = sha1_file_to_archive(args, path, sha1, old_mode, &type, &size);\n+\t\tif (!buffer)\n+\t\t\treturn error(\"cannot read %s\", sha1_to_hex(sha1));\n+\t} else {\n+\t\tbuffer = NULL;\n+\t\tsize = 0;\n+\t}\n+\n+\tif (S_ISLNK(mode)) {\n \t\tif (size > sizeof(header.linkname)) {\n \t\t\tsprintf(header.linkname, \"see %s.paxheader\",\n \t\t\t sha1_to_hex(sha1));\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 02d1f37..f8039ba 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -121,8 +121,9 @@ static void *zlib_deflate(void *data, unsigned long size,\n }\n \n static int write_zip_entry(struct archiver_args *args,\n-\t\tconst unsigned char *sha1, const char *path, size_t pathlen,\n-\t\tunsigned int mode, void *buffer, unsigned long size)\n+\t\t\t const unsigned char *sha1,\n+\t\t\t const char *path, size_t pathlen,\n+\t\t\t unsigned int mode)\n {\n \tstruct zip_local_header header;\n \tstruct zip_dir_header dirent;\n@@ -134,6 +135,8 @@ static int write_zip_entry(struct archiver_args *args,\n \tint method;\n \tunsigned char *out;\n \tvoid *deflated = NULL;\n+\tvoid *buffer;\n+\tunsigned long size;\n \n \tcrc = crc32(0, NULL, 0);\n \n@@ -148,7 +151,14 @@ static int write_zip_entry(struct archiver_args *args,\n \t\tout = NULL;\n \t\tuncompressed_size = 0;\n \t\tcompressed_size = 0;\n+\t\tbuffer = NULL;\n+\t\tsize = 0;\n \t} else if (S_ISREG(mode) || S_ISLNK(mode)) {\n+\t\tenum object_type type;\n+\t\tbuffer = sha1_file_to_archive(args, path, sha1, mode, &type, &size);\n+\t\tif (!buffer)\n+\t\t\treturn error(\"cannot read %s\", sha1_to_hex(sha1));\n+\n \t\tmethod = 0;\n \t\tattr2 = S_ISLNK(mode) ? ((mode | 0777) << 16) :\n \t\t\t(mode & 0111) ? ((mode) << 16) : 0;\ndiff --git a/archive.c b/archive.c\nindex 1ee837d..cd083ea 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -59,12 +59,15 @@ static void format_subst(const struct commit *commit,\n \tfree(to_free);\n }\n \n-static void *sha1_file_to_archive(const char *path, const unsigned char *sha1,\n-\t\tunsigned int mode, enum object_type *type,\n-\t\tunsigned long *sizep, const struct commit *commit)\n+void *sha1_file_to_archive(const struct archiver_args *args,\n+\t\t\t const char *path, const unsigned char *sha1,\n+\t\t\t unsigned int mode, enum object_type *type,\n+\t\t\t unsigned long *sizep)\n {\n \tvoid *buffer;\n+\tconst struct commit *commit = args->convert ? args->commit : NULL;\n \n+\tpath += args->baselen;\n \tbuffer = read_sha1_file(sha1, type, sizep);\n \tif (buffer && S_ISREG(mode)) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n@@ -109,12 +112,9 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \twrite_archive_entry_fn_t write_entry = c->write_entry;\n \tstruct git_attr_check check[2];\n \tconst char *path_without_prefix;\n-\tint convert = 0;\n \tint err;\n-\tenum object_type type;\n-\tunsigned long size;\n-\tvoid *buffer;\n \n+\targs->convert = 0;\n \tstrbuf_reset(&path);\n \tstrbuf_grow(&path, PATH_MAX);\n \tstrbuf_add(&path, args->base, args->baselen);\n@@ -126,28 +126,22 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \tif (!git_check_attr(path_without_prefix, ARRAY_SIZE(check), check)) {\n \t\tif (ATTR_TRUE(check[0].value))\n \t\t\treturn 0;\n-\t\tconvert = ATTR_TRUE(check[1].value);\n+\t\targs->convert = ATTR_TRUE(check[1].value);\n \t}\n \n \tif (S_ISDIR(mode) || S_ISGITLINK(mode)) {\n \t\tstrbuf_addch(&path, '/');\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"%.*s\\n\", (int)path.len, path.buf);\n-\t\terr = write_entry(args, sha1, path.buf, path.len, mode, NULL, 0);\n+\t\terr = write_entry(args, sha1, path.buf, path.len, mode);\n \t\tif (err)\n \t\t\treturn err;\n \t\treturn (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);\n \t}\n \n-\tbuffer = sha1_file_to_archive(path_without_prefix, sha1, mode,\n-\t\t\t&type, &size, convert ? args->commit : NULL);\n-\tif (!buffer)\n-\t\treturn error(\"cannot read %s\", sha1_to_hex(sha1));\n \tif (args->verbose)\n \t\tfprintf(stderr, \"%.*s\\n\", (int)path.len, path.buf);\n-\terr = write_entry(args, sha1, path.buf, path.len, mode, buffer, size);\n-\tfree(buffer);\n-\treturn err;\n+\treturn write_entry(args, sha1, path.buf, path.len, mode);\n }\n \n int write_archive_entries(struct archiver_args *args,\n@@ -167,7 +161,7 @@ int write_archive_entries(struct archiver_args *args,\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"%.*s\\n\", (int)len, args->base);\n \t\terr = write_entry(args, args->tree->object.sha1, args->base,\n-\t\t\t\tlen, 040777, NULL, 0);\n+\t\t\t\t len, 040777);\n \t\tif (err)\n \t\t\treturn err;\n \t}\ndiff --git a/archive.h b/archive.h\nindex 2b0884f..895afcd 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -11,6 +11,7 @@ struct archiver_args {\n \tconst char **pathspec;\n \tunsigned int verbose : 1;\n \tunsigned int worktree_attributes : 1;\n+\tunsigned int convert : 1;\n \tint compression_level;\n };\n \n@@ -27,11 +28,18 @@ extern void register_archiver(struct archiver *);\n extern void init_tar_archiver(void);\n extern void init_zip_archiver(void);\n \n-typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, const char *path, size_t pathlen, unsigned int mode, void *buffer, unsigned long size);\n+typedef int (*write_archive_entry_fn_t)(struct archiver_args *args,\n+\t\t\t\t\tconst unsigned char *sha1,\n+\t\t\t\t\tconst char *path, size_t pathlen,\n+\t\t\t\t\tunsigned int mode);\n \n extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix, const char *name_hint, int remote);\n \n const char *archive_format_from_filename(const char *filename);\n+extern void *sha1_file_to_archive(const struct archiver_args *args,\n+\t\t\t\t const char *path, const unsigned char *sha1,\n+\t\t\t\t unsigned int mode, enum object_type *type,\n+\t\t\t\t unsigned long *sizep);\n \n #endif\t/* ARCHIVE_H */\n-- \n1.7.8.36.g69ee2\n"},{"id":"190277","messageId":"1335761837-12482-5-git-send-email-pclouds@gmail.com","threadId":"30361","inReplyTo":"1335761837-12482-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 4/5] archive-tar: stream large blobs to tar file","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-30T04:57:16Z","receivedAt":"2012-04-30T04:57:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n archive-tar.c | 38 +++++++++++++++++++++++++++++++++++---\n t/t1050-large.sh | 4 ++++\n 2 files changed, 39 insertions(+), 3 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 61821f4..865ef6d 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -4,6 +4,7 @@\n #include \"cache.h\"\n #include \"tar.h\"\n #include \"archive.h\"\n+#include \"streaming.h\"\n #include \"run-command.h\"\n \n #define RECORDSIZE\t(512)\n@@ -62,6 +63,29 @@ static void write_blocked(const void *data, unsigned long size)\n \twrite_if_needed();\n }\n \n+static int stream_blob_to_file(const unsigned char *sha1)\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\treturn error(\"cannot stream blob %s\", sha1_to_hex(sha1));\n+\tfor (;;) {\n+\t\tchar buf[BLOCKSIZE];\n+\t\tssize_t readlen;\n+\n+\t\treadlen = read_istream(st, buf, sizeof(buf));\n+\n+\t\tif (readlen <= 0)\n+\t\t\treturn readlen;\n+\t\twrite_blocked(buf, readlen);\n+\t}\n+\tclose_istream(st);\n+\treturn 0;\n+}\n+\n /*\n * The end of tar archives is marked by 2*512 nul bytes and after that\n * follows the rest of the block (if any).\n@@ -203,7 +227,11 @@ static int write_tar_entry(struct archiver_args *args,\n \t} else\n \t\tmemcpy(header.name, path, pathlen);\n \n-\tif (S_ISLNK(mode) || S_ISREG(mode)) {\n+\tif (S_ISREG(mode) && !args->convert &&\n+\t sha1_object_info(sha1, &size) == OBJ_BLOB &&\n+\t size > big_file_threshold)\n+\t\tbuffer = NULL;\n+\telse if (S_ISLNK(mode) || S_ISREG(mode)) {\n \t\tenum object_type type;\n \t\tbuffer = sha1_file_to_archive(args, path, sha1, old_mode, &type, &size);\n \t\tif (!buffer)\n@@ -233,8 +261,12 @@ static int write_tar_entry(struct archiver_args *args,\n \t}\n \tstrbuf_release(&ext_header);\n \twrite_blocked(&header, sizeof(header));\n-\tif (S_ISREG(mode) && buffer && size > 0)\n-\t\twrite_blocked(buffer, size);\n+\tif (S_ISREG(mode) && size > 0) {\n+\t\tif (buffer)\n+\t\t\twrite_blocked(buffer, size);\n+\t\telse\n+\t\t\terr = stream_blob_to_file(sha1);\n+\t}\n \treturn err;\n }\n \ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 4d127f1..fe47554 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -134,4 +134,8 @@ test_expect_success 'repack' '\n \tgit repack -ad\n '\n \n+test_expect_success 'tar achiving' '\n+\tgit archive --format=tar HEAD >/dev/null\n+'\n+\n test_done\n-- \n1.7.8.36.g69ee2\n"},{"id":"190278","messageId":"1335761837-12482-6-git-send-email-pclouds@gmail.com","threadId":"30361","inReplyTo":"1335761837-12482-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 5/5] archive-zip: stream large blobs into zip file","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-30T04:57:17Z","receivedAt":"2012-04-30T04:57:17Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"A large blob will be read twice. One for calculating crc32, one for\nactual writing. Large blobs are written uncompressed for simplicity.\n\nWriting compressed large blobs is possible. But a naive implementation\nwould need to decompress/compress the blob twice: one to calculate\ncompressed size, one for actual writing, assuming compressed blobs are\nstill over large file limit.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n I think we could extract compressed size from pack index, then stream\n the compressed blob directly from pack to zip file. But that makes\n git-archive sensitive to pack format. And to be honest I don't care\n that much about large file support to do it. This patch is good\n enough for me.\n\n Documentation/git-archive.txt | 3 ++\n archive-zip.c | 42 ++++++++++++++++++++++++++++++++++++++++-\n t/t1050-large.sh | 4 +++\n 3 files changed, 48 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex ac7006e..6df85a6 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -120,6 +120,9 @@ tar..remote::\n \tuser-defined formats, but true for the \"tar.gz\" and \"tgz\"\n \tformats.\n \n+core.bigFileThreshold::\n+\tFiles larger than this size are stored uncompressed in zip format.\n+\n ATTRIBUTES\n ----------\n \ndiff --git a/archive-zip.c b/archive-zip.c\nindex f8039ba..ee58bda 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -3,6 +3,7 @@\n */\n #include \"cache.h\"\n #include \"archive.h\"\n+#include \"streaming.h\"\n \n static int zip_date;\n static int zip_time;\n@@ -120,6 +121,29 @@ static void *zlib_deflate(void *data, unsigned long size,\n \treturn buffer;\n }\n \n+static int crc32_stream(const unsigned char *sha1, unsigned long *crc)\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\treturn error(\"cannot stream blob %s\", sha1_to_hex(sha1));\n+\tfor (;;) {\n+\t\tchar buf[1024];\n+\t\tssize_t readlen;\n+\n+\t\treadlen = read_istream(st, buf, sizeof(buf));\n+\n+\t\tif (readlen <= 0)\n+\t\t\treturn readlen;\n+\t\t*crc = crc32(*crc, (unsigned char*)buf, readlen);\n+\t}\n+\tclose_istream(st);\n+\treturn 0;\n+}\n+\n static int write_zip_entry(struct archiver_args *args,\n \t\t\t const unsigned char *sha1,\n \t\t\t const char *path, size_t pathlen,\n@@ -153,6 +177,19 @@ static int write_zip_entry(struct archiver_args *args,\n \t\tcompressed_size = 0;\n \t\tbuffer = NULL;\n \t\tsize = 0;\n+\t} else if (!args->convert && S_ISREG(mode) &&\n+\t\t sha1_object_info(sha1, &size) == OBJ_BLOB &&\n+\t\t size > big_file_threshold) {\n+\t\tbuffer = NULL;\n+\t\tmethod = 0;\n+\t\tattr2 = S_ISLNK(mode) ? ((mode | 0777) << 16) :\n+\t\t\t(mode & 0111) ? ((mode) << 16) : 0;\n+\t\tif (crc32_stream(sha1, &crc) < 0)\n+\t\t\treturn error(\"failed to calculate crc32 from blob %s, SHA1 %s\",\n+\t\t\t\t path, sha1_to_hex(sha1));\n+\t\tout = buffer;\n+\t\tuncompressed_size = size;\n+\t\tcompressed_size = size;\n \t} else if (S_ISREG(mode) || S_ISLNK(mode)) {\n \t\tenum object_type type;\n \t\tbuffer = sha1_file_to_archive(args, path, sha1, mode, &type, &size);\n@@ -234,7 +271,10 @@ static int write_zip_entry(struct archiver_args *args,\n \twrite_or_die(1, path, pathlen);\n \tzip_offset += pathlen;\n \tif (compressed_size > 0) {\n-\t\twrite_or_die(1, out, compressed_size);\n+\t\tif (out)\n+\t\t\twrite_or_die(1, out, compressed_size);\n+\t\telse\n+\t\t\tstream_blob_to_fd(1, sha1, NULL, 0);\n \t\tzip_offset += compressed_size;\n \t}\n \ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex fe47554..458fdde 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -138,4 +138,8 @@ test_expect_success 'tar achiving' '\n \tgit archive --format=tar HEAD >/dev/null\n '\n \n+test_expect_success 'zip achiving' '\n+\tgit archive --format=zip HEAD >/dev/null\n+'\n+\n test_done\n-- \n1.7.8.36.g69ee2\n"},{"id":"190325","messageId":"7vr4v5f5n2.fsf@alter.siamese.dyndns.org","threadId":"30361","inReplyTo":"1335761837-12482-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/5] archive-tar: turn write_tar_entry into blob-writing only","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-30T18:15:13Z","receivedAt":"2012-04-30T18:15:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Before this patch write_tar_entry() can:\n>\n> - write global header\n> by write_global_extended_header() calling write_tar_entry with\n> with both sha1 and path == NULL\n>\n> - write extended header for symlinks, by write_tar_entry() calling\n> itself with sha1 != NULL and path == NULL\n>\n> - write a normal blob. In this case both sha1 and path are valid.\n>\n> After this patch, the first two call sites are modified to write the\n> header without calling write_tar_entry(). The function is now for\n> writing blobs only.\n\nNice.\n\nI am kind of surprised how hacky the original code that switched on !sha1\nand !path was, especially given that it came from René at ae64bbc\n(tar-tree: Introduce write_entry(), 2006-03-25) --- it even claims that\nthese are \"reasonable\" magic values ;-).\n"},{"id":"190330","messageId":"7vmx5tf3h6.fsf@alter.siamese.dyndns.org","threadId":"30361","inReplyTo":"1335761837-12482-5-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 4/5] archive-tar: stream large blobs to tar file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-30T19:01:57Z","receivedAt":"2012-04-30T19:01:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> archive-tar.c | 38 +++++++++++++++++++++++++++++++++++---\n> t/t1050-large.sh | 4 ++++\n> 2 files changed, 39 insertions(+), 3 deletions(-)\n>\n> diff --git a/archive-tar.c b/archive-tar.c\n> index 61821f4..865ef6d 100644\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -4,6 +4,7 @@\n> #include \"cache.h\"\n> #include \"tar.h\"\n> #include \"archive.h\"\n> +#include \"streaming.h\"\n> #include \"run-command.h\"\n> \n> #define RECORDSIZE\t(512)\n> @@ -62,6 +63,29 @@ static void write_blocked(const void *data, unsigned long size)\n> \twrite_if_needed();\n> }\n> \n> +static int stream_blob_to_file(const unsigned char *sha1)\n\nThe name of this function is misleading and caused me to look for an\nexisting implementation that streams blob contents to a given fd while\nwondering how that fd is passed to this function.\n\nI do not think it would make sense to reuse stream_blob_to_fd() here, as\nis it too much hassle to account for blocking factors. In order to make\nit clear for future futzers that this is not something they can turn into\nglobal function and reuse outside the context of generating tar archive\noutput, please rename it either to stream_blob_to_tarfile(), which is more\nspecific, or stream_blocked(), which is too vague and nobody sane would\nthink about exporting from the file just like existing write_blocked() is.\n"},{"id":"190333","messageId":"7vipghf2z0.fsf@alter.siamese.dyndns.org","threadId":"30361","inReplyTo":"1335761837-12482-6-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 5/5] archive-zip: stream large blobs into zip file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-30T19:12:51Z","receivedAt":"2012-04-30T19:12:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> A large blob will be read twice. One for calculating crc32, one for\n> actual writing.\n\nIs that because you need the checksum before the payload? That is\nunfortunate. It would be nice (read: not a rejection of this patch---it\nis a good first step to do it stupid but correct way before trying to\noptimize it) to avoid it when the output is seekable, especially because\nwe are talking about a *large* payload.\n\n> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n> index fe47554..458fdde 100755\n> --- a/t/t1050-large.sh\n> +++ b/t/t1050-large.sh\n> @@ -138,4 +138,8 @@ test_expect_success 'tar achiving' '\n> \tgit archive --format=tar HEAD >/dev/null\n> '\n> \n> +test_expect_success 'zip achiving' '\n> +\tgit archive --format=zip HEAD >/dev/null\n> +'\n\nCan't we do better than \"we only check if it finishes without barfing; we\ncannot afford to check the correctness of the output\"? The same comment\napplies to all the tests you added to this file in the past 3 months.\n"},{"id":"190334","messageId":"7vehr5f2uo.fsf@alter.siamese.dyndns.org","threadId":"30361","inReplyTo":"1335761837-12482-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 0/5] Large file support for git-archive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-30T19:15:27Z","receivedAt":"2012-04-30T19:15:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Overall this was a very pleasant read. Thanks. Will queue.\n"},{"id":"190345","messageId":"4F9EFF22.4060105@lsrfire.ath.cx","threadId":"30361","inReplyTo":"1335761837-12482-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 3/5] archive: delegate blob reading to backend","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T21:07:46Z","receivedAt":"2012-04-30T21:07:46Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 30.04.2012 06:57, schrieb Nguyễn Thái Ngọc Duy:\n> archive-tar.c and archive-zip.c now perform conversion check, with\n> help of sha1_file_to_archive() from archive.c\n> \n> This gives backends more freedom in dealing with (streaming) large\n> blobs.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy\n> ---\n> archive-tar.c | 20 +++++++++++++++++---\n> archive-zip.c | 14 ++++++++++++--\n> archive.c | 28 +++++++++++-----------------\n> archive.h | 10 +++++++++-\n> 4 files changed, 49 insertions(+), 23 deletions(-)\n> \n> diff --git a/archive-tar.c b/archive-tar.c\n> index 6c8a0bd..61821f4 100644\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -161,11 +161,15 @@ static int write_extended_header(struct archiver_args *args,\n> }\n> \n> static int write_tar_entry(struct archiver_args *args,\n> -\t\tconst unsigned char *sha1, const char *path, size_t pathlen,\n> -\t\tunsigned int mode, void *buffer, unsigned long size)\n> +\t\t\t const unsigned char *sha1,\n> +\t\t\t const char *path, size_t pathlen,\n> +\t\t\t unsigned int mode)\n> {\n> \tstruct ustar_header header;\n> \tstruct strbuf ext_header = STRBUF_INIT;\n> +\tunsigned int old_mode = mode;\n> +\tunsigned long size;\n> +\tvoid *buffer;\n> \tint err = 0;\n> \n> \tmemset(&header, 0, sizeof(header));\n> @@ -199,7 +203,17 @@ static int write_tar_entry(struct archiver_args *args,\n> \t} else\n> \t\tmemcpy(header.name, path, pathlen);\n> \n> -\tif (S_ISLNK(mode)&& buffer) {\n> +\tif (S_ISLNK(mode) || S_ISREG(mode)) {\n> +\t\tenum object_type type;\n> +\t\tbuffer = sha1_file_to_archive(args, path, sha1, old_mode,&type,&size);\n\nThis buffer is never freed.\n\n> +\t\tif (!buffer)\n> +\t\t\treturn error(\"cannot read %s\", sha1_to_hex(sha1));\n> +\t} else {\n> +\t\tbuffer = NULL;\n> +\t\tsize = 0;\n> +\t}\n> +\n> +\tif (S_ISLNK(mode)) {\n> \t\tif (size> sizeof(header.linkname)) {\n> \t\t\tsprintf(header.linkname, \"see %s.paxheader\",\n> \t\t\t sha1_to_hex(sha1));\n> diff --git a/archive-zip.c b/archive-zip.c\n> index 02d1f37..f8039ba 100644\n> --- a/archive-zip.c\n> +++ b/archive-zip.c\n> @@ -121,8 +121,9 @@ static void *zlib_deflate(void *data, unsigned long size,\n> }\n> \n> static int write_zip_entry(struct archiver_args *args,\n> -\t\tconst unsigned char *sha1, const char *path, size_t pathlen,\n> -\t\tunsigned int mode, void *buffer, unsigned long size)\n> +\t\t\t const unsigned char *sha1,\n> +\t\t\t const char *path, size_t pathlen,\n> +\t\t\t unsigned int mode)\n> {\n> \tstruct zip_local_header header;\n> \tstruct zip_dir_header dirent;\n> @@ -134,6 +135,8 @@ static int write_zip_entry(struct archiver_args *args,\n> \tint method;\n> \tunsigned char *out;\n> \tvoid *deflated = NULL;\n> +\tvoid *buffer;\n> +\tunsigned long size;\n> \n> \tcrc = crc32(0, NULL, 0);\n> \n> @@ -148,7 +151,14 @@ static int write_zip_entry(struct archiver_args *args,\n> \t\tout = NULL;\n> \t\tuncompressed_size = 0;\n> \t\tcompressed_size = 0;\n> +\t\tbuffer = NULL;\n> +\t\tsize = 0;\n> \t} else if (S_ISREG(mode) || S_ISLNK(mode)) {\n> +\t\tenum object_type type;\n> +\t\tbuffer = sha1_file_to_archive(args, path, sha1, mode,&type,&size);\n\nThis one is leaked as well.\n\n> +\t\tif (!buffer)\n> +\t\t\treturn error(\"cannot read %s\", sha1_to_hex(sha1));\n> +\n> \t\tmethod = 0;\n> \t\tattr2 = S_ISLNK(mode) ? ((mode | 0777)<< 16) :\n> \t\t\t(mode& 0111) ? ((mode)<< 16) : 0;\n\nYou could squash this in:\n\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 61821f4..bb320ad 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -235,6 +235,7 @@ static int write_tar_entry(struct archiver_args *args,\n \twrite_blocked(&header, sizeof(header));\n \tif (S_ISREG(mode) && buffer && size > 0)\n \t\twrite_blocked(buffer, size);\n+\tfree(buffer);\n \treturn err;\n }\n \ndiff --git a/archive-zip.c b/archive-zip.c\nindex f8039ba..716cc42 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -239,6 +239,7 @@ static int write_zip_entry(struct archiver_args *args,\n \t}\n \n \tfree(deflated);\n+\tfree(buffer);\n \n \treturn 0;\n }\n"},{"id":"190346","messageId":"4F9EFF2D.8080505@lsrfire.ath.cx","threadId":"30361","inReplyTo":"1335761837-12482-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 0/5] Large file support for git-archive","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T21:07:57Z","receivedAt":"2012-04-30T21:07:57Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 30.04.2012 06:57, schrieb Nguyễn Thái Ngọc Duy:\n> This is a spin-off from the large file support series I posted some\n> time ago [1]. Both tar and zip format now support streaming large\n> blobs. Although zip will store uncompressed large blobs.\n>\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/191605\n>\n> Nguyễn Thái Ngọc Duy (5):\n> archive-tar: turn write_tar_entry into blob-writing only\n> archive-tar: unindent write_tar_entry by one level\n> archive: delegate blob reading to backend\n> archive-tar: stream large blobs to tar file\n> archive-zip: stream large blobs into zip file\n>\n> Documentation/git-archive.txt | 3 +\n> archive-tar.c | 184 ++++++++++++++++++++++++++++-------------\n> archive-zip.c | 56 ++++++++++++-\n> archive.c | 28 +++----\n> archive.h | 10 ++-\n> t/t1050-large.sh | 8 ++\n> 6 files changed, 209 insertions(+), 80 deletions(-)\n\nI like the cleanups in the first two patches.\n\nI'm neutral to positive on the third one; it certainly simplifies the \ninterface to the backends with only little code duplication.\n\nThe ZIP format supports streaming natively (look for \"Data descriptor\" \nin http://www.pkware.com/documents/casestudies/APPNOTE.TXT). \nIncidentally, I had been working on supporting that, but without any \npresentable results. So far. I've adapted the pieces I had to your \nseries now; will send shortly.\n\nRené\n"},{"id":"190347","messageId":"4F9EFF39.4010804@lsrfire.ath.cx","threadId":"30361","inReplyTo":"1335761837-12482-5-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 4/5] archive-tar: stream large blobs to tar file","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T21:08:09Z","receivedAt":"2012-04-30T21:08:09Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 30.04.2012 06:57, schrieb Nguyễn Thái Ngọc Duy:\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy\n> ---\n> archive-tar.c | 38 +++++++++++++++++++++++++++++++++++---\n> t/t1050-large.sh | 4 ++++\n> 2 files changed, 39 insertions(+), 3 deletions(-)\n> \n> diff --git a/archive-tar.c b/archive-tar.c\n> index 61821f4..865ef6d 100644\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -4,6 +4,7 @@\n> #include \"cache.h\"\n> #include \"tar.h\"\n> #include \"archive.h\"\n> +#include \"streaming.h\"\n> #include \"run-command.h\"\n> \n> #define RECORDSIZE\t(512)\n> @@ -62,6 +63,29 @@ static void write_blocked(const void *data, unsigned long size)\n> \twrite_if_needed();\n> }\n> \n> +static int stream_blob_to_file(const unsigned char *sha1)\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\treturn error(\"cannot stream blob %s\", sha1_to_hex(sha1));\n> +\tfor (;;) {\n> +\t\tchar buf[BLOCKSIZE];\n> +\t\tssize_t readlen;\n> +\n> +\t\treadlen = read_istream(st, buf, sizeof(buf));\n> +\n> +\t\tif (readlen <= 0)\n> +\t\t\treturn readlen;\n> +\t\twrite_blocked(buf, readlen);\n> +\t}\n> +\tclose_istream(st);\n> +\treturn 0;\n> +}\n\nThe stream is never closed. Perhaps squash this in?\n\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 506c8cb..6109fd3 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -66,6 +66,7 @@ static void write_blocked(const void *data, unsigned long size)\n static int stream_blob_to_file(const unsigned char *sha1)\n {\n \tstruct git_istream *st;\n+\tssize_t readlen;\n \tenum object_type type;\n \tunsigned long sz;\n \n@@ -74,16 +75,15 @@ static int stream_blob_to_file(const unsigned char *sha1)\n \t\treturn error(\"cannot stream blob %s\", sha1_to_hex(sha1));\n \tfor (;;) {\n \t\tchar buf[BLOCKSIZE];\n-\t\tssize_t readlen;\n \n \t\treadlen = read_istream(st, buf, sizeof(buf));\n \n \t\tif (readlen <= 0)\n-\t\t\treturn readlen;\n+\t\t\tbreak;\n \t\twrite_blocked(buf, readlen);\n \t}\n \tclose_istream(st);\n-\treturn 0;\n+\treturn readlen;\n }\n \n /*\n"},{"id":"190349","messageId":"7vhaw0ewb8.fsf@alter.siamese.dyndns.org","threadId":"30361","inReplyTo":"4F9EFF39.4010804@lsrfire.ath.cx","subject":"Re: [PATCH 4/5] archive-tar: stream large blobs to tar file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-30T21:36:43Z","receivedAt":"2012-04-30T21:36:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n>> +static int stream_blob_to_file(const unsigned char *sha1)\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\treturn error(\"cannot stream blob %s\", sha1_to_hex(sha1));\n>> +\tfor (;;) {\n>> +\t\tchar buf[BLOCKSIZE];\n>> +\t\tssize_t readlen;\n>> +\n>> +\t\treadlen = read_istream(st, buf, sizeof(buf));\n>> +\n>> +\t\tif (readlen <= 0)\n>> +\t\t\treturn readlen;\n>> +\t\twrite_blocked(buf, readlen);\n>> +\t}\n>> +\tclose_istream(st);\n>> +\treturn 0;\n>> +}\n>\n> The stream is never closed. Perhaps squash this in?\n>\n> diff --git a/archive-tar.c b/archive-tar.c\n> index 506c8cb..6109fd3 100644\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -66,6 +66,7 @@ static void write_blocked(const void *data, unsigned long size)\n> static int stream_blob_to_file(const unsigned char *sha1)\n> {\n> \tstruct git_istream *st;\n> +\tssize_t readlen;\n> \tenum object_type type;\n> \tunsigned long sz;\n> \n> @@ -74,16 +75,15 @@ static int stream_blob_to_file(const unsigned char *sha1)\n> \t\treturn error(\"cannot stream blob %s\", sha1_to_hex(sha1));\n> \tfor (;;) {\n> \t\tchar buf[BLOCKSIZE];\n> -\t\tssize_t readlen;\n> \n> \t\treadlen = read_istream(st, buf, sizeof(buf));\n> \n> \t\tif (readlen <= 0)\n> -\t\t\treturn readlen;\n> +\t\t\tbreak;\n> \t\twrite_blocked(buf, readlen);\n> \t}\n> \tclose_istream(st);\n> -\treturn 0;\n> +\treturn readlen;\n> }\n\nYour patch on top obviouly is the right thing to do, but reading the code\nagain, I am not sure if the original is correct. read_istream() itself\ndoes not promise that it will always fill the buffer before returning (it\ncould return with a short read). It seems incorrect that the caller does\nnot loop to avoid padding a short read with padding by calling\nwrite_blocked().\n"},{"id":"190352","messageId":"4F9F0E21.3080407@lsrfire.ath.cx","threadId":"30361","inReplyTo":"7vr4v5f5n2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/5] archive-tar: turn write_tar_entry into blob-writing only","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T22:11:45Z","receivedAt":"2012-04-30T22:11:45Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 30.04.2012 20:15, schrieb Junio C Hamano:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> Before this patch write_tar_entry() can:\n>>\n>> - write global header\n>> by write_global_extended_header() calling write_tar_entry with\n>> with both sha1 and path == NULL\n>>\n>> - write extended header for symlinks, by write_tar_entry() calling\n>> itself with sha1 != NULL and path == NULL\n>>\n>> - write a normal blob. In this case both sha1 and path are valid.\n>>\n>> After this patch, the first two call sites are modified to write the\n>> header without calling write_tar_entry(). The function is now for\n>> writing blobs only.\n>\n> Nice.\n>\n> I am kind of surprised how hacky the original code that switched on !sha1\n> and !path was, especially given that it came from René at ae64bbc\n> (tar-tree: Introduce write_entry(), 2006-03-25) --- it even claims that\n> these are \"reasonable\" magic values ;-).\n\nYeah, and there are still opportunities for cleanup. Can't say what I \nwas thinking back then, but a few cleanup attempts since then invariably \nmade the code even more ugly, so I never sent them.\n\nRené\n"},{"id":"190353","messageId":"4F9F0E2A.1060004@lsrfire.ath.cx","threadId":"30361","inReplyTo":"1335761837-12482-6-git-send-email-pclouds@gmail.com","subject":"[PATCH 5a/5] streaming: void pointer instead of char pointer","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T22:11:54Z","receivedAt":"2012-04-30T22:11:54Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Allow any kind of buffer to be fed to read_istream() without an explicit\ncast by making it's buf argument a void pointer. It's about arbitrary\ndata, not only characters.\n\nSigned-off-by: Rene Scharfe \n---\n streaming.c | 2 +-\n streaming.h | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/streaming.c b/streaming.c\nindex 7e7ee2b..3a3cd12 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -99,7 +99,7 @@ int close_istream(struct git_istream *st)\n \treturn r;\n }\n \n-ssize_t read_istream(struct git_istream *st, char *buf, size_t sz)\n+ssize_t read_istream(struct git_istream *st, void *buf, size_t sz)\n {\n \treturn st->vtbl->read(st, buf, sz);\n }\ndiff --git a/streaming.h b/streaming.h\nindex 3e82770..1d05c2a 100644\n--- a/streaming.h\n+++ b/streaming.h\n@@ -10,7 +10,7 @@ struct git_istream;\n \n extern struct git_istream *open_istream(const unsigned char *, enum object_type *, unsigned long *, struct stream_filter *);\n extern int close_istream(struct git_istream *);\n-extern ssize_t read_istream(struct git_istream *, char *, size_t);\n+extern ssize_t read_istream(struct git_istream *, void *, size_t);\n \n extern int stream_blob_to_fd(int fd, const unsigned char *, struct stream_filter *, int can_seek);\n \n-- \n1.7.10\n"},{"id":"190354","messageId":"4F9F0E30.70403@lsrfire.ath.cx","threadId":"30361","inReplyTo":"7vhaw0ewb8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/5] archive-tar: stream large blobs to tar file","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T22:12:00Z","receivedAt":"2012-04-30T22:12:00Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 30.04.2012 23:36, schrieb Junio C Hamano:\n> René Scharfe writes:\n>\n>>> +static int stream_blob_to_file(const unsigned char *sha1)\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\treturn error(\"cannot stream blob %s\", sha1_to_hex(sha1));\n>>> +\tfor (;;) {\n>>> +\t\tchar buf[BLOCKSIZE];\n>>> +\t\tssize_t readlen;\n>>> +\n>>> +\t\treadlen = read_istream(st, buf, sizeof(buf));\n>>> +\n>>> +\t\tif (readlen<= 0)\n>>> +\t\t\treturn readlen;\n>>> +\t\twrite_blocked(buf, readlen);\n>>> +\t}\n>>> +\tclose_istream(st);\n>>> +\treturn 0;\n>>> +}\n>>\n>> The stream is never closed. Perhaps squash this in?\n>>\n>> diff --git a/archive-tar.c b/archive-tar.c\n>> index 506c8cb..6109fd3 100644\n>> --- a/archive-tar.c\n>> +++ b/archive-tar.c\n>> @@ -66,6 +66,7 @@ static void write_blocked(const void *data, unsigned long size)\n>> static int stream_blob_to_file(const unsigned char *sha1)\n>> {\n>> \tstruct git_istream *st;\n>> +\tssize_t readlen;\n>> \tenum object_type type;\n>> \tunsigned long sz;\n>>\n>> @@ -74,16 +75,15 @@ static int stream_blob_to_file(const unsigned char *sha1)\n>> \t\treturn error(\"cannot stream blob %s\", sha1_to_hex(sha1));\n>> \tfor (;;) {\n>> \t\tchar buf[BLOCKSIZE];\n>> -\t\tssize_t readlen;\n>>\n>> \t\treadlen = read_istream(st, buf, sizeof(buf));\n>>\n>> \t\tif (readlen<= 0)\n>> -\t\t\treturn readlen;\n>> +\t\t\tbreak;\n>> \t\twrite_blocked(buf, readlen);\n>> \t}\n>> \tclose_istream(st);\n>> -\treturn 0;\n>> +\treturn readlen;\n>> }\n>\n> Your patch on top obviouly is the right thing to do, but reading the code\n> again, I am not sure if the original is correct. read_istream() itself\n> does not promise that it will always fill the buffer before returning (it\n> could return with a short read). It seems incorrect that the caller does\n> not loop to avoid padding a short read with padding by calling\n> write_blocked().\n\nYes, indeed, good catch. It could also write to block directly and \navoid copying the buffer again. The tail clearing part of \nwrite_blocked() can certainly be reused, but the rest will probably have \nto be reimplemented around read_istream().\n\nRené\n"},{"id":"190355","messageId":"4F9F0E37.6040406@lsrfire.ath.cx","threadId":"30361","inReplyTo":"1335761837-12482-6-git-send-email-pclouds@gmail.com","subject":"[PATCH 6a/5] archive-zip: remove uncompressed_size","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T22:12:07Z","receivedAt":"2012-04-30T22:12:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"We only need size and compressed_size.\n\nSigned-off-by: Rene Scharfe \n---\n archive-zip.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 716cc42..400ba38 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -129,7 +129,6 @@ static int write_zip_entry(struct archiver_args *args,\n \tstruct zip_dir_header dirent;\n \tunsigned long attr2;\n \tunsigned long compressed_size;\n-\tunsigned long uncompressed_size;\n \tunsigned long crc;\n \tunsigned long direntsize;\n \tint method;\n@@ -149,7 +148,7 @@ static int write_zip_entry(struct archiver_args *args,\n \t\tmethod = 0;\n \t\tattr2 = 16;\n \t\tout = NULL;\n-\t\tuncompressed_size = 0;\n+\t\tsize = 0;\n \t\tcompressed_size = 0;\n \t\tbuffer = NULL;\n \t\tsize = 0;\n@@ -166,7 +165,6 @@ static int write_zip_entry(struct archiver_args *args,\n \t\t\tmethod = 8;\n \t\tcrc = crc32(crc, buffer, size);\n \t\tout = buffer;\n-\t\tuncompressed_size = size;\n \t\tcompressed_size = size;\n \t} else {\n \t\treturn error(\"unsupported file mode: 0%o (SHA1: %s)\", mode,\n@@ -204,7 +202,7 @@ static int write_zip_entry(struct archiver_args *args,\n \tcopy_le16(dirent.mdate, zip_date);\n \tcopy_le32(dirent.crc32, crc);\n \tcopy_le32(dirent.compressed_size, compressed_size);\n-\tcopy_le32(dirent.size, uncompressed_size);\n+\tcopy_le32(dirent.size, size);\n \tcopy_le16(dirent.filename_length, pathlen);\n \tcopy_le16(dirent.extra_length, 0);\n \tcopy_le16(dirent.comment_length, 0);\n@@ -226,7 +224,7 @@ static int write_zip_entry(struct archiver_args *args,\n \tcopy_le16(header.mdate, zip_date);\n \tcopy_le32(header.crc32, crc);\n \tcopy_le32(header.compressed_size, compressed_size);\n-\tcopy_le32(header.size, uncompressed_size);\n+\tcopy_le32(header.size, size);\n \tcopy_le16(header.filename_length, pathlen);\n \tcopy_le16(header.extra_length, 0);\n \twrite_or_die(1, &header, ZIP_LOCAL_HEADER_SIZE);\n-- \n1.7.10\n"},{"id":"190356","messageId":"4F9F0E40.8010809@lsrfire.ath.cx","threadId":"30361","inReplyTo":"1335761837-12482-6-git-send-email-pclouds@gmail.com","subject":"[PATCH 7a/5] archive-zip: factor out helpers for writing sizes and CRC","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T22:12:16Z","receivedAt":"2012-04-30T22:12:16Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"We're going to reuse them soon for streaming. Also, update the ZIP\ndirectory only at the very end, which will also make streaming easier.\n\nSigned-off-by: Rene Scharfe \n---\n archive-zip.c | 39 ++++++++++++++++++++++++++++-----------\n 1 file changed, 28 insertions(+), 11 deletions(-)\n\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 400ba38..678569a 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -120,6 +120,26 @@ static void *zlib_deflate(void *data, unsigned long size,\n \treturn buffer;\n }\n \n+static void set_zip_dir_data_desc(struct zip_dir_header *header,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t unsigned long compressed_size,\n+\t\t\t\t unsigned long crc)\n+{\n+\tcopy_le32(header->crc32, crc);\n+\tcopy_le32(header->compressed_size, compressed_size);\n+\tcopy_le32(header->size, size);\n+}\n+\n+static void set_zip_header_data_desc(struct zip_local_header *header,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t unsigned long compressed_size,\n+\t\t\t\t unsigned long crc)\n+{\n+\tcopy_le32(header->crc32, crc);\n+\tcopy_le32(header->compressed_size, compressed_size);\n+\tcopy_le32(header->size, size);\n+}\n+\n static int write_zip_entry(struct archiver_args *args,\n \t\t\t const unsigned char *sha1,\n \t\t\t const char *path, size_t pathlen,\n@@ -200,9 +220,7 @@ static int write_zip_entry(struct archiver_args *args,\n \tcopy_le16(dirent.compression_method, method);\n \tcopy_le16(dirent.mtime, zip_time);\n \tcopy_le16(dirent.mdate, zip_date);\n-\tcopy_le32(dirent.crc32, crc);\n-\tcopy_le32(dirent.compressed_size, compressed_size);\n-\tcopy_le32(dirent.size, size);\n+\tset_zip_dir_data_desc(&dirent, size, compressed_size, crc);\n \tcopy_le16(dirent.filename_length, pathlen);\n \tcopy_le16(dirent.extra_length, 0);\n \tcopy_le16(dirent.comment_length, 0);\n@@ -210,11 +228,6 @@ static int write_zip_entry(struct archiver_args *args,\n \tcopy_le16(dirent.attr1, 0);\n \tcopy_le32(dirent.attr2, attr2);\n \tcopy_le32(dirent.offset, zip_offset);\n-\tmemcpy(zip_dir + zip_dir_offset, &dirent, ZIP_DIR_HEADER_SIZE);\n-\tzip_dir_offset += ZIP_DIR_HEADER_SIZE;\n-\tmemcpy(zip_dir + zip_dir_offset, path, pathlen);\n-\tzip_dir_offset += pathlen;\n-\tzip_dir_entries++;\n \n \tcopy_le32(header.magic, 0x04034b50);\n \tcopy_le16(header.version, 10);\n@@ -222,9 +235,7 @@ static int write_zip_entry(struct archiver_args *args,\n \tcopy_le16(header.compression_method, method);\n \tcopy_le16(header.mtime, zip_time);\n \tcopy_le16(header.mdate, zip_date);\n-\tcopy_le32(header.crc32, crc);\n-\tcopy_le32(header.compressed_size, compressed_size);\n-\tcopy_le32(header.size, size);\n+\tset_zip_header_data_desc(&header, size, compressed_size, crc);\n \tcopy_le16(header.filename_length, pathlen);\n \tcopy_le16(header.extra_length, 0);\n \twrite_or_die(1, &header, ZIP_LOCAL_HEADER_SIZE);\n@@ -239,6 +250,12 @@ static int write_zip_entry(struct archiver_args *args,\n \tfree(deflated);\n \tfree(buffer);\n \n+\tmemcpy(zip_dir + zip_dir_offset, &dirent, ZIP_DIR_HEADER_SIZE);\n+\tzip_dir_offset += ZIP_DIR_HEADER_SIZE;\n+\tmemcpy(zip_dir + zip_dir_offset, path, pathlen);\n+\tzip_dir_offset += pathlen;\n+\tzip_dir_entries++;\n+\n \treturn 0;\n }\n \n-- \n1.7.10\n"},{"id":"190357","messageId":"4F9F0E45.6070108@lsrfire.ath.cx","threadId":"30361","inReplyTo":"1335761837-12482-6-git-send-email-pclouds@gmail.com","subject":"[PATCH 8a/5] archive-zip: streaming for stored files","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T22:12:21Z","receivedAt":"2012-04-30T22:12:21Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Write a data descriptor containing the CRC of the entry and its sizes\nafter streaming it out. For simplicity, do that only if we're storing\nfiles (option -0) for now.\n\nSigned-off-by: Rene Scharfe \n---\n archive-zip.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++--------\n t/t1050-large.sh | 4 +++\n 2 files changed, 82 insertions(+), 12 deletions(-)\n\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 678569a..1c6c39d 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -3,6 +3,7 @@\n */\n #include \"cache.h\"\n #include \"archive.h\"\n+#include \"streaming.h\"\n \n static int zip_date;\n static int zip_time;\n@@ -15,6 +16,7 @@ static unsigned int zip_dir_offset;\n static unsigned int zip_dir_entries;\n \n #define ZIP_DIRECTORY_MIN_SIZE\t(1024 * 1024)\n+#define ZIP_STREAM (8)\n \n struct zip_local_header {\n \tunsigned char magic[4];\n@@ -31,6 +33,14 @@ struct zip_local_header {\n \tunsigned char _end[1];\n };\n \n+struct zip_data_desc {\n+\tunsigned char magic[4];\n+\tunsigned char crc32[4];\n+\tunsigned char compressed_size[4];\n+\tunsigned char size[4];\n+\tunsigned char _end[1];\n+};\n+\n struct zip_dir_header {\n \tunsigned char magic[4];\n \tunsigned char creator_version[2];\n@@ -70,6 +80,7 @@ struct zip_dir_trailer {\n * we're interested in.\n */\n #define ZIP_LOCAL_HEADER_SIZE\toffsetof(struct zip_local_header, _end)\n+#define ZIP_DATA_DESC_SIZE\toffsetof(struct zip_data_desc, _end)\n #define ZIP_DIR_HEADER_SIZE\toffsetof(struct zip_dir_header, _end)\n #define ZIP_DIR_TRAILER_SIZE\toffsetof(struct zip_dir_trailer, _end)\n \n@@ -120,6 +131,19 @@ static void *zlib_deflate(void *data, unsigned long size,\n \treturn buffer;\n }\n \n+static void write_zip_data_desc(unsigned long size,\n+\t\t\t\tunsigned long compressed_size,\n+\t\t\t\tunsigned long crc)\n+{\n+\tstruct zip_data_desc trailer;\n+\n+\tcopy_le32(trailer.magic, 0x08074b50);\n+\tcopy_le32(trailer.crc32, crc);\n+\tcopy_le32(trailer.compressed_size, compressed_size);\n+\tcopy_le32(trailer.size, size);\n+\twrite_or_die(1, &trailer, ZIP_DATA_DESC_SIZE);\n+}\n+\n static void set_zip_dir_data_desc(struct zip_dir_header *header,\n \t\t\t\t unsigned long size,\n \t\t\t\t unsigned long compressed_size,\n@@ -140,6 +164,8 @@ static void set_zip_header_data_desc(struct zip_local_header *header,\n \tcopy_le32(header->size, size);\n }\n \n+#define STREAM_BUFFER_SIZE (1024 * 16)\n+\n static int write_zip_entry(struct archiver_args *args,\n \t\t\t const unsigned char *sha1,\n \t\t\t const char *path, size_t pathlen,\n@@ -155,6 +181,8 @@ static int write_zip_entry(struct archiver_args *args,\n \tunsigned char *out;\n \tvoid *deflated = NULL;\n \tvoid *buffer;\n+\tstruct git_istream *stream = NULL;\n+\tunsigned long flags = 0;\n \tunsigned long size;\n \n \tcrc = crc32(0, NULL, 0);\n@@ -173,25 +201,38 @@ static int write_zip_entry(struct archiver_args *args,\n \t\tbuffer = NULL;\n \t\tsize = 0;\n \t} else if (S_ISREG(mode) || S_ISLNK(mode)) {\n-\t\tenum object_type type;\n-\t\tbuffer = sha1_file_to_archive(args, path, sha1, mode, &type, &size);\n-\t\tif (!buffer)\n-\t\t\treturn error(\"cannot read %s\", sha1_to_hex(sha1));\n+\t\tenum object_type type = sha1_object_info(sha1, &size);\n \n \t\tmethod = 0;\n \t\tattr2 = S_ISLNK(mode) ? ((mode | 0777) << 16) :\n \t\t\t(mode & 0111) ? ((mode) << 16) : 0;\n-\t\tif (S_ISREG(mode) && args->compression_level != 0)\n+\t\tif (S_ISREG(mode) && args->compression_level != 0 && size > 0)\n \t\t\tmethod = 8;\n-\t\tcrc = crc32(crc, buffer, size);\n-\t\tout = buffer;\n \t\tcompressed_size = size;\n+\n+\t\tif (S_ISREG(mode) && type == OBJ_BLOB && !args->convert &&\n+\t\t size > big_file_threshold && method == 0) {\n+\t\t\tstream = open_istream(sha1, &type, &size, NULL);\n+\t\t\tif (!stream)\n+\t\t\t\treturn error(\"cannot stream blob %s\",\n+\t\t\t\t\t sha1_to_hex(sha1));\n+\t\t\tflags |= ZIP_STREAM;\n+\t\t\tout = buffer = NULL;\n+\t\t} else {\n+\t\t\tbuffer = sha1_file_to_archive(args, path, sha1, mode,\n+\t\t\t\t\t\t &type, &size);\n+\t\t\tif (!buffer)\n+\t\t\t\treturn error(\"cannot read %s\",\n+\t\t\t\t\t sha1_to_hex(sha1));\n+\t\t\tcrc = crc32(crc, buffer, size);\n+\t\t\tout = buffer;\n+\t\t}\n \t} else {\n \t\treturn error(\"unsupported file mode: 0%o (SHA1: %s)\", mode,\n \t\t\t\tsha1_to_hex(sha1));\n \t}\n \n-\tif (method == 8) {\n+\tif (buffer && method == 8) {\n \t\tdeflated = zlib_deflate(buffer, size, args->compression_level,\n \t\t\t\t&compressed_size);\n \t\tif (deflated && compressed_size - 6 < size) {\n@@ -216,7 +257,7 @@ static int write_zip_entry(struct archiver_args *args,\n \tcopy_le16(dirent.creator_version,\n \t\tS_ISLNK(mode) || (S_ISREG(mode) && (mode & 0111)) ? 0x0317 : 0);\n \tcopy_le16(dirent.version, 10);\n-\tcopy_le16(dirent.flags, 0);\n+\tcopy_le16(dirent.flags, flags);\n \tcopy_le16(dirent.compression_method, method);\n \tcopy_le16(dirent.mtime, zip_time);\n \tcopy_le16(dirent.mdate, zip_date);\n@@ -231,18 +272,43 @@ static int write_zip_entry(struct archiver_args *args,\n \n \tcopy_le32(header.magic, 0x04034b50);\n \tcopy_le16(header.version, 10);\n-\tcopy_le16(header.flags, 0);\n+\tcopy_le16(header.flags, flags);\n \tcopy_le16(header.compression_method, method);\n \tcopy_le16(header.mtime, zip_time);\n \tcopy_le16(header.mdate, zip_date);\n-\tset_zip_header_data_desc(&header, size, compressed_size, crc);\n+\tif (flags & ZIP_STREAM)\n+\t\tset_zip_header_data_desc(&header, 0, 0, 0);\n+\telse\n+\t\tset_zip_header_data_desc(&header, size, compressed_size, crc);\n \tcopy_le16(header.filename_length, pathlen);\n \tcopy_le16(header.extra_length, 0);\n \twrite_or_die(1, &header, ZIP_LOCAL_HEADER_SIZE);\n \tzip_offset += ZIP_LOCAL_HEADER_SIZE;\n \twrite_or_die(1, path, pathlen);\n \tzip_offset += pathlen;\n-\tif (compressed_size > 0) {\n+\tif (stream && method == 0) {\n+\t\tunsigned char buf[STREAM_BUFFER_SIZE];\n+\t\tssize_t readlen;\n+\n+\t\tfor (;;) {\n+\t\t\treadlen = read_istream(stream, buf, sizeof(buf));\n+\t\t\tif (readlen <= 0)\n+\t\t\t\tbreak;\n+\t\t\tcrc = crc32(crc, buf, readlen);\n+\t\t\twrite_or_die(1, buf, readlen);\n+\t\t}\n+\t\tclose_istream(stream);\n+\t\tif (readlen)\n+\t\t\treturn readlen;\n+\n+\t\tcompressed_size = size;\n+\t\tzip_offset += compressed_size;\n+\n+\t\twrite_zip_data_desc(size, compressed_size, crc);\n+\t\tzip_offset += ZIP_DATA_DESC_SIZE;\n+\n+\t\tset_zip_dir_data_desc(&dirent, size, compressed_size, crc);\n+\t} else if (compressed_size > 0) {\n \t\twrite_or_die(1, out, compressed_size);\n \t\tzip_offset += compressed_size;\n \t}\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex fe47554..9db54b5 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -138,4 +138,8 @@ test_expect_success 'tar achiving' '\n \tgit archive --format=tar HEAD >/dev/null\n '\n \n+test_expect_success 'zip achiving, store only' '\n+\tgit archive --format=zip -0 HEAD >/dev/null\n+'\n+\n test_done\n-- \n1.7.10\n"},{"id":"190358","messageId":"4F9F0E4B.40206@lsrfire.ath.cx","threadId":"30361","inReplyTo":"1335761837-12482-6-git-send-email-pclouds@gmail.com","subject":"[PATCH 9a/5] archive-zip: streaming for deflated files","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T22:12:27Z","receivedAt":"2012-04-30T22:12:27Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"After an entry has been streamed out, its CRC and sizes are written as\npart of a data descriptor.\n\nFor simplicity, we make the buffer for the compressed chunks twice as\nbig as for the uncompressed ones, to be sure the result fit in even\nif deflate makes them bigger.\n\nSigned-off-by: Rene Scharfe \n---\n archive-zip.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t1050-large.sh | 4 ++++\n 2 files changed, 67 insertions(+), 1 deletion(-)\n\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 1c6c39d..f5af81f 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -211,7 +211,7 @@ static int write_zip_entry(struct archiver_args *args,\n \t\tcompressed_size = size;\n \n \t\tif (S_ISREG(mode) && type == OBJ_BLOB && !args->convert &&\n-\t\t size > big_file_threshold && method == 0) {\n+\t\t size > big_file_threshold) {\n \t\t\tstream = open_istream(sha1, &type, &size, NULL);\n \t\t\tif (!stream)\n \t\t\t\treturn error(\"cannot stream blob %s\",\n@@ -308,6 +308,68 @@ static int write_zip_entry(struct archiver_args *args,\n \t\tzip_offset += ZIP_DATA_DESC_SIZE;\n \n \t\tset_zip_dir_data_desc(&dirent, size, compressed_size, crc);\n+\t} else if (stream && method == 8) {\n+\t\tunsigned char buf[STREAM_BUFFER_SIZE];\n+\t\tssize_t readlen;\n+\t\tgit_zstream zstream;\n+\t\tint result;\n+\t\tsize_t out_len;\n+\t\tunsigned char compressed[STREAM_BUFFER_SIZE * 2];\n+\n+\t\tmemset(&zstream, 0, sizeof(zstream));\n+\t\tgit_deflate_init(&zstream, args->compression_level);\n+\n+\t\tcompressed_size = 0;\n+\t\tzstream.next_out = compressed;\n+\t\tzstream.avail_out = sizeof(compressed);\n+\n+\t\tfor (;;) {\n+\t\t\treadlen = read_istream(stream, buf, sizeof(buf));\n+\t\t\tif (readlen <= 0)\n+\t\t\t\tbreak;\n+\t\t\tcrc = crc32(crc, buf, readlen);\n+\n+\t\t\tzstream.next_in = buf;\n+\t\t\tzstream.avail_in = readlen;\n+\t\t\tresult = git_deflate(&zstream, 0);\n+\t\t\tif (result != Z_OK)\n+\t\t\t\tdie(\"deflate error (%d)\", result);\n+\t\t\tout = compressed;\n+\t\t\tif (!compressed_size)\n+\t\t\t\tout += 2;\n+\t\t\tout_len = zstream.next_out - out;\n+\n+\t\t\tif (out_len > 0) {\n+\t\t\t\twrite_or_die(1, out, out_len);\n+\t\t\t\tcompressed_size += out_len;\n+\t\t\t\tzstream.next_out = compressed;\n+\t\t\t\tzstream.avail_out = sizeof(compressed);\n+\t\t\t}\n+\n+\t\t}\n+\t\tclose_istream(stream);\n+\t\tif (readlen)\n+\t\t\treturn readlen;\n+\n+\t\tzstream.next_in = buf;\n+\t\tzstream.avail_in = 0;\n+\t\tresult = git_deflate(&zstream, Z_FINISH);\n+\t\tif (result != Z_STREAM_END)\n+\t\t\tdie(\"deflate error (%d)\", result);\n+\n+\t\tgit_deflate_end(&zstream);\n+\t\tout = compressed;\n+\t\tif (!compressed_size)\n+\t\t\tout += 2;\n+\t\tout_len = zstream.next_out - out - 4;\n+\t\twrite_or_die(1, out, out_len);\n+\t\tcompressed_size += out_len;\n+\t\tzip_offset += compressed_size;\n+\n+\t\twrite_zip_data_desc(size, compressed_size, crc);\n+\t\tzip_offset += ZIP_DATA_DESC_SIZE;\n+\n+\t\tset_zip_dir_data_desc(&dirent, size, compressed_size, crc);\n \t} else if (compressed_size > 0) {\n \t\twrite_or_die(1, out, compressed_size);\n \t\tzip_offset += compressed_size;\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 9db54b5..55ed955 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -142,4 +142,8 @@ test_expect_success 'zip achiving, store only' '\n \tgit archive --format=zip -0 HEAD >/dev/null\n '\n \n+test_expect_success 'zip achiving, deflate' '\n+\tgit archive --format=zip HEAD >/dev/null\n+'\n+\n test_done\n-- \n1.7.10\n"},{"id":"190361","messageId":"4F9F1809.1060803@lsrfire.ath.cx","threadId":"30361","inReplyTo":"7vipghf2z0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/5] archive-zip: stream large blobs into zip file","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-04-30T22:54:01Z","receivedAt":"2012-04-30T22:54:01Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 30.04.2012 21:12, schrieb Junio C Hamano:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> A large blob will be read twice. One for calculating crc32, one for\n>> actual writing.\n>\n> Is that because you need the checksum before the payload? That is\n> unfortunate. It would be nice (read: not a rejection of this patch---it\n> is a good first step to do it stupid but correct way before trying to\n> optimize it) to avoid it when the output is seekable, especially because\n> we are talking about a *large* payload.\n\nThe ZIP format optionally allows writing the CRC and the sizes after the \ndata. This adds a data descriptor with a size of 16 bytes to each \nstreamed entry. Seeking back and correcting these values in an output \nfile would avoid that.\n\n>> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n>> index fe47554..458fdde 100755\n>> --- a/t/t1050-large.sh\n>> +++ b/t/t1050-large.sh\n>> @@ -138,4 +138,8 @@ test_expect_success 'tar achiving' '\n>> \tgit archive --format=tar HEAD>/dev/null\n>> '\n>>\n>> +test_expect_success 'zip achiving' '\n>> +\tgit archive --format=zip HEAD>/dev/null\n>> +'\n>\n> Can't we do better than \"we only check if it finishes without barfing; we\n> cannot afford to check the correctness of the output\"? The same comment\n> applies to all the tests you added to this file in the past 3 months.\n\nStreaming to tar can be tested by setting core.big_file_threshold big \nenough, creating a non-streamed version of the archive and comparing it \nto the streamed one. With the seek trick, this would work for ZIP as \nwell. For streaming with an added data descriptor we'd need to actually \nunpack the ZIP file, though.\n\nRené\n"},{"id":"190414","messageId":"CACsJy8DXMSZE+ZHEXcCKteKxheLhkNu_O0JM7YQFsFMxbm-A7w@mail.gmail.com","threadId":"30361","inReplyTo":"4F9EFF2D.8080505@lsrfire.ath.cx","subject":"Re: [PATCH 0/5] Large file support for git-archive","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-01T10:19:01Z","receivedAt":"2012-05-01T10:19:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Thanks both. Will resend with fixes in the first patches and René's\npatches with better tests that also verify output. I think I will move\nzip tests back to t5000 though because it may change output structure.\n\n2012/5/1 René Scharfe :\n> Am 30.04.2012 06:57, schrieb Nguyễn Thái Ngọc Duy:\n>\n>> This is a spin-off from the large file support series I posted some\n>> time ago [1]. Both tar and zip format now support streaming large\n>> blobs. Although zip will store uncompressed large blobs.\n>>\n>> [1] http://thread.gmane.org/gmane.comp.version-control.git/191605\n>>\n>> Nguyễn Thái Ngọc Duy (5):\n>>   archive-tar: turn write_tar_entry into blob-writing only\n>>   archive-tar: unindent write_tar_entry by one level\n>>   archive: delegate blob reading to backend\n>>   archive-tar: stream large blobs to tar file\n>>   archive-zip: stream large blobs into zip file\n>>\n>>  Documentation/git-archive.txt |    3 +\n>>  archive-tar.c                 |  184\n>> ++++++++++++++++++++++++++++-------------\n>>  archive-zip.c                 |   56 ++++++++++++-\n>>  archive.c                     |   28 +++----\n>>  archive.h                     |   10 ++-\n>>  t/t1050-large.sh              |    8 ++\n>>  6 files changed, 209 insertions(+), 80 deletions(-)\n>\n>\n> I like the cleanups in the first two patches.\n>\n> I'm neutral to positive on the third one; it certainly simplifies the\n> interface to the backends with only little code duplication.\n>\n> The ZIP format supports streaming natively (look for \"Data descriptor\" in\n> http://www.pkware.com/documents/casestudies/APPNOTE.TXT). Incidentally, I\n> had been working on supporting that, but without any presentable results.\n>  So far.  I've adapted the pieces I had to your series now; will send\n> shortly.\n>\n> René\n\n\n\n-- \nDuy\n"}]}