{"thread":{"id":"13966","subject":"[PATCH 0/4] Teach \"git clone\" to pack refs","startedAt":"2008-06-15T14:02:03Z","lastAt":"2008-06-16T09:57:50Z","messageCount":13,"participants":["Johan Herland","Jeff King","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"79934","messageId":"200806151602.03445.johan@herland.net","threadId":"13966","inReplyTo":null,"subject":"[PATCH 0/4] Teach \"git clone\" to pack refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-15T14:02:03Z","receivedAt":"2008-06-15T14:02:03Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"(This is meant for AFTER v1.5.6.)\n\nThis is a re-post of the series I posted while builtin-clone was still\nunder development. The rationale for this series can be found in earlier\nthreads, but it roughly boils down to the following:\n\n1. \"git clone\" currently creates \"loose\" refs for every ref in the cloned\n   repo. A subsequent \"git gc\" will pack these refs into a packed-refs file.\n   Having \"git clone\" produce the \"packed\" refs in the first place seems\n   more efficient.\n\n2. For repos with few refs the performance difference between writing loose\n   refs and packed refs is negligible. However, for repos with thousands of\n   refs [1], the difference between writing one packed-refs file and\n   thousands of \"loose\" refs files is definitely noticeable. Even more so\n   on Windows.\n\n3. When the user updates a ref, a \"loose\" ref is written, and the\n   corresponding packed ref (if any) is left unused. By making \"git clone\"\n   write packed refs, we increase the overhead of unused packed refs\n   (proportionally to the number of refs updated by the user). However,\n   the number of refs updated by the user is typically small. If the user\n   updates tens - or even hundreds - of refs, I still expect this overhead\n   to be negligible, and in any case outweighed by the added performance\n   when cloning repos with many refs.\n\nThe series is based on current 'next'.\n\nJohan Herland (4):\n  Incorporate fetched packs in future object traversal\n  Move pack_refs() and friends into libgit\n  Prepare testsuite for a \"git clone\" that packs refs\n  Teach \"git clone\" to pack refs\n\n Makefile                     |    2 +\n builtin-clone.c              |    8 ++-\n builtin-fetch-pack.c         |    1 +\n builtin-pack-refs.c          |  121 +-----------------------------------------\n pack-refs.c                  |  117 ++++++++++++++++++++++++++++++++++++++++\n pack-refs.h                  |   18 ++++++\n t/t5515-fetch-merge-logic.sh |   19 +++++++\n 7 files changed, 164 insertions(+), 122 deletions(-)\n create mode 100644 pack-refs.c\n create mode 100644 pack-refs.h\n\n\nHave fun! :)\n\n...Johan\n\n\n[1]: At $dayjob I'm converting old CVS modules with up to ~10 years of\n     history, and some of the resulting repos have ~30000 refs (mostly\n     build tags). When cloning these repos, the speedup of this series\n     can be measured in seconds on Linux; minutes on Windows.\n"},{"id":"79935","messageId":"200806151604.20810.johan@herland.net","threadId":"13966","inReplyTo":"200806151602.03445.johan@herland.net","subject":"[PATCH 1/4] Incorporate fetched packs in future object traversal","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-15T14:04:20Z","receivedAt":"2008-06-15T14:04:20Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Immediately after fetching a pack, we should call reprepare_packed_git() to\nmake sure the objects in the pack are reachable. Otherwise, we will fail to\nlook up objects that are present only in the fetched pack.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n builtin-fetch-pack.c |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c\nindex de1e8d1..f4dbcf0 100644\n--- a/builtin-fetch-pack.c\n+++ b/builtin-fetch-pack.c\n@@ -820,5 +820,6 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,\n \t\t}\n \t}\n \n+\treprepare_packed_git();\n \treturn ref_cpy;\n }\n-- \n1.5.6.rc2.128.gf64ae\n"},{"id":"79937","messageId":"200806151605.07008.johan@herland.net","threadId":"13966","inReplyTo":"200806151602.03445.johan@herland.net","subject":"[PATCH 2/4] Move pack_refs() and friends into libgit","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-15T14:05:06Z","receivedAt":"2008-06-15T14:05:06Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"This moves pack_refs() and underlying functionality into the library,\nto make pack-refs functionality easily available to all git programs.\n\nMost of builtin-pack-refs.c has been moved verbatim into a new file\npack-refs.c that is compiled into libgit.a. A corresponding header\nfile, pack-refs.h, has also been added, declaring pack_refs() and\nthe #defines associated with the flags parameter to pack_refs().\n\nThis patch introduces no other changes in functionality.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n Makefile            |    2 +\n builtin-pack-refs.c |  121 +--------------------------------------------------\n pack-refs.c         |  117 +++++++++++++++++++++++++++++++++++++++++++++++++\n pack-refs.h         |   18 ++++++++\n 4 files changed, 138 insertions(+), 120 deletions(-)\n create mode 100644 pack-refs.c\n create mode 100644 pack-refs.h\n\ndiff --git a/Makefile b/Makefile\nindex 1937507..4a78388 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -354,6 +354,7 @@ LIB_H += log-tree.h\n LIB_H += mailmap.h\n LIB_H += object.h\n LIB_H += pack.h\n+LIB_H += pack-refs.h\n LIB_H += pack-revindex.h\n LIB_H += parse-options.h\n LIB_H += patch-ids.h\n@@ -429,6 +430,7 @@ LIB_OBJS += merge-file.o\n LIB_OBJS += name-hash.o\n LIB_OBJS += object.o\n LIB_OBJS += pack-check.o\n+LIB_OBJS += pack-refs.o\n LIB_OBJS += pack-revindex.o\n LIB_OBJS += pack-write.o\n LIB_OBJS += pager.o\ndiff --git a/builtin-pack-refs.c b/builtin-pack-refs.c\nindex 1aaa76d..ff90aef 100644\n--- a/builtin-pack-refs.c\n+++ b/builtin-pack-refs.c\n@@ -1,125 +1,6 @@\n-#include \"builtin.h\"\n #include \"cache.h\"\n-#include \"refs.h\"\n-#include \"object.h\"\n-#include \"tag.h\"\n #include \"parse-options.h\"\n-\n-struct ref_to_prune {\n-\tstruct ref_to_prune *next;\n-\tunsigned char sha1[20];\n-\tchar name[FLEX_ARRAY];\n-};\n-\n-#define PACK_REFS_PRUNE\t0x0001\n-#define PACK_REFS_ALL\t0x0002\n-\n-struct pack_refs_cb_data {\n-\tunsigned int flags;\n-\tstruct ref_to_prune *ref_to_prune;\n-\tFILE *refs_file;\n-};\n-\n-static int do_not_prune(int flags)\n-{\n-\t/* If it is already packed or if it is a symref,\n-\t * do not prune it.\n-\t */\n-\treturn (flags & (REF_ISSYMREF|REF_ISPACKED));\n-}\n-\n-static int handle_one_ref(const char *path, const unsigned char *sha1,\n-\t\t\t  int flags, void *cb_data)\n-{\n-\tstruct pack_refs_cb_data *cb = cb_data;\n-\tint is_tag_ref;\n-\n-\t/* Do not pack the symbolic refs */\n-\tif ((flags & REF_ISSYMREF))\n-\t\treturn 0;\n-\tis_tag_ref = !prefixcmp(path, \"refs/tags/\");\n-\n-\t/* ALWAYS pack refs that were already packed or are tags */\n-\tif (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))\n-\t\treturn 0;\n-\n-\tfprintf(cb->refs_file, \"%s %s\\n\", sha1_to_hex(sha1), path);\n-\tif (is_tag_ref) {\n-\t\tstruct object *o = parse_object(sha1);\n-\t\tif (o->type == OBJ_TAG) {\n-\t\t\to = deref_tag(o, path, 0);\n-\t\t\tif (o)\n-\t\t\t\tfprintf(cb->refs_file, \"^%s\\n\",\n-\t\t\t\t\tsha1_to_hex(o->sha1));\n-\t\t}\n-\t}\n-\n-\tif ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {\n-\t\tint namelen = strlen(path) + 1;\n-\t\tstruct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);\n-\t\thashcpy(n->sha1, sha1);\n-\t\tstrcpy(n->name, path);\n-\t\tn->next = cb->ref_to_prune;\n-\t\tcb->ref_to_prune = n;\n-\t}\n-\treturn 0;\n-}\n-\n-/* make sure nobody touched the ref, and unlink */\n-static void prune_ref(struct ref_to_prune *r)\n-{\n-\tstruct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);\n-\n-\tif (lock) {\n-\t\tunlink(git_path(\"%s\", r->name));\n-\t\tunlock_ref(lock);\n-\t}\n-}\n-\n-static void prune_refs(struct ref_to_prune *r)\n-{\n-\twhile (r) {\n-\t\tprune_ref(r);\n-\t\tr = r->next;\n-\t}\n-}\n-\n-static struct lock_file packed;\n-\n-static int pack_refs(unsigned int flags)\n-{\n-\tint fd;\n-\tstruct pack_refs_cb_data cbdata;\n-\n-\tmemset(&cbdata, 0, sizeof(cbdata));\n-\tcbdata.flags = flags;\n-\n-\tfd = hold_lock_file_for_update(&packed, git_path(\"packed-refs\"), 1);\n-\tcbdata.refs_file = fdopen(fd, \"w\");\n-\tif (!cbdata.refs_file)\n-\t\tdie(\"unable to create ref-pack file structure (%s)\",\n-\t\t    strerror(errno));\n-\n-\t/* perhaps other traits later as well */\n-\tfprintf(cbdata.refs_file, \"# pack-refs with: peeled \\n\");\n-\n-\tfor_each_ref(handle_one_ref, &cbdata);\n-\tif (ferror(cbdata.refs_file))\n-\t\tdie(\"failed to write ref-pack file\");\n-\tif (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))\n-\t\tdie(\"failed to write ref-pack file (%s)\", strerror(errno));\n-\t/*\n-\t * Since the lock file was fdopen()'ed and then fclose()'ed above,\n-\t * assign -1 to the lock file descriptor so that commit_lock_file()\n-\t * won't try to close() it.\n-\t */\n-\tpacked.fd = -1;\n-\tif (commit_lock_file(&packed) < 0)\n-\t\tdie(\"unable to overwrite old ref-pack file (%s)\", strerror(errno));\n-\tif (cbdata.flags & PACK_REFS_PRUNE)\n-\t\tprune_refs(cbdata.ref_to_prune);\n-\treturn 0;\n-}\n+#include \"pack-refs.h\"\n \n static char const * const pack_refs_usage[] = {\n \t\"git-pack-refs [options]\",\ndiff --git a/pack-refs.c b/pack-refs.c\nnew file mode 100644\nindex 0000000..848d311\n--- /dev/null\n+++ b/pack-refs.c\n@@ -0,0 +1,117 @@\n+#include \"cache.h\"\n+#include \"refs.h\"\n+#include \"tag.h\"\n+#include \"pack-refs.h\"\n+\n+struct ref_to_prune {\n+\tstruct ref_to_prune *next;\n+\tunsigned char sha1[20];\n+\tchar name[FLEX_ARRAY];\n+};\n+\n+struct pack_refs_cb_data {\n+\tunsigned int flags;\n+\tstruct ref_to_prune *ref_to_prune;\n+\tFILE *refs_file;\n+};\n+\n+static int do_not_prune(int flags)\n+{\n+\t/* If it is already packed or if it is a symref,\n+\t * do not prune it.\n+\t */\n+\treturn (flags & (REF_ISSYMREF|REF_ISPACKED));\n+}\n+\n+static int handle_one_ref(const char *path, const unsigned char *sha1,\n+\t\t\t  int flags, void *cb_data)\n+{\n+\tstruct pack_refs_cb_data *cb = cb_data;\n+\tint is_tag_ref;\n+\n+\t/* Do not pack the symbolic refs */\n+\tif ((flags & REF_ISSYMREF))\n+\t\treturn 0;\n+\tis_tag_ref = !prefixcmp(path, \"refs/tags/\");\n+\n+\t/* ALWAYS pack refs that were already packed or are tags */\n+\tif (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))\n+\t\treturn 0;\n+\n+\tfprintf(cb->refs_file, \"%s %s\\n\", sha1_to_hex(sha1), path);\n+\tif (is_tag_ref) {\n+\t\tstruct object *o = parse_object(sha1);\n+\t\tif (o->type == OBJ_TAG) {\n+\t\t\to = deref_tag(o, path, 0);\n+\t\t\tif (o)\n+\t\t\t\tfprintf(cb->refs_file, \"^%s\\n\",\n+\t\t\t\t\tsha1_to_hex(o->sha1));\n+\t\t}\n+\t}\n+\n+\tif ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {\n+\t\tint namelen = strlen(path) + 1;\n+\t\tstruct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);\n+\t\thashcpy(n->sha1, sha1);\n+\t\tstrcpy(n->name, path);\n+\t\tn->next = cb->ref_to_prune;\n+\t\tcb->ref_to_prune = n;\n+\t}\n+\treturn 0;\n+}\n+\n+/* make sure nobody touched the ref, and unlink */\n+static void prune_ref(struct ref_to_prune *r)\n+{\n+\tstruct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);\n+\n+\tif (lock) {\n+\t\tunlink(git_path(\"%s\", r->name));\n+\t\tunlock_ref(lock);\n+\t}\n+}\n+\n+static void prune_refs(struct ref_to_prune *r)\n+{\n+\twhile (r) {\n+\t\tprune_ref(r);\n+\t\tr = r->next;\n+\t}\n+}\n+\n+static struct lock_file packed;\n+\n+int pack_refs(unsigned int flags)\n+{\n+\tint fd;\n+\tstruct pack_refs_cb_data cbdata;\n+\n+\tmemset(&cbdata, 0, sizeof(cbdata));\n+\tcbdata.flags = flags;\n+\n+\tfd = hold_lock_file_for_update(&packed, git_path(\"packed-refs\"), 1);\n+\tcbdata.refs_file = fdopen(fd, \"w\");\n+\tif (!cbdata.refs_file)\n+\t\tdie(\"unable to create ref-pack file structure (%s)\",\n+\t\t    strerror(errno));\n+\n+\t/* perhaps other traits later as well */\n+\tfprintf(cbdata.refs_file, \"# pack-refs with: peeled \\n\");\n+\n+\tfor_each_ref(handle_one_ref, &cbdata);\n+\tif (ferror(cbdata.refs_file))\n+\t\tdie(\"failed to write ref-pack file\");\n+\tif (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))\n+\t\tdie(\"failed to write ref-pack file (%s)\", strerror(errno));\n+\t/*\n+\t * Since the lock file was fdopen()'ed and then fclose()'ed above,\n+\t * assign -1 to the lock file descriptor so that commit_lock_file()\n+\t * won't try to close() it.\n+\t */\n+\tpacked.fd = -1;\n+\tif (commit_lock_file(&packed) < 0)\n+\t\tdie(\"unable to overwrite old ref-pack file (%s)\", strerror(errno));\n+\tif (cbdata.flags & PACK_REFS_PRUNE)\n+\t\tprune_refs(cbdata.ref_to_prune);\n+\treturn 0;\n+}\ndiff --git a/pack-refs.h b/pack-refs.h\nnew file mode 100644\nindex 0000000..518acfb\n--- /dev/null\n+++ b/pack-refs.h\n@@ -0,0 +1,18 @@\n+#ifndef PACK_REFS_H\n+#define PACK_REFS_H\n+\n+/*\n+ * Flags for controlling behaviour of pack_refs()\n+ * PACK_REFS_PRUNE: Prune loose refs after packing\n+ * PACK_REFS_ALL:   Pack _all_ refs, not just tags and already packed refs\n+ */\n+#define PACK_REFS_PRUNE 0x0001\n+#define PACK_REFS_ALL   0x0002\n+\n+/*\n+ * Write a packed-refs file for the current repository.\n+ * flags: Combination of the above PACK_REFS_* flags.\n+ */\n+int pack_refs(unsigned int flags);\n+\n+#endif /* PACK_REFS_H */\n-- \n1.5.6.rc2.128.gf64ae\n"},{"id":"79936","messageId":"200806151605.47214.johan@herland.net","threadId":"13966","inReplyTo":"200806151602.03445.johan@herland.net","subject":"[PATCH 3/4] Prepare testsuite for a \"git clone\" that packs refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-15T14:05:47Z","receivedAt":"2008-06-15T14:05:47Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"t5515-fetch-merge-logic removes many, but not all, refs between each test.\nThis is done by removing the corresponding refs/foo/* files in the .git/refs\nhierarchy. However, once \"git clone\" starts producing packed refs, these refs\nwill no longer be in the .git/refs hierarchy, but rather listed in\n.git/packed-refs. This patch therefore teaches t5515-fetch-merge-logic to also\nremove the refs in question from the packed-refs file.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n t/t5515-fetch-merge-logic.sh |   19 +++++++++++++++++++\n 1 files changed, 19 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh\nindex 3def75e..b3e1b97 100755\n--- a/t/t5515-fetch-merge-logic.sh\n+++ b/t/t5515-fetch-merge-logic.sh\n@@ -145,6 +145,25 @@ do\n \t\t\trm -f .git/refs/heads/*\n \t\t\trm -f .git/refs/remotes/rem/*\n \t\t\trm -f .git/refs/tags/*\n+\t\t\tcat .git/packed-refs | \\\n+\t\t\twhile read sha1 refname\n+\t\t\tdo\n+\t\t\t\tcase \"$sha1\" in\n+\t\t\t\t^*) # remove peeled tags\n+\t\t\t\t\t;;\n+\t\t\t\t*)\n+\t\t\t\t\tcase \"$refname\" in\n+\t\t\t\t\trefs/heads/*|\\\n+\t\t\t\t\trefs/remotes/rem/*|\\\n+\t\t\t\t\trefs/tags/*) # remove same as above\n+\t\t\t\t\t\t;;\n+\t\t\t\t\t*) # keep everything else\n+\t\t\t\t\t\techo \"$sha1 $refname\"\n+\t\t\t\t\t\t;;\n+\t\t\t\t\tesac\n+\t\t\t\tesac\n+\t\t\tdone > .git/packed-refs.new\n+\t\t\tmv .git/packed-refs.new .git/packed-refs\n \t\t\tgit fetch \"$@\" >/dev/null\n \t\t\tcat .git/FETCH_HEAD\n \t\t} >\"$actual_f\" &&\n-- \n1.5.6.rc2.128.gf64ae\n"},{"id":"79938","messageId":"200806151606.16380.johan@herland.net","threadId":"13966","inReplyTo":"200806151602.03445.johan@herland.net","subject":"[PATCH 4/4] Teach \"git clone\" to pack refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-15T14:06:16Z","receivedAt":"2008-06-15T14:06:16Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"In repos with many refs, it is unlikely that most refs will ever change.\nThis fact is already exploited by \"git gc\" by executing \"git pack-refs\"\nto consolidate all refs into a single file.\n\nWhen cloning a repo with many refs, it does not make sense to create the\nloose refs in the first place, just to have the next \"git gc\" consolidate\nthem into one file. Instead, make \"git clone\" create the packed refs file\nimmediately, and forego the loose refs completely.\n\nSigned-off-by: Johan Herland <johan@herland.net>\nAcked-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n builtin-clone.c |    8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 7190952..5c5acb4 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -18,6 +18,7 @@\n #include \"transport.h\"\n #include \"strbuf.h\"\n #include \"dir.h\"\n+#include \"pack-refs.h\"\n \n /*\n  * Overall FIXMEs:\n@@ -321,8 +322,11 @@ static struct ref *write_remote_refs(const struct ref *refs,\n \tget_fetch_map(refs, tag_refspec, &tail, 0);\n \n \tfor (r = local_refs; r; r = r->next)\n-\t\tupdate_ref(reflog,\n-\t\t\t   r->peer_ref->name, r->old_sha1, NULL, 0, DIE_ON_ERR);\n+\t\tadd_extra_ref(r->peer_ref->name, r->old_sha1, 0);\n+\n+\tpack_refs(PACK_REFS_ALL);\n+\tclear_extra_refs();\n+\n \treturn local_refs;\n }\n \n-- \n1.5.6.rc2.128.gf64ae\n"},{"id":"79941","messageId":"20080615175159.GA6127@sigill.intra.peff.net","threadId":"13966","inReplyTo":"200806151605.07008.johan@herland.net","subject":"Re: [PATCH 2/4] Move pack_refs() and friends into libgit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-15T17:52:00Z","receivedAt":"2008-06-15T17:52:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 15, 2008 at 04:05:06PM +0200, Johan Herland wrote:\n\n> ---\n>  Makefile            |    2 +\n>  builtin-pack-refs.c |  121 +-----------------------------------------------\n>  pack-refs.c         |  117 ++++++++++++++++++++++++++++++++++++++++++++++\n>  pack-refs.h         |   18 ++++++++\n>  4 files changed, 138 insertions(+), 120 deletions(-)\n>  create mode 100644 pack-refs.c\n>  create mode 100644 pack-refs.h\n\nThis patch would have been a lot easier to read, btw, if it had been\ngenerated with '-C' (then patch to pack-refs.c is based on\nbuiltin-pack-refs instead of /dev/null).\n\n-Peff\n"},{"id":"79942","messageId":"20080615175418.GB6127@sigill.intra.peff.net","threadId":"13966","inReplyTo":"200806151605.47214.johan@herland.net","subject":"Re: [PATCH 3/4] Prepare testsuite for a \"git clone\" that packs refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-15T17:54:18Z","receivedAt":"2008-06-15T17:54:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 15, 2008 at 04:05:47PM +0200, Johan Herland wrote:\n\n> .git/packed-refs. This patch therefore teaches t5515-fetch-merge-logic\n> to also remove the refs in question from the packed-refs file.\n> [...]\n> +\t\t\tcat .git/packed-refs | \\\n> +\t\t\twhile read sha1 refname\n> +\t\t\tdo\n> +\t\t\t\tcase \"$sha1\" in\n> +\t\t\t\t^*) # remove peeled tags\n> +\t\t\t\t\t;;\n> +\t\t\t\t*)\n> +\t\t\t\t\tcase \"$refname\" in\n> +\t\t\t\t\trefs/heads/*|\\\n> +\t\t\t\t\trefs/remotes/rem/*|\\\n> +\t\t\t\t\trefs/tags/*) # remove same as above\n> +\t\t\t\t\t\t;;\n> +\t\t\t\t\t*) # keep everything else\n> +\t\t\t\t\t\techo \"$sha1 $refname\"\n> +\t\t\t\t\t\t;;\n> +\t\t\t\t\tesac\n> +\t\t\t\tesac\n> +\t\t\tdone > .git/packed-refs.new\n> +\t\t\tmv .git/packed-refs.new .git/packed-refs\n\nMight it not be simpler to just convert it to use plumbing to delete the\nrefs? Something like piping for-each-ref into update-ref -d?\n\n-Peff\n"},{"id":"79943","messageId":"20080615175613.GC6127@sigill.intra.peff.net","threadId":"13966","inReplyTo":"200806151606.16380.johan@herland.net","subject":"Re: [PATCH 4/4] Teach \"git clone\" to pack refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-15T17:56:14Z","receivedAt":"2008-06-15T17:56:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 15, 2008 at 04:06:16PM +0200, Johan Herland wrote:\n\n> +\tpack_refs(PACK_REFS_ALL);\n\nI haven't looked carefully at the pack_refs code, but my understanding\nwas that this would pack _all_ refs, including branches. Don't we\ngenerally try to leave branches unpacked, since they change a lot? IOW,\nshouldn't this just be \"pack_refs(0)\"?\n\n-Peff\n"},{"id":"79945","messageId":"m3bq22poby.fsf@localhost.localdomain","threadId":"13966","inReplyTo":"20080615175418.GB6127@sigill.intra.peff.net","subject":"Re: [PATCH 3/4] Prepare testsuite for a \"git clone\" that packs refs","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-15T18:04:07Z","receivedAt":"2008-06-15T18:04:07Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sun, Jun 15, 2008 at 04:05:47PM +0200, Johan Herland wrote:\n> \n> > .git/packed-refs. This patch therefore teaches t5515-fetch-merge-logic\n> > to also remove the refs in question from the packed-refs file.\n> > [...]\n> > +\t\t\tcat .git/packed-refs | \\\n> > +\t\t\twhile read sha1 refname\n> > +\t\t\tdo\n> > +\t\t\t\tcase \"$sha1\" in\n> > +\t\t\t\t^*) # remove peeled tags\n> > +\t\t\t\t\t;;\n> > +\t\t\t\t*)\n> > +\t\t\t\t\tcase \"$refname\" in\n> > +\t\t\t\t\trefs/heads/*|\\\n> > +\t\t\t\t\trefs/remotes/rem/*|\\\n> > +\t\t\t\t\trefs/tags/*) # remove same as above\n> > +\t\t\t\t\t\t;;\n> > +\t\t\t\t\t*) # keep everything else\n> > +\t\t\t\t\t\techo \"$sha1 $refname\"\n> > +\t\t\t\t\t\t;;\n> > +\t\t\t\t\tesac\n> > +\t\t\t\tesac\n> > +\t\t\tdone > .git/packed-refs.new\n> > +\t\t\tmv .git/packed-refs.new .git/packed-refs\n> \n> Might it not be simpler to just convert it to use plumbing to delete the\n> refs? Something like piping for-each-ref into update-ref -d?\n\nOr use git-for-each-ref with --shell option to generate code for\ndeleting refs?\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"79962","messageId":"200806152327.24586.johan@herland.net","threadId":"13966","inReplyTo":"20080615175159.GA6127@sigill.intra.peff.net","subject":"[PATCH 2/4 v2] Move pack_refs() and friends into libgit","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-15T21:27:24Z","receivedAt":"2008-06-15T21:27:24Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"This moves pack_refs() and underlying functionality into the library,\nto make pack-refs functionality easily available to all git programs.\n\nMost of builtin-pack-refs.c has been moved verbatim into a new file\npack-refs.c that is compiled into libgit.a. A corresponding header\nfile, pack-refs.h, has also been added, declaring pack_refs() and\nthe #defines associated with the flags parameter to pack_refs().\n\nThis patch introduces no other changes in functionality.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n\nOn Sunday 15 June 2008, Jeff King wrote:\n> This patch would have been a lot easier to read, btw, if it had been\n> generated with '-C' (then patch to pack-refs.c is based on\n> builtin-pack-refs instead of /dev/null).\n\nSorry about that. Here you go.\n\n...Johan\t\n\n\n Makefile                           |    2 +\n builtin-pack-refs.c                |  121 +-----------------------------------\n builtin-pack-refs.c => pack-refs.c |   27 +--------\n pack-refs.h                        |   18 +++++\n 4 files changed, 23 insertions(+), 145 deletions(-)\n copy builtin-pack-refs.c => pack-refs.c (80%)\n create mode 100644 pack-refs.h\n\ndiff --git a/Makefile b/Makefile\nindex 1937507..4a78388 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -354,6 +354,7 @@ LIB_H += log-tree.h\n LIB_H += mailmap.h\n LIB_H += object.h\n LIB_H += pack.h\n+LIB_H += pack-refs.h\n LIB_H += pack-revindex.h\n LIB_H += parse-options.h\n LIB_H += patch-ids.h\n@@ -429,6 +430,7 @@ LIB_OBJS += merge-file.o\n LIB_OBJS += name-hash.o\n LIB_OBJS += object.o\n LIB_OBJS += pack-check.o\n+LIB_OBJS += pack-refs.o\n LIB_OBJS += pack-revindex.o\n LIB_OBJS += pack-write.o\n LIB_OBJS += pager.o\ndiff --git a/builtin-pack-refs.c b/builtin-pack-refs.c\nindex 1aaa76d..ff90aef 100644\n--- a/builtin-pack-refs.c\n+++ b/builtin-pack-refs.c\n@@ -1,125 +1,6 @@\n-#include \"builtin.h\"\n #include \"cache.h\"\n-#include \"refs.h\"\n-#include \"object.h\"\n-#include \"tag.h\"\n #include \"parse-options.h\"\n-\n-struct ref_to_prune {\n-\tstruct ref_to_prune *next;\n-\tunsigned char sha1[20];\n-\tchar name[FLEX_ARRAY];\n-};\n-\n-#define PACK_REFS_PRUNE\t0x0001\n-#define PACK_REFS_ALL\t0x0002\n-\n-struct pack_refs_cb_data {\n-\tunsigned int flags;\n-\tstruct ref_to_prune *ref_to_prune;\n-\tFILE *refs_file;\n-};\n-\n-static int do_not_prune(int flags)\n-{\n-\t/* If it is already packed or if it is a symref,\n-\t * do not prune it.\n-\t */\n-\treturn (flags & (REF_ISSYMREF|REF_ISPACKED));\n-}\n-\n-static int handle_one_ref(const char *path, const unsigned char *sha1,\n-\t\t\t  int flags, void *cb_data)\n-{\n-\tstruct pack_refs_cb_data *cb = cb_data;\n-\tint is_tag_ref;\n-\n-\t/* Do not pack the symbolic refs */\n-\tif ((flags & REF_ISSYMREF))\n-\t\treturn 0;\n-\tis_tag_ref = !prefixcmp(path, \"refs/tags/\");\n-\n-\t/* ALWAYS pack refs that were already packed or are tags */\n-\tif (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))\n-\t\treturn 0;\n-\n-\tfprintf(cb->refs_file, \"%s %s\\n\", sha1_to_hex(sha1), path);\n-\tif (is_tag_ref) {\n-\t\tstruct object *o = parse_object(sha1);\n-\t\tif (o->type == OBJ_TAG) {\n-\t\t\to = deref_tag(o, path, 0);\n-\t\t\tif (o)\n-\t\t\t\tfprintf(cb->refs_file, \"^%s\\n\",\n-\t\t\t\t\tsha1_to_hex(o->sha1));\n-\t\t}\n-\t}\n-\n-\tif ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {\n-\t\tint namelen = strlen(path) + 1;\n-\t\tstruct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);\n-\t\thashcpy(n->sha1, sha1);\n-\t\tstrcpy(n->name, path);\n-\t\tn->next = cb->ref_to_prune;\n-\t\tcb->ref_to_prune = n;\n-\t}\n-\treturn 0;\n-}\n-\n-/* make sure nobody touched the ref, and unlink */\n-static void prune_ref(struct ref_to_prune *r)\n-{\n-\tstruct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);\n-\n-\tif (lock) {\n-\t\tunlink(git_path(\"%s\", r->name));\n-\t\tunlock_ref(lock);\n-\t}\n-}\n-\n-static void prune_refs(struct ref_to_prune *r)\n-{\n-\twhile (r) {\n-\t\tprune_ref(r);\n-\t\tr = r->next;\n-\t}\n-}\n-\n-static struct lock_file packed;\n-\n-static int pack_refs(unsigned int flags)\n-{\n-\tint fd;\n-\tstruct pack_refs_cb_data cbdata;\n-\n-\tmemset(&cbdata, 0, sizeof(cbdata));\n-\tcbdata.flags = flags;\n-\n-\tfd = hold_lock_file_for_update(&packed, git_path(\"packed-refs\"), 1);\n-\tcbdata.refs_file = fdopen(fd, \"w\");\n-\tif (!cbdata.refs_file)\n-\t\tdie(\"unable to create ref-pack file structure (%s)\",\n-\t\t    strerror(errno));\n-\n-\t/* perhaps other traits later as well */\n-\tfprintf(cbdata.refs_file, \"# pack-refs with: peeled \\n\");\n-\n-\tfor_each_ref(handle_one_ref, &cbdata);\n-\tif (ferror(cbdata.refs_file))\n-\t\tdie(\"failed to write ref-pack file\");\n-\tif (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))\n-\t\tdie(\"failed to write ref-pack file (%s)\", strerror(errno));\n-\t/*\n-\t * Since the lock file was fdopen()'ed and then fclose()'ed above,\n-\t * assign -1 to the lock file descriptor so that commit_lock_file()\n-\t * won't try to close() it.\n-\t */\n-\tpacked.fd = -1;\n-\tif (commit_lock_file(&packed) < 0)\n-\t\tdie(\"unable to overwrite old ref-pack file (%s)\", strerror(errno));\n-\tif (cbdata.flags & PACK_REFS_PRUNE)\n-\t\tprune_refs(cbdata.ref_to_prune);\n-\treturn 0;\n-}\n+#include \"pack-refs.h\"\n \n static char const * const pack_refs_usage[] = {\n \t\"git-pack-refs [options]\",\ndiff --git a/builtin-pack-refs.c b/pack-refs.c\nsimilarity index 80%\ncopy from builtin-pack-refs.c\ncopy to pack-refs.c\nindex 1aaa76d..848d311 100644\n--- a/builtin-pack-refs.c\n+++ b/pack-refs.c\n@@ -1,9 +1,7 @@\n-#include \"builtin.h\"\n #include \"cache.h\"\n #include \"refs.h\"\n-#include \"object.h\"\n #include \"tag.h\"\n-#include \"parse-options.h\"\n+#include \"pack-refs.h\"\n \n struct ref_to_prune {\n \tstruct ref_to_prune *next;\n@@ -11,9 +9,6 @@ struct ref_to_prune {\n \tchar name[FLEX_ARRAY];\n };\n \n-#define PACK_REFS_PRUNE\t0x0001\n-#define PACK_REFS_ALL\t0x0002\n-\n struct pack_refs_cb_data {\n \tunsigned int flags;\n \tstruct ref_to_prune *ref_to_prune;\n@@ -86,7 +81,7 @@ static void prune_refs(struct ref_to_prune *r)\n \n static struct lock_file packed;\n \n-static int pack_refs(unsigned int flags)\n+int pack_refs(unsigned int flags)\n {\n \tint fd;\n \tstruct pack_refs_cb_data cbdata;\n@@ -120,21 +115,3 @@ static int pack_refs(unsigned int flags)\n \t\tprune_refs(cbdata.ref_to_prune);\n \treturn 0;\n }\n-\n-static char const * const pack_refs_usage[] = {\n-\t\"git-pack-refs [options]\",\n-\tNULL\n-};\n-\n-int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n-{\n-\tunsigned int flags = PACK_REFS_PRUNE;\n-\tstruct option opts[] = {\n-\t\tOPT_BIT(0, \"all\",   &flags, \"pack everything\", PACK_REFS_ALL),\n-\t\tOPT_BIT(0, \"prune\", &flags, \"prune loose refs (default)\", PACK_REFS_PRUNE),\n-\t\tOPT_END(),\n-\t};\n-\tif (parse_options(argc, argv, opts, pack_refs_usage, 0))\n-\t\tusage_with_options(pack_refs_usage, opts);\n-\treturn pack_refs(flags);\n-}\ndiff --git a/pack-refs.h b/pack-refs.h\nnew file mode 100644\nindex 0000000..518acfb\n--- /dev/null\n+++ b/pack-refs.h\n@@ -0,0 +1,18 @@\n+#ifndef PACK_REFS_H\n+#define PACK_REFS_H\n+\n+/*\n+ * Flags for controlling behaviour of pack_refs()\n+ * PACK_REFS_PRUNE: Prune loose refs after packing\n+ * PACK_REFS_ALL:   Pack _all_ refs, not just tags and already packed refs\n+ */\n+#define PACK_REFS_PRUNE 0x0001\n+#define PACK_REFS_ALL   0x0002\n+\n+/*\n+ * Write a packed-refs file for the current repository.\n+ * flags: Combination of the above PACK_REFS_* flags.\n+ */\n+int pack_refs(unsigned int flags);\n+\n+#endif /* PACK_REFS_H */\n-- \n1.5.6.rc2.128.gf64ae\n"},{"id":"79986","messageId":"200806160003.06300.johan@herland.net","threadId":"13966","inReplyTo":"20080615175613.GC6127@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] Teach \"git clone\" to pack refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-15T22:03:06Z","receivedAt":"2008-06-15T22:03:06Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Sunday 15 June 2008, Jeff King wrote:\n> On Sun, Jun 15, 2008 at 04:06:16PM +0200, Johan Herland wrote:\n> > +\tpack_refs(PACK_REFS_ALL);\n>\n> I haven't looked carefully at the pack_refs code, but my understanding\n> was that this would pack _all_ refs, including branches. Don't we\n> generally try to leave branches unpacked, since they change a lot? IOW,\n> shouldn't this just be \"pack_refs(0)\"?\n\nYes, for many repos it does not make much sense to pack branches. But in the \ncase where the repo has many inactive branches (I have repos with 1000 \nbranches where at most 5-10 are still active), I'd much rather pack all \nbranches and then later \"unpack\" the active ones, than write all \nthose \"loose\" refs as separate files onto the filesystem (e.g. in CygWin \n*shudder*). In any case, the user normally does not work actively on \nhundreds of branches, so the overhead of \"unpacking\" active branches should \nbe fairly negligible in any case.\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"79991","messageId":"200806160116.53446.johan@herland.net","threadId":"13966","inReplyTo":"m3bq22poby.fsf@localhost.localdomain","subject":"[PATCH 3/4 v2] Prepare testsuite for a \"git clone\" that packs refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-15T23:16:53Z","receivedAt":"2008-06-15T23:16:53Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"t5515-fetch-merge-logic removes many, but not all, refs between each test.\nThis is done by removing the corresponding refs/foo/* files in the .git/refs\nhierarchy. However, once \"git clone\" starts producing packed refs, these refs\nwill no longer be in the .git/refs hierarchy, but rather listed in\n.git/packed-refs. This patch teaches t5515-fetch-merge-logic to remove the\nrefs using \"git update-ref -d\" which properly handles packed refs.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n\nOn Sunday 15 June 2008, Jakub Narebski wrote:\n> Jeff King <peff@peff.net> writes:\n> > Might it not be simpler to just convert it to use plumbing to delete\n> > the refs? Something like piping for-each-ref into update-ref -d?\n>\n> Or use git-for-each-ref with --shell option to generate code for\n> deleting refs?\n\nThanks for the tip. The following seems to do the job, and looks much better.\n\n\nHave fun! :)\n\n...Johan\n\n t/t5515-fetch-merge-logic.sh |    9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh\nindex 3def75e..9de8408 100755\n--- a/t/t5515-fetch-merge-logic.sh\n+++ b/t/t5515-fetch-merge-logic.sh\n@@ -142,9 +142,12 @@ do\n \t\t\tset x $cmd; shift\n \t\t\tgit symbolic-ref HEAD refs/heads/$1 ; shift\n \t\t\trm -f .git/FETCH_HEAD\n-\t\t\trm -f .git/refs/heads/*\n-\t\t\trm -f .git/refs/remotes/rem/*\n-\t\t\trm -f .git/refs/tags/*\n+\t\t\tgit for-each-ref --format=\"%(refname)\" \\\n+\t\t\t\trefs/heads refs/remotes/rem refs/tags | \\\n+\t\t\twhile read refname\n+\t\t\tdo\n+\t\t\t\tgit update-ref -d \"$refname\"\n+\t\t\tdone\n \t\t\tgit fetch \"$@\" >/dev/null\n \t\t\tcat .git/FETCH_HEAD\n \t\t} >\"$actual_f\" &&\n-- \n1.5.6.rc2.128.gf64ae\n"},{"id":"80010","messageId":"20080616095750.GA24116@sigill.intra.peff.net","threadId":"13966","inReplyTo":"200806160003.06300.johan@herland.net","subject":"Re: [PATCH 4/4] Teach \"git clone\" to pack refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-16T09:57:50Z","receivedAt":"2008-06-16T09:57:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 16, 2008 at 12:03:06AM +0200, Johan Herland wrote:\n\n> Yes, for many repos it does not make much sense to pack branches. But in the \n> case where the repo has many inactive branches (I have repos with 1000 \n> branches where at most 5-10 are still active), I'd much rather pack all \n> branches and then later \"unpack\" the active ones, than write all \n> those \"loose\" refs as separate files onto the filesystem (e.g. in CygWin \n> *shudder*). In any case, the user normally does not work actively on \n> hundreds of branches, so the overhead of \"unpacking\" active branches should \n> be fairly negligible in any case.\n\nWhat I was concerned about was that pack-refs would continue to pack\nactive branches forever, because by default it repacks branches that are\nalready packed.\n\nHowever, it seems that git-gc just passes --all anyway, so it presumably\nis not a problem. And at the very least, clone is then consistent with\nthe auto-gc behavior, which makes sense.\n\nSo I withdraw my complaint.\n\n-Peff\n"}]}