{"thread":{"id":"27295","subject":"[PATCH v0 1/3] index_fd(): turn write_object and format_check arguments into one flag","startedAt":"2011-05-08T08:47:32Z","lastAt":"2011-05-29T20:29:41Z","messageCount":18,"participants":["Junio C Hamano","Nguyen Thai Ngoc Duy","Shawn Pearce","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":0,"patchTotal":3},"messages":[{"id":"167353","messageId":"1304844455-23570-1-git-send-email-gitster@pobox.com","threadId":"27295","inReplyTo":null,"subject":"[PATCH v0 0/3] git add a-Big-file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-08T08:47:32Z","receivedAt":"2011-05-08T08:47:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is merely a POC, hoping that interested parties might fill in the\nblanks aka update the NEEDSWORK part of the patch.\n\nJunio C Hamano (3):\n  index_fd(): turn write_object and format_check arguments into one flag\n  index_fd(): split into two helper functions\n  Bigfile: teach \"git add\" to send a large file straight to a pack\n\n builtin/hash-object.c  |    5 +-\n builtin/update-index.c |    3 +-\n cache.h                |    7 ++-\n notes-merge.c          |    2 +-\n read-cache.c           |    4 +-\n sha1_file.c            |  165 +++++++++++++++++++++++++++++++++++++++++-------\n t/t1050-large.sh       |   22 +++++++\n 7 files changed, 177 insertions(+), 31 deletions(-)\n create mode 100755 t/t1050-large.sh\n\n-- \n1.7.5.1.268.gce5bd\n"},{"id":"167351","messageId":"1304844455-23570-2-git-send-email-gitster@pobox.com","threadId":"27295","inReplyTo":"1304844455-23570-1-git-send-email-gitster@pobox.com","subject":"[PATCH v0 1/3] index_fd(): turn write_object and format_check arguments into one flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-08T08:47:33Z","receivedAt":"2011-05-08T08:47:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"format_check\" parameter tucked after the existing parameters is too\nugly an afterthought to live in any reasonable API.\n\nCombine it with the other boolean parameter \"write_object\" into a single\n\"flags\" parameter.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/hash-object.c  |    5 ++++-\n builtin/update-index.c |    3 ++-\n cache.h                |    7 +++++--\n notes-merge.c          |    2 +-\n read-cache.c           |    4 ++--\n sha1_file.c            |   29 +++++++++++++----------------\n 6 files changed, 27 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/hash-object.c b/builtin/hash-object.c\nindex b96f46a..33911fd 100644\n--- a/builtin/hash-object.c\n+++ b/builtin/hash-object.c\n@@ -14,8 +14,11 @@ static void hash_fd(int fd, const char *type, int write_object, const char *path\n {\n \tstruct stat st;\n \tunsigned char sha1[20];\n+\tunsigned flags = (HASH_FORMAT_CHECK |\n+\t\t\t  (write_object ? HASH_WRITE_OBJECT : 0));\n+\n \tif (fstat(fd, &st) < 0 ||\n-\t    index_fd(sha1, fd, &st, write_object, type_from_string(type), path, 1))\n+\t    index_fd(sha1, fd, &st, type_from_string(type), path, flags))\n \t\tdie(write_object\n \t\t    ? \"Unable to add %s to database\"\n \t\t    : \"Unable to hash %s\", path);\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex d7850c6..f14bc90 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -99,7 +99,8 @@ static int add_one_path(struct cache_entry *old, const char *path, int len, stru\n \tfill_stat_cache_info(ce, st);\n \tce->ce_mode = ce_mode_from_stat(old, st->st_mode);\n \n-\tif (index_path(ce->sha1, path, st, !info_only))\n+\tif (index_path(ce->sha1, path, st,\n+\t\t       info_only ? 0 : HASH_WRITE_OBJECT))\n \t\treturn -1;\n \toption = allow_add ? ADD_CACHE_OK_TO_ADD : 0;\n \toption |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;\ndiff --git a/cache.h b/cache.h\nindex 2b34116..2be5333 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -518,8 +518,11 @@ struct pathspec {\n extern int init_pathspec(struct pathspec *, const char **);\n extern void free_pathspec(struct pathspec *);\n extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);\n-extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, enum object_type type, const char *path, int format_check);\n-extern int index_path(unsigned char *sha1, const char *path, struct stat *st, int write_object);\n+\n+#define HASH_WRITE_OBJECT 1\n+#define HASH_FORMAT_CHECK 2\n+extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);\n+extern int index_path(unsigned char *sha1, const char *path, struct stat *st, unsigned flags);\n extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);\n \n #define REFRESH_REALLY\t\t0x0001\t/* ignore_valid */\ndiff --git a/notes-merge.c b/notes-merge.c\nindex 28046a9..e1aaf43 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -707,7 +707,7 @@ int notes_merge_commit(struct notes_merge_options *o,\n \t\t/* write file as blob, and add to partial_tree */\n \t\tif (stat(ent->name, &st))\n \t\t\tdie_errno(\"Failed to stat '%s'\", ent->name);\n-\t\tif (index_path(blob_sha1, ent->name, &st, 1))\n+\t\tif (index_path(blob_sha1, ent->name, &st, HASH_WRITE_OBJECT))\n \t\t\tdie(\"Failed to write blob object from '%s'\", ent->name);\n \t\tif (add_note(partial_tree, obj_sha1, blob_sha1, NULL))\n \t\t\tdie(\"Failed to add resolved note '%s' to notes tree\",\ndiff --git a/read-cache.c b/read-cache.c\nindex f38471c..4ac9a03 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -92,7 +92,7 @@ static int ce_compare_data(struct cache_entry *ce, struct stat *st)\n \n \tif (fd >= 0) {\n \t\tunsigned char sha1[20];\n-\t\tif (!index_fd(sha1, fd, st, 0, OBJ_BLOB, ce->name, 0))\n+\t\tif (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, 0))\n \t\t\tmatch = hashcmp(sha1, ce->sha1);\n \t\t/* index_fd() closed the file descriptor already */\n \t}\n@@ -641,7 +641,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \t\treturn 0;\n \t}\n \tif (!intent_only) {\n-\t\tif (index_path(ce->sha1, path, st, 1))\n+\t\tif (index_path(ce->sha1, path, st, HASH_WRITE_OBJECT))\n \t\t\treturn error(\"unable to index file %s\", path);\n \t} else\n \t\trecord_intent_to_add(ce);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 889fe71..17c179c 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2581,10 +2581,11 @@ static void check_tag(const void *buf, size_t size)\n }\n \n static int index_mem(unsigned char *sha1, void *buf, size_t size,\n-\t\t     int write_object, enum object_type type,\n-\t\t     const char *path, int format_check)\n+\t\t     enum object_type type,\n+\t\t     const char *path, unsigned flags)\n {\n \tint ret, re_allocated = 0;\n+\tint write_object = flags & HASH_WRITE_OBJECT;\n \n \tif (!type)\n \t\ttype = OBJ_BLOB;\n@@ -2600,7 +2601,7 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,\n \t\t\tre_allocated = 1;\n \t\t}\n \t}\n-\tif (format_check) {\n+\tif (flags & HASH_FORMAT_CHECK) {\n \t\tif (type == OBJ_TREE)\n \t\t\tcheck_tree(buf, size);\n \t\tif (type == OBJ_COMMIT)\n@@ -2620,8 +2621,8 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,\n \n #define SMALL_FILE_SIZE (32*1024)\n \n-int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,\n-\t     enum object_type type, const char *path, int format_check)\n+int index_fd(unsigned char *sha1, int fd, struct stat *st,\n+\t     enum object_type type, const char *path, unsigned flags)\n {\n \tint ret;\n \tsize_t size = xsize_t(st->st_size);\n@@ -2629,33 +2630,29 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,\n \tif (!S_ISREG(st->st_mode)) {\n \t\tstruct strbuf sbuf = STRBUF_INIT;\n \t\tif (strbuf_read(&sbuf, fd, 4096) >= 0)\n-\t\t\tret = index_mem(sha1, sbuf.buf, sbuf.len, write_object,\n-\t\t\t\t\ttype, path, format_check);\n+\t\t\tret = index_mem(sha1, sbuf.buf, sbuf.len, type,\tpath, flags);\n \t\telse\n \t\t\tret = -1;\n \t\tstrbuf_release(&sbuf);\n \t} else if (!size) {\n-\t\tret = index_mem(sha1, NULL, size, write_object, type, path,\n-\t\t\t\tformat_check);\n+\t\tret = index_mem(sha1, NULL, size, type, path, flags);\n \t} else if (size <= SMALL_FILE_SIZE) {\n \t\tchar *buf = xmalloc(size);\n \t\tif (size == read_in_full(fd, buf, size))\n-\t\t\tret = index_mem(sha1, buf, size, write_object, type,\n-\t\t\t\t\tpath, format_check);\n+\t\t\tret = index_mem(sha1, buf, size, type, path, flags);\n \t\telse\n \t\t\tret = error(\"short read %s\", strerror(errno));\n \t\tfree(buf);\n \t} else {\n \t\tvoid *buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);\n-\t\tret = index_mem(sha1, buf, size, write_object, type, path,\n-\t\t\t\tformat_check);\n+\t\tret = index_mem(sha1, buf, size, type, path, flags);\n \t\tmunmap(buf, size);\n \t}\n \tclose(fd);\n \treturn ret;\n }\n \n-int index_path(unsigned char *sha1, const char *path, struct stat *st, int write_object)\n+int index_path(unsigned char *sha1, const char *path, struct stat *st, unsigned flags)\n {\n \tint fd;\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -2666,7 +2663,7 @@ int index_path(unsigned char *sha1, const char *path, struct stat *st, int write\n \t\tif (fd < 0)\n \t\t\treturn error(\"open(\\\"%s\\\"): %s\", path,\n \t\t\t\t     strerror(errno));\n-\t\tif (index_fd(sha1, fd, st, write_object, OBJ_BLOB, path, 0) < 0)\n+\t\tif (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)\n \t\t\treturn error(\"%s: failed to insert into database\",\n \t\t\t\t     path);\n \t\tbreak;\n@@ -2676,7 +2673,7 @@ int index_path(unsigned char *sha1, const char *path, struct stat *st, int write\n \t\t\treturn error(\"readlink(\\\"%s\\\"): %s\", path,\n \t\t\t             errstr);\n \t\t}\n-\t\tif (!write_object)\n+\t\tif (!(flags & HASH_WRITE_OBJECT))\n \t\t\thash_sha1_file(sb.buf, sb.len, blob_type, sha1);\n \t\telse if (write_sha1_file(sb.buf, sb.len, blob_type, sha1))\n \t\t\treturn error(\"%s: failed to insert into database\",\n-- \n1.7.5.1.268.gce5bd\n"},{"id":"167352","messageId":"1304844455-23570-3-git-send-email-gitster@pobox.com","threadId":"27295","inReplyTo":"1304844455-23570-1-git-send-email-gitster@pobox.com","subject":"[PATCH v0 2/3] index_fd(): split into two helper functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-08T08:47:34Z","receivedAt":"2011-05-08T08:47:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Split out the case where we do not know the size of the input (hence we\nread everything into a strbuf before doing anything) to index_pipe(), and\nthe other case where we mmap or read the whole data to index_bulk().\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_file.c |   42 +++++++++++++++++++++++++++++++-----------\n 1 files changed, 31 insertions(+), 11 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 17c179c..49416b0 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2619,22 +2619,29 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,\n \treturn ret;\n }\n \n+static int index_pipe(unsigned char *sha1, int fd, enum object_type type,\n+\t\t      const char *path, unsigned flags)\n+{\n+\tstruct strbuf sbuf = STRBUF_INIT;\n+\tint ret;\n+\n+\tif (strbuf_read(&sbuf, fd, 4096) >= 0)\n+\t\tret = index_mem(sha1, sbuf.buf, sbuf.len, type,\tpath, flags);\n+\telse\n+\t\tret = -1;\n+\tstrbuf_release(&sbuf);\n+\treturn ret;\n+}\n+\n #define SMALL_FILE_SIZE (32*1024)\n \n-int index_fd(unsigned char *sha1, int fd, struct stat *st,\n-\t     enum object_type type, const char *path, unsigned flags)\n+static int index_core(unsigned char *sha1, int fd, size_t size,\n+\t\t      enum object_type type, const char *path,\n+\t\t      unsigned flags)\n {\n \tint ret;\n-\tsize_t size = xsize_t(st->st_size);\n \n-\tif (!S_ISREG(st->st_mode)) {\n-\t\tstruct strbuf sbuf = STRBUF_INIT;\n-\t\tif (strbuf_read(&sbuf, fd, 4096) >= 0)\n-\t\t\tret = index_mem(sha1, sbuf.buf, sbuf.len, type,\tpath, flags);\n-\t\telse\n-\t\t\tret = -1;\n-\t\tstrbuf_release(&sbuf);\n-\t} else if (!size) {\n+\tif (!size) {\n \t\tret = index_mem(sha1, NULL, size, type, path, flags);\n \t} else if (size <= SMALL_FILE_SIZE) {\n \t\tchar *buf = xmalloc(size);\n@@ -2648,6 +2655,19 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,\n \t\tret = index_mem(sha1, buf, size, type, path, flags);\n \t\tmunmap(buf, size);\n \t}\n+\treturn ret;\n+}\n+\n+int index_fd(unsigned char *sha1, int fd, struct stat *st,\n+\t     enum object_type type, const char *path, unsigned flags)\n+{\n+\tint ret;\n+\tsize_t size = xsize_t(st->st_size);\n+\n+\tif (!S_ISREG(st->st_mode))\n+\t\tret = index_pipe(sha1, fd, type, path, flags);\n+\telse\n+\t\tret = index_core(sha1, fd, size, type, path, flags);\n \tclose(fd);\n \treturn ret;\n }\n-- \n1.7.5.1.268.gce5bd\n"},{"id":"167354","messageId":"1304844455-23570-4-git-send-email-gitster@pobox.com","threadId":"27295","inReplyTo":"1304844455-23570-1-git-send-email-gitster@pobox.com","subject":"[PATCH v0 3/3] Bigfile: teach \"git add\" to send a large file straight to a pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-08T08:47:35Z","receivedAt":"2011-05-08T08:47:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When adding a new content to the repository, we have always slurped\nthe blob in its entirety in-core first, and computed the object name\nand compressed it into a loose object file.  Handling large binary\nfiles (e.g.  video and audio asset for games) has been problematic\nbecause of this design.\n\nAt the middle level of \"git add\" callchain is an internal API\nindex_fd() that takes an open file descriptor to read from the\nworking tree file being added with its size. Teach it to call out to\nfast-import when adding a large blob.\n\nThis is merely a POC that has two large \"NEEDSWORK\" items.\n\n * The code in this patch runs fast-import via start_command() API;\n   because the caller needs the object name, we can only stuff one\n   object per pack (see in-code comments for future directions for a\n   possible solution).\n\n * The decision to stream in this patch is based on the size of the\n   blob, but it should be tied to an attribute, \"bigdata\". The\n   attribute should also mean the blob will not undergo any\n   convert-to-git processing.\n\nThe write-out codepath in entry.c::write_entry() should be taught to\nstream, instead of reading everything in core. This should not be so\nhard to implement, especially if we limit ourselves only to loose\nobject files and non-delta representation in packfiles.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_file.c      |  102 +++++++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t1050-large.sh |   22 ++++++++++++\n 2 files changed, 123 insertions(+), 1 deletions(-)\n create mode 100755 t/t1050-large.sh\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 49416b0..ef1a698 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -11,6 +11,7 @@\n #include \"pack.h\"\n #include \"blob.h\"\n #include \"commit.h\"\n+#include \"run-command.h\"\n #include \"tag.h\"\n #include \"tree.h\"\n #include \"tree-walk.h\"\n@@ -2658,6 +2659,103 @@ static int index_core(unsigned char *sha1, int fd, size_t size,\n \treturn ret;\n }\n \n+/*\n+ * NEEDSWORK: This creates one packfile per large blob, because the\n+ * caller immediately wants the result sha1, and fast-import can\n+ * report the object name via marks mechanism only by closing the\n+ * created packfile. We should instead add an internal \"stuff objects\n+ * into a single pack, all in non-delta representation, keeping track\n+ * of only <object-name, offset> tuples in core\" API, that keeps one\n+ * append-only packfile open at a time.  Have the first call to this\n+ * function open a packfile on demand, and make sure the caller calls\n+ * another function in the API to close the packfile at end, at which\n+ * point the in-core tuples of <object-name, offset> should be written\n+ * out as a corresponding pack .idx file and the tentative .pack file\n+ * renamed to the final name.\n+ *\n+ * This also bypasses the usual \"convert-to-git\" dance, but that is on\n+ * purpose. We could write a streaming version of the converting\n+ * functions and insert that before feeding the data to fast-import\n+ * (or equivalent in-core API described above), but the primary\n+ * motivation for trying to stream from the working tree file and to\n+ * avoid mmaping it in core is to deal with large binary blobs, and\n+ * by definition they do _not_ want to get any conversion.\n+ */\n+static int index_stream(unsigned char *sha1, int fd, size_t size,\n+\t\t\tenum object_type type, const char *path,\n+\t\t\tunsigned flags)\n+{\n+\tstruct child_process fast_import;\n+\tchar export_marks[512];\n+\tconst char *argv[] = { \"fast-import\", \"--quiet\", export_marks, NULL };\n+\tchar tmpfile[512];\n+\tchar fast_import_cmd[512];\n+\tchar buf[512];\n+\tint len, tmpfd;\n+\n+\tstrcpy(tmpfile, git_path(\"hashstream_XXXXXX\"));\n+\ttmpfd = git_mkstemp_mode(tmpfile, 0600);\n+\tif (tmpfd < 0)\n+\t\tdie_errno(\"cannot create tempfile: %s\", tmpfile);\n+\tif (close(tmpfd))\n+\t\tdie_errno(\"cannot close tempfile: %s\", tmpfile);\n+\tsprintf(export_marks, \"--export-marks=%s\", tmpfile);\n+\n+\tmemset(&fast_import, 0, sizeof(fast_import));\n+\tfast_import.in = -1;\n+\tfast_import.argv = argv;\n+\tfast_import.git_cmd = 1;\n+\tif (start_command(&fast_import))\n+\t\tdie_errno(\"index-stream: git fast-import failed\");\n+\n+\tlen = sprintf(fast_import_cmd, \"blob\\nmark :1\\ndata %lu\\n\",\n+\t\t      (unsigned long) size);\n+\twrite_or_whine(fast_import.in, fast_import_cmd, len,\n+\t\t       \"index-stream: feeding fast-import\");\n+\twhile (size) {\n+\t\tchar buf[10240];\n+\t\tsize_t sz = size < sizeof(buf) ? size : sizeof(buf);\n+\t\tsize_t actual;\n+\n+\t\tactual = read_in_full(fd, buf, sz);\n+\t\tif (actual < 0)\n+\t\t\tdie_errno(\"index-stream: reading input\");\n+\t\tif (write_in_full(fast_import.in, buf, actual) != actual)\n+\t\t\tdie_errno(\"index-stream: feeding fast-import\");\n+\t\tsize -= actual;\n+\t}\n+\tif (close(fast_import.in))\n+\t\tdie_errno(\"index-stream: closing fast-import\");\n+\tif (finish_command(&fast_import))\n+\t\tdie_errno(\"index-stream: finishing fast-import\");\n+\n+\ttmpfd = open(tmpfile, O_RDONLY);\n+\tif (tmpfd < 0)\n+\t\tdie_errno(\"index-stream: cannot open fast-import mark\");\n+\tlen = read(tmpfd, buf, sizeof(buf));\n+\tif (len < 0)\n+\t\tdie_errno(\"index-stream: reading fast-import mark\");\n+\tif (close(tmpfd) < 0)\n+\t\tdie_errno(\"index-stream: closing fast-import mark\");\n+\tif (unlink(tmpfile))\n+\t\tdie_errno(\"index-stream: unlinking fast-import mark\");\n+\tif (len != 44 ||\n+\t    memcmp(\":1 \", buf, 3) ||\n+\t    get_sha1_hex(buf + 3, sha1))\n+\t\tdie_errno(\"index-stream: unexpected fast-import mark: <%s>\", buf);\n+\treturn 0;\n+}\n+\n+/*\n+ * NEEDSWORK: Currently, we choose blobs that are bigger than\n+ * core.bigFileThreshold to send out to a pack, but we might want to\n+ * control this by choosing any blob that has \"bigdata\" attribute on\n+ * path, regardless of its size. By design, index_stream() ignores\n+ * any \"convert-to-git\" conversions, so associating an attribute with\n+ * the logic to choose which paths to give index_stream() makes it\n+ * easier to explain---we can say \"bigdata will trump all the crlf,\n+ * clean, smudge, ident and any other conversion related crap.\n+ */\n int index_fd(unsigned char *sha1, int fd, struct stat *st,\n \t     enum object_type type, const char *path, unsigned flags)\n {\n@@ -2666,8 +2764,10 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,\n \n \tif (!S_ISREG(st->st_mode))\n \t\tret = index_pipe(sha1, fd, type, path, flags);\n-\telse\n+\telse if (size <= big_file_threshold && type != OBJ_BLOB)\n \t\tret = index_core(sha1, fd, size, type, path, flags);\n+\telse\n+\t\tret = index_stream(sha1, fd, size, type, path, flags);\n \tclose(fd);\n \treturn ret;\n }\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nnew file mode 100755\nindex 0000000..489c0f0\n--- /dev/null\n+++ b/t/t1050-large.sh\n@@ -0,0 +1,22 @@\n+#!/bin/sh\n+# Copyright (c) 2011, Google Inc.\n+\n+test_description='adding and checking out large blobs'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tgit config core.bigfilethreshold 200k &&\n+\techo X | dd of=large bs=1k seek=2000\n+'\n+\n+test_expect_success 'add a large file' '\n+\tgit add large &&\n+\tgit cat-file blob :large >actual &&\n+\t# make sure we got a packfile and no loose objects\n+\ttest -f .git/objects/pack/pack-*.pack &&\n+\ttest ! -f .git/objects/??/?????????????????????????????????????? &&\n+\tcmp -s large actual  # This should be \"cmp\", not \"test_cmp\"\n+'\n+\n+test_done\n-- \n1.7.5.1.268.gce5bd\n"},{"id":"167359","messageId":"20110508101925.GA19525@do","threadId":"27295","inReplyTo":"1304844455-23570-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v0 3/3] Bigfile: teach \"git add\" to send a large file straight to a pack","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-08T10:19:25Z","receivedAt":"2011-05-08T10:19:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, May 08, 2011 at 01:47:35AM -0700, Junio C Hamano wrote:\n>  * The decision to stream in this patch is based on the size of the\n>    blob, but it should be tied to an attribute, \"bigdata\". The\n>    attribute should also mean the blob will not undergo any\n>    convert-to-git processing.\n\nInstead we could detect if convert-from/to-git is unnecessary and\nstream big blobs based on core.bigFileThresold. The below patch may\nhelp. I made it in order to checkout files without inflating in memory\nbut that part's still not ready.\n\nBy the way, is there any way we can mark an object in the pack as\n\"non-compressed\" so we can read it without inflating? zlib compression\nlevel == 0 still splits the object into multiple parts.\n\n-- 8< --\nSubject: [PATCH] convert: add functions to determine whether conversion is needed\n\nBlob conversion from/to repository requires the entire blob in memory.\nThe conversion is rarely used most of the time and that requirement\ncould put more pressure on memory for large blobs.\n\nAdd two functions to determine early whether we can bypass conversion\nwithout looking at the content.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n cache.h   |    3 ++\n convert.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++-------------\n 2 files changed, 65 insertions(+), 17 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 08a9022..be3845d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1091,7 +1091,10 @@ extern void trace_repo_setup(const char *prefix);\n /* returns 1 if *dst was used */\n extern int convert_to_git(const char *path, const char *src, size_t len,\n                           struct strbuf *dst, enum safe_crlf checksafe);\n+extern int convert_to_git_needed(const char *path, size_t len,\n+\t\t\t\t enum safe_crlf checksafe);\n extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);\n+extern int convert_to_working_tree_needed(const char *path, size_t len);\n extern int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst);\n \n /* add */\ndiff --git a/convert.c b/convert.c\nindex d5aebed..39545ed 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -188,7 +188,8 @@ static int has_cr_in_index(const char *path)\n }\n \n static int crlf_to_git(const char *path, const char *src, size_t len,\n-\t\t       struct strbuf *buf, enum action action, enum safe_crlf checksafe)\n+\t\t       struct strbuf *buf, enum action action,\n+\t\t       enum safe_crlf checksafe, int dry_run)\n {\n \tstruct text_stat stats;\n \tchar *dst;\n@@ -197,6 +198,9 @@ static int crlf_to_git(const char *path, const char *src, size_t len,\n \t    (action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE) || !len)\n \t\treturn 0;\n \n+\tif (dry_run)\n+\t\treturn 1;\n+\n \tgather_stats(src, len, &stats);\n \n \tif (action == CRLF_AUTO || action == CRLF_GUESS) {\n@@ -257,7 +261,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,\n }\n \n static int crlf_to_worktree(const char *path, const char *src, size_t len,\n-\t\t\t    struct strbuf *buf, enum action action)\n+\t\t\t    struct strbuf *buf, enum action action, int dry_run)\n {\n \tchar *to_free = NULL;\n \tstruct text_stat stats;\n@@ -265,6 +269,9 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,\n \tif (!len || determine_output_conversion(action) != EOL_CRLF)\n \t\treturn 0;\n \n+\tif (dry_run)\n+\t\treturn 1;\n+\n \tgather_stats(src, len, &stats);\n \n \t/* No LF? Nothing to convert, regardless. */\n@@ -374,7 +381,7 @@ static int filter_buffer(int in, int out, void *data)\n }\n \n static int apply_filter(const char *path, const char *src, size_t len,\n-                        struct strbuf *dst, const char *cmd)\n+\t\t\tstruct strbuf *dst, const char *cmd, int dry_run)\n {\n \t/*\n \t * Create a pipeline to have the command filter the buffer's\n@@ -390,6 +397,9 @@ static int apply_filter(const char *path, const char *src, size_t len,\n \tif (!cmd)\n \t\treturn 0;\n \n+\tif (dry_run)\n+\t\treturn 1;\n+\n \tmemset(&async, 0, sizeof(async));\n \tasync.proc = filter_buffer;\n \tasync.data = &params;\n@@ -541,11 +551,17 @@ static int count_ident(const char *cp, unsigned long size)\n }\n \n static int ident_to_git(const char *path, const char *src, size_t len,\n-                        struct strbuf *buf, int ident)\n+\t\t\tstruct strbuf *buf, int ident, int dry_run)\n {\n \tchar *dst, *dollar;\n \n-\tif (!ident || !count_ident(src, len))\n+\tif (!ident)\n+\t\treturn 0;\n+\n+\tif (dry_run)\n+\t\treturn 1;\n+\n+\tif(!count_ident(src, len))\n \t\treturn 0;\n \n \t/* only grow if not in place */\n@@ -582,7 +598,7 @@ static int ident_to_git(const char *path, const char *src, size_t len,\n }\n \n static int ident_to_worktree(const char *path, const char *src, size_t len,\n-                             struct strbuf *buf, int ident)\n+\t\t\t     struct strbuf *buf, int ident, int dry_run)\n {\n \tunsigned char sha1[20];\n \tchar *to_free = NULL, *dollar, *spc;\n@@ -591,6 +607,9 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,\n \tif (!ident)\n \t\treturn 0;\n \n+\tif (dry_run)\n+\t\treturn 1;\n+\n \tcnt = count_ident(src, len);\n \tif (!cnt)\n \t\treturn 0;\n@@ -726,8 +745,9 @@ static enum action determine_action(enum action text_attr, enum eol eol_attr)\n \treturn text_attr;\n }\n \n-int convert_to_git(const char *path, const char *src, size_t len,\n-                   struct strbuf *dst, enum safe_crlf checksafe)\n+static int convert_to_git_1(const char *path, const char *src, size_t len,\n+\t\t\t    struct strbuf *dst, enum safe_crlf checksafe,\n+\t\t\t    int dry_run)\n {\n \tstruct git_attr_check check[5];\n \tenum action action = CRLF_GUESS;\n@@ -748,23 +768,39 @@ int convert_to_git(const char *path, const char *src, size_t len,\n \t\t\tfilter = drv->clean;\n \t}\n \n-\tret |= apply_filter(path, src, len, dst, filter);\n+\tret |= apply_filter(path, src, len, dst, filter, dry_run);\n \tif (ret) {\n+\t\tif (dry_run)\n+\t\t\treturn 1;\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n \taction = determine_action(action, eol_attr);\n-\tret |= crlf_to_git(path, src, len, dst, action, checksafe);\n+\tret |= crlf_to_git(path, src, len, dst, action, checksafe, dry_run);\n \tif (ret) {\n+\t\tif (dry_run)\n+\t\t\treturn 1;\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\treturn ret | ident_to_git(path, src, len, dst, ident);\n+\treturn ret | ident_to_git(path, src, len, dst, ident, dry_run);\n+}\n+\n+int convert_to_git(const char *path, const char *src, size_t len,\n+\t\t   struct strbuf *dst, enum safe_crlf checksafe)\n+{\n+\treturn convert_to_git_1(path, src, len, dst, checksafe, 0);\n+}\n+\n+int convert_to_git_needed(const char *path, size_t len,\n+\t\t\t  enum safe_crlf checksafe)\n+{\n+\treturn convert_to_git_1(path, NULL, len, NULL, checksafe, 1);\n }\n \n static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t\t\t\t    size_t len, struct strbuf *dst,\n-\t\t\t\t\t    int normalizing)\n+\t\t\t\t\t    int normalizing, int dry_run)\n {\n \tstruct git_attr_check check[5];\n \tenum action action = CRLF_GUESS;\n@@ -785,8 +821,10 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t\tfilter = drv->smudge;\n \t}\n \n-\tret |= ident_to_worktree(path, src, len, dst, ident);\n+\tret |= ident_to_worktree(path, src, len, dst, ident, dry_run);\n \tif (ret) {\n+\t\tif (dry_run)\n+\t\t\treturn 1;\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n@@ -796,23 +834,30 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t */\n \tif (filter || !normalizing) {\n \t\taction = determine_action(action, eol_attr);\n-\t\tret |= crlf_to_worktree(path, src, len, dst, action);\n+\t\tret |= crlf_to_worktree(path, src, len, dst, action, dry_run);\n \t\tif (ret) {\n+\t\t\tif (dry_run)\n+\t\t\t\treturn 1;\n \t\t\tsrc = dst->buf;\n \t\t\tlen = dst->len;\n \t\t}\n \t}\n-\treturn ret | apply_filter(path, src, len, dst, filter);\n+\treturn ret | apply_filter(path, src, len, dst, filter, dry_run);\n }\n \n int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)\n {\n-\treturn convert_to_working_tree_internal(path, src, len, dst, 0);\n+\treturn convert_to_working_tree_internal(path, src, len, dst, 0, 0);\n+}\n+\n+int convert_to_working_tree_needed(const char *path, size_t len)\n+{\n+\treturn convert_to_working_tree_internal(path, NULL, len, NULL, 0, 1);\n }\n \n int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst)\n {\n-\tint ret = convert_to_working_tree_internal(path, src, len, dst, 1);\n+\tint ret = convert_to_working_tree_internal(path, src, len, dst, 1, 0);\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n-- \n1.7.4.74.g639db\n\n\n-- 8< --\n\n-- \nDuy\n"},{"id":"167434","messageId":"7vk4e1t7p4.fsf@alter.siamese.dyndns.org","threadId":"27295","inReplyTo":"20110508101925.GA19525@do","subject":"Re: [PATCH v0 3/3] Bigfile: teach \"git add\" to send a large file straight to a pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-08T17:37:11Z","receivedAt":"2011-05-08T17:37:11Z","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> By the way, is there any way we can mark an object in the pack as\n> \"non-compressed\" so we can read it without inflating?\n\nNo. Such a packfile will be unusable by existing git, so I would suggest\nyou not even think about going there.  It will not happen.\n"},{"id":"167500","messageId":"BANLkTimo_D_s-=zjPAbStvETRh8d_Tpa_w@mail.gmail.com","threadId":"27295","inReplyTo":"1304844455-23570-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v0 3/3] Bigfile: teach \"git add\" to send a large file straight to a pack","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2011-05-09T14:05:00Z","receivedAt":"2011-05-09T14:05:00Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Sun, May 8, 2011 at 01:47, Junio C Hamano <gitster@pobox.com> wrote:\n> + * NEEDSWORK: This creates one packfile per large blob, because the\n> + * caller immediately wants the result sha1, and fast-import can\n> + * report the object name via marks mechanism only by closing the\n> + * created packfile. We should instead add an internal \"stuff objects\n> + * into a single pack, all in non-delta representation, keeping track\n> + * of only <object-name, offset> tuples in core\" API, that keeps one\n> + * append-only packfile open at a time.  Have the first call to this\n> + * function open a packfile on demand, and make sure the caller calls\n> + * another function in the API to close the packfile at end, at which\n> + * point the in-core tuples of <object-name, offset> should be written\n> + * out as a corresponding pack .idx file and the tentative .pack file\n> + * renamed to the final name.\n\nThe other problem here is the caller cannot access the written objects\nuntil the pack is closed. That is one of the things that has made\nfast-import difficult for git-svn to use, because git-svn expects the\nobject to be available immediately. I assume that within a single git\nadd or git update-index process we don't need to worry about this, so\nits probably a non-issue.\n\n-- \nShawn.\n"},{"id":"167506","messageId":"7v4o53rhkv.fsf@alter.siamese.dyndns.org","threadId":"27295","inReplyTo":"BANLkTimo_D_s-=zjPAbStvETRh8d_Tpa_w@mail.gmail.com","subject":"Re: [PATCH v0 3/3] Bigfile: teach \"git add\" to send a large file straight to a pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T15:58:56Z","receivedAt":"2011-05-09T15:58:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce <spearce@spearce.org> writes:\n\n> The other problem here is the caller cannot access the written objects\n> until the pack is closed. That is one of the things that has made\n> fast-import difficult for git-svn to use, because git-svn expects the\n> object to be available immediately. I assume that within a single git\n> add or git update-index process we don't need to worry about this, so\n> its probably a non-issue.\n\nYes, it is part of a possible issue to be addressed in the plan.\n\nI envisioned that the \"API\" I talked about in the NEEDSWORK you quoted\nwould keep an open file descriptor to the \"currently being built\" packfile\nwrapped in a \"struct packed_git\", with an in-core index_data that is\nadjusted every time you add a straight-to-pack kind of object. Upon a\n\"finalize\" call, it would determines the final pack name, write the real\npack .idx file out, and rename the \"being built\" packfile to the final\nname to make it available to the outside world.\n\nWithin a single git process that approach would give access to the set of\nobjects that are going straight to the pack.  When it needs to spawn a git\nsubprocess, it however would need to finalize the pack to give access to\nthe new object, just like when fast-import flushes when asked to expose\nthe marks.\n\nAfter all, this topic is about handling large binary files that would not\nfit in core at once (we do not support them now at all). It may not too\nbad to say we stuff one object per packfile and immediately close the\npackfile (which is what the use of fast-import by the POC patch\ndoes). Once the packfile is closed, the object in it is automatically\navailable to the outside world, and it is just the matter of making a\nreprepare_packed_git() call to make it available to ourselves. When there\nare many such objects, as they would exceed bigfilethreashold, repacking\nthem would just amount to copying the already compressed data literally (I\nhaven't re-checked the code though) and the cost shouldn't be more than\nproportional to the size of the data. Expecting any system to do better\nthan that is asking for moon and I am not willing to bend backwards to\ncater to such demands before running out of other better things to do ;-).\n\nSo I am tempted to keep the \"spawn an external fast-import\" code at least\nfor now, and give it a higher priority to make the other side (writing out\nthe blob to a working tree) streamable.\n"},{"id":"167507","messageId":"7vzkmvq2b2.fsf@alter.siamese.dyndns.org","threadId":"27295","inReplyTo":"7v4o53rhkv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v0 3/3] Bigfile: teach \"git add\" to send a large file straight to a pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T16:14:09Z","receivedAt":"2011-05-09T16:14:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I envisioned that the \"API\" I talked about in the NEEDSWORK you quoted\n> would keep an open file descriptor to the \"currently being built\" packfile\n> wrapped in a \"struct packed_git\", with an in-core index_data that is\n> adjusted every time you add a straight-to-pack kind of object. Upon a\n> \"finalize\" call, it would determines the final pack name, write the real\n> pack .idx file out, and rename the \"being built\" packfile to the final\n> name to make it available to the outside world.\n>\n> Within a single git process that approach would give access to the set of\n> objects that are going straight to the pack.  When it needs to spawn a git\n> subprocess, it however would need to finalize the pack to give access to\n> the new object, just like when fast-import flushes when asked to expose\n> the marks.\n>\n> After all, this topic is about handling large binary files that would not\n> fit in core at once (we do not support them now at all). It may not too\n> bad to say we stuff one object per packfile and immediately close the\n> packfile (which is what the use of fast-import by the POC patch\n> does).\n\nA (tentatively final) side note.\n\nThe primary reason why I wanted to think about using a single packfile\nthat is kept open and add multiple objects to the pack was because we may\nlater want to use this kind of set-up for \"initial import\", regardless of\nthe size of the object being added.  But now I think about it I do not\nthink that use case matters a lot.  The resulting single pack would have\nmuch worse object density, compared to the case where you add them\nnormally, initially creating loose object files and then repack/gc at\nwhich time you are likely to have more than one rev to sanely deltify.\n\nUsing one pack per large object while creating is not too bad to begin\nwith.  If you had a large enough core to hold such a large binary file,\nthe current system would store it as a single loose object file, so it is\nnot like we are making things any worse.  In either form, these \"single\nobject per a file\" initial storage will find their more permanent home\nupon the first repack/gc.\n"},{"id":"167534","messageId":"1304978701-19310-1-git-send-email-gitster@pobox.com","threadId":"27295","inReplyTo":"20110508101925.GA19525@do","subject":"[PATCH 0/4] convert.c clean-up","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T22:04:57Z","receivedAt":"2011-05-09T22:04:57Z","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> Instead we could detect if convert-from/to-git is unnecessary and\n> stream big blobs based on core.bigFileThresold. The below patch may\n> help.\n\nYeah, I think that would probably make more sense. If a file is small\nenough, it is more sensible to send it to a loose object just like any\nother files. We do not want to invite users to make a mistake of marking\nit as bigdata and send it straight to a packfile. Having one less knob to\ntweak is always a good thing to do.\n\nHowever, while reviewing your patch, I noticed that convert.c was littered\nwith misnamed types, variables and functions to the point to make it\nalmost unreadble as the result of its evolution.  I originally wrote this\nseries so that I can add \"bigdata\" sensibly, and it turns out that there\nis no benefit to do so for now, but the clean-up by itself would be worth\nit.\n\nSo there...\n\nJunio C Hamano (4):\n  convert: rename the \"eol\" global variable to \"core_eol\"\n  convert: give saner names to crlf/eol variables, types and functions\n  convert: make it safer to add conversion attributes\n  convert: make it harder to screw up adding a conversion attribute\n\n cache.h       |    2 +-\n config.c      |   12 ++--\n convert.c     |  158 +++++++++++++++++++++++++++-----------------------------\n environment.c |    2 +-\n 4 files changed, 84 insertions(+), 90 deletions(-)\n\n-- \n1.7.5.1.288.g93ebc\n"},{"id":"167532","messageId":"1304978701-19310-2-git-send-email-gitster@pobox.com","threadId":"27295","inReplyTo":"1304978701-19310-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/4] convert: rename the \"eol\" global variable to \"core_eol\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T22:04:58Z","receivedAt":"2011-05-09T22:04:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yes, it is clear that \"eol\" wants to mean some sort of end-of-line thing,\nbut as the name of a global variable, it is way too short to describe what\nkind of end-of-line thing it wants to represent. Besides, there are many\ncodepaths that want to use their own local \"char *eol\" variable to point\nat the end of the current line they are processing.\n\nThis global variable holds what we read from core.eol configuration\nvariable. Name it as such.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n cache.h       |    2 +-\n config.c      |   12 ++++++------\n convert.c     |    4 ++--\n environment.c |    2 +-\n 4 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 2b34116..4e9123b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -606,7 +606,7 @@ enum eol {\n #endif\n };\n \n-extern enum eol eol;\n+extern enum eol core_eol;\n \n enum branch_track {\n \tBRANCH_TRACK_UNSPECIFIED = -1,\ndiff --git a/config.c b/config.c\nindex 5f9ec28..671c8df 100644\n--- a/config.c\n+++ b/config.c\n@@ -583,7 +583,7 @@ static int git_default_core_config(const char *var, const char *value)\n \n \tif (!strcmp(var, \"core.autocrlf\")) {\n \t\tif (value && !strcasecmp(value, \"input\")) {\n-\t\t\tif (eol == EOL_CRLF)\n+\t\t\tif (core_eol == EOL_CRLF)\n \t\t\t\treturn error(\"core.autocrlf=input conflicts with core.eol=crlf\");\n \t\t\tauto_crlf = AUTO_CRLF_INPUT;\n \t\t\treturn 0;\n@@ -603,14 +603,14 @@ static int git_default_core_config(const char *var, const char *value)\n \n \tif (!strcmp(var, \"core.eol\")) {\n \t\tif (value && !strcasecmp(value, \"lf\"))\n-\t\t\teol = EOL_LF;\n+\t\t\tcore_eol = EOL_LF;\n \t\telse if (value && !strcasecmp(value, \"crlf\"))\n-\t\t\teol = EOL_CRLF;\n+\t\t\tcore_eol = EOL_CRLF;\n \t\telse if (value && !strcasecmp(value, \"native\"))\n-\t\t\teol = EOL_NATIVE;\n+\t\t\tcore_eol = EOL_NATIVE;\n \t\telse\n-\t\t\teol = EOL_UNSET;\n-\t\tif (eol == EOL_CRLF && auto_crlf == AUTO_CRLF_INPUT)\n+\t\t\tcore_eol = EOL_UNSET;\n+\t\tif (core_eol == EOL_CRLF && auto_crlf == AUTO_CRLF_INPUT)\n \t\t\treturn error(\"core.autocrlf=input conflicts with core.eol=crlf\");\n \t\treturn 0;\n \t}\ndiff --git a/convert.c b/convert.c\nindex 7eb51b1..4dba329 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -113,10 +113,10 @@ static enum eol determine_output_conversion(enum action action)\n \t\t\treturn EOL_CRLF;\n \t\telse if (auto_crlf == AUTO_CRLF_INPUT)\n \t\t\treturn EOL_LF;\n-\t\telse if (eol == EOL_UNSET)\n+\t\telse if (core_eol == EOL_UNSET)\n \t\t\treturn EOL_NATIVE;\n \t}\n-\treturn eol;\n+\treturn core_eol;\n }\n \n static void check_safe_crlf(const char *path, enum action action,\ndiff --git a/environment.c b/environment.c\nindex 40185bc..7fe9f10 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -43,7 +43,7 @@ const char *askpass_program;\n const char *excludes_file;\n enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;\n int read_replace_refs = 1;\n-enum eol eol = EOL_UNSET;\n+enum eol core_eol = EOL_UNSET;\n enum safe_crlf safe_crlf = SAFE_CRLF_WARN;\n unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n-- \n1.7.5.1.288.g93ebc\n"},{"id":"167530","messageId":"1304978701-19310-3-git-send-email-gitster@pobox.com","threadId":"27295","inReplyTo":"1304978701-19310-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/4] convert: give saner names to crlf/eol variables, types and functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T22:04:59Z","receivedAt":"2011-05-09T22:04:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Back when the conversion was only about the end-of-line convention, it\nmight have made sense to call what we do upon seeing CR/LF simply an\n\"action\", but these days the conversion routines do a lot more than just\ntweaking the line ending.  Raname \"action\" to \"crlf_action\".\n\nThe function that decides what end of line conversion to use on the output\ncodepath was called \"determine_output_conversion\", as if there is no other\nkind of output conversion.  Rename it to \"output_eol\"; it is a function\nthat returns what EOL convention is to be used.\n\nA function that decides what \"crlf_action\" needs to be used on the input\ncodepath, given what conversion attribute is set to the path and global\nend-of-line convention, was called \"determine_action\".  Rename it to\n\"input_crlf_action\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n convert.c |   61 +++++++++++++++++++++++++++++++------------------------------\n 1 files changed, 31 insertions(+), 30 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 4dba329..e0ee245 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -12,7 +12,7 @@\n  * translation when the \"text\" attribute or \"auto_crlf\" option is set.\n  */\n \n-enum action {\n+enum crlf_action {\n \tCRLF_GUESS = -1,\n \tCRLF_BINARY = 0,\n \tCRLF_TEXT,\n@@ -94,9 +94,9 @@ static int is_binary(unsigned long size, struct text_stat *stats)\n \treturn 0;\n }\n \n-static enum eol determine_output_conversion(enum action action)\n+static enum eol output_eol(enum crlf_action crlf_action)\n {\n-\tswitch (action) {\n+\tswitch (crlf_action) {\n \tcase CRLF_BINARY:\n \t\treturn EOL_UNSET;\n \tcase CRLF_CRLF:\n@@ -119,13 +119,13 @@ static enum eol determine_output_conversion(enum action action)\n \treturn core_eol;\n }\n \n-static void check_safe_crlf(const char *path, enum action action,\n+static void check_safe_crlf(const char *path, enum crlf_action crlf_action,\n                             struct text_stat *stats, enum safe_crlf checksafe)\n {\n \tif (!checksafe)\n \t\treturn;\n \n-\tif (determine_output_conversion(action) == EOL_LF) {\n+\tif (output_eol(crlf_action) == EOL_LF) {\n \t\t/*\n \t\t * CRLFs would not be restored by checkout:\n \t\t * check if we'd remove CRLFs\n@@ -136,7 +136,7 @@ static void check_safe_crlf(const char *path, enum action action,\n \t\t\telse /* i.e. SAFE_CRLF_FAIL */\n \t\t\t\tdie(\"CRLF would be replaced by LF in %s.\", path);\n \t\t}\n-\t} else if (determine_output_conversion(action) == EOL_CRLF) {\n+\t} else if (output_eol(crlf_action) == EOL_CRLF) {\n \t\t/*\n \t\t * CRLFs would be added by checkout:\n \t\t * check if we have \"naked\" LFs\n@@ -188,18 +188,19 @@ static int has_cr_in_index(const char *path)\n }\n \n static int crlf_to_git(const char *path, const char *src, size_t len,\n-\t\t       struct strbuf *buf, enum action action, enum safe_crlf checksafe)\n+\t\t       struct strbuf *buf,\n+\t\t       enum crlf_action crlf_action, enum safe_crlf checksafe)\n {\n \tstruct text_stat stats;\n \tchar *dst;\n \n-\tif (action == CRLF_BINARY ||\n-\t    (action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE) || !len)\n+\tif (crlf_action == CRLF_BINARY ||\n+\t    (crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE) || !len)\n \t\treturn 0;\n \n \tgather_stats(src, len, &stats);\n \n-\tif (action == CRLF_AUTO || action == CRLF_GUESS) {\n+\tif (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) {\n \t\t/*\n \t\t * We're currently not going to even try to convert stuff\n \t\t * that has bare CR characters. Does anybody do that crazy\n@@ -214,7 +215,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,\n \t\tif (is_binary(len, &stats))\n \t\t\treturn 0;\n \n-\t\tif (action == CRLF_GUESS) {\n+\t\tif (crlf_action == CRLF_GUESS) {\n \t\t\t/*\n \t\t\t * If the file in the index has any CR in it, do not convert.\n \t\t\t * This is the new safer autocrlf handling.\n@@ -224,7 +225,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,\n \t\t}\n \t}\n \n-\tcheck_safe_crlf(path, action, &stats, checksafe);\n+\tcheck_safe_crlf(path, crlf_action, &stats, checksafe);\n \n \t/* Optimization: No CR? Nothing to convert, regardless. */\n \tif (!stats.cr)\n@@ -234,7 +235,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,\n \tif (strbuf_avail(buf) + buf->len < len)\n \t\tstrbuf_grow(buf, len - buf->len);\n \tdst = buf->buf;\n-\tif (action == CRLF_AUTO || action == CRLF_GUESS) {\n+\tif (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) {\n \t\t/*\n \t\t * If we guessed, we already know we rejected a file with\n \t\t * lone CR, and we can strip a CR without looking at what\n@@ -257,12 +258,12 @@ static int crlf_to_git(const char *path, const char *src, size_t len,\n }\n \n static int crlf_to_worktree(const char *path, const char *src, size_t len,\n-\t\t\t    struct strbuf *buf, enum action action)\n+\t\t\t    struct strbuf *buf, enum crlf_action crlf_action)\n {\n \tchar *to_free = NULL;\n \tstruct text_stat stats;\n \n-\tif (!len || determine_output_conversion(action) != EOL_CRLF)\n+\tif (!len || output_eol(crlf_action) != EOL_CRLF)\n \t\treturn 0;\n \n \tgather_stats(src, len, &stats);\n@@ -275,8 +276,8 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,\n \tif (stats.lf == stats.crlf)\n \t\treturn 0;\n \n-\tif (action == CRLF_AUTO || action == CRLF_GUESS) {\n-\t\tif (action == CRLF_GUESS) {\n+\tif (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) {\n+\t\tif (crlf_action == CRLF_GUESS) {\n \t\t\t/* If we have any CR or CRLF line endings, we do not touch it */\n \t\t\t/* This is the new safer autocrlf-handling */\n \t\t\tif (stats.cr > 0 || stats.crlf > 0)\n@@ -715,7 +716,7 @@ static int git_path_check_ident(const char *path, struct git_attr_check *check)\n \treturn !!ATTR_TRUE(value);\n }\n \n-static enum action determine_action(enum action text_attr, enum eol eol_attr)\n+static enum crlf_action input_crlf_action(enum crlf_action text_attr, enum eol eol_attr)\n {\n \tif (text_attr == CRLF_BINARY)\n \t\treturn CRLF_BINARY;\n@@ -730,7 +731,7 @@ int convert_to_git(const char *path, const char *src, size_t len,\n                    struct strbuf *dst, enum safe_crlf checksafe)\n {\n \tstruct git_attr_check check[5];\n-\tenum action action = CRLF_GUESS;\n+\tenum crlf_action crlf_action = CRLF_GUESS;\n \tenum eol eol_attr = EOL_UNSET;\n \tint ident = 0, ret = 0;\n \tconst char *filter = NULL;\n@@ -738,9 +739,9 @@ int convert_to_git(const char *path, const char *src, size_t len,\n \tsetup_convert_check(check);\n \tif (!git_checkattr(path, ARRAY_SIZE(check), check)) {\n \t\tstruct convert_driver *drv;\n-\t\taction = git_path_check_crlf(path, check + 4);\n-\t\tif (action == CRLF_GUESS)\n-\t\t\taction = git_path_check_crlf(path, check + 0);\n+\t\tcrlf_action = git_path_check_crlf(path, check + 4);\n+\t\tif (crlf_action == CRLF_GUESS)\n+\t\t\tcrlf_action = git_path_check_crlf(path, check + 0);\n \t\tident = git_path_check_ident(path, check + 1);\n \t\tdrv = git_path_check_convert(path, check + 2);\n \t\teol_attr = git_path_check_eol(path, check + 3);\n@@ -753,8 +754,8 @@ int convert_to_git(const char *path, const char *src, size_t len,\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\taction = determine_action(action, eol_attr);\n-\tret |= crlf_to_git(path, src, len, dst, action, checksafe);\n+\tcrlf_action = input_crlf_action(crlf_action, eol_attr);\n+\tret |= crlf_to_git(path, src, len, dst, crlf_action, checksafe);\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n@@ -767,7 +768,7 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t\t\t\t    int normalizing)\n {\n \tstruct git_attr_check check[5];\n-\tenum action action = CRLF_GUESS;\n+\tenum crlf_action crlf_action = CRLF_GUESS;\n \tenum eol eol_attr = EOL_UNSET;\n \tint ident = 0, ret = 0;\n \tconst char *filter = NULL;\n@@ -775,9 +776,9 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \tsetup_convert_check(check);\n \tif (!git_checkattr(path, ARRAY_SIZE(check), check)) {\n \t\tstruct convert_driver *drv;\n-\t\taction = git_path_check_crlf(path, check + 4);\n-\t\tif (action == CRLF_GUESS)\n-\t\t\taction = git_path_check_crlf(path, check + 0);\n+\t\tcrlf_action = git_path_check_crlf(path, check + 4);\n+\t\tif (crlf_action == CRLF_GUESS)\n+\t\t\tcrlf_action = git_path_check_crlf(path, check + 0);\n \t\tident = git_path_check_ident(path, check + 1);\n \t\tdrv = git_path_check_convert(path, check + 2);\n \t\teol_attr = git_path_check_eol(path, check + 3);\n@@ -795,8 +796,8 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t * is a smudge filter.  The filter might expect CRLFs.\n \t */\n \tif (filter || !normalizing) {\n-\t\taction = determine_action(action, eol_attr);\n-\t\tret |= crlf_to_worktree(path, src, len, dst, action);\n+\t\tcrlf_action = input_crlf_action(crlf_action, eol_attr);\n+\t\tret |= crlf_to_worktree(path, src, len, dst, crlf_action);\n \t\tif (ret) {\n \t\t\tsrc = dst->buf;\n \t\t\tlen = dst->len;\n-- \n1.7.5.1.288.g93ebc\n"},{"id":"167533","messageId":"1304978701-19310-4-git-send-email-gitster@pobox.com","threadId":"27295","inReplyTo":"1304978701-19310-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/4] convert: make it safer to add conversion attributes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T22:05:00Z","receivedAt":"2011-05-09T22:05:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The places that need to pass an array of \"struct git_attr_check\" needed to\nbe careful to pass a large enough array and know what index each element\nlied.  Make it safer and easier to code these.\n\nBesides, the hard-coded sequence of initializing various attributes was\ntoo ugly after we gained more than a few attributes.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n convert.c |   48 ++++++++++++++++++++++--------------------------\n 1 files changed, 22 insertions(+), 26 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex e0ee245..a05820b 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -475,30 +475,6 @@ static int read_convert_config(const char *var, const char *value, void *cb)\n \treturn 0;\n }\n \n-static void setup_convert_check(struct git_attr_check *check)\n-{\n-\tstatic struct git_attr *attr_text;\n-\tstatic struct git_attr *attr_crlf;\n-\tstatic struct git_attr *attr_eol;\n-\tstatic struct git_attr *attr_ident;\n-\tstatic struct git_attr *attr_filter;\n-\n-\tif (!attr_text) {\n-\t\tattr_text = git_attr(\"text\");\n-\t\tattr_crlf = git_attr(\"crlf\");\n-\t\tattr_eol = git_attr(\"eol\");\n-\t\tattr_ident = git_attr(\"ident\");\n-\t\tattr_filter = git_attr(\"filter\");\n-\t\tuser_convert_tail = &user_convert;\n-\t\tgit_config(read_convert_config, NULL);\n-\t}\n-\tcheck[0].attr = attr_crlf;\n-\tcheck[1].attr = attr_ident;\n-\tcheck[2].attr = attr_filter;\n-\tcheck[3].attr = attr_eol;\n-\tcheck[4].attr = attr_text;\n-}\n-\n static int count_ident(const char *cp, unsigned long size)\n {\n \t/*\n@@ -727,10 +703,30 @@ static enum crlf_action input_crlf_action(enum crlf_action text_attr, enum eol e\n \treturn text_attr;\n }\n \n+static const char *conv_attr_name[] = {\n+\t\"crlf\", \"ident\", \"filter\", \"eol\", \"text\",\n+};\n+#define NUM_CONV_ATTRS ARRAY_SIZE(conv_attr_name)\n+\n+static void setup_convert_check(struct git_attr_check *check)\n+{\n+\tint i;\n+\tstatic struct git_attr_check ccheck[NUM_CONV_ATTRS];\n+\n+\tif (!ccheck[0].attr) {\n+\t\tfor (i = 0; i < NUM_CONV_ATTRS; i++)\n+\t\t\tccheck[i].attr = git_attr(conv_attr_name[i]);\n+\t\tuser_convert_tail = &user_convert;\n+\t\tgit_config(read_convert_config, NULL);\n+\t}\n+\tfor (i = 0; i < NUM_CONV_ATTRS; i++)\n+\t\tcheck[i].attr = ccheck[i].attr;\n+}\n+\n int convert_to_git(const char *path, const char *src, size_t len,\n                    struct strbuf *dst, enum safe_crlf checksafe)\n {\n-\tstruct git_attr_check check[5];\n+\tstruct git_attr_check check[NUM_CONV_ATTRS];\n \tenum crlf_action crlf_action = CRLF_GUESS;\n \tenum eol eol_attr = EOL_UNSET;\n \tint ident = 0, ret = 0;\n@@ -767,7 +763,7 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t\t\t\t    size_t len, struct strbuf *dst,\n \t\t\t\t\t    int normalizing)\n {\n-\tstruct git_attr_check check[5];\n+\tstruct git_attr_check check[NUM_CONV_ATTRS];\n \tenum crlf_action crlf_action = CRLF_GUESS;\n \tenum eol eol_attr = EOL_UNSET;\n \tint ident = 0, ret = 0;\n-- \n1.7.5.1.288.g93ebc\n"},{"id":"167531","messageId":"1304978701-19310-5-git-send-email-gitster@pobox.com","threadId":"27295","inReplyTo":"1304978701-19310-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/4] convert: make it harder to screw up adding a conversion attribute","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T22:05:01Z","receivedAt":"2011-05-09T22:05:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The current internal API requires the callers of setup_convert_check() to\nsupply the git_attr_check structures (hence they need to know how many to\nallocate), but they grab the same set of attributes for given path.\n\nDefine a new convert_attrs() API that fills a higher level information that\nthe callers (convert_to_git and convert_to_working_tree) really want, and\nmove the common code to interact with the attributes system to it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n convert.c |   79 +++++++++++++++++++++++++++++-------------------------------\n 1 files changed, 38 insertions(+), 41 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex a05820b..efc7e07 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -703,12 +703,19 @@ static enum crlf_action input_crlf_action(enum crlf_action text_attr, enum eol e\n \treturn text_attr;\n }\n \n+struct conv_attrs {\n+\tstruct convert_driver *drv;\n+\tenum crlf_action crlf_action;\n+\tenum eol eol_attr;\n+\tint ident;\n+};\n+\n static const char *conv_attr_name[] = {\n \t\"crlf\", \"ident\", \"filter\", \"eol\", \"text\",\n };\n #define NUM_CONV_ATTRS ARRAY_SIZE(conv_attr_name)\n \n-static void setup_convert_check(struct git_attr_check *check)\n+static void convert_attrs(struct conv_attrs *ca, const char *path)\n {\n \tint i;\n \tstatic struct git_attr_check ccheck[NUM_CONV_ATTRS];\n@@ -719,70 +726,60 @@ static void setup_convert_check(struct git_attr_check *check)\n \t\tuser_convert_tail = &user_convert;\n \t\tgit_config(read_convert_config, NULL);\n \t}\n-\tfor (i = 0; i < NUM_CONV_ATTRS; i++)\n-\t\tcheck[i].attr = ccheck[i].attr;\n+\n+\tif (!git_checkattr(path, NUM_CONV_ATTRS, ccheck)) {\n+\t\tca->crlf_action = git_path_check_crlf(path, ccheck + 4);\n+\t\tif (ca->crlf_action == CRLF_GUESS)\n+\t\t\tca->crlf_action = git_path_check_crlf(path, ccheck + 0);\n+\t\tca->ident = git_path_check_ident(path, ccheck + 1);\n+\t\tca->drv = git_path_check_convert(path, ccheck + 2);\n+\t\tca->eol_attr = git_path_check_eol(path, ccheck + 3);\n+\t} else {\n+\t\tca->drv = NULL;\n+\t\tca->crlf_action = CRLF_GUESS;\n+\t\tca->eol_attr = EOL_UNSET;\n+\t\tca->ident = 0;\n+\t}\n }\n \n int convert_to_git(const char *path, const char *src, size_t len,\n                    struct strbuf *dst, enum safe_crlf checksafe)\n {\n-\tstruct git_attr_check check[NUM_CONV_ATTRS];\n-\tenum crlf_action crlf_action = CRLF_GUESS;\n-\tenum eol eol_attr = EOL_UNSET;\n-\tint ident = 0, ret = 0;\n+\tint ret = 0;\n \tconst char *filter = NULL;\n+\tstruct conv_attrs ca;\n \n-\tsetup_convert_check(check);\n-\tif (!git_checkattr(path, ARRAY_SIZE(check), check)) {\n-\t\tstruct convert_driver *drv;\n-\t\tcrlf_action = git_path_check_crlf(path, check + 4);\n-\t\tif (crlf_action == CRLF_GUESS)\n-\t\t\tcrlf_action = git_path_check_crlf(path, check + 0);\n-\t\tident = git_path_check_ident(path, check + 1);\n-\t\tdrv = git_path_check_convert(path, check + 2);\n-\t\teol_attr = git_path_check_eol(path, check + 3);\n-\t\tif (drv && drv->clean)\n-\t\t\tfilter = drv->clean;\n-\t}\n+\tconvert_attrs(&ca, path);\n+\tif (ca.drv)\n+\t\tfilter = ca.drv->clean;\n \n \tret |= apply_filter(path, src, len, dst, filter);\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\tcrlf_action = input_crlf_action(crlf_action, eol_attr);\n-\tret |= crlf_to_git(path, src, len, dst, crlf_action, checksafe);\n+\tca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);\n+\tret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe);\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\treturn ret | ident_to_git(path, src, len, dst, ident);\n+\treturn ret | ident_to_git(path, src, len, dst, ca.ident);\n }\n \n static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t\t\t\t    size_t len, struct strbuf *dst,\n \t\t\t\t\t    int normalizing)\n {\n-\tstruct git_attr_check check[NUM_CONV_ATTRS];\n-\tenum crlf_action crlf_action = CRLF_GUESS;\n-\tenum eol eol_attr = EOL_UNSET;\n-\tint ident = 0, ret = 0;\n+\tint ret = 0;\n \tconst char *filter = NULL;\n+\tstruct conv_attrs ca;\n \n-\tsetup_convert_check(check);\n-\tif (!git_checkattr(path, ARRAY_SIZE(check), check)) {\n-\t\tstruct convert_driver *drv;\n-\t\tcrlf_action = git_path_check_crlf(path, check + 4);\n-\t\tif (crlf_action == CRLF_GUESS)\n-\t\t\tcrlf_action = git_path_check_crlf(path, check + 0);\n-\t\tident = git_path_check_ident(path, check + 1);\n-\t\tdrv = git_path_check_convert(path, check + 2);\n-\t\teol_attr = git_path_check_eol(path, check + 3);\n-\t\tif (drv && drv->smudge)\n-\t\t\tfilter = drv->smudge;\n-\t}\n+\tconvert_attrs(&ca, path);\n+\tif (ca.drv)\n+\t\tfilter = ca.drv->smudge;\n \n-\tret |= ident_to_worktree(path, src, len, dst, ident);\n+\tret |= ident_to_worktree(path, src, len, dst, ca.ident);\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n@@ -792,8 +789,8 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t * is a smudge filter.  The filter might expect CRLFs.\n \t */\n \tif (filter || !normalizing) {\n-\t\tcrlf_action = input_crlf_action(crlf_action, eol_attr);\n-\t\tret |= crlf_to_worktree(path, src, len, dst, crlf_action);\n+\t\tca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);\n+\t\tret |= crlf_to_worktree(path, src, len, dst, ca.crlf_action);\n \t\tif (ret) {\n \t\t\tsrc = dst->buf;\n \t\t\tlen = dst->len;\n-- \n1.7.5.1.288.g93ebc\n"},{"id":"167598","messageId":"BANLkTi=fjLkqWB2MTGtKE93qSjXU76cpvA@mail.gmail.com","threadId":"27295","inReplyTo":"1304978701-19310-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/4] convert.c clean-up","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-10T12:59:29Z","receivedAt":"2011-05-10T12:59:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, May 10, 2011 at 5:04 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Yeah, I think that would probably make more sense. If a file is small\n> enough, it is more sensible to send it to a loose object just like any\n> other files. We do not want to invite users to make a mistake of marking\n> it as bigdata and send it straight to a packfile. Having one less knob to\n> tweak is always a good thing to do.\n>\n> However, while reviewing your patch, I noticed that convert.c was littered\n> with misnamed types, variables and functions to the point to make it\n> almost unreadble as the result of its evolution.  I originally wrote this\n> series so that I can add \"bigdata\" sensibly, and it turns out that there\n> is no benefit to do so for now, but the clean-up by itself would be worth\n> it.\n\nI still don't like \"bigdata\" attribute. It sounds overlapping with\nbigFileThreshold we already have. Maybe \"inPack\", \"packed\" or\n\"noLoose\" a better name? It makes it quite clear that this attribute\nsends objects to a pack. If they want to process tiny files this way\nby setting inPack/noLoose, I don't care. But files larger than\ncore.bigFileThreshold should be automatically marked \"inPack/noLoose\".\n\n> So there...\n\nYeah, I wish you did this before I touched convert.c. Anyway it looks\nbetter from now on.\n-- \nDuy\n"},{"id":"167602","messageId":"7voc3alk9c.fsf@alter.siamese.dyndns.org","threadId":"27295","inReplyTo":"BANLkTi=fjLkqWB2MTGtKE93qSjXU76cpvA@mail.gmail.com","subject":"Re: [PATCH 0/4] convert.c clean-up","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-10T14:09:51Z","receivedAt":"2011-05-10T14:09:51Z","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 10, 2011 at 5:04 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Yeah, I think that would probably make more sense. If a file is small\n>> enough, it is more sensible to send it to a loose object just like any\n>> other files. We do not want to invite users to make a mistake of marking\n>> it as bigdata and send it straight to a packfile. Having one less knob to\n>> tweak is always a good thing to do.\n>>\n>> However, while reviewing your patch, I noticed that convert.c was littered\n>> with misnamed types, variables and functions to the point to make it\n>> almost unreadble as the result of its evolution. I originally wrote this\n>> series so that I can add \"bigdata\" sensibly, and it turns out that there\n>> is no benefit to do so for now, but the clean-up by itself would be worth\n>> it.\n>\n> I still don't like \"bigdata\" attribute.\n\nI think we are in agreement. I do not like it anymore and that was what I\ntried to say in the first paragraph, and that was why I said \"it turns out\nthat there is no benefit\" in the second paragraph.\n"},{"id":"168946","messageId":"BANLkTikB3VhB4nTZhjE+3znfVTuBYsnehg@mail.gmail.com","threadId":"27295","inReplyTo":"1304844455-23570-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v0 3/3] Bigfile: teach \"git add\" to send a large file straight to a pack","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-05-29T18:20:01Z","receivedAt":"2011-05-29T18:20:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, May 8, 2011 at 10:47, Junio C Hamano <gitster@pobox.com> wrote:\n\n> +               char buf[10240];\n> +               size_t sz = size < sizeof(buf) ? size : sizeof(buf);\n> +               size_t actual;\n> +\n> +               actual = read_in_full(fd, buf, sz);\n> +               if (actual < 0)\n> +                       die_errno(\"index-stream: reading input\");\n\nFrom clang:\n\n    sha1_file.c:2710:14: warning: comparison of unsigned expression <\n0 is always false [-Wtautological-compare]\n                    if (actual < 0)\n                        ~~~~~~ ^ ~\n\nLooks like it's right. size_t is unsigned according to the standard,\nso that die_errno() is never reached.\n"},{"id":"168953","messageId":"7v62otp7wq.fsf@alter.siamese.dyndns.org","threadId":"27295","inReplyTo":"BANLkTikB3VhB4nTZhjE+3znfVTuBYsnehg@mail.gmail.com","subject":"Re: [PATCH v0 3/3] Bigfile: teach \"git add\" to send a large file straight to a pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-29T20:29:41Z","receivedAt":"2011-05-29T20:29:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> On Sun, May 8, 2011 at 10:47, Junio C Hamano <gitster@pobox.com> wrote:\n>\n>> +               char buf[10240];\n>> +               size_t sz = size < sizeof(buf) ? size : sizeof(buf);\n>> +               size_t actual;\n>> +\n>> +               actual = read_in_full(fd, buf, sz);\n>> +               if (actual < 0)\n>> +                       die_errno(\"index-stream: reading input\");\n\nIt already has been fixed with 23c7df6 (sha1_file: use the correct type\n(ssize_t, not size_t) for read-style function, 2011-05-26).\n\nScanning the list and updating your copy of 'pu' from k.org would often\nsave your time for topics that are relatively new.\n\nThanks.\n"}]}