{"thread":{"id":"45565","subject":"[PATCH v2 00/20] Separate `ref_cache` into a separate module","startedAt":"2017-03-31T14:11:39Z","lastAt":"2017-04-16T06:15:49Z","messageCount":42,"participants":["Michael Haggerty","Junio C Hamano","Stefan Beller","Jeff King","Ramsay Jones","Duy Nguyen"],"isPatch":true,"patchVersion":2,"patchTotal":20},"messages":[{"id":"315894","messageId":"cover.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":null,"subject":"[PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:10:58Z","receivedAt":"2017-03-31T14:11:39Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This is v2 of this patch series. Thanks to Peff, Junio, Stefan and\nÆvar for their comments about v1 [1].\n\nThis version literally only contains a few commit message changes and\none minor comment changes relative to v1. The code is identical. I\nwasn't sure whether it is even worth sending this patch series to the\nML again; Junio, if you'd prefer I just send a link to a published\nbranch in such cases, please let me know.\n\n* I picked up the \"area:\" prefixes that Junio added to a couple of\n  commit subject lines.\n\n* I clarified the justification for keeping a pointer to the\n  `ref_store` in `ref_dir`.\n\n* I added a long blurb about the removal of an internal consistency\n  check in the commit message for\n\n      [18/20] commit_packed_refs(): use reference iteration\n\n* I changed \"loose refs cache\" to \"loose ref cache\" in a comment in\n\n      [19/20] files_pack_refs(): use reference iteration\n\nThis patch series is also available from my GitHub fork [2] as branch\n\"separate-ref-cache\". These patches depend on Duy's\nnd/files-backend-git-dir branch.\n\n[1] http://public-inbox.org/git/cover.1490026594.git.mhagger@alum.mit.edu/\n[2] https://github.com/mhagger/git\n\nMichael Haggerty (20):\n  get_ref_dir(): don't call read_loose_refs() for \"refs/bisect\"\n  refs_read_raw_ref(): new function\n  refs_ref_iterator_begin(): new function\n  refs_verify_refname_available(): implement once for all backends\n  refs_verify_refname_available(): use function in more places\n  ref-cache: rename `add_ref()` to `add_ref_entry()`\n  ref-cache: rename `find_ref()` to `find_ref_entry()`\n  ref-cache: rename `remove_entry()` to `remove_entry_from_dir()`\n  refs: split `ref_cache` code into separate files\n  ref-cache: introduce a new type, ref_cache\n  refs: record the ref_store in ref_cache, not ref_dir\n  ref-cache: use a callback function to fill the cache\n  refs: handle \"refs/bisect/\" in `loose_fill_ref_dir()`\n  do_for_each_entry_in_dir(): eliminate `offset` argument\n  get_loose_ref_dir(): function renamed from get_loose_refs()\n  get_loose_ref_cache(): new function\n  cache_ref_iterator_begin(): make function smarter\n  commit_packed_refs(): use reference iteration\n  files_pack_refs(): use reference iteration\n  do_for_each_entry_in_dir(): delete function\n\n Makefile             |    1 +\n refs.c               |  111 ++++-\n refs.h               |    2 +-\n refs/files-backend.c | 1229 +++++++-------------------------------------------\n refs/ref-cache.c     |  523 +++++++++++++++++++++\n refs/ref-cache.h     |  267 +++++++++++\n refs/refs-internal.h |   22 +-\n 7 files changed, 1066 insertions(+), 1089 deletions(-)\n create mode 100644 refs/ref-cache.c\n create mode 100644 refs/ref-cache.h\n\n-- \n2.11.0\n\n"},{"id":"315895","messageId":"53e0eed5e13a04f502dcaeda83b2cbb3035f1396.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 01/20] get_ref_dir(): don't call read_loose_refs() for \"refs/bisect\"","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:10:59Z","receivedAt":"2017-03-31T14:11:41Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Since references under \"refs/bisect/\" are per-worktree, they have to\nbe sought in the worktree rather than in the main repository. But\nsince loose references are found by traversing directories, the\nreference iterator won't even get the idea to look for a\n\"refs/bisect/\" directory in the worktree if there is not a directory\nwith that name in the main repository. Thus `get_ref_dir()` manually\ninserts a dir_entry for \"refs/bisect/\" whenever it reads the entry for\n\"refs/\".\n\nThe current code then immediately calls `read_loose_refs()` on that\ndirectory. But since the dir_entry is created with its `incomplete`\nflag set, any traversal that gets to this point will read the\ndirectory automatically. So there is no need to call\n`read_loose_refs()` explicitly; the lazy mechanism suffices.\n\nAnd in fact, the attempt to `read_loose_refs()` was broken anyway.\nThat function needs its `dirname` argument to have a trailing `/`\ncharacter, but the invocation here was passing it \"refs/bisect\"\nwithout a trailing slash. So `read_loose_refs()` would read\n`$GIT_DIR/refs/bisect\" correctly, but if it found an entry \"foo\" in\nthat directory, it would try to read \"$GIT_DIR/refs/bisectfoo\".\nNormally it wouldn't find anything at that path, but the failure was\ncanceled out because `get_ref_dir()` *also* forgot to reset the\n`REF_INCOMPLETE` bit on the dir_entry. So the read was attempted again\nwhen it was accessed, via the lazy mechanism, and this time the read\nwas done correctly.\n\nThis code has been broken since it was first introduced.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 4242486118..e7a075e864 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -191,8 +191,6 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n \t\t\t\t\t\t\t       \"refs/bisect/\",\n \t\t\t\t\t\t\t       12, 1);\n \t\t\t\tadd_entry_to_dir(dir, child_entry);\n-\t\t\t\tread_loose_refs(\"refs/bisect\",\n-\t\t\t\t\t\t&child_entry->u.subdir);\n \t\t\t}\n \t\t}\n \t\tentry->flag &= ~REF_INCOMPLETE;\n-- \n2.11.0\n\n"},{"id":"315896","messageId":"3962e229cf0eb9daad8b5ed55b98fc9dbdd0d64a.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 02/20] refs_read_raw_ref(): new function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:00Z","receivedAt":"2017-03-31T14:11:44Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Extract a new function from `refs_resolve_ref_unsafe()`. It will be\nuseful elsewhere.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c               | 11 +++++++++--\n refs/refs-internal.h |  4 ++++\n 2 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 77a39f8b17..0ed6c3c7a4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1326,6 +1326,13 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)\n \treturn refs_for_each_rawref(get_main_ref_store(), fn, cb_data);\n }\n \n+int refs_read_raw_ref(struct ref_store *ref_store,\n+\t\t      const char *refname, unsigned char *sha1,\n+\t\t      struct strbuf *referent, unsigned int *type)\n+{\n+\treturn ref_store->be->read_raw_ref(ref_store, refname, sha1, referent, type);\n+}\n+\n /* This function needs to return a meaningful errno on failure */\n const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t    const char *refname,\n@@ -1362,8 +1369,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \tfor (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {\n \t\tunsigned int read_flags = 0;\n \n-\t\tif (refs->be->read_raw_ref(refs, refname,\n-\t\t\t\t\t   sha1, &sb_refname, &read_flags)) {\n+\t\tif (refs_read_raw_ref(refs, refname,\n+\t\t\t\t      sha1, &sb_refname, &read_flags)) {\n \t\t\t*flags |= read_flags;\n \t\t\tif (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))\n \t\t\t\treturn NULL;\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 690498698e..6ee9f20dbc 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -165,6 +165,10 @@ struct ref_update {\n \tconst char refname[FLEX_ARRAY];\n };\n \n+int refs_read_raw_ref(struct ref_store *ref_store,\n+\t\t      const char *refname, unsigned char *sha1,\n+\t\t      struct strbuf *referent, unsigned int *type);\n+\n /*\n  * Add a ref_update with the specified properties to transaction, and\n  * return a pointer to the new object. This function does not verify\n-- \n2.11.0\n\n"},{"id":"315897","messageId":"22abd274bfdada94b3654a811ee209822640765f.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 04/20] refs_verify_refname_available(): implement once for all backends","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:02Z","receivedAt":"2017-03-31T14:11:46Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It turns out that we can now implement\n`refs_verify_refname_available()` based on the other virtual\nfunctions, so there is no need for it to be defined at the backend\nlevel. Instead, define it once in `refs.c` and remove the\n`files_backend` definition.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c               | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++--\n refs.h               |  2 +-\n refs/files-backend.c | 39 +++++-------------------\n refs/refs-internal.h |  7 -----\n 4 files changed, 92 insertions(+), 41 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex aeb704ab92..2cf531b9c7 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -5,6 +5,7 @@\n #include \"cache.h\"\n #include \"hashmap.h\"\n #include \"lockfile.h\"\n+#include \"iterator.h\"\n #include \"refs.h\"\n #include \"refs/refs-internal.h\"\n #include \"object.h\"\n@@ -1661,11 +1662,91 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \n int refs_verify_refname_available(struct ref_store *refs,\n \t\t\t\t  const char *refname,\n-\t\t\t\t  const struct string_list *extra,\n+\t\t\t\t  const struct string_list *extras,\n \t\t\t\t  const struct string_list *skip,\n \t\t\t\t  struct strbuf *err)\n {\n-\treturn refs->be->verify_refname_available(refs, refname, extra, skip, err);\n+\tconst char *slash;\n+\tconst char *extra_refname;\n+\tstruct strbuf dirname = STRBUF_INIT;\n+\tstruct strbuf referent = STRBUF_INIT;\n+\tstruct object_id oid;\n+\tunsigned int type;\n+\tstruct ref_iterator *iter;\n+\tint ok;\n+\tint ret = -1;\n+\n+\t/*\n+\t * For the sake of comments in this function, suppose that\n+\t * refname is \"refs/foo/bar\".\n+\t */\n+\n+\tassert(err);\n+\n+\tstrbuf_grow(&dirname, strlen(refname) + 1);\n+\tfor (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {\n+\t\t/* Expand dirname to the new prefix, not including the trailing slash: */\n+\t\tstrbuf_add(&dirname, refname + dirname.len, slash - refname - dirname.len);\n+\n+\t\t/*\n+\t\t * We are still at a leading dir of the refname (e.g.,\n+\t\t * \"refs/foo\"; if there is a reference with that name,\n+\t\t * it is a conflict, *unless* it is in skip.\n+\t\t */\n+\t\tif (skip && string_list_has_string(skip, dirname.buf))\n+\t\t\tcontinue;\n+\n+\t\tif (!refs_read_raw_ref(refs, dirname.buf, oid.hash, &referent, &type)) {\n+\t\t\tstrbuf_addf(err, \"'%s' exists; cannot create '%s'\",\n+\t\t\t\t    dirname.buf, refname);\n+\t\t\tgoto cleanup;\n+\t\t}\n+\n+\t\tif (extras && string_list_has_string(extras, dirname.buf)) {\n+\t\t\tstrbuf_addf(err, \"cannot process '%s' and '%s' at the same time\",\n+\t\t\t\t    refname, dirname.buf);\n+\t\t\tgoto cleanup;\n+\t\t}\n+\t}\n+\n+\t/*\n+\t * We are at the leaf of our refname (e.g., \"refs/foo/bar\").\n+\t * There is no point in searching for a reference with that\n+\t * name, because a refname isn't considered to conflict with\n+\t * itself. But we still need to check for references whose\n+\t * names are in the \"refs/foo/bar/\" namespace, because they\n+\t * *do* conflict.\n+\t */\n+\tstrbuf_addstr(&dirname, refname + dirname.len);\n+\tstrbuf_addch(&dirname, '/');\n+\n+\titer = refs_ref_iterator_begin(refs, dirname.buf, 0,\n+\t\t\t\t       DO_FOR_EACH_INCLUDE_BROKEN);\n+\twhile ((ok = ref_iterator_advance(iter)) == ITER_OK) {\n+\t\tif (skip &&\n+\t\t    string_list_has_string(skip, iter->refname))\n+\t\t\tcontinue;\n+\n+\t\tstrbuf_addf(err, \"'%s' exists; cannot create '%s'\",\n+\t\t\t    iter->refname, refname);\n+\t\tok = ref_iterator_abort(iter);\n+\t\tgoto cleanup;\n+\t}\n+\n+\tif (ok != ITER_DONE)\n+\t\tdie(\"BUG: error while iterating over references\");\n+\n+\textra_refname = find_descendant_ref(dirname.buf, extras, skip);\n+\tif (extra_refname)\n+\t\tstrbuf_addf(err, \"cannot process '%s' and '%s' at the same time\",\n+\t\t\t    refname, extra_refname);\n+\telse\n+\t\tret = 0;\n+\n+cleanup:\n+\tstrbuf_release(&referent);\n+\tstrbuf_release(&dirname);\n+\treturn ret;\n }\n \n int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)\ndiff --git a/refs.h b/refs.h\nindex 49e97d7d5f..07cf4cd41b 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -97,7 +97,7 @@ int read_ref(const char *refname, unsigned char *sha1);\n \n int refs_verify_refname_available(struct ref_store *refs,\n \t\t\t\t  const char *refname,\n-\t\t\t\t  const struct string_list *extra,\n+\t\t\t\t  const struct string_list *extras,\n \t\t\t\t  const struct string_list *skip,\n \t\t\t\t  struct strbuf *err);\n \ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex e7a075e864..447f2767f3 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1725,10 +1725,9 @@ static int lock_raw_ref(struct files_ref_store *refs,\n \t\t\t\tgoto error_return;\n \t\t\t} else if (remove_dir_recursively(&ref_file,\n \t\t\t\t\t\t\t  REMOVE_DIR_EMPTY_ONLY)) {\n-\t\t\t\tif (verify_refname_available_dir(\n-\t\t\t\t\t\t    refname, extras, skip,\n-\t\t\t\t\t\t    get_loose_refs(refs),\n-\t\t\t\t\t\t    err)) {\n+\t\t\t\tif (refs_verify_refname_available(\n+\t\t\t\t\t\t    &refs->base, refname,\n+\t\t\t\t\t\t    extras, skip, err)) {\n \t\t\t\t\t/*\n \t\t\t\t\t * The error message set by\n \t\t\t\t\t * verify_refname_available() is OK.\n@@ -2097,9 +2096,9 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,\n \t\t */\n \t\tif (remove_empty_directories(&ref_file)) {\n \t\t\tlast_errno = errno;\n-\t\t\tif (!verify_refname_available_dir(\n-\t\t\t\t\t    refname, extras, skip,\n-\t\t\t\t\t    get_loose_refs(refs), err))\n+\t\t\tif (!refs_verify_refname_available(\n+\t\t\t\t\t    &refs->base,\n+\t\t\t\t\t    refname, extras, skip, err))\n \t\t\t\tstrbuf_addf(err, \"there are still refs under '%s'\",\n \t\t\t\t\t    refname);\n \t\t\tgoto error_return;\n@@ -2111,9 +2110,8 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,\n \tif (!resolved) {\n \t\tlast_errno = errno;\n \t\tif (last_errno != ENOTDIR ||\n-\t\t    !verify_refname_available_dir(\n-\t\t\t\t    refname, extras, skip,\n-\t\t\t\t    get_loose_refs(refs), err))\n+\t\t    !refs_verify_refname_available(&refs->base, refname,\n+\t\t\t\t\t\t   extras, skip, err))\n \t\t\tstrbuf_addf(err, \"unable to resolve reference '%s': %s\",\n \t\t\t\t    refname, strerror(last_errno));\n \n@@ -2609,26 +2607,6 @@ static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname)\n \treturn ret;\n }\n \n-static int files_verify_refname_available(struct ref_store *ref_store,\n-\t\t\t\t\t  const char *newname,\n-\t\t\t\t\t  const struct string_list *extras,\n-\t\t\t\t\t  const struct string_list *skip,\n-\t\t\t\t\t  struct strbuf *err)\n-{\n-\tstruct files_ref_store *refs =\n-\t\tfiles_downcast(ref_store, REF_STORE_READ, \"verify_refname_available\");\n-\tstruct ref_dir *packed_refs = get_packed_refs(refs);\n-\tstruct ref_dir *loose_refs = get_loose_refs(refs);\n-\n-\tif (verify_refname_available_dir(newname, extras, skip,\n-\t\t\t\t\t packed_refs, err) ||\n-\t    verify_refname_available_dir(newname, extras, skip,\n-\t\t\t\t\t loose_refs, err))\n-\t\treturn -1;\n-\n-\treturn 0;\n-}\n-\n static int write_ref_to_lockfile(struct ref_lock *lock,\n \t\t\t\t const unsigned char *sha1, struct strbuf *err);\n static int commit_ref_update(struct files_ref_store *refs,\n@@ -4291,7 +4269,6 @@ struct ref_storage_be refs_be_files = {\n \n \tfiles_ref_iterator_begin,\n \tfiles_read_raw_ref,\n-\tfiles_verify_refname_available,\n \n \tfiles_reflog_iterator_begin,\n \tfiles_for_each_reflog_ent,\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 545989ae7f..3d46131efb 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -590,12 +590,6 @@ typedef int read_raw_ref_fn(struct ref_store *ref_store,\n \t\t\t    const char *refname, unsigned char *sha1,\n \t\t\t    struct strbuf *referent, unsigned int *type);\n \n-typedef int verify_refname_available_fn(struct ref_store *ref_store,\n-\t\t\t\t\tconst char *newname,\n-\t\t\t\t\tconst struct string_list *extras,\n-\t\t\t\t\tconst struct string_list *skip,\n-\t\t\t\t\tstruct strbuf *err);\n-\n struct ref_storage_be {\n \tstruct ref_storage_be *next;\n \tconst char *name;\n@@ -612,7 +606,6 @@ struct ref_storage_be {\n \n \tref_iterator_begin_fn *iterator_begin;\n \tread_raw_ref_fn *read_raw_ref;\n-\tverify_refname_available_fn *verify_refname_available;\n \n \treflog_iterator_begin_fn *reflog_iterator_begin;\n \tfor_each_reflog_ent_fn *for_each_reflog_ent;\n-- \n2.11.0\n\n"},{"id":"315898","messageId":"b37435be49b285431166d4829d03faedb446817e.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 06/20] ref-cache: rename `add_ref()` to `add_ref_entry()`","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:04Z","receivedAt":"2017-03-31T14:11:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This function's visibility is about to be increased, so give it a more\ndistinctive name.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex cad56efb04..4d579cbdac 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -455,7 +455,7 @@ static int remove_entry(struct ref_dir *dir, const char *refname)\n  * subdirectories as necessary.  dir must represent the top-level\n  * directory.  Return 0 on success.\n  */\n-static int add_ref(struct ref_dir *dir, struct ref_entry *ref)\n+static int add_ref_entry(struct ref_dir *dir, struct ref_entry *ref)\n {\n \tdir = find_containing_dir(dir, ref->name, 1);\n \tif (!dir)\n@@ -994,7 +994,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)\n \t\t\tif (peeled == PEELED_FULLY ||\n \t\t\t    (peeled == PEELED_TAGS && starts_with(refname, \"refs/tags/\")))\n \t\t\t\tlast->flag |= REF_KNOWS_PEELED;\n-\t\t\tadd_ref(dir, last);\n+\t\t\tadd_ref_entry(dir, last);\n \t\t\tcontinue;\n \t\t}\n \t\tif (last &&\n@@ -1116,7 +1116,7 @@ static void add_packed_ref(struct files_ref_store *refs,\n \n \tif (!packed_ref_cache->lock)\n \t\tdie(\"internal error: packed refs not locked\");\n-\tadd_ref(get_packed_ref_dir(packed_ref_cache),\n+\tadd_ref_entry(get_packed_ref_dir(packed_ref_cache),\n \t\tcreate_ref_entry(refname, sha1, REF_ISPACKED, 1));\n }\n \n@@ -2179,7 +2179,7 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)\n \t} else {\n \t\tpacked_entry = create_ref_entry(entry->name, entry->u.value.oid.hash,\n \t\t\t\t\t\tREF_ISPACKED | REF_KNOWS_PEELED, 0);\n-\t\tadd_ref(cb->packed_refs, packed_entry);\n+\t\tadd_ref_entry(cb->packed_refs, packed_entry);\n \t}\n \toidcpy(&packed_entry->u.value.peeled, &entry->u.value.peeled);\n \n-- \n2.11.0\n\n"},{"id":"315899","messageId":"279df1fe591349db97aa4a5dd30ec57a0dba3c2e.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 07/20] ref-cache: rename `find_ref()` to `find_ref_entry()`","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:05Z","receivedAt":"2017-03-31T14:11:50Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This function's visibility is about to be increased, so give it a more\ndistinctive name.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 4d579cbdac..6768c8c86b 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -385,7 +385,7 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n  * and recursing into subdirectories as necessary.  If the name is not\n  * found or it corresponds to a directory entry, return NULL.\n  */\n-static struct ref_entry *find_ref(struct ref_dir *dir, const char *refname)\n+static struct ref_entry *find_ref_entry(struct ref_dir *dir, const char *refname)\n {\n \tint entry_index;\n \tstruct ref_entry *entry;\n@@ -1227,7 +1227,7 @@ static struct ref_dir *get_loose_refs(struct files_ref_store *refs)\n static struct ref_entry *get_packed_ref(struct files_ref_store *refs,\n \t\t\t\t\tconst char *refname)\n {\n-\treturn find_ref(get_packed_refs(refs), refname);\n+\treturn find_ref_entry(get_packed_refs(refs), refname);\n }\n \n /*\n@@ -2171,7 +2171,7 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)\n \tif (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)\n \t\tdie(\"internal error peeling reference %s (%s)\",\n \t\t    entry->name, oid_to_hex(&entry->u.value.oid));\n-\tpacked_entry = find_ref(cb->packed_refs, entry->name);\n+\tpacked_entry = find_ref_entry(cb->packed_refs, entry->name);\n \tif (packed_entry) {\n \t\t/* Overwrite existing packed entry with info from loose entry */\n \t\tpacked_entry->flag = REF_ISPACKED | REF_KNOWS_PEELED;\n-- \n2.11.0\n\n"},{"id":"315900","messageId":"d68fe455742381b9c64fcd25130011385ef1c6a1.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 08/20] ref-cache: rename `remove_entry()` to `remove_entry_from_dir()`","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:06Z","receivedAt":"2017-03-31T14:11:54Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This function's visibility is about to be increased, so give it a more\ndistinctive name.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 6768c8c86b..b4c11afadf 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -413,7 +413,7 @@ static struct ref_entry *find_ref_entry(struct ref_dir *dir, const char *refname\n  * empty by the removal.  dir must represent the top-level directory\n  * and must already be complete.\n  */\n-static int remove_entry(struct ref_dir *dir, const char *refname)\n+static int remove_entry_from_dir(struct ref_dir *dir, const char *refname)\n {\n \tint refname_len = strlen(refname);\n \tint entry_index;\n@@ -2338,7 +2338,7 @@ static int repack_without_refs(struct files_ref_store *refs,\n \n \t/* Remove refnames from the cache */\n \tfor_each_string_list_item(refname, refnames)\n-\t\tif (remove_entry(packed, refname->string) != -1)\n+\t\tif (remove_entry_from_dir(packed, refname->string) != -1)\n \t\t\tremoved = 1;\n \tif (!removed) {\n \t\t/*\n-- \n2.11.0\n\n"},{"id":"315901","messageId":"840b199ad58f11a690c684ad110807ed81d3b0eb.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 03/20] refs_ref_iterator_begin(): new function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:01Z","receivedAt":"2017-03-31T14:11:57Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Extract a new function from `do_for_each_ref()`. It will be useful\nelsewhere.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c               | 15 +++++++++++++--\n refs/refs-internal.h | 11 +++++++++++\n 2 files changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 0ed6c3c7a4..aeb704ab92 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1230,6 +1230,18 @@ int head_ref(each_ref_fn fn, void *cb_data)\n \treturn head_ref_submodule(NULL, fn, cb_data);\n }\n \n+struct ref_iterator *refs_ref_iterator_begin(\n+\t\tstruct ref_store *refs,\n+\t\tconst char *prefix, int trim, int flags)\n+{\n+\tstruct ref_iterator *iter;\n+\n+\titer = refs->be->iterator_begin(refs, prefix, flags);\n+\titer = prefix_ref_iterator_begin(iter, prefix, trim);\n+\n+\treturn iter;\n+}\n+\n /*\n  * Call fn for each reference in the specified submodule for which the\n  * refname begins with prefix. If trim is non-zero, then trim that\n@@ -1247,8 +1259,7 @@ static int do_for_each_ref(struct ref_store *refs, const char *prefix,\n \tif (!refs)\n \t\treturn 0;\n \n-\titer = refs->be->iterator_begin(refs, prefix, flags);\n-\titer = prefix_ref_iterator_begin(iter, prefix, trim);\n+\titer = refs_ref_iterator_begin(refs, prefix, trim, flags);\n \n \treturn do_for_each_ref_iterator(iter, fn, cb_data);\n }\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 6ee9f20dbc..545989ae7f 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -335,6 +335,17 @@ struct ref_iterator *empty_ref_iterator_begin(void);\n  */\n int is_empty_ref_iterator(struct ref_iterator *ref_iterator);\n \n+/*\n+ * Return an iterator that goes over each reference in `refs` for\n+ * which the refname begins with prefix. If trim is non-zero, then\n+ * trim that many characters off the beginning of each refname. flags\n+ * can be DO_FOR_EACH_INCLUDE_BROKEN to include broken references in\n+ * the iteration.\n+ */\n+struct ref_iterator *refs_ref_iterator_begin(\n+\t\tstruct ref_store *refs,\n+\t\tconst char *prefix, int trim, int flags);\n+\n /*\n  * A callback function used to instruct merge_ref_iterator how to\n  * interleave the entries from iter0 and iter1. The function should\n-- \n2.11.0\n\n"},{"id":"315902","messageId":"80808305cd86f4fb3b8b06cd78ba8d0a9cbf11a5.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 05/20] refs_verify_refname_available(): use function in more places","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:03Z","receivedAt":"2017-03-31T14:11:59Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Change `lock_raw_ref()` and `lock_ref_sha1_basic()` to use\n`refs_verify_refname_available()` instead of\n`verify_refname_available_dir()`. This means that those callsites now\ncheck for conflicts with all references rather than just packed refs,\nbut the performance cost shouldn't be significant (and will be\nregained later).\n\nThese were the last callers of `verify_refname_available_dir()`, so\nalso delete that (very complicated) function.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 171 ++++-----------------------------------------------\n 1 file changed, 11 insertions(+), 160 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 447f2767f3..cad56efb04 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -738,152 +738,6 @@ static struct ref_iterator *cache_ref_iterator_begin(struct ref_dir *dir)\n \treturn ref_iterator;\n }\n \n-struct nonmatching_ref_data {\n-\tconst struct string_list *skip;\n-\tconst char *conflicting_refname;\n-};\n-\n-static int nonmatching_ref_fn(struct ref_entry *entry, void *vdata)\n-{\n-\tstruct nonmatching_ref_data *data = vdata;\n-\n-\tif (data->skip && string_list_has_string(data->skip, entry->name))\n-\t\treturn 0;\n-\n-\tdata->conflicting_refname = entry->name;\n-\treturn 1;\n-}\n-\n-/*\n- * Return 0 if a reference named refname could be created without\n- * conflicting with the name of an existing reference in dir.\n- * See verify_refname_available for more information.\n- */\n-static int verify_refname_available_dir(const char *refname,\n-\t\t\t\t\tconst struct string_list *extras,\n-\t\t\t\t\tconst struct string_list *skip,\n-\t\t\t\t\tstruct ref_dir *dir,\n-\t\t\t\t\tstruct strbuf *err)\n-{\n-\tconst char *slash;\n-\tconst char *extra_refname;\n-\tint pos;\n-\tstruct strbuf dirname = STRBUF_INIT;\n-\tint ret = -1;\n-\n-\t/*\n-\t * For the sake of comments in this function, suppose that\n-\t * refname is \"refs/foo/bar\".\n-\t */\n-\n-\tassert(err);\n-\n-\tstrbuf_grow(&dirname, strlen(refname) + 1);\n-\tfor (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {\n-\t\t/* Expand dirname to the new prefix, not including the trailing slash: */\n-\t\tstrbuf_add(&dirname, refname + dirname.len, slash - refname - dirname.len);\n-\n-\t\t/*\n-\t\t * We are still at a leading dir of the refname (e.g.,\n-\t\t * \"refs/foo\"; if there is a reference with that name,\n-\t\t * it is a conflict, *unless* it is in skip.\n-\t\t */\n-\t\tif (dir) {\n-\t\t\tpos = search_ref_dir(dir, dirname.buf, dirname.len);\n-\t\t\tif (pos >= 0 &&\n-\t\t\t    (!skip || !string_list_has_string(skip, dirname.buf))) {\n-\t\t\t\t/*\n-\t\t\t\t * We found a reference whose name is\n-\t\t\t\t * a proper prefix of refname; e.g.,\n-\t\t\t\t * \"refs/foo\", and is not in skip.\n-\t\t\t\t */\n-\t\t\t\tstrbuf_addf(err, \"'%s' exists; cannot create '%s'\",\n-\t\t\t\t\t    dirname.buf, refname);\n-\t\t\t\tgoto cleanup;\n-\t\t\t}\n-\t\t}\n-\n-\t\tif (extras && string_list_has_string(extras, dirname.buf) &&\n-\t\t    (!skip || !string_list_has_string(skip, dirname.buf))) {\n-\t\t\tstrbuf_addf(err, \"cannot process '%s' and '%s' at the same time\",\n-\t\t\t\t    refname, dirname.buf);\n-\t\t\tgoto cleanup;\n-\t\t}\n-\n-\t\t/*\n-\t\t * Otherwise, we can try to continue our search with\n-\t\t * the next component. So try to look up the\n-\t\t * directory, e.g., \"refs/foo/\". If we come up empty,\n-\t\t * we know there is nothing under this whole prefix,\n-\t\t * but even in that case we still have to continue the\n-\t\t * search for conflicts with extras.\n-\t\t */\n-\t\tstrbuf_addch(&dirname, '/');\n-\t\tif (dir) {\n-\t\t\tpos = search_ref_dir(dir, dirname.buf, dirname.len);\n-\t\t\tif (pos < 0) {\n-\t\t\t\t/*\n-\t\t\t\t * There was no directory \"refs/foo/\",\n-\t\t\t\t * so there is nothing under this\n-\t\t\t\t * whole prefix. So there is no need\n-\t\t\t\t * to continue looking for conflicting\n-\t\t\t\t * references. But we need to continue\n-\t\t\t\t * looking for conflicting extras.\n-\t\t\t\t */\n-\t\t\t\tdir = NULL;\n-\t\t\t} else {\n-\t\t\t\tdir = get_ref_dir(dir->entries[pos]);\n-\t\t\t}\n-\t\t}\n-\t}\n-\n-\t/*\n-\t * We are at the leaf of our refname (e.g., \"refs/foo/bar\").\n-\t * There is no point in searching for a reference with that\n-\t * name, because a refname isn't considered to conflict with\n-\t * itself. But we still need to check for references whose\n-\t * names are in the \"refs/foo/bar/\" namespace, because they\n-\t * *do* conflict.\n-\t */\n-\tstrbuf_addstr(&dirname, refname + dirname.len);\n-\tstrbuf_addch(&dirname, '/');\n-\n-\tif (dir) {\n-\t\tpos = search_ref_dir(dir, dirname.buf, dirname.len);\n-\n-\t\tif (pos >= 0) {\n-\t\t\t/*\n-\t\t\t * We found a directory named \"$refname/\"\n-\t\t\t * (e.g., \"refs/foo/bar/\"). It is a problem\n-\t\t\t * iff it contains any ref that is not in\n-\t\t\t * \"skip\".\n-\t\t\t */\n-\t\t\tstruct nonmatching_ref_data data;\n-\n-\t\t\tdata.skip = skip;\n-\t\t\tdata.conflicting_refname = NULL;\n-\t\t\tdir = get_ref_dir(dir->entries[pos]);\n-\t\t\tsort_ref_dir(dir);\n-\t\t\tif (do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, &data)) {\n-\t\t\t\tstrbuf_addf(err, \"'%s' exists; cannot create '%s'\",\n-\t\t\t\t\t    data.conflicting_refname, refname);\n-\t\t\t\tgoto cleanup;\n-\t\t\t}\n-\t\t}\n-\t}\n-\n-\textra_refname = find_descendant_ref(dirname.buf, extras, skip);\n-\tif (extra_refname)\n-\t\tstrbuf_addf(err, \"cannot process '%s' and '%s' at the same time\",\n-\t\t\t    refname, extra_refname);\n-\telse\n-\t\tret = 0;\n-\n-cleanup:\n-\tstrbuf_release(&dirname);\n-\treturn ret;\n-}\n-\n struct packed_ref_cache {\n \tstruct ref_entry *root;\n \n@@ -1563,7 +1417,7 @@ static void unlock_ref(struct ref_lock *lock)\n  *\n  * If the reference doesn't already exist, verify that refname doesn't\n  * have a D/F conflict with any existing references. extras and skip\n- * are passed to verify_refname_available_dir() for this check.\n+ * are passed to refs_verify_refname_available() for this check.\n  *\n  * If mustexist is not set and the reference is not found or is\n  * broken, lock the reference anyway but clear sha1.\n@@ -1578,7 +1432,7 @@ static void unlock_ref(struct ref_lock *lock)\n  *\n  * but it includes a lot more code to\n  * - Deal with possible races with other processes\n- * - Avoid calling verify_refname_available_dir() when it can be\n+ * - Avoid calling refs_verify_refname_available() when it can be\n  *   avoided, namely if we were successfully able to read the ref\n  * - Generate informative error messages in the case of failure\n  */\n@@ -1635,7 +1489,8 @@ static int lock_raw_ref(struct files_ref_store *refs,\n \t\t\t} else {\n \t\t\t\t/*\n \t\t\t\t * The error message set by\n-\t\t\t\t * verify_refname_available_dir() is OK.\n+\t\t\t\t * refs_verify_refname_available() is\n+\t\t\t\t * OK.\n \t\t\t\t */\n \t\t\t\tret = TRANSACTION_NAME_CONFLICT;\n \t\t\t}\n@@ -1759,16 +1614,13 @@ static int lock_raw_ref(struct files_ref_store *refs,\n \n \t\t/*\n \t\t * If the ref did not exist and we are creating it,\n-\t\t * make sure there is no existing packed ref whose\n-\t\t * name begins with our refname, nor a packed ref\n-\t\t * whose name is a proper prefix of our refname.\n+\t\t * make sure there is no existing ref that conflicts\n+\t\t * with refname:\n \t\t */\n-\t\tif (verify_refname_available_dir(\n-\t\t\t\t    refname, extras, skip,\n-\t\t\t\t    get_packed_refs(refs),\n-\t\t\t\t    err)) {\n+\t\tif (refs_verify_refname_available(\n+\t\t\t\t    &refs->base, refname,\n+\t\t\t\t    extras, skip, err))\n \t\t\tgoto error_return;\n-\t\t}\n \t}\n \n \tret = 0;\n@@ -2125,9 +1977,8 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,\n \t * our refname.\n \t */\n \tif (is_null_oid(&lock->old_oid) &&\n-\t    verify_refname_available_dir(refname, extras, skip,\n-\t\t\t\t\t get_packed_refs(refs),\n-\t\t\t\t\t err)) {\n+\t    refs_verify_refname_available(&refs->base, refname,\n+\t\t\t\t\t  extras, skip, err)) {\n \t\tlast_errno = ENOTDIR;\n \t\tgoto error_return;\n \t}\n-- \n2.11.0\n\n"},{"id":"315903","messageId":"b5754fdf848f4b924cc3d682c854ea4fea723016.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 12/20] ref-cache: use a callback function to fill the cache","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:10Z","receivedAt":"2017-03-31T14:12:03Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It is a leveling violation for `ref_cache` to know about\n`files_ref_store` or that it should call `read_loose_refs()` to lazily\nfill cache directories. So instead, have its constructor take as an\nargument a callback function that it should use for lazy-filling, and\nchange `files_ref_store` to supply a pointer to function\n`read_loose_refs` (renamed to `loose_fill_ref_dir`) when creating the\nref cache for its loose refs.\n\nThis means that we can generify the type of the back-pointer in\n`struct ref_cache` from `files_ref_store` to `ref_store`.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 10 ++++++----\n refs/ref-cache.c     | 12 +++++++-----\n refs/ref-cache.h     | 29 +++++++++++++++++++++++++----\n 3 files changed, 38 insertions(+), 13 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 78572e55a0..e4d78393ac 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -387,7 +387,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref\n \n \t\trefs->packed = xcalloc(1, sizeof(*refs->packed));\n \t\tacquire_packed_ref_cache(refs->packed);\n-\t\trefs->packed->cache = create_ref_cache(refs);\n+\t\trefs->packed->cache = create_ref_cache(&refs->base, NULL);\n \t\trefs->packed->cache->root->flag &= ~REF_INCOMPLETE;\n \t\tf = fopen(packed_refs_file, \"r\");\n \t\tif (f) {\n@@ -431,9 +431,11 @@ static void add_packed_ref(struct files_ref_store *refs,\n  * (without recursing).  dirname must end with '/'.  dir must be the\n  * directory entry corresponding to dirname.\n  */\n-void read_loose_refs(const char *dirname, struct ref_dir *dir)\n+static void loose_fill_ref_dir(struct ref_store *ref_store,\n+\t\t\t       struct ref_dir *dir, const char *dirname)\n {\n-\tstruct files_ref_store *refs = dir->cache->ref_store;\n+\tstruct files_ref_store *refs =\n+\t\tfiles_downcast(ref_store, REF_STORE_READ, \"fill_ref_dir\");\n \tDIR *d;\n \tstruct dirent *de;\n \tint dirnamelen = strlen(dirname);\n@@ -516,7 +518,7 @@ static struct ref_dir *get_loose_refs(struct files_ref_store *refs)\n \t\t * are about to read the only subdirectory that can\n \t\t * hold references:\n \t\t */\n-\t\trefs->loose = create_ref_cache(refs);\n+\t\trefs->loose = create_ref_cache(&refs->base, loose_fill_ref_dir);\n \n \t\t/* We're going to fill the top level ourselves: */\n \t\trefs->loose->root->flag &= ~REF_INCOMPLETE;\ndiff --git a/refs/ref-cache.c b/refs/ref-cache.c\nindex 96da094788..7f247b9170 100644\n--- a/refs/ref-cache.c\n+++ b/refs/ref-cache.c\n@@ -4,9 +4,6 @@\n #include \"ref-cache.h\"\n #include \"../iterator.h\"\n \n-/* FIXME: This declaration shouldn't be here */\n-void read_loose_refs(const char *dirname, struct ref_dir *dir);\n-\n void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry)\n {\n \tALLOC_GROW(dir->entries, dir->nr + 1, dir->alloc);\n@@ -25,7 +22,10 @@ struct ref_dir *get_ref_dir(struct ref_entry *entry)\n \tassert(entry->flag & REF_DIR);\n \tdir = &entry->u.subdir;\n \tif (entry->flag & REF_INCOMPLETE) {\n-\t\tread_loose_refs(entry->name, dir);\n+\t\tif (!dir->cache->fill_ref_dir)\n+\t\t\tdie(\"BUG: incomplete ref_store without fill_ref_dir function\");\n+\n+\t\tdir->cache->fill_ref_dir(dir->cache->ref_store, dir, entry->name);\n \n \t\t/*\n \t\t * Manually add refs/bisect, which, being\n@@ -63,11 +63,13 @@ struct ref_entry *create_ref_entry(const char *refname,\n \treturn ref;\n }\n \n-struct ref_cache *create_ref_cache(struct files_ref_store *refs)\n+struct ref_cache *create_ref_cache(struct ref_store *refs,\n+\t\t\t\t   fill_ref_dir_fn *fill_ref_dir)\n {\n \tstruct ref_cache *ret = xcalloc(1, sizeof(*ret));\n \n \tret->ref_store = refs;\n+\tret->fill_ref_dir = fill_ref_dir;\n \tret->root = create_dir_entry(ret, \"\", 0, 1);\n \treturn ret;\n }\ndiff --git a/refs/ref-cache.h b/refs/ref-cache.h\nindex 83051854ff..ed51e80d88 100644\n--- a/refs/ref-cache.h\n+++ b/refs/ref-cache.h\n@@ -1,11 +1,27 @@\n #ifndef REFS_REF_CACHE_H\n #define REFS_REF_CACHE_H\n \n+struct ref_dir;\n+\n+/*\n+ * If this ref_cache is filled lazily, this function is used to load\n+ * information into the specified ref_dir (shallow or deep, at the\n+ * option of the ref_store). dirname includes a trailing slash.\n+ */\n+typedef void fill_ref_dir_fn(struct ref_store *ref_store,\n+\t\t\t     struct ref_dir *dir, const char *dirname);\n+\n struct ref_cache {\n \tstruct ref_entry *root;\n \n-\t/* A pointer to the files_ref_store whose cache this is: */\n-\tstruct files_ref_store *ref_store;\n+\t/* A pointer to the ref_store whose cache this is: */\n+\tstruct ref_store *ref_store;\n+\n+\t/*\n+\t * Function used (if necessary) to lazily-fill cache. May be\n+\t * NULL.\n+\t */\n+\tfill_ref_dir_fn *fill_ref_dir;\n };\n \n /*\n@@ -174,9 +190,14 @@ struct ref_entry *create_ref_entry(const char *refname,\n \n /*\n  * Return a pointer to a new `ref_cache`. Its top-level starts out\n- * marked incomplete.\n+ * marked incomplete. If `fill_ref_dir` is non-NULL, it is the\n+ * function called to fill in incomplete directories in the\n+ * `ref_cache` when they are accessed. If it is NULL, then the whole\n+ * `ref_cache` must be filled (including clearing its directories'\n+ * `REF_INCOMPLETE` bits) before it is used.\n  */\n-struct ref_cache *create_ref_cache(struct files_ref_store *refs);\n+struct ref_cache *create_ref_cache(struct ref_store *refs,\n+\t\t\t\t   fill_ref_dir_fn *fill_ref_dir);\n \n /*\n  * Free the `ref_cache` and all of its associated data.\n-- \n2.11.0\n\n"},{"id":"315904","messageId":"37e7ece105c11f33a3c1640221e24a7432cca3b9.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 09/20] refs: split `ref_cache` code into separate files","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:07Z","receivedAt":"2017-03-31T14:12:07Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The `ref_cache` code is currently too tightly coupled to\n`files-backend`, making the code harder to understand and making it\nawkward for new code to use `ref_cache` (as we indeed have planned).\nStart loosening that coupling by splitting `ref_cache` into a separate\nmodule.\n\nThis commit moves code, adds declarations, and changes the visibility\nof some functions, but doesn't change any code.\n\nThe modules are still too tightly coupled, but the situation will be\nimproved in subsequent commits.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n Makefile             |   1 +\n refs/files-backend.c | 736 +--------------------------------------------------\n refs/ref-cache.c     | 512 +++++++++++++++++++++++++++++++++++\n refs/ref-cache.h     | 251 ++++++++++++++++++\n 4 files changed, 767 insertions(+), 733 deletions(-)\n create mode 100644 refs/ref-cache.c\n create mode 100644 refs/ref-cache.h\n\ndiff --git a/Makefile b/Makefile\nindex 5f3844e33e..2f30580cde 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -807,6 +807,7 @@ LIB_OBJS += reflog-walk.o\n LIB_OBJS += refs.o\n LIB_OBJS += refs/files-backend.o\n LIB_OBJS += refs/iterator.o\n+LIB_OBJS += refs/ref-cache.o\n LIB_OBJS += ref-filter.o\n LIB_OBJS += remote.o\n LIB_OBJS += replace_object.o\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex b4c11afadf..f07e93d522 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1,6 +1,7 @@\n #include \"../cache.h\"\n #include \"../refs.h\"\n #include \"refs-internal.h\"\n+#include \"ref-cache.h\"\n #include \"../iterator.h\"\n #include \"../dir-iterator.h\"\n #include \"../lockfile.h\"\n@@ -13,509 +14,6 @@ struct ref_lock {\n \tstruct object_id old_oid;\n };\n \n-struct ref_entry;\n-\n-/*\n- * Information used (along with the information in ref_entry) to\n- * describe a single cached reference.  This data structure only\n- * occurs embedded in a union in struct ref_entry, and only when\n- * (ref_entry->flag & REF_DIR) is zero.\n- */\n-struct ref_value {\n-\t/*\n-\t * The name of the object to which this reference resolves\n-\t * (which may be a tag object).  If REF_ISBROKEN, this is\n-\t * null.  If REF_ISSYMREF, then this is the name of the object\n-\t * referred to by the last reference in the symlink chain.\n-\t */\n-\tstruct object_id oid;\n-\n-\t/*\n-\t * If REF_KNOWS_PEELED, then this field holds the peeled value\n-\t * of this reference, or null if the reference is known not to\n-\t * be peelable.  See the documentation for peel_ref() for an\n-\t * exact definition of \"peelable\".\n-\t */\n-\tstruct object_id peeled;\n-};\n-\n-struct files_ref_store;\n-\n-/*\n- * Information used (along with the information in ref_entry) to\n- * describe a level in the hierarchy of references.  This data\n- * structure only occurs embedded in a union in struct ref_entry, and\n- * only when (ref_entry.flag & REF_DIR) is set.  In that case,\n- * (ref_entry.flag & REF_INCOMPLETE) determines whether the references\n- * in the directory have already been read:\n- *\n- *     (ref_entry.flag & REF_INCOMPLETE) unset -- a directory of loose\n- *         or packed references, already read.\n- *\n- *     (ref_entry.flag & REF_INCOMPLETE) set -- a directory of loose\n- *         references that hasn't been read yet (nor has any of its\n- *         subdirectories).\n- *\n- * Entries within a directory are stored within a growable array of\n- * pointers to ref_entries (entries, nr, alloc).  Entries 0 <= i <\n- * sorted are sorted by their component name in strcmp() order and the\n- * remaining entries are unsorted.\n- *\n- * Loose references are read lazily, one directory at a time.  When a\n- * directory of loose references is read, then all of the references\n- * in that directory are stored, and REF_INCOMPLETE stubs are created\n- * for any subdirectories, but the subdirectories themselves are not\n- * read.  The reading is triggered by get_ref_dir().\n- */\n-struct ref_dir {\n-\tint nr, alloc;\n-\n-\t/*\n-\t * Entries with index 0 <= i < sorted are sorted by name.  New\n-\t * entries are appended to the list unsorted, and are sorted\n-\t * only when required; thus we avoid the need to sort the list\n-\t * after the addition of every reference.\n-\t */\n-\tint sorted;\n-\n-\t/* A pointer to the files_ref_store that contains this ref_dir. */\n-\tstruct files_ref_store *ref_store;\n-\n-\tstruct ref_entry **entries;\n-};\n-\n-/*\n- * Bit values for ref_entry::flag.  REF_ISSYMREF=0x01,\n- * REF_ISPACKED=0x02, REF_ISBROKEN=0x04 and REF_BAD_NAME=0x08 are\n- * public values; see refs.h.\n- */\n-\n-/*\n- * The field ref_entry->u.value.peeled of this value entry contains\n- * the correct peeled value for the reference, which might be\n- * null_sha1 if the reference is not a tag or if it is broken.\n- */\n-#define REF_KNOWS_PEELED 0x10\n-\n-/* ref_entry represents a directory of references */\n-#define REF_DIR 0x20\n-\n-/*\n- * Entry has not yet been read from disk (used only for REF_DIR\n- * entries representing loose references)\n- */\n-#define REF_INCOMPLETE 0x40\n-\n-/*\n- * A ref_entry represents either a reference or a \"subdirectory\" of\n- * references.\n- *\n- * Each directory in the reference namespace is represented by a\n- * ref_entry with (flags & REF_DIR) set and containing a subdir member\n- * that holds the entries in that directory that have been read so\n- * far.  If (flags & REF_INCOMPLETE) is set, then the directory and\n- * its subdirectories haven't been read yet.  REF_INCOMPLETE is only\n- * used for loose reference directories.\n- *\n- * References are represented by a ref_entry with (flags & REF_DIR)\n- * unset and a value member that describes the reference's value.  The\n- * flag member is at the ref_entry level, but it is also needed to\n- * interpret the contents of the value field (in other words, a\n- * ref_value object is not very much use without the enclosing\n- * ref_entry).\n- *\n- * Reference names cannot end with slash and directories' names are\n- * always stored with a trailing slash (except for the top-level\n- * directory, which is always denoted by \"\").  This has two nice\n- * consequences: (1) when the entries in each subdir are sorted\n- * lexicographically by name (as they usually are), the references in\n- * a whole tree can be generated in lexicographic order by traversing\n- * the tree in left-to-right, depth-first order; (2) the names of\n- * references and subdirectories cannot conflict, and therefore the\n- * presence of an empty subdirectory does not block the creation of a\n- * similarly-named reference.  (The fact that reference names with the\n- * same leading components can conflict *with each other* is a\n- * separate issue that is regulated by verify_refname_available().)\n- *\n- * Please note that the name field contains the fully-qualified\n- * reference (or subdirectory) name.  Space could be saved by only\n- * storing the relative names.  But that would require the full names\n- * to be generated on the fly when iterating in do_for_each_ref(), and\n- * would break callback functions, who have always been able to assume\n- * that the name strings that they are passed will not be freed during\n- * the iteration.\n- */\n-struct ref_entry {\n-\tunsigned char flag; /* ISSYMREF? ISPACKED? */\n-\tunion {\n-\t\tstruct ref_value value; /* if not (flags&REF_DIR) */\n-\t\tstruct ref_dir subdir; /* if (flags&REF_DIR) */\n-\t} u;\n-\t/*\n-\t * The full name of the reference (e.g., \"refs/heads/master\")\n-\t * or the full name of the directory with a trailing slash\n-\t * (e.g., \"refs/heads/\"):\n-\t */\n-\tchar name[FLEX_ARRAY];\n-};\n-\n-static void read_loose_refs(const char *dirname, struct ref_dir *dir);\n-static int search_ref_dir(struct ref_dir *dir, const char *refname, size_t len);\n-static struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,\n-\t\t\t\t\t  const char *dirname, size_t len,\n-\t\t\t\t\t  int incomplete);\n-static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);\n-static int files_log_ref_write(struct files_ref_store *refs,\n-\t\t\t       const char *refname, const unsigned char *old_sha1,\n-\t\t\t       const unsigned char *new_sha1, const char *msg,\n-\t\t\t       int flags, struct strbuf *err);\n-\n-static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n-{\n-\tstruct ref_dir *dir;\n-\tassert(entry->flag & REF_DIR);\n-\tdir = &entry->u.subdir;\n-\tif (entry->flag & REF_INCOMPLETE) {\n-\t\tread_loose_refs(entry->name, dir);\n-\n-\t\t/*\n-\t\t * Manually add refs/bisect, which, being\n-\t\t * per-worktree, might not appear in the directory\n-\t\t * listing for refs/ in the main repo.\n-\t\t */\n-\t\tif (!strcmp(entry->name, \"refs/\")) {\n-\t\t\tint pos = search_ref_dir(dir, \"refs/bisect/\", 12);\n-\t\t\tif (pos < 0) {\n-\t\t\t\tstruct ref_entry *child_entry;\n-\t\t\t\tchild_entry = create_dir_entry(dir->ref_store,\n-\t\t\t\t\t\t\t       \"refs/bisect/\",\n-\t\t\t\t\t\t\t       12, 1);\n-\t\t\t\tadd_entry_to_dir(dir, child_entry);\n-\t\t\t}\n-\t\t}\n-\t\tentry->flag &= ~REF_INCOMPLETE;\n-\t}\n-\treturn dir;\n-}\n-\n-static struct ref_entry *create_ref_entry(const char *refname,\n-\t\t\t\t\t  const unsigned char *sha1, int flag,\n-\t\t\t\t\t  int check_name)\n-{\n-\tstruct ref_entry *ref;\n-\n-\tif (check_name &&\n-\t    check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))\n-\t\tdie(\"Reference has invalid format: '%s'\", refname);\n-\tFLEX_ALLOC_STR(ref, name, refname);\n-\thashcpy(ref->u.value.oid.hash, sha1);\n-\toidclr(&ref->u.value.peeled);\n-\tref->flag = flag;\n-\treturn ref;\n-}\n-\n-static void clear_ref_dir(struct ref_dir *dir);\n-\n-static void free_ref_entry(struct ref_entry *entry)\n-{\n-\tif (entry->flag & REF_DIR) {\n-\t\t/*\n-\t\t * Do not use get_ref_dir() here, as that might\n-\t\t * trigger the reading of loose refs.\n-\t\t */\n-\t\tclear_ref_dir(&entry->u.subdir);\n-\t}\n-\tfree(entry);\n-}\n-\n-/*\n- * Add a ref_entry to the end of dir (unsorted).  Entry is always\n- * stored directly in dir; no recursion into subdirectories is\n- * done.\n- */\n-static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry)\n-{\n-\tALLOC_GROW(dir->entries, dir->nr + 1, dir->alloc);\n-\tdir->entries[dir->nr++] = entry;\n-\t/* optimize for the case that entries are added in order */\n-\tif (dir->nr == 1 ||\n-\t    (dir->nr == dir->sorted + 1 &&\n-\t     strcmp(dir->entries[dir->nr - 2]->name,\n-\t\t    dir->entries[dir->nr - 1]->name) < 0))\n-\t\tdir->sorted = dir->nr;\n-}\n-\n-/*\n- * Clear and free all entries in dir, recursively.\n- */\n-static void clear_ref_dir(struct ref_dir *dir)\n-{\n-\tint i;\n-\tfor (i = 0; i < dir->nr; i++)\n-\t\tfree_ref_entry(dir->entries[i]);\n-\tfree(dir->entries);\n-\tdir->sorted = dir->nr = dir->alloc = 0;\n-\tdir->entries = NULL;\n-}\n-\n-/*\n- * Create a struct ref_entry object for the specified dirname.\n- * dirname is the name of the directory with a trailing slash (e.g.,\n- * \"refs/heads/\") or \"\" for the top-level directory.\n- */\n-static struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,\n-\t\t\t\t\t  const char *dirname, size_t len,\n-\t\t\t\t\t  int incomplete)\n-{\n-\tstruct ref_entry *direntry;\n-\tFLEX_ALLOC_MEM(direntry, name, dirname, len);\n-\tdirentry->u.subdir.ref_store = ref_store;\n-\tdirentry->flag = REF_DIR | (incomplete ? REF_INCOMPLETE : 0);\n-\treturn direntry;\n-}\n-\n-static int ref_entry_cmp(const void *a, const void *b)\n-{\n-\tstruct ref_entry *one = *(struct ref_entry **)a;\n-\tstruct ref_entry *two = *(struct ref_entry **)b;\n-\treturn strcmp(one->name, two->name);\n-}\n-\n-static void sort_ref_dir(struct ref_dir *dir);\n-\n-struct string_slice {\n-\tsize_t len;\n-\tconst char *str;\n-};\n-\n-static int ref_entry_cmp_sslice(const void *key_, const void *ent_)\n-{\n-\tconst struct string_slice *key = key_;\n-\tconst struct ref_entry *ent = *(const struct ref_entry * const *)ent_;\n-\tint cmp = strncmp(key->str, ent->name, key->len);\n-\tif (cmp)\n-\t\treturn cmp;\n-\treturn '\\0' - (unsigned char)ent->name[key->len];\n-}\n-\n-/*\n- * Return the index of the entry with the given refname from the\n- * ref_dir (non-recursively), sorting dir if necessary.  Return -1 if\n- * no such entry is found.  dir must already be complete.\n- */\n-static int search_ref_dir(struct ref_dir *dir, const char *refname, size_t len)\n-{\n-\tstruct ref_entry **r;\n-\tstruct string_slice key;\n-\n-\tif (refname == NULL || !dir->nr)\n-\t\treturn -1;\n-\n-\tsort_ref_dir(dir);\n-\tkey.len = len;\n-\tkey.str = refname;\n-\tr = bsearch(&key, dir->entries, dir->nr, sizeof(*dir->entries),\n-\t\t    ref_entry_cmp_sslice);\n-\n-\tif (r == NULL)\n-\t\treturn -1;\n-\n-\treturn r - dir->entries;\n-}\n-\n-/*\n- * Search for a directory entry directly within dir (without\n- * recursing).  Sort dir if necessary.  subdirname must be a directory\n- * name (i.e., end in '/').  If mkdir is set, then create the\n- * directory if it is missing; otherwise, return NULL if the desired\n- * directory cannot be found.  dir must already be complete.\n- */\n-static struct ref_dir *search_for_subdir(struct ref_dir *dir,\n-\t\t\t\t\t const char *subdirname, size_t len,\n-\t\t\t\t\t int mkdir)\n-{\n-\tint entry_index = search_ref_dir(dir, subdirname, len);\n-\tstruct ref_entry *entry;\n-\tif (entry_index == -1) {\n-\t\tif (!mkdir)\n-\t\t\treturn NULL;\n-\t\t/*\n-\t\t * Since dir is complete, the absence of a subdir\n-\t\t * means that the subdir really doesn't exist;\n-\t\t * therefore, create an empty record for it but mark\n-\t\t * the record complete.\n-\t\t */\n-\t\tentry = create_dir_entry(dir->ref_store, subdirname, len, 0);\n-\t\tadd_entry_to_dir(dir, entry);\n-\t} else {\n-\t\tentry = dir->entries[entry_index];\n-\t}\n-\treturn get_ref_dir(entry);\n-}\n-\n-/*\n- * If refname is a reference name, find the ref_dir within the dir\n- * tree that should hold refname.  If refname is a directory name\n- * (i.e., ends in '/'), then return that ref_dir itself.  dir must\n- * represent the top-level directory and must already be complete.\n- * Sort ref_dirs and recurse into subdirectories as necessary.  If\n- * mkdir is set, then create any missing directories; otherwise,\n- * return NULL if the desired directory cannot be found.\n- */\n-static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n-\t\t\t\t\t   const char *refname, int mkdir)\n-{\n-\tconst char *slash;\n-\tfor (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {\n-\t\tsize_t dirnamelen = slash - refname + 1;\n-\t\tstruct ref_dir *subdir;\n-\t\tsubdir = search_for_subdir(dir, refname, dirnamelen, mkdir);\n-\t\tif (!subdir) {\n-\t\t\tdir = NULL;\n-\t\t\tbreak;\n-\t\t}\n-\t\tdir = subdir;\n-\t}\n-\n-\treturn dir;\n-}\n-\n-/*\n- * Find the value entry with the given name in dir, sorting ref_dirs\n- * and recursing into subdirectories as necessary.  If the name is not\n- * found or it corresponds to a directory entry, return NULL.\n- */\n-static struct ref_entry *find_ref_entry(struct ref_dir *dir, const char *refname)\n-{\n-\tint entry_index;\n-\tstruct ref_entry *entry;\n-\tdir = find_containing_dir(dir, refname, 0);\n-\tif (!dir)\n-\t\treturn NULL;\n-\tentry_index = search_ref_dir(dir, refname, strlen(refname));\n-\tif (entry_index == -1)\n-\t\treturn NULL;\n-\tentry = dir->entries[entry_index];\n-\treturn (entry->flag & REF_DIR) ? NULL : entry;\n-}\n-\n-/*\n- * Remove the entry with the given name from dir, recursing into\n- * subdirectories as necessary.  If refname is the name of a directory\n- * (i.e., ends with '/'), then remove the directory and its contents.\n- * If the removal was successful, return the number of entries\n- * remaining in the directory entry that contained the deleted entry.\n- * If the name was not found, return -1.  Please note that this\n- * function only deletes the entry from the cache; it does not delete\n- * it from the filesystem or ensure that other cache entries (which\n- * might be symbolic references to the removed entry) are updated.\n- * Nor does it remove any containing dir entries that might be made\n- * empty by the removal.  dir must represent the top-level directory\n- * and must already be complete.\n- */\n-static int remove_entry_from_dir(struct ref_dir *dir, const char *refname)\n-{\n-\tint refname_len = strlen(refname);\n-\tint entry_index;\n-\tstruct ref_entry *entry;\n-\tint is_dir = refname[refname_len - 1] == '/';\n-\tif (is_dir) {\n-\t\t/*\n-\t\t * refname represents a reference directory.  Remove\n-\t\t * the trailing slash; otherwise we will get the\n-\t\t * directory *representing* refname rather than the\n-\t\t * one *containing* it.\n-\t\t */\n-\t\tchar *dirname = xmemdupz(refname, refname_len - 1);\n-\t\tdir = find_containing_dir(dir, dirname, 0);\n-\t\tfree(dirname);\n-\t} else {\n-\t\tdir = find_containing_dir(dir, refname, 0);\n-\t}\n-\tif (!dir)\n-\t\treturn -1;\n-\tentry_index = search_ref_dir(dir, refname, refname_len);\n-\tif (entry_index == -1)\n-\t\treturn -1;\n-\tentry = dir->entries[entry_index];\n-\n-\tmemmove(&dir->entries[entry_index],\n-\t\t&dir->entries[entry_index + 1],\n-\t\t(dir->nr - entry_index - 1) * sizeof(*dir->entries)\n-\t\t);\n-\tdir->nr--;\n-\tif (dir->sorted > entry_index)\n-\t\tdir->sorted--;\n-\tfree_ref_entry(entry);\n-\treturn dir->nr;\n-}\n-\n-/*\n- * Add a ref_entry to the ref_dir (unsorted), recursing into\n- * subdirectories as necessary.  dir must represent the top-level\n- * directory.  Return 0 on success.\n- */\n-static int add_ref_entry(struct ref_dir *dir, struct ref_entry *ref)\n-{\n-\tdir = find_containing_dir(dir, ref->name, 1);\n-\tif (!dir)\n-\t\treturn -1;\n-\tadd_entry_to_dir(dir, ref);\n-\treturn 0;\n-}\n-\n-/*\n- * Emit a warning and return true iff ref1 and ref2 have the same name\n- * and the same sha1.  Die if they have the same name but different\n- * sha1s.\n- */\n-static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2)\n-{\n-\tif (strcmp(ref1->name, ref2->name))\n-\t\treturn 0;\n-\n-\t/* Duplicate name; make sure that they don't conflict: */\n-\n-\tif ((ref1->flag & REF_DIR) || (ref2->flag & REF_DIR))\n-\t\t/* This is impossible by construction */\n-\t\tdie(\"Reference directory conflict: %s\", ref1->name);\n-\n-\tif (oidcmp(&ref1->u.value.oid, &ref2->u.value.oid))\n-\t\tdie(\"Duplicated ref, and SHA1s don't match: %s\", ref1->name);\n-\n-\twarning(\"Duplicated ref: %s\", ref1->name);\n-\treturn 1;\n-}\n-\n-/*\n- * Sort the entries in dir non-recursively (if they are not already\n- * sorted) and remove any duplicate entries.\n- */\n-static void sort_ref_dir(struct ref_dir *dir)\n-{\n-\tint i, j;\n-\tstruct ref_entry *last = NULL;\n-\n-\t/*\n-\t * This check also prevents passing a zero-length array to qsort(),\n-\t * which is a problem on some platforms.\n-\t */\n-\tif (dir->sorted == dir->nr)\n-\t\treturn;\n-\n-\tQSORT(dir->entries, dir->nr, ref_entry_cmp);\n-\n-\t/* Remove any duplicates: */\n-\tfor (i = 0, j = 0; j < dir->nr; j++) {\n-\t\tstruct ref_entry *entry = dir->entries[j];\n-\t\tif (last && is_dup_ref(last, entry))\n-\t\t\tfree_ref_entry(entry);\n-\t\telse\n-\t\t\tlast = dir->entries[i++] = entry;\n-\t}\n-\tdir->sorted = dir->nr = i;\n-}\n-\n /*\n  * Return true if refname, which has the specified oid and flags, can\n  * be resolved to an object in the database. If the referred-to object\n@@ -545,199 +43,6 @@ static int entry_resolves_to_object(struct ref_entry *entry)\n \t\t\t\t      &entry->u.value.oid, entry->flag);\n }\n \n-typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);\n-\n-/*\n- * Call fn for each reference in dir that has index in the range\n- * offset <= index < dir->nr.  Recurse into subdirectories that are in\n- * that index range, sorting them before iterating.  This function\n- * does not sort dir itself; it should be sorted beforehand.  fn is\n- * called for all references, including broken ones.\n- */\n-static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,\n-\t\t\t\t    each_ref_entry_fn fn, void *cb_data)\n-{\n-\tint i;\n-\tassert(dir->sorted == dir->nr);\n-\tfor (i = offset; i < dir->nr; i++) {\n-\t\tstruct ref_entry *entry = dir->entries[i];\n-\t\tint retval;\n-\t\tif (entry->flag & REF_DIR) {\n-\t\t\tstruct ref_dir *subdir = get_ref_dir(entry);\n-\t\t\tsort_ref_dir(subdir);\n-\t\t\tretval = do_for_each_entry_in_dir(subdir, 0, fn, cb_data);\n-\t\t} else {\n-\t\t\tretval = fn(entry, cb_data);\n-\t\t}\n-\t\tif (retval)\n-\t\t\treturn retval;\n-\t}\n-\treturn 0;\n-}\n-\n-/*\n- * Load all of the refs from the dir into our in-memory cache. The hard work\n- * of loading loose refs is done by get_ref_dir(), so we just need to recurse\n- * through all of the sub-directories. We do not even need to care about\n- * sorting, as traversal order does not matter to us.\n- */\n-static void prime_ref_dir(struct ref_dir *dir)\n-{\n-\tint i;\n-\tfor (i = 0; i < dir->nr; i++) {\n-\t\tstruct ref_entry *entry = dir->entries[i];\n-\t\tif (entry->flag & REF_DIR)\n-\t\t\tprime_ref_dir(get_ref_dir(entry));\n-\t}\n-}\n-\n-/*\n- * A level in the reference hierarchy that is currently being iterated\n- * through.\n- */\n-struct cache_ref_iterator_level {\n-\t/*\n-\t * The ref_dir being iterated over at this level. The ref_dir\n-\t * is sorted before being stored here.\n-\t */\n-\tstruct ref_dir *dir;\n-\n-\t/*\n-\t * The index of the current entry within dir (which might\n-\t * itself be a directory). If index == -1, then the iteration\n-\t * hasn't yet begun. If index == dir->nr, then the iteration\n-\t * through this level is over.\n-\t */\n-\tint index;\n-};\n-\n-/*\n- * Represent an iteration through a ref_dir in the memory cache. The\n- * iteration recurses through subdirectories.\n- */\n-struct cache_ref_iterator {\n-\tstruct ref_iterator base;\n-\n-\t/*\n-\t * The number of levels currently on the stack. This is always\n-\t * at least 1, because when it becomes zero the iteration is\n-\t * ended and this struct is freed.\n-\t */\n-\tsize_t levels_nr;\n-\n-\t/* The number of levels that have been allocated on the stack */\n-\tsize_t levels_alloc;\n-\n-\t/*\n-\t * A stack of levels. levels[0] is the uppermost level that is\n-\t * being iterated over in this iteration. (This is not\n-\t * necessary the top level in the references hierarchy. If we\n-\t * are iterating through a subtree, then levels[0] will hold\n-\t * the ref_dir for that subtree, and subsequent levels will go\n-\t * on from there.)\n-\t */\n-\tstruct cache_ref_iterator_level *levels;\n-};\n-\n-static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator)\n-{\n-\tstruct cache_ref_iterator *iter =\n-\t\t(struct cache_ref_iterator *)ref_iterator;\n-\n-\twhile (1) {\n-\t\tstruct cache_ref_iterator_level *level =\n-\t\t\t&iter->levels[iter->levels_nr - 1];\n-\t\tstruct ref_dir *dir = level->dir;\n-\t\tstruct ref_entry *entry;\n-\n-\t\tif (level->index == -1)\n-\t\t\tsort_ref_dir(dir);\n-\n-\t\tif (++level->index == level->dir->nr) {\n-\t\t\t/* This level is exhausted; pop up a level */\n-\t\t\tif (--iter->levels_nr == 0)\n-\t\t\t\treturn ref_iterator_abort(ref_iterator);\n-\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\tentry = dir->entries[level->index];\n-\n-\t\tif (entry->flag & REF_DIR) {\n-\t\t\t/* push down a level */\n-\t\t\tALLOC_GROW(iter->levels, iter->levels_nr + 1,\n-\t\t\t\t   iter->levels_alloc);\n-\n-\t\t\tlevel = &iter->levels[iter->levels_nr++];\n-\t\t\tlevel->dir = get_ref_dir(entry);\n-\t\t\tlevel->index = -1;\n-\t\t} else {\n-\t\t\titer->base.refname = entry->name;\n-\t\t\titer->base.oid = &entry->u.value.oid;\n-\t\t\titer->base.flags = entry->flag;\n-\t\t\treturn ITER_OK;\n-\t\t}\n-\t}\n-}\n-\n-static enum peel_status peel_entry(struct ref_entry *entry, int repeel);\n-\n-static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator,\n-\t\t\t\t   struct object_id *peeled)\n-{\n-\tstruct cache_ref_iterator *iter =\n-\t\t(struct cache_ref_iterator *)ref_iterator;\n-\tstruct cache_ref_iterator_level *level;\n-\tstruct ref_entry *entry;\n-\n-\tlevel = &iter->levels[iter->levels_nr - 1];\n-\n-\tif (level->index == -1)\n-\t\tdie(\"BUG: peel called before advance for cache iterator\");\n-\n-\tentry = level->dir->entries[level->index];\n-\n-\tif (peel_entry(entry, 0))\n-\t\treturn -1;\n-\toidcpy(peeled, &entry->u.value.peeled);\n-\treturn 0;\n-}\n-\n-static int cache_ref_iterator_abort(struct ref_iterator *ref_iterator)\n-{\n-\tstruct cache_ref_iterator *iter =\n-\t\t(struct cache_ref_iterator *)ref_iterator;\n-\n-\tfree(iter->levels);\n-\tbase_ref_iterator_free(ref_iterator);\n-\treturn ITER_DONE;\n-}\n-\n-static struct ref_iterator_vtable cache_ref_iterator_vtable = {\n-\tcache_ref_iterator_advance,\n-\tcache_ref_iterator_peel,\n-\tcache_ref_iterator_abort\n-};\n-\n-static struct ref_iterator *cache_ref_iterator_begin(struct ref_dir *dir)\n-{\n-\tstruct cache_ref_iterator *iter;\n-\tstruct ref_iterator *ref_iterator;\n-\tstruct cache_ref_iterator_level *level;\n-\n-\titer = xcalloc(1, sizeof(*iter));\n-\tref_iterator = &iter->base;\n-\tbase_ref_iterator_init(ref_iterator, &cache_ref_iterator_vtable);\n-\tALLOC_GROW(iter->levels, 10, iter->levels_alloc);\n-\n-\titer->levels_nr = 1;\n-\tlevel = &iter->levels[0];\n-\tlevel->index = -1;\n-\tlevel->dir = dir;\n-\n-\treturn ref_iterator;\n-}\n-\n struct packed_ref_cache {\n \tstruct ref_entry *root;\n \n@@ -1117,7 +422,7 @@ static void add_packed_ref(struct files_ref_store *refs,\n \tif (!packed_ref_cache->lock)\n \t\tdie(\"internal error: packed refs not locked\");\n \tadd_ref_entry(get_packed_ref_dir(packed_ref_cache),\n-\t\tcreate_ref_entry(refname, sha1, REF_ISPACKED, 1));\n+\t\t      create_ref_entry(refname, sha1, REF_ISPACKED, 1));\n }\n \n /*\n@@ -1125,7 +430,7 @@ static void add_packed_ref(struct files_ref_store *refs,\n  * (without recursing).  dirname must end with '/'.  dir must be the\n  * directory entry corresponding to dirname.\n  */\n-static void read_loose_refs(const char *dirname, struct ref_dir *dir)\n+void read_loose_refs(const char *dirname, struct ref_dir *dir)\n {\n \tstruct files_ref_store *refs = dir->ref_store;\n \tDIR *d;\n@@ -1635,41 +940,6 @@ static int lock_raw_ref(struct files_ref_store *refs,\n \treturn ret;\n }\n \n-/*\n- * Peel the entry (if possible) and return its new peel_status.  If\n- * repeel is true, re-peel the entry even if there is an old peeled\n- * value that is already stored in it.\n- *\n- * It is OK to call this function with a packed reference entry that\n- * might be stale and might even refer to an object that has since\n- * been garbage-collected.  In such a case, if the entry has\n- * REF_KNOWS_PEELED then leave the status unchanged and return\n- * PEEL_PEELED or PEEL_NON_TAG; otherwise, return PEEL_INVALID.\n- */\n-static enum peel_status peel_entry(struct ref_entry *entry, int repeel)\n-{\n-\tenum peel_status status;\n-\n-\tif (entry->flag & REF_KNOWS_PEELED) {\n-\t\tif (repeel) {\n-\t\t\tentry->flag &= ~REF_KNOWS_PEELED;\n-\t\t\toidclr(&entry->u.value.peeled);\n-\t\t} else {\n-\t\t\treturn is_null_oid(&entry->u.value.peeled) ?\n-\t\t\t\tPEEL_NON_TAG : PEEL_PEELED;\n-\t\t}\n-\t}\n-\tif (entry->flag & REF_ISBROKEN)\n-\t\treturn PEEL_BROKEN;\n-\tif (entry->flag & REF_ISSYMREF)\n-\t\treturn PEEL_IS_SYMREF;\n-\n-\tstatus = peel_object(entry->u.value.oid.hash, entry->u.value.peeled.hash);\n-\tif (status == PEEL_PEELED || status == PEEL_NON_TAG)\n-\t\tentry->flag |= REF_KNOWS_PEELED;\n-\treturn status;\n-}\n-\n static int files_peel_ref(struct ref_store *ref_store,\n \t\t\t  const char *refname, unsigned char *sha1)\n {\ndiff --git a/refs/ref-cache.c b/refs/ref-cache.c\nnew file mode 100644\nindex 0000000000..4274a43a9b\n--- /dev/null\n+++ b/refs/ref-cache.c\n@@ -0,0 +1,512 @@\n+#include \"../cache.h\"\n+#include \"../refs.h\"\n+#include \"refs-internal.h\"\n+#include \"ref-cache.h\"\n+#include \"../iterator.h\"\n+\n+/* FIXME: This declaration shouldn't be here */\n+void read_loose_refs(const char *dirname, struct ref_dir *dir);\n+\n+void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry)\n+{\n+\tALLOC_GROW(dir->entries, dir->nr + 1, dir->alloc);\n+\tdir->entries[dir->nr++] = entry;\n+\t/* optimize for the case that entries are added in order */\n+\tif (dir->nr == 1 ||\n+\t    (dir->nr == dir->sorted + 1 &&\n+\t     strcmp(dir->entries[dir->nr - 2]->name,\n+\t\t    dir->entries[dir->nr - 1]->name) < 0))\n+\t\tdir->sorted = dir->nr;\n+}\n+\n+struct ref_dir *get_ref_dir(struct ref_entry *entry)\n+{\n+\tstruct ref_dir *dir;\n+\tassert(entry->flag & REF_DIR);\n+\tdir = &entry->u.subdir;\n+\tif (entry->flag & REF_INCOMPLETE) {\n+\t\tread_loose_refs(entry->name, dir);\n+\n+\t\t/*\n+\t\t * Manually add refs/bisect, which, being\n+\t\t * per-worktree, might not appear in the directory\n+\t\t * listing for refs/ in the main repo.\n+\t\t */\n+\t\tif (!strcmp(entry->name, \"refs/\")) {\n+\t\t\tint pos = search_ref_dir(dir, \"refs/bisect/\", 12);\n+\t\t\tif (pos < 0) {\n+\t\t\t\tstruct ref_entry *child_entry;\n+\t\t\t\tchild_entry = create_dir_entry(dir->ref_store,\n+\t\t\t\t\t\t\t       \"refs/bisect/\",\n+\t\t\t\t\t\t\t       12, 1);\n+\t\t\t\tadd_entry_to_dir(dir, child_entry);\n+\t\t\t}\n+\t\t}\n+\t\tentry->flag &= ~REF_INCOMPLETE;\n+\t}\n+\treturn dir;\n+}\n+\n+struct ref_entry *create_ref_entry(const char *refname,\n+\t\t\t\t   const unsigned char *sha1, int flag,\n+\t\t\t\t   int check_name)\n+{\n+\tstruct ref_entry *ref;\n+\n+\tif (check_name &&\n+\t    check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))\n+\t\tdie(\"Reference has invalid format: '%s'\", refname);\n+\tFLEX_ALLOC_STR(ref, name, refname);\n+\thashcpy(ref->u.value.oid.hash, sha1);\n+\toidclr(&ref->u.value.peeled);\n+\tref->flag = flag;\n+\treturn ref;\n+}\n+\n+static void clear_ref_dir(struct ref_dir *dir);\n+\n+void free_ref_entry(struct ref_entry *entry)\n+{\n+\tif (entry->flag & REF_DIR) {\n+\t\t/*\n+\t\t * Do not use get_ref_dir() here, as that might\n+\t\t * trigger the reading of loose refs.\n+\t\t */\n+\t\tclear_ref_dir(&entry->u.subdir);\n+\t}\n+\tfree(entry);\n+}\n+\n+/*\n+ * Clear and free all entries in dir, recursively.\n+ */\n+static void clear_ref_dir(struct ref_dir *dir)\n+{\n+\tint i;\n+\tfor (i = 0; i < dir->nr; i++)\n+\t\tfree_ref_entry(dir->entries[i]);\n+\tfree(dir->entries);\n+\tdir->sorted = dir->nr = dir->alloc = 0;\n+\tdir->entries = NULL;\n+}\n+\n+struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,\n+\t\t\t\t   const char *dirname, size_t len,\n+\t\t\t\t   int incomplete)\n+{\n+\tstruct ref_entry *direntry;\n+\tFLEX_ALLOC_MEM(direntry, name, dirname, len);\n+\tdirentry->u.subdir.ref_store = ref_store;\n+\tdirentry->flag = REF_DIR | (incomplete ? REF_INCOMPLETE : 0);\n+\treturn direntry;\n+}\n+\n+static int ref_entry_cmp(const void *a, const void *b)\n+{\n+\tstruct ref_entry *one = *(struct ref_entry **)a;\n+\tstruct ref_entry *two = *(struct ref_entry **)b;\n+\treturn strcmp(one->name, two->name);\n+}\n+\n+static void sort_ref_dir(struct ref_dir *dir);\n+\n+struct string_slice {\n+\tsize_t len;\n+\tconst char *str;\n+};\n+\n+static int ref_entry_cmp_sslice(const void *key_, const void *ent_)\n+{\n+\tconst struct string_slice *key = key_;\n+\tconst struct ref_entry *ent = *(const struct ref_entry * const *)ent_;\n+\tint cmp = strncmp(key->str, ent->name, key->len);\n+\tif (cmp)\n+\t\treturn cmp;\n+\treturn '\\0' - (unsigned char)ent->name[key->len];\n+}\n+\n+int search_ref_dir(struct ref_dir *dir, const char *refname, size_t len)\n+{\n+\tstruct ref_entry **r;\n+\tstruct string_slice key;\n+\n+\tif (refname == NULL || !dir->nr)\n+\t\treturn -1;\n+\n+\tsort_ref_dir(dir);\n+\tkey.len = len;\n+\tkey.str = refname;\n+\tr = bsearch(&key, dir->entries, dir->nr, sizeof(*dir->entries),\n+\t\t    ref_entry_cmp_sslice);\n+\n+\tif (r == NULL)\n+\t\treturn -1;\n+\n+\treturn r - dir->entries;\n+}\n+\n+/*\n+ * Search for a directory entry directly within dir (without\n+ * recursing).  Sort dir if necessary.  subdirname must be a directory\n+ * name (i.e., end in '/').  If mkdir is set, then create the\n+ * directory if it is missing; otherwise, return NULL if the desired\n+ * directory cannot be found.  dir must already be complete.\n+ */\n+static struct ref_dir *search_for_subdir(struct ref_dir *dir,\n+\t\t\t\t\t const char *subdirname, size_t len,\n+\t\t\t\t\t int mkdir)\n+{\n+\tint entry_index = search_ref_dir(dir, subdirname, len);\n+\tstruct ref_entry *entry;\n+\tif (entry_index == -1) {\n+\t\tif (!mkdir)\n+\t\t\treturn NULL;\n+\t\t/*\n+\t\t * Since dir is complete, the absence of a subdir\n+\t\t * means that the subdir really doesn't exist;\n+\t\t * therefore, create an empty record for it but mark\n+\t\t * the record complete.\n+\t\t */\n+\t\tentry = create_dir_entry(dir->ref_store, subdirname, len, 0);\n+\t\tadd_entry_to_dir(dir, entry);\n+\t} else {\n+\t\tentry = dir->entries[entry_index];\n+\t}\n+\treturn get_ref_dir(entry);\n+}\n+\n+struct ref_dir *find_containing_dir(struct ref_dir *dir,\n+\t\t\t\t    const char *refname, int mkdir)\n+{\n+\tconst char *slash;\n+\tfor (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {\n+\t\tsize_t dirnamelen = slash - refname + 1;\n+\t\tstruct ref_dir *subdir;\n+\t\tsubdir = search_for_subdir(dir, refname, dirnamelen, mkdir);\n+\t\tif (!subdir) {\n+\t\t\tdir = NULL;\n+\t\t\tbreak;\n+\t\t}\n+\t\tdir = subdir;\n+\t}\n+\n+\treturn dir;\n+}\n+\n+struct ref_entry *find_ref_entry(struct ref_dir *dir, const char *refname)\n+{\n+\tint entry_index;\n+\tstruct ref_entry *entry;\n+\tdir = find_containing_dir(dir, refname, 0);\n+\tif (!dir)\n+\t\treturn NULL;\n+\tentry_index = search_ref_dir(dir, refname, strlen(refname));\n+\tif (entry_index == -1)\n+\t\treturn NULL;\n+\tentry = dir->entries[entry_index];\n+\treturn (entry->flag & REF_DIR) ? NULL : entry;\n+}\n+\n+int remove_entry_from_dir(struct ref_dir *dir, const char *refname)\n+{\n+\tint refname_len = strlen(refname);\n+\tint entry_index;\n+\tstruct ref_entry *entry;\n+\tint is_dir = refname[refname_len - 1] == '/';\n+\tif (is_dir) {\n+\t\t/*\n+\t\t * refname represents a reference directory.  Remove\n+\t\t * the trailing slash; otherwise we will get the\n+\t\t * directory *representing* refname rather than the\n+\t\t * one *containing* it.\n+\t\t */\n+\t\tchar *dirname = xmemdupz(refname, refname_len - 1);\n+\t\tdir = find_containing_dir(dir, dirname, 0);\n+\t\tfree(dirname);\n+\t} else {\n+\t\tdir = find_containing_dir(dir, refname, 0);\n+\t}\n+\tif (!dir)\n+\t\treturn -1;\n+\tentry_index = search_ref_dir(dir, refname, refname_len);\n+\tif (entry_index == -1)\n+\t\treturn -1;\n+\tentry = dir->entries[entry_index];\n+\n+\tmemmove(&dir->entries[entry_index],\n+\t\t&dir->entries[entry_index + 1],\n+\t\t(dir->nr - entry_index - 1) * sizeof(*dir->entries)\n+\t\t);\n+\tdir->nr--;\n+\tif (dir->sorted > entry_index)\n+\t\tdir->sorted--;\n+\tfree_ref_entry(entry);\n+\treturn dir->nr;\n+}\n+\n+int add_ref_entry(struct ref_dir *dir, struct ref_entry *ref)\n+{\n+\tdir = find_containing_dir(dir, ref->name, 1);\n+\tif (!dir)\n+\t\treturn -1;\n+\tadd_entry_to_dir(dir, ref);\n+\treturn 0;\n+}\n+\n+/*\n+ * Emit a warning and return true iff ref1 and ref2 have the same name\n+ * and the same sha1.  Die if they have the same name but different\n+ * sha1s.\n+ */\n+static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2)\n+{\n+\tif (strcmp(ref1->name, ref2->name))\n+\t\treturn 0;\n+\n+\t/* Duplicate name; make sure that they don't conflict: */\n+\n+\tif ((ref1->flag & REF_DIR) || (ref2->flag & REF_DIR))\n+\t\t/* This is impossible by construction */\n+\t\tdie(\"Reference directory conflict: %s\", ref1->name);\n+\n+\tif (oidcmp(&ref1->u.value.oid, &ref2->u.value.oid))\n+\t\tdie(\"Duplicated ref, and SHA1s don't match: %s\", ref1->name);\n+\n+\twarning(\"Duplicated ref: %s\", ref1->name);\n+\treturn 1;\n+}\n+\n+/*\n+ * Sort the entries in dir non-recursively (if they are not already\n+ * sorted) and remove any duplicate entries.\n+ */\n+static void sort_ref_dir(struct ref_dir *dir)\n+{\n+\tint i, j;\n+\tstruct ref_entry *last = NULL;\n+\n+\t/*\n+\t * This check also prevents passing a zero-length array to qsort(),\n+\t * which is a problem on some platforms.\n+\t */\n+\tif (dir->sorted == dir->nr)\n+\t\treturn;\n+\n+\tQSORT(dir->entries, dir->nr, ref_entry_cmp);\n+\n+\t/* Remove any duplicates: */\n+\tfor (i = 0, j = 0; j < dir->nr; j++) {\n+\t\tstruct ref_entry *entry = dir->entries[j];\n+\t\tif (last && is_dup_ref(last, entry))\n+\t\t\tfree_ref_entry(entry);\n+\t\telse\n+\t\t\tlast = dir->entries[i++] = entry;\n+\t}\n+\tdir->sorted = dir->nr = i;\n+}\n+\n+int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,\n+\t\t\t     each_ref_entry_fn fn, void *cb_data)\n+{\n+\tint i;\n+\tassert(dir->sorted == dir->nr);\n+\tfor (i = offset; i < dir->nr; i++) {\n+\t\tstruct ref_entry *entry = dir->entries[i];\n+\t\tint retval;\n+\t\tif (entry->flag & REF_DIR) {\n+\t\t\tstruct ref_dir *subdir = get_ref_dir(entry);\n+\t\t\tsort_ref_dir(subdir);\n+\t\t\tretval = do_for_each_entry_in_dir(subdir, 0, fn, cb_data);\n+\t\t} else {\n+\t\t\tretval = fn(entry, cb_data);\n+\t\t}\n+\t\tif (retval)\n+\t\t\treturn retval;\n+\t}\n+\treturn 0;\n+}\n+\n+void prime_ref_dir(struct ref_dir *dir)\n+{\n+\t/*\n+\t * The hard work of loading loose refs is done by get_ref_dir(), so we\n+\t * just need to recurse through all of the sub-directories. We do not\n+\t * even need to care about sorting, as traversal order does not matter\n+\t * to us.\n+\t */\n+\tint i;\n+\tfor (i = 0; i < dir->nr; i++) {\n+\t\tstruct ref_entry *entry = dir->entries[i];\n+\t\tif (entry->flag & REF_DIR)\n+\t\t\tprime_ref_dir(get_ref_dir(entry));\n+\t}\n+}\n+\n+/*\n+ * A level in the reference hierarchy that is currently being iterated\n+ * through.\n+ */\n+struct cache_ref_iterator_level {\n+\t/*\n+\t * The ref_dir being iterated over at this level. The ref_dir\n+\t * is sorted before being stored here.\n+\t */\n+\tstruct ref_dir *dir;\n+\n+\t/*\n+\t * The index of the current entry within dir (which might\n+\t * itself be a directory). If index == -1, then the iteration\n+\t * hasn't yet begun. If index == dir->nr, then the iteration\n+\t * through this level is over.\n+\t */\n+\tint index;\n+};\n+\n+/*\n+ * Represent an iteration through a ref_dir in the memory cache. The\n+ * iteration recurses through subdirectories.\n+ */\n+struct cache_ref_iterator {\n+\tstruct ref_iterator base;\n+\n+\t/*\n+\t * The number of levels currently on the stack. This is always\n+\t * at least 1, because when it becomes zero the iteration is\n+\t * ended and this struct is freed.\n+\t */\n+\tsize_t levels_nr;\n+\n+\t/* The number of levels that have been allocated on the stack */\n+\tsize_t levels_alloc;\n+\n+\t/*\n+\t * A stack of levels. levels[0] is the uppermost level that is\n+\t * being iterated over in this iteration. (This is not\n+\t * necessary the top level in the references hierarchy. If we\n+\t * are iterating through a subtree, then levels[0] will hold\n+\t * the ref_dir for that subtree, and subsequent levels will go\n+\t * on from there.)\n+\t */\n+\tstruct cache_ref_iterator_level *levels;\n+};\n+\n+static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator)\n+{\n+\tstruct cache_ref_iterator *iter =\n+\t\t(struct cache_ref_iterator *)ref_iterator;\n+\n+\twhile (1) {\n+\t\tstruct cache_ref_iterator_level *level =\n+\t\t\t&iter->levels[iter->levels_nr - 1];\n+\t\tstruct ref_dir *dir = level->dir;\n+\t\tstruct ref_entry *entry;\n+\n+\t\tif (level->index == -1)\n+\t\t\tsort_ref_dir(dir);\n+\n+\t\tif (++level->index == level->dir->nr) {\n+\t\t\t/* This level is exhausted; pop up a level */\n+\t\t\tif (--iter->levels_nr == 0)\n+\t\t\t\treturn ref_iterator_abort(ref_iterator);\n+\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tentry = dir->entries[level->index];\n+\n+\t\tif (entry->flag & REF_DIR) {\n+\t\t\t/* push down a level */\n+\t\t\tALLOC_GROW(iter->levels, iter->levels_nr + 1,\n+\t\t\t\t   iter->levels_alloc);\n+\n+\t\t\tlevel = &iter->levels[iter->levels_nr++];\n+\t\t\tlevel->dir = get_ref_dir(entry);\n+\t\t\tlevel->index = -1;\n+\t\t} else {\n+\t\t\titer->base.refname = entry->name;\n+\t\t\titer->base.oid = &entry->u.value.oid;\n+\t\t\titer->base.flags = entry->flag;\n+\t\t\treturn ITER_OK;\n+\t\t}\n+\t}\n+}\n+\n+enum peel_status peel_entry(struct ref_entry *entry, int repeel)\n+{\n+\tenum peel_status status;\n+\n+\tif (entry->flag & REF_KNOWS_PEELED) {\n+\t\tif (repeel) {\n+\t\t\tentry->flag &= ~REF_KNOWS_PEELED;\n+\t\t\toidclr(&entry->u.value.peeled);\n+\t\t} else {\n+\t\t\treturn is_null_oid(&entry->u.value.peeled) ?\n+\t\t\t\tPEEL_NON_TAG : PEEL_PEELED;\n+\t\t}\n+\t}\n+\tif (entry->flag & REF_ISBROKEN)\n+\t\treturn PEEL_BROKEN;\n+\tif (entry->flag & REF_ISSYMREF)\n+\t\treturn PEEL_IS_SYMREF;\n+\n+\tstatus = peel_object(entry->u.value.oid.hash, entry->u.value.peeled.hash);\n+\tif (status == PEEL_PEELED || status == PEEL_NON_TAG)\n+\t\tentry->flag |= REF_KNOWS_PEELED;\n+\treturn status;\n+}\n+\n+static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator,\n+\t\t\t\t   struct object_id *peeled)\n+{\n+\tstruct cache_ref_iterator *iter =\n+\t\t(struct cache_ref_iterator *)ref_iterator;\n+\tstruct cache_ref_iterator_level *level;\n+\tstruct ref_entry *entry;\n+\n+\tlevel = &iter->levels[iter->levels_nr - 1];\n+\n+\tif (level->index == -1)\n+\t\tdie(\"BUG: peel called before advance for cache iterator\");\n+\n+\tentry = level->dir->entries[level->index];\n+\n+\tif (peel_entry(entry, 0))\n+\t\treturn -1;\n+\toidcpy(peeled, &entry->u.value.peeled);\n+\treturn 0;\n+}\n+\n+static int cache_ref_iterator_abort(struct ref_iterator *ref_iterator)\n+{\n+\tstruct cache_ref_iterator *iter =\n+\t\t(struct cache_ref_iterator *)ref_iterator;\n+\n+\tfree(iter->levels);\n+\tbase_ref_iterator_free(ref_iterator);\n+\treturn ITER_DONE;\n+}\n+\n+static struct ref_iterator_vtable cache_ref_iterator_vtable = {\n+\tcache_ref_iterator_advance,\n+\tcache_ref_iterator_peel,\n+\tcache_ref_iterator_abort\n+};\n+\n+struct ref_iterator *cache_ref_iterator_begin(struct ref_dir *dir)\n+{\n+\tstruct cache_ref_iterator *iter;\n+\tstruct ref_iterator *ref_iterator;\n+\tstruct cache_ref_iterator_level *level;\n+\n+\titer = xcalloc(1, sizeof(*iter));\n+\tref_iterator = &iter->base;\n+\tbase_ref_iterator_init(ref_iterator, &cache_ref_iterator_vtable);\n+\tALLOC_GROW(iter->levels, 10, iter->levels_alloc);\n+\n+\titer->levels_nr = 1;\n+\tlevel = &iter->levels[0];\n+\tlevel->index = -1;\n+\tlevel->dir = dir;\n+\n+\treturn ref_iterator;\n+}\ndiff --git a/refs/ref-cache.h b/refs/ref-cache.h\nnew file mode 100644\nindex 0000000000..2e7b1a366e\n--- /dev/null\n+++ b/refs/ref-cache.h\n@@ -0,0 +1,251 @@\n+#ifndef REFS_REF_CACHE_H\n+#define REFS_REF_CACHE_H\n+\n+/*\n+ * Information used (along with the information in ref_entry) to\n+ * describe a single cached reference.  This data structure only\n+ * occurs embedded in a union in struct ref_entry, and only when\n+ * (ref_entry->flag & REF_DIR) is zero.\n+ */\n+struct ref_value {\n+\t/*\n+\t * The name of the object to which this reference resolves\n+\t * (which may be a tag object).  If REF_ISBROKEN, this is\n+\t * null.  If REF_ISSYMREF, then this is the name of the object\n+\t * referred to by the last reference in the symlink chain.\n+\t */\n+\tstruct object_id oid;\n+\n+\t/*\n+\t * If REF_KNOWS_PEELED, then this field holds the peeled value\n+\t * of this reference, or null if the reference is known not to\n+\t * be peelable.  See the documentation for peel_ref() for an\n+\t * exact definition of \"peelable\".\n+\t */\n+\tstruct object_id peeled;\n+};\n+\n+/*\n+ * Information used (along with the information in ref_entry) to\n+ * describe a level in the hierarchy of references.  This data\n+ * structure only occurs embedded in a union in struct ref_entry, and\n+ * only when (ref_entry.flag & REF_DIR) is set.  In that case,\n+ * (ref_entry.flag & REF_INCOMPLETE) determines whether the references\n+ * in the directory have already been read:\n+ *\n+ *     (ref_entry.flag & REF_INCOMPLETE) unset -- a directory of loose\n+ *         or packed references, already read.\n+ *\n+ *     (ref_entry.flag & REF_INCOMPLETE) set -- a directory of loose\n+ *         references that hasn't been read yet (nor has any of its\n+ *         subdirectories).\n+ *\n+ * Entries within a directory are stored within a growable array of\n+ * pointers to ref_entries (entries, nr, alloc).  Entries 0 <= i <\n+ * sorted are sorted by their component name in strcmp() order and the\n+ * remaining entries are unsorted.\n+ *\n+ * Loose references are read lazily, one directory at a time.  When a\n+ * directory of loose references is read, then all of the references\n+ * in that directory are stored, and REF_INCOMPLETE stubs are created\n+ * for any subdirectories, but the subdirectories themselves are not\n+ * read.  The reading is triggered by get_ref_dir().\n+ */\n+struct ref_dir {\n+\tint nr, alloc;\n+\n+\t/*\n+\t * Entries with index 0 <= i < sorted are sorted by name.  New\n+\t * entries are appended to the list unsorted, and are sorted\n+\t * only when required; thus we avoid the need to sort the list\n+\t * after the addition of every reference.\n+\t */\n+\tint sorted;\n+\n+\t/* A pointer to the files_ref_store that contains this ref_dir. */\n+\tstruct files_ref_store *ref_store;\n+\n+\tstruct ref_entry **entries;\n+};\n+\n+/*\n+ * Bit values for ref_entry::flag.  REF_ISSYMREF=0x01,\n+ * REF_ISPACKED=0x02, REF_ISBROKEN=0x04 and REF_BAD_NAME=0x08 are\n+ * public values; see refs.h.\n+ */\n+\n+/*\n+ * The field ref_entry->u.value.peeled of this value entry contains\n+ * the correct peeled value for the reference, which might be\n+ * null_sha1 if the reference is not a tag or if it is broken.\n+ */\n+#define REF_KNOWS_PEELED 0x10\n+\n+/* ref_entry represents a directory of references */\n+#define REF_DIR 0x20\n+\n+/*\n+ * Entry has not yet been read from disk (used only for REF_DIR\n+ * entries representing loose references)\n+ */\n+#define REF_INCOMPLETE 0x40\n+\n+/*\n+ * A ref_entry represents either a reference or a \"subdirectory\" of\n+ * references.\n+ *\n+ * Each directory in the reference namespace is represented by a\n+ * ref_entry with (flags & REF_DIR) set and containing a subdir member\n+ * that holds the entries in that directory that have been read so\n+ * far.  If (flags & REF_INCOMPLETE) is set, then the directory and\n+ * its subdirectories haven't been read yet.  REF_INCOMPLETE is only\n+ * used for loose reference directories.\n+ *\n+ * References are represented by a ref_entry with (flags & REF_DIR)\n+ * unset and a value member that describes the reference's value.  The\n+ * flag member is at the ref_entry level, but it is also needed to\n+ * interpret the contents of the value field (in other words, a\n+ * ref_value object is not very much use without the enclosing\n+ * ref_entry).\n+ *\n+ * Reference names cannot end with slash and directories' names are\n+ * always stored with a trailing slash (except for the top-level\n+ * directory, which is always denoted by \"\").  This has two nice\n+ * consequences: (1) when the entries in each subdir are sorted\n+ * lexicographically by name (as they usually are), the references in\n+ * a whole tree can be generated in lexicographic order by traversing\n+ * the tree in left-to-right, depth-first order; (2) the names of\n+ * references and subdirectories cannot conflict, and therefore the\n+ * presence of an empty subdirectory does not block the creation of a\n+ * similarly-named reference.  (The fact that reference names with the\n+ * same leading components can conflict *with each other* is a\n+ * separate issue that is regulated by refs_verify_refname_available().)\n+ *\n+ * Please note that the name field contains the fully-qualified\n+ * reference (or subdirectory) name.  Space could be saved by only\n+ * storing the relative names.  But that would require the full names\n+ * to be generated on the fly when iterating in do_for_each_ref(), and\n+ * would break callback functions, who have always been able to assume\n+ * that the name strings that they are passed will not be freed during\n+ * the iteration.\n+ */\n+struct ref_entry {\n+\tunsigned char flag; /* ISSYMREF? ISPACKED? */\n+\tunion {\n+\t\tstruct ref_value value; /* if not (flags&REF_DIR) */\n+\t\tstruct ref_dir subdir; /* if (flags&REF_DIR) */\n+\t} u;\n+\t/*\n+\t * The full name of the reference (e.g., \"refs/heads/master\")\n+\t * or the full name of the directory with a trailing slash\n+\t * (e.g., \"refs/heads/\"):\n+\t */\n+\tchar name[FLEX_ARRAY];\n+};\n+\n+/*\n+ * Return the index of the entry with the given refname from the\n+ * ref_dir (non-recursively), sorting dir if necessary.  Return -1 if\n+ * no such entry is found.  dir must already be complete.\n+ */\n+int search_ref_dir(struct ref_dir *dir, const char *refname, size_t len);\n+\n+struct ref_dir *get_ref_dir(struct ref_entry *entry);\n+\n+/*\n+ * Create a struct ref_entry object for the specified dirname.\n+ * dirname is the name of the directory with a trailing slash (e.g.,\n+ * \"refs/heads/\") or \"\" for the top-level directory.\n+ */\n+struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,\n+\t\t\t\t   const char *dirname, size_t len,\n+\t\t\t\t   int incomplete);\n+\n+struct ref_entry *create_ref_entry(const char *refname,\n+\t\t\t\t   const unsigned char *sha1, int flag,\n+\t\t\t\t   int check_name);\n+\n+void free_ref_entry(struct ref_entry *entry);\n+\n+/*\n+ * Add a ref_entry to the end of dir (unsorted).  Entry is always\n+ * stored directly in dir; no recursion into subdirectories is\n+ * done.\n+ */\n+void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);\n+\n+/*\n+ * Remove the entry with the given name from dir, recursing into\n+ * subdirectories as necessary.  If refname is the name of a directory\n+ * (i.e., ends with '/'), then remove the directory and its contents.\n+ * If the removal was successful, return the number of entries\n+ * remaining in the directory entry that contained the deleted entry.\n+ * If the name was not found, return -1.  Please note that this\n+ * function only deletes the entry from the cache; it does not delete\n+ * it from the filesystem or ensure that other cache entries (which\n+ * might be symbolic references to the removed entry) are updated.\n+ * Nor does it remove any containing dir entries that might be made\n+ * empty by the removal.  dir must represent the top-level directory\n+ * and must already be complete.\n+ */\n+int remove_entry_from_dir(struct ref_dir *dir, const char *refname);\n+\n+/*\n+ * Add a ref_entry to the ref_dir (unsorted), recursing into\n+ * subdirectories as necessary.  dir must represent the top-level\n+ * directory.  Return 0 on success.\n+ */\n+int add_ref_entry(struct ref_dir *dir, struct ref_entry *ref);\n+\n+/*\n+ * If refname is a reference name, find the ref_dir within the dir\n+ * tree that should hold refname. If refname is a directory name\n+ * (i.e., it ends in '/'), then return that ref_dir itself. dir must\n+ * represent the top-level directory and must already be complete.\n+ * Sort ref_dirs and recurse into subdirectories as necessary. If\n+ * mkdir is set, then create any missing directories; otherwise,\n+ * return NULL if the desired directory cannot be found.\n+ */\n+struct ref_dir *find_containing_dir(struct ref_dir *dir,\n+\t\t\t\t    const char *refname, int mkdir);\n+\n+/*\n+ * Find the value entry with the given name in dir, sorting ref_dirs\n+ * and recursing into subdirectories as necessary.  If the name is not\n+ * found or it corresponds to a directory entry, return NULL.\n+ */\n+struct ref_entry *find_ref_entry(struct ref_dir *dir, const char *refname);\n+\n+struct ref_iterator *cache_ref_iterator_begin(struct ref_dir *dir);\n+\n+typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);\n+\n+/*\n+ * Call fn for each reference in dir that has index in the range\n+ * offset <= index < dir->nr.  Recurse into subdirectories that are in\n+ * that index range, sorting them before iterating.  This function\n+ * does not sort dir itself; it should be sorted beforehand.  fn is\n+ * called for all references, including broken ones.\n+ */\n+int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,\n+\t\t\t     each_ref_entry_fn fn, void *cb_data);\n+\n+/*\n+ * Peel the entry (if possible) and return its new peel_status.  If\n+ * repeel is true, re-peel the entry even if there is an old peeled\n+ * value that is already stored in it.\n+ *\n+ * It is OK to call this function with a packed reference entry that\n+ * might be stale and might even refer to an object that has since\n+ * been garbage-collected.  In such a case, if the entry has\n+ * REF_KNOWS_PEELED then leave the status unchanged and return\n+ * PEEL_PEELED or PEEL_NON_TAG; otherwise, return PEEL_INVALID.\n+ */\n+enum peel_status peel_entry(struct ref_entry *entry, int repeel);\n+\n+/*\n+ * Load all of the refs from `dir` into our in-memory cache.\n+ */\n+void prime_ref_dir(struct ref_dir *dir);\n+\n+#endif /* REFS_REF_CACHE_H */\n-- \n2.11.0\n\n"},{"id":"315905","messageId":"7e0466b41ae2f3c74a83319e456ccf36e86e970d.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 15/20] get_loose_ref_dir(): function renamed from get_loose_refs()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:13Z","receivedAt":"2017-03-31T14:12:09Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The new name is more analogous to `get_packed_ref_dir()`.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 0ff5df6b46..0a16f6196c 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -525,7 +525,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,\n \t}\n }\n \n-static struct ref_dir *get_loose_refs(struct files_ref_store *refs)\n+static struct ref_dir *get_loose_ref_dir(struct files_ref_store *refs)\n {\n \tif (!refs->loose) {\n \t\t/*\n@@ -1113,7 +1113,7 @@ static struct ref_iterator *files_ref_iterator_begin(\n \t * date with what is on disk, and re-reads it if not.\n \t */\n \n-\tloose_dir = get_loose_refs(refs);\n+\tloose_dir = get_loose_ref_dir(refs);\n \n \tif (prefix && *prefix)\n \t\tloose_dir = find_containing_dir(loose_dir, prefix, 0);\n@@ -1584,7 +1584,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)\n \tlock_packed_refs(refs, LOCK_DIE_ON_ERROR);\n \tcbdata.packed_refs = get_packed_refs(refs);\n \n-\tdo_for_each_entry_in_dir(get_loose_refs(refs),\n+\tdo_for_each_entry_in_dir(get_loose_ref_dir(refs),\n \t\t\t\t pack_if_possible_fn, &cbdata);\n \n \tif (commit_packed_refs(refs))\n-- \n2.11.0\n\n"},{"id":"315906","messageId":"fe76ca723956d8285dce8ad6e6845031a974ac36.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 16/20] get_loose_ref_cache(): new function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:14Z","receivedAt":"2017-03-31T14:12:13Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Extract a new function, `get_loose_ref_cache()`, from\nget_loose_ref_dir(). The function returns the `ref_cache` for the\nloose refs of a `files_ref_store`.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 0a16f6196c..fefc29433a 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -525,7 +525,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,\n \t}\n }\n \n-static struct ref_dir *get_loose_ref_dir(struct files_ref_store *refs)\n+static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)\n {\n \tif (!refs->loose) {\n \t\t/*\n@@ -545,7 +545,12 @@ static struct ref_dir *get_loose_ref_dir(struct files_ref_store *refs)\n \t\tadd_entry_to_dir(get_ref_dir(refs->loose->root),\n \t\t\t\t create_dir_entry(refs->loose, \"refs/\", 5, 1));\n \t}\n-\treturn get_ref_dir(refs->loose->root);\n+\treturn refs->loose;\n+}\n+\n+static struct ref_dir *get_loose_ref_dir(struct files_ref_store *refs)\n+{\n+\treturn get_ref_dir(get_loose_ref_cache(refs)->root);\n }\n \n /*\n-- \n2.11.0\n\n"},{"id":"315907","messageId":"1d131a33fd155f14908ee1d41e98be8a1309c6b9.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 19/20] files_pack_refs(): use reference iteration","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:17Z","receivedAt":"2017-03-31T14:12:16Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Use reference iteration rather than do_for_each_entry_in_dir() in the\ndefinition of files_pack_refs().\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 143 +++++++++++++++++++++------------------------------\n 1 file changed, 60 insertions(+), 83 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 0ea42826c8..8420d0fc5b 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -32,17 +32,6 @@ static int ref_resolves_to_object(const char *refname,\n \treturn 1;\n }\n \n-/*\n- * Return true if the reference described by entry can be resolved to\n- * an object in the database; otherwise, emit a warning and return\n- * false.\n- */\n-static int entry_resolves_to_object(struct ref_entry *entry)\n-{\n-\treturn ref_resolves_to_object(entry->name,\n-\t\t\t\t      &entry->u.value.oid, entry->flag);\n-}\n-\n struct packed_ref_cache {\n \tstruct ref_cache *cache;\n \n@@ -548,11 +537,6 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)\n \treturn refs->loose;\n }\n \n-static struct ref_dir *get_loose_ref_dir(struct files_ref_store *refs)\n-{\n-\treturn get_ref_dir(get_loose_ref_cache(refs)->root);\n-}\n-\n /*\n  * Return the ref_entry for the given refname from the packed\n  * references.  If it does not exist, return NULL.\n@@ -1411,65 +1395,6 @@ struct ref_to_prune {\n \tchar name[FLEX_ARRAY];\n };\n \n-struct pack_refs_cb_data {\n-\tunsigned int flags;\n-\tstruct ref_dir *packed_refs;\n-\tstruct ref_to_prune *ref_to_prune;\n-};\n-\n-/*\n- * An each_ref_entry_fn that is run over loose references only.  If\n- * the loose reference can be packed, add an entry in the packed ref\n- * cache.  If the reference should be pruned, also add it to\n- * ref_to_prune in the pack_refs_cb_data.\n- */\n-static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)\n-{\n-\tstruct pack_refs_cb_data *cb = cb_data;\n-\tenum peel_status peel_status;\n-\tstruct ref_entry *packed_entry;\n-\tint is_tag_ref = starts_with(entry->name, \"refs/tags/\");\n-\n-\t/* Do not pack per-worktree refs: */\n-\tif (ref_type(entry->name) != REF_TYPE_NORMAL)\n-\t\treturn 0;\n-\n-\t/* ALWAYS pack tags */\n-\tif (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref)\n-\t\treturn 0;\n-\n-\t/* Do not pack symbolic or broken refs: */\n-\tif ((entry->flag & REF_ISSYMREF) || !entry_resolves_to_object(entry))\n-\t\treturn 0;\n-\n-\t/* Add a packed ref cache entry equivalent to the loose entry. */\n-\tpeel_status = peel_entry(entry, 1);\n-\tif (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)\n-\t\tdie(\"internal error peeling reference %s (%s)\",\n-\t\t    entry->name, oid_to_hex(&entry->u.value.oid));\n-\tpacked_entry = find_ref_entry(cb->packed_refs, entry->name);\n-\tif (packed_entry) {\n-\t\t/* Overwrite existing packed entry with info from loose entry */\n-\t\tpacked_entry->flag = REF_ISPACKED | REF_KNOWS_PEELED;\n-\t\toidcpy(&packed_entry->u.value.oid, &entry->u.value.oid);\n-\t} else {\n-\t\tpacked_entry = create_ref_entry(entry->name, entry->u.value.oid.hash,\n-\t\t\t\t\t\tREF_ISPACKED | REF_KNOWS_PEELED, 0);\n-\t\tadd_ref_entry(cb->packed_refs, packed_entry);\n-\t}\n-\toidcpy(&packed_entry->u.value.peeled, &entry->u.value.peeled);\n-\n-\t/* Schedule the loose reference for pruning if requested. */\n-\tif ((cb->flags & PACK_REFS_PRUNE)) {\n-\t\tstruct ref_to_prune *n;\n-\t\tFLEX_ALLOC_STR(n, name, entry->name);\n-\t\thashcpy(n->sha1, entry->u.value.oid.hash);\n-\t\tn->next = cb->ref_to_prune;\n-\t\tcb->ref_to_prune = n;\n-\t}\n-\treturn 0;\n-}\n-\n enum {\n \tREMOVE_EMPTY_PARENTS_REF = 0x01,\n \tREMOVE_EMPTY_PARENTS_REFLOG = 0x02\n@@ -1559,21 +1484,73 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)\n \tstruct files_ref_store *refs =\n \t\tfiles_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,\n \t\t\t       \"pack_refs\");\n-\tstruct pack_refs_cb_data cbdata;\n-\n-\tmemset(&cbdata, 0, sizeof(cbdata));\n-\tcbdata.flags = flags;\n+\tstruct ref_iterator *iter;\n+\tstruct ref_dir *packed_refs;\n+\tint ok;\n+\tstruct ref_to_prune *refs_to_prune = NULL;\n \n \tlock_packed_refs(refs, LOCK_DIE_ON_ERROR);\n-\tcbdata.packed_refs = get_packed_refs(refs);\n+\tpacked_refs = get_packed_refs(refs);\n+\n+\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL, 0);\n+\twhile ((ok = ref_iterator_advance(iter)) == ITER_OK) {\n+\t\t/*\n+\t\t * If the loose reference can be packed, add an entry\n+\t\t * in the packed ref cache. If the reference should be\n+\t\t * pruned, also add it to refs_to_prune.\n+\t\t */\n+\t\tstruct ref_entry *packed_entry;\n+\t\tint is_tag_ref = starts_with(iter->refname, \"refs/tags/\");\n+\n+\t\t/* Do not pack per-worktree refs: */\n+\t\tif (ref_type(iter->refname) != REF_TYPE_NORMAL)\n+\t\t\tcontinue;\n+\n+\t\t/* ALWAYS pack tags */\n+\t\tif (!(flags & PACK_REFS_ALL) && !is_tag_ref)\n+\t\t\tcontinue;\n+\n+\t\t/* Do not pack symbolic or broken refs: */\n+\t\tif (iter->flags & REF_ISSYMREF)\n+\t\t\tcontinue;\n \n-\tdo_for_each_entry_in_dir(get_loose_ref_dir(refs),\n-\t\t\t\t pack_if_possible_fn, &cbdata);\n+\t\tif (!ref_resolves_to_object(iter->refname, iter->oid, iter->flags))\n+\t\t\tcontinue;\n+\n+\t\t/*\n+\t\t * Create an entry in the packed-refs cache equivalent\n+\t\t * to the one from the loose ref cache, except that\n+\t\t * we don't copy the peeled status, because we want it\n+\t\t * to be re-peeled.\n+\t\t */\n+\t\tpacked_entry = find_ref_entry(packed_refs, iter->refname);\n+\t\tif (packed_entry) {\n+\t\t\t/* Overwrite existing packed entry with info from loose entry */\n+\t\t\tpacked_entry->flag = REF_ISPACKED;\n+\t\t\toidcpy(&packed_entry->u.value.oid, iter->oid);\n+\t\t} else {\n+\t\t\tpacked_entry = create_ref_entry(iter->refname, iter->oid->hash,\n+\t\t\t\t\t\t\tREF_ISPACKED, 0);\n+\t\t\tadd_ref_entry(packed_refs, packed_entry);\n+\t\t}\n+\t\toidclr(&packed_entry->u.value.peeled);\n+\n+\t\t/* Schedule the loose reference for pruning if requested. */\n+\t\tif ((flags & PACK_REFS_PRUNE)) {\n+\t\t\tstruct ref_to_prune *n;\n+\t\t\tFLEX_ALLOC_STR(n, name, iter->refname);\n+\t\t\thashcpy(n->sha1, iter->oid->hash);\n+\t\t\tn->next = refs_to_prune;\n+\t\t\trefs_to_prune = n;\n+\t\t}\n+\t}\n+\tif (ok != ITER_DONE)\n+\t\tdie(\"error while iterating over references\");\n \n \tif (commit_packed_refs(refs))\n \t\tdie_errno(\"unable to overwrite old ref-pack file\");\n \n-\tprune_refs(refs, cbdata.ref_to_prune);\n+\tprune_refs(refs, refs_to_prune);\n \treturn 0;\n }\n \n-- \n2.11.0\n\n"},{"id":"315908","messageId":"4be692203dbb726612efbafd1ea983feed350760.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 17/20] cache_ref_iterator_begin(): make function smarter","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:15Z","receivedAt":"2017-03-31T14:12:18Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Change `cache_ref_iterator_begin()` to take two new arguments:\n\n* `prefix` -- to iterate only over references with the specified\n  prefix.\n\n* `prime_dir` -- to \"prime\" (i.e., pre-load) the cache before starting\n  the iteration.\n\nThe new functionality makes it possible for\n`files_ref_iterator_begin()` to be made more ignorant of the internals\nof `ref_cache`, and `find_containing_dir()` and `prime_ref_dir()` to\nbe made private.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 44 +++++++++++++-------------------------------\n refs/ref-cache.c     | 38 ++++++++++++++++++++++++++++++++++----\n refs/ref-cache.h     | 27 +++++++++------------------\n 3 files changed, 56 insertions(+), 53 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex fefc29433a..736a6c9ff7 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1083,7 +1083,6 @@ static struct ref_iterator *files_ref_iterator_begin(\n \t\tconst char *prefix, unsigned int flags)\n {\n \tstruct files_ref_store *refs;\n-\tstruct ref_dir *loose_dir, *packed_dir;\n \tstruct ref_iterator *loose_iter, *packed_iter;\n \tstruct files_ref_iterator *iter;\n \tstruct ref_iterator *ref_iterator;\n@@ -1109,41 +1108,24 @@ static struct ref_iterator *files_ref_iterator_begin(\n \t * condition if loose refs are migrated to the packed-refs\n \t * file by a simultaneous process, but our in-memory view is\n \t * from before the migration. We ensure this as follows:\n-\t * First, we call prime_ref_dir(), which pre-reads the loose\n-\t * references for the subtree into the cache. (If they've\n-\t * already been read, that's OK; we only need to guarantee\n-\t * that they're read before the packed refs, not *how much*\n-\t * before.) After that, we call get_packed_ref_cache(), which\n-\t * internally checks whether the packed-ref cache is up to\n-\t * date with what is on disk, and re-reads it if not.\n+\t * First, we call start the loose refs iteration with its\n+\t * `prime_ref` argument set to true. This causes the loose\n+\t * references in the subtree to be pre-read into the cache.\n+\t * (If they've already been read, that's OK; we only need to\n+\t * guarantee that they're read before the packed refs, not\n+\t * *how much* before.) After that, we call\n+\t * get_packed_ref_cache(), which internally checks whether the\n+\t * packed-ref cache is up to date with what is on disk, and\n+\t * re-reads it if not.\n \t */\n \n-\tloose_dir = get_loose_ref_dir(refs);\n-\n-\tif (prefix && *prefix)\n-\t\tloose_dir = find_containing_dir(loose_dir, prefix, 0);\n-\n-\tif (loose_dir) {\n-\t\tprime_ref_dir(loose_dir);\n-\t\tloose_iter = cache_ref_iterator_begin(loose_dir);\n-\t} else {\n-\t\t/* There's nothing to iterate over. */\n-\t\tloose_iter = empty_ref_iterator_begin();\n-\t}\n+\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),\n+\t\t\t\t\t      prefix, 1);\n \n \titer->packed_ref_cache = get_packed_ref_cache(refs);\n \tacquire_packed_ref_cache(iter->packed_ref_cache);\n-\tpacked_dir = get_packed_ref_dir(iter->packed_ref_cache);\n-\n-\tif (prefix && *prefix)\n-\t\tpacked_dir = find_containing_dir(packed_dir, prefix, 0);\n-\n-\tif (packed_dir) {\n-\t\tpacked_iter = cache_ref_iterator_begin(packed_dir);\n-\t} else {\n-\t\t/* There's nothing to iterate over. */\n-\t\tpacked_iter = empty_ref_iterator_begin();\n-\t}\n+\tpacked_iter = cache_ref_iterator_begin(iter->packed_ref_cache->cache,\n+\t\t\t\t\t       prefix, 0);\n \n \titer->iter0 = overlay_ref_iterator_begin(loose_iter, packed_iter);\n \titer->flags = flags;\ndiff --git a/refs/ref-cache.c b/refs/ref-cache.c\nindex 38d4c31985..b3a30350d7 100644\n--- a/refs/ref-cache.c\n+++ b/refs/ref-cache.c\n@@ -177,8 +177,17 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir,\n \treturn get_ref_dir(entry);\n }\n \n-struct ref_dir *find_containing_dir(struct ref_dir *dir,\n-\t\t\t\t    const char *refname, int mkdir)\n+/*\n+ * If refname is a reference name, find the ref_dir within the dir\n+ * tree that should hold refname. If refname is a directory name\n+ * (i.e., it ends in '/'), then return that ref_dir itself. dir must\n+ * represent the top-level directory and must already be complete.\n+ * Sort ref_dirs and recurse into subdirectories as necessary. If\n+ * mkdir is set, then create any missing directories; otherwise,\n+ * return NULL if the desired directory cannot be found.\n+ */\n+static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n+\t\t\t\t\t   const char *refname, int mkdir)\n {\n \tconst char *slash;\n \tfor (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {\n@@ -328,7 +337,11 @@ int do_for_each_entry_in_dir(struct ref_dir *dir,\n \treturn 0;\n }\n \n-void prime_ref_dir(struct ref_dir *dir)\n+/*\n+ * Load all of the refs from `dir` (recursively) into our in-memory\n+ * cache.\n+ */\n+static void prime_ref_dir(struct ref_dir *dir)\n {\n \t/*\n \t * The hard work of loading loose refs is done by get_ref_dir(), so we\n@@ -494,12 +507,25 @@ static struct ref_iterator_vtable cache_ref_iterator_vtable = {\n \tcache_ref_iterator_abort\n };\n \n-struct ref_iterator *cache_ref_iterator_begin(struct ref_dir *dir)\n+struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache,\n+\t\t\t\t\t      const char *prefix,\n+\t\t\t\t\t      int prime_dir)\n {\n+\tstruct ref_dir *dir;\n \tstruct cache_ref_iterator *iter;\n \tstruct ref_iterator *ref_iterator;\n \tstruct cache_ref_iterator_level *level;\n \n+\tdir = get_ref_dir(cache->root);\n+\tif (prefix && *prefix)\n+\t\tdir = find_containing_dir(dir, prefix, 0);\n+\tif (!dir)\n+\t\t/* There's nothing to iterate over. */\n+\t\treturn  empty_ref_iterator_begin();\n+\n+\tif (prime_dir)\n+\t\tprime_ref_dir(dir);\n+\n \titer = xcalloc(1, sizeof(*iter));\n \tref_iterator = &iter->base;\n \tbase_ref_iterator_init(ref_iterator, &cache_ref_iterator_vtable);\n@@ -510,5 +536,9 @@ struct ref_iterator *cache_ref_iterator_begin(struct ref_dir *dir)\n \tlevel->index = -1;\n \tlevel->dir = dir;\n \n+\tif (prefix && *prefix)\n+\t\tref_iterator = prefix_ref_iterator_begin(ref_iterator,\n+\t\t\t\t\t\t\t prefix, 0);\n+\n \treturn ref_iterator;\n }\ndiff --git a/refs/ref-cache.h b/refs/ref-cache.h\nindex 6eecdf4276..5e7a918ac0 100644\n--- a/refs/ref-cache.h\n+++ b/refs/ref-cache.h\n@@ -234,18 +234,6 @@ int remove_entry_from_dir(struct ref_dir *dir, const char *refname);\n  */\n int add_ref_entry(struct ref_dir *dir, struct ref_entry *ref);\n \n-/*\n- * If refname is a reference name, find the ref_dir within the dir\n- * tree that should hold refname. If refname is a directory name\n- * (i.e., it ends in '/'), then return that ref_dir itself. dir must\n- * represent the top-level directory and must already be complete.\n- * Sort ref_dirs and recurse into subdirectories as necessary. If\n- * mkdir is set, then create any missing directories; otherwise,\n- * return NULL if the desired directory cannot be found.\n- */\n-struct ref_dir *find_containing_dir(struct ref_dir *dir,\n-\t\t\t\t    const char *refname, int mkdir);\n-\n /*\n  * Find the value entry with the given name in dir, sorting ref_dirs\n  * and recursing into subdirectories as necessary.  If the name is not\n@@ -253,7 +241,15 @@ struct ref_dir *find_containing_dir(struct ref_dir *dir,\n  */\n struct ref_entry *find_ref_entry(struct ref_dir *dir, const char *refname);\n \n-struct ref_iterator *cache_ref_iterator_begin(struct ref_dir *dir);\n+/*\n+ * Start iterating over references in `cache`. If `prefix` is\n+ * specified, only include references whose names start with that\n+ * prefix. If `prime_dir` is true, then fill any incomplete\n+ * directories before beginning the iteration.\n+ */\n+struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache,\n+\t\t\t\t\t      const char *prefix,\n+\t\t\t\t\t      int prime_dir);\n \n typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);\n \n@@ -279,9 +275,4 @@ int do_for_each_entry_in_dir(struct ref_dir *dir,\n  */\n enum peel_status peel_entry(struct ref_entry *entry, int repeel);\n \n-/*\n- * Load all of the refs from `dir` into our in-memory cache.\n- */\n-void prime_ref_dir(struct ref_dir *dir);\n-\n #endif /* REFS_REF_CACHE_H */\n-- \n2.11.0\n\n"},{"id":"315909","messageId":"9c338fe52cac277e87a8d663e0082ccb765ff3f0.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 18/20] commit_packed_refs(): use reference iteration","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:16Z","receivedAt":"2017-03-31T14:12:20Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Use reference iteration rather than do_for_each_entry_in_dir() in the\ndefinition of commit_packed_refs().\n\nNote that an internal consistency check that was previously done in\n`write_packed_entry_fn()` is not there anymore. This is actually an\nimprovement:\n\nThe old error message was emitted when there is an entry in the\npacked-ref cache that is not `REF_KNOWS_PEELED`, and when we attempted\nto peel the reference, the result was `PEEL_INVALID`,\n`PEEL_IS_SYMREF`, or `PEEL_BROKEN`. Since a packed ref cannot be a\nsymref, `PEEL_IS_SYMREF` and `PEEL_BROKEN` can be ruled out. So we're\nleft with `PEEL_INVALID`.\n\nAn entry without `REF_KNOWS_PEELED` can get into the packed-refs cache\nin the following two ways:\n\n* The reference was read from a `packed-refs` file that didn't have\n  the `fully-peeled` attribute. In that case, we *don't want* to emit\n  an error, because the broken value is presumably a stale value of\n  the reference that is now masked by a loose version of the same\n  reference (which we just don't happen to be packing this time). This\n  is a perfectly legitimate situation and doesn't indicate that the\n  repository is corrupt. The old code incorrectly emits an error\n  message in this case. (It was probably never reported as a bug\n  because this scenario is rare.)\n\n* The reference was a loose reference that was just added to the\n  packed ref cache by `files_packed_refs()` via\n  `pack_if_possible_fn()` in preparation for being packed. The latter\n  function refuses to pack a reference for which\n  `entry_resolves_to_object()` returns false, and otherwise calls\n  `peel_entry()` itself and checks the return value. So an entry added\n  this way should always have `REF_KNOWS_PEELED` and shouldn't trigger\n  the error message in either the old code or the new.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 38 +++++++++++++++++---------------------\n 1 file changed, 17 insertions(+), 21 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 736a6c9ff7..0ea42826c8 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1293,30 +1293,15 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,\n  * Write an entry to the packed-refs file for the specified refname.\n  * If peeled is non-NULL, write it as the entry's peeled value.\n  */\n-static void write_packed_entry(FILE *fh, char *refname, unsigned char *sha1,\n-\t\t\t       unsigned char *peeled)\n+static void write_packed_entry(FILE *fh, const char *refname,\n+\t\t\t       const unsigned char *sha1,\n+\t\t\t       const unsigned char *peeled)\n {\n \tfprintf_or_die(fh, \"%s %s\\n\", sha1_to_hex(sha1), refname);\n \tif (peeled)\n \t\tfprintf_or_die(fh, \"^%s\\n\", sha1_to_hex(peeled));\n }\n \n-/*\n- * An each_ref_entry_fn that writes the entry to a packed-refs file.\n- */\n-static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)\n-{\n-\tenum peel_status peel_status = peel_entry(entry, 0);\n-\n-\tif (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)\n-\t\terror(\"internal error: %s is not a valid packed reference!\",\n-\t\t      entry->name);\n-\twrite_packed_entry(cb_data, entry->name, entry->u.value.oid.hash,\n-\t\t\t   peel_status == PEEL_PEELED ?\n-\t\t\t   entry->u.value.peeled.hash : NULL);\n-\treturn 0;\n-}\n-\n /*\n  * Lock the packed-refs file for writing. Flags is passed to\n  * hold_lock_file_for_update(). Return 0 on success. On errors, set\n@@ -1362,9 +1347,10 @@ static int commit_packed_refs(struct files_ref_store *refs)\n {\n \tstruct packed_ref_cache *packed_ref_cache =\n \t\tget_packed_ref_cache(refs);\n-\tint error = 0;\n+\tint ok, error = 0;\n \tint save_errno = 0;\n \tFILE *out;\n+\tstruct ref_iterator *iter;\n \n \tfiles_assert_main_repository(refs, \"commit_packed_refs\");\n \n@@ -1376,8 +1362,18 @@ static int commit_packed_refs(struct files_ref_store *refs)\n \t\tdie_errno(\"unable to fdopen packed-refs descriptor\");\n \n \tfprintf_or_die(out, \"%s\", PACKED_REFS_HEADER);\n-\tdo_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),\n-\t\t\t\t write_packed_entry_fn, out);\n+\n+\titer = cache_ref_iterator_begin(packed_ref_cache->cache, NULL, 0);\n+\twhile ((ok = ref_iterator_advance(iter)) == ITER_OK) {\n+\t\tstruct object_id peeled;\n+\t\tint peel_error = ref_iterator_peel(iter, &peeled);\n+\n+\t\twrite_packed_entry(out, iter->refname, iter->oid->hash,\n+\t\t\t\t   peel_error ? NULL : peeled.hash);\n+\t}\n+\n+\tif (ok != ITER_DONE)\n+\t\tdie(\"error while iterating over references\");\n \n \tif (commit_lock_file(packed_ref_cache->lock)) {\n \t\tsave_errno = errno;\n-- \n2.11.0\n\n"},{"id":"315910","messageId":"1254a76cd06f6fbf0d5820b7d0ccc148413c4b16.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 13/20] refs: handle \"refs/bisect/\" in `loose_fill_ref_dir()`","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:11Z","receivedAt":"2017-03-31T14:12:28Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"That \"refs/bisect/\" has to be handled specially when filling the\nref_cache for loose references is a peculiarity of the files backend,\nand the ref-cache code shouldn't need to know about it. So move this\ncode to the callback function, `loose_fill_ref_dir()`.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 15 +++++++++++++++\n refs/ref-cache.c     | 16 ----------------\n 2 files changed, 15 insertions(+), 16 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex e4d78393ac..7b5f5c1240 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -508,6 +508,21 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,\n \tstrbuf_release(&refname);\n \tstrbuf_release(&path);\n \tclosedir(d);\n+\n+\t/*\n+\t * Manually add refs/bisect, which, being per-worktree, might\n+\t * not appear in the directory listing for refs/ in the main\n+\t * repo.\n+\t */\n+\tif (!strcmp(dirname, \"refs/\")) {\n+\t\tint pos = search_ref_dir(dir, \"refs/bisect/\", 12);\n+\n+\t\tif (pos < 0) {\n+\t\t\tstruct ref_entry *child_entry = create_dir_entry(\n+\t\t\t\t\tdir->cache, \"refs/bisect/\", 12, 1);\n+\t\t\tadd_entry_to_dir(dir, child_entry);\n+\t\t}\n+\t}\n }\n \n static struct ref_dir *get_loose_refs(struct files_ref_store *refs)\ndiff --git a/refs/ref-cache.c b/refs/ref-cache.c\nindex 7f247b9170..44440e0c13 100644\n--- a/refs/ref-cache.c\n+++ b/refs/ref-cache.c\n@@ -26,22 +26,6 @@ struct ref_dir *get_ref_dir(struct ref_entry *entry)\n \t\t\tdie(\"BUG: incomplete ref_store without fill_ref_dir function\");\n \n \t\tdir->cache->fill_ref_dir(dir->cache->ref_store, dir, entry->name);\n-\n-\t\t/*\n-\t\t * Manually add refs/bisect, which, being\n-\t\t * per-worktree, might not appear in the directory\n-\t\t * listing for refs/ in the main repo.\n-\t\t */\n-\t\tif (!strcmp(entry->name, \"refs/\")) {\n-\t\t\tint pos = search_ref_dir(dir, \"refs/bisect/\", 12);\n-\t\t\tif (pos < 0) {\n-\t\t\t\tstruct ref_entry *child_entry;\n-\t\t\t\tchild_entry = create_dir_entry(dir->cache,\n-\t\t\t\t\t\t\t       \"refs/bisect/\",\n-\t\t\t\t\t\t\t       12, 1);\n-\t\t\t\tadd_entry_to_dir(dir, child_entry);\n-\t\t\t}\n-\t\t}\n \t\tentry->flag &= ~REF_INCOMPLETE;\n \t}\n \treturn dir;\n-- \n2.11.0\n\n"},{"id":"315911","messageId":"6d22bdb112ab62e03701f6181ee05203da82f686.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 20/20] do_for_each_entry_in_dir(): delete function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:18Z","receivedAt":"2017-03-31T14:12:29Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Its only remaining caller was itself.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/ref-cache.c | 21 ---------------------\n refs/ref-cache.h | 11 -----------\n 2 files changed, 32 deletions(-)\n\ndiff --git a/refs/ref-cache.c b/refs/ref-cache.c\nindex b3a30350d7..6059362f1d 100644\n--- a/refs/ref-cache.c\n+++ b/refs/ref-cache.c\n@@ -316,27 +316,6 @@ static void sort_ref_dir(struct ref_dir *dir)\n \tdir->sorted = dir->nr = i;\n }\n \n-int do_for_each_entry_in_dir(struct ref_dir *dir,\n-\t\t\t     each_ref_entry_fn fn, void *cb_data)\n-{\n-\tint i;\n-\tassert(dir->sorted == dir->nr);\n-\tfor (i = 0; i < dir->nr; i++) {\n-\t\tstruct ref_entry *entry = dir->entries[i];\n-\t\tint retval;\n-\t\tif (entry->flag & REF_DIR) {\n-\t\t\tstruct ref_dir *subdir = get_ref_dir(entry);\n-\t\t\tsort_ref_dir(subdir);\n-\t\t\tretval = do_for_each_entry_in_dir(subdir, fn, cb_data);\n-\t\t} else {\n-\t\t\tretval = fn(entry, cb_data);\n-\t\t}\n-\t\tif (retval)\n-\t\t\treturn retval;\n-\t}\n-\treturn 0;\n-}\n-\n /*\n  * Load all of the refs from `dir` (recursively) into our in-memory\n  * cache.\ndiff --git a/refs/ref-cache.h b/refs/ref-cache.h\nindex 5e7a918ac0..ffdc54f3f0 100644\n--- a/refs/ref-cache.h\n+++ b/refs/ref-cache.h\n@@ -251,17 +251,6 @@ struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache,\n \t\t\t\t\t      const char *prefix,\n \t\t\t\t\t      int prime_dir);\n \n-typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);\n-\n-/*\n- * Call `fn` for each reference in `dir`. Recurse into subdirectories,\n- * sorting them before iterating. This function does not sort `dir`\n- * itself; it should be sorted beforehand. `fn` is called for all\n- * references, including broken ones.\n- */\n-int do_for_each_entry_in_dir(struct ref_dir *dir,\n-\t\t\t     each_ref_entry_fn fn, void *cb_data);\n-\n /*\n  * Peel the entry (if possible) and return its new peel_status.  If\n  * repeel is true, re-peel the entry even if there is an old peeled\n-- \n2.11.0\n\n"},{"id":"315912","messageId":"e90f9afd20cbfc07f8844ba4f81e7369b4b9bb00.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 10/20] ref-cache: introduce a new type, ref_cache","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:08Z","receivedAt":"2017-03-31T14:12:31Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"For now, it just wraps a `ref_entry *` that points at the root of the\ntree. Soon it will hold more information.\n\nAdd two new functions, `create_ref_cache()` and `free_ref_cache()`.\nMake `free_ref_entry()` private.\n\nChange files-backend to use this type to hold its caches.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 28 +++++++++++++++++-----------\n refs/ref-cache.c     | 16 +++++++++++++++-\n refs/ref-cache.h     | 15 ++++++++++++++-\n 3 files changed, 46 insertions(+), 13 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex f07e93d522..a7d912ae39 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -44,7 +44,7 @@ static int entry_resolves_to_object(struct ref_entry *entry)\n }\n \n struct packed_ref_cache {\n-\tstruct ref_entry *root;\n+\tstruct ref_cache *cache;\n \n \t/*\n \t * Count of references to the data structure in this instance,\n@@ -79,7 +79,7 @@ struct files_ref_store {\n \tchar *gitcommondir;\n \tchar *packed_refs_path;\n \n-\tstruct ref_entry *loose;\n+\tstruct ref_cache *loose;\n \tstruct packed_ref_cache *packed;\n };\n \n@@ -101,7 +101,7 @@ static void acquire_packed_ref_cache(struct packed_ref_cache *packed_refs)\n static int release_packed_ref_cache(struct packed_ref_cache *packed_refs)\n {\n \tif (!--packed_refs->referrers) {\n-\t\tfree_ref_entry(packed_refs->root);\n+\t\tfree_ref_cache(packed_refs->cache);\n \t\tstat_validity_clear(&packed_refs->validity);\n \t\tfree(packed_refs);\n \t\treturn 1;\n@@ -125,7 +125,7 @@ static void clear_packed_ref_cache(struct files_ref_store *refs)\n static void clear_loose_ref_cache(struct files_ref_store *refs)\n {\n \tif (refs->loose) {\n-\t\tfree_ref_entry(refs->loose);\n+\t\tfree_ref_cache(refs->loose);\n \t\trefs->loose = NULL;\n \t}\n }\n@@ -387,11 +387,12 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref\n \n \t\trefs->packed = xcalloc(1, sizeof(*refs->packed));\n \t\tacquire_packed_ref_cache(refs->packed);\n-\t\trefs->packed->root = create_dir_entry(refs, \"\", 0, 0);\n+\t\trefs->packed->cache = create_ref_cache(refs);\n+\t\trefs->packed->cache->root->flag &= ~REF_INCOMPLETE;\n \t\tf = fopen(packed_refs_file, \"r\");\n \t\tif (f) {\n \t\t\tstat_validity_update(&refs->packed->validity, fileno(f));\n-\t\t\tread_packed_refs(f, get_ref_dir(refs->packed->root));\n+\t\t\tread_packed_refs(f, get_ref_dir(refs->packed->cache->root));\n \t\t\tfclose(f);\n \t\t}\n \t}\n@@ -400,7 +401,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref\n \n static struct ref_dir *get_packed_ref_dir(struct packed_ref_cache *packed_ref_cache)\n {\n-\treturn get_ref_dir(packed_ref_cache->root);\n+\treturn get_ref_dir(packed_ref_cache->cache->root);\n }\n \n static struct ref_dir *get_packed_refs(struct files_ref_store *refs)\n@@ -515,14 +516,19 @@ static struct ref_dir *get_loose_refs(struct files_ref_store *refs)\n \t\t * are about to read the only subdirectory that can\n \t\t * hold references:\n \t\t */\n-\t\trefs->loose = create_dir_entry(refs, \"\", 0, 0);\n+\t\trefs->loose = create_ref_cache(refs);\n+\n+\t\t/* We're going to fill the top level ourselves: */\n+\t\trefs->loose->root->flag &= ~REF_INCOMPLETE;\n+\n \t\t/*\n-\t\t * Create an incomplete entry for \"refs/\":\n+\t\t * Add an incomplete entry for \"refs/\" (to be filled\n+\t\t * lazily):\n \t\t */\n-\t\tadd_entry_to_dir(get_ref_dir(refs->loose),\n+\t\tadd_entry_to_dir(get_ref_dir(refs->loose->root),\n \t\t\t\t create_dir_entry(refs, \"refs/\", 5, 1));\n \t}\n-\treturn get_ref_dir(refs->loose);\n+\treturn get_ref_dir(refs->loose->root);\n }\n \n /*\ndiff --git a/refs/ref-cache.c b/refs/ref-cache.c\nindex 4274a43a9b..bf911028c8 100644\n--- a/refs/ref-cache.c\n+++ b/refs/ref-cache.c\n@@ -63,9 +63,17 @@ struct ref_entry *create_ref_entry(const char *refname,\n \treturn ref;\n }\n \n+struct ref_cache *create_ref_cache(struct files_ref_store *refs)\n+{\n+\tstruct ref_cache *ret = xcalloc(1, sizeof(*ret));\n+\n+\tret->root = create_dir_entry(refs, \"\", 0, 1);\n+\treturn ret;\n+}\n+\n static void clear_ref_dir(struct ref_dir *dir);\n \n-void free_ref_entry(struct ref_entry *entry)\n+static void free_ref_entry(struct ref_entry *entry)\n {\n \tif (entry->flag & REF_DIR) {\n \t\t/*\n@@ -77,6 +85,12 @@ void free_ref_entry(struct ref_entry *entry)\n \tfree(entry);\n }\n \n+void free_ref_cache(struct ref_cache *cache)\n+{\n+\tfree_ref_entry(cache->root);\n+\tfree(cache);\n+}\n+\n /*\n  * Clear and free all entries in dir, recursively.\n  */\ndiff --git a/refs/ref-cache.h b/refs/ref-cache.h\nindex 2e7b1a366e..da5388c136 100644\n--- a/refs/ref-cache.h\n+++ b/refs/ref-cache.h\n@@ -1,6 +1,10 @@\n #ifndef REFS_REF_CACHE_H\n #define REFS_REF_CACHE_H\n \n+struct ref_cache {\n+\tstruct ref_entry *root;\n+};\n+\n /*\n  * Information used (along with the information in ref_entry) to\n  * describe a single cached reference.  This data structure only\n@@ -165,7 +169,16 @@ struct ref_entry *create_ref_entry(const char *refname,\n \t\t\t\t   const unsigned char *sha1, int flag,\n \t\t\t\t   int check_name);\n \n-void free_ref_entry(struct ref_entry *entry);\n+/*\n+ * Return a pointer to a new `ref_cache`. Its top-level starts out\n+ * marked incomplete.\n+ */\n+struct ref_cache *create_ref_cache(struct files_ref_store *refs);\n+\n+/*\n+ * Free the `ref_cache` and all of its associated data.\n+ */\n+void free_ref_cache(struct ref_cache *cache);\n \n /*\n  * Add a ref_entry to the end of dir (unsorted).  Entry is always\n-- \n2.11.0\n\n"},{"id":"315913","messageId":"5f739eb95a8635210dffb12d122551d40795706e.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 14/20] do_for_each_entry_in_dir(): eliminate `offset` argument","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:12Z","receivedAt":"2017-03-31T14:12:33Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It was never used.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c |  4 ++--\n refs/ref-cache.c     |  6 +++---\n refs/ref-cache.h     | 11 +++++------\n 3 files changed, 10 insertions(+), 11 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 7b5f5c1240..0ff5df6b46 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1390,7 +1390,7 @@ static int commit_packed_refs(struct files_ref_store *refs)\n \n \tfprintf_or_die(out, \"%s\", PACKED_REFS_HEADER);\n \tdo_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),\n-\t\t\t\t 0, write_packed_entry_fn, out);\n+\t\t\t\t write_packed_entry_fn, out);\n \n \tif (commit_lock_file(packed_ref_cache->lock)) {\n \t\tsave_errno = errno;\n@@ -1584,7 +1584,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)\n \tlock_packed_refs(refs, LOCK_DIE_ON_ERROR);\n \tcbdata.packed_refs = get_packed_refs(refs);\n \n-\tdo_for_each_entry_in_dir(get_loose_refs(refs), 0,\n+\tdo_for_each_entry_in_dir(get_loose_refs(refs),\n \t\t\t\t pack_if_possible_fn, &cbdata);\n \n \tif (commit_packed_refs(refs))\ndiff --git a/refs/ref-cache.c b/refs/ref-cache.c\nindex 44440e0c13..38d4c31985 100644\n--- a/refs/ref-cache.c\n+++ b/refs/ref-cache.c\n@@ -307,18 +307,18 @@ static void sort_ref_dir(struct ref_dir *dir)\n \tdir->sorted = dir->nr = i;\n }\n \n-int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,\n+int do_for_each_entry_in_dir(struct ref_dir *dir,\n \t\t\t     each_ref_entry_fn fn, void *cb_data)\n {\n \tint i;\n \tassert(dir->sorted == dir->nr);\n-\tfor (i = offset; i < dir->nr; i++) {\n+\tfor (i = 0; i < dir->nr; i++) {\n \t\tstruct ref_entry *entry = dir->entries[i];\n \t\tint retval;\n \t\tif (entry->flag & REF_DIR) {\n \t\t\tstruct ref_dir *subdir = get_ref_dir(entry);\n \t\t\tsort_ref_dir(subdir);\n-\t\t\tretval = do_for_each_entry_in_dir(subdir, 0, fn, cb_data);\n+\t\t\tretval = do_for_each_entry_in_dir(subdir, fn, cb_data);\n \t\t} else {\n \t\t\tretval = fn(entry, cb_data);\n \t\t}\ndiff --git a/refs/ref-cache.h b/refs/ref-cache.h\nindex ed51e80d88..6eecdf4276 100644\n--- a/refs/ref-cache.h\n+++ b/refs/ref-cache.h\n@@ -258,13 +258,12 @@ struct ref_iterator *cache_ref_iterator_begin(struct ref_dir *dir);\n typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);\n \n /*\n- * Call fn for each reference in dir that has index in the range\n- * offset <= index < dir->nr.  Recurse into subdirectories that are in\n- * that index range, sorting them before iterating.  This function\n- * does not sort dir itself; it should be sorted beforehand.  fn is\n- * called for all references, including broken ones.\n+ * Call `fn` for each reference in `dir`. Recurse into subdirectories,\n+ * sorting them before iterating. This function does not sort `dir`\n+ * itself; it should be sorted beforehand. `fn` is called for all\n+ * references, including broken ones.\n  */\n-int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,\n+int do_for_each_entry_in_dir(struct ref_dir *dir,\n \t\t\t     each_ref_entry_fn fn, void *cb_data);\n \n /*\n-- \n2.11.0\n\n"},{"id":"315914","messageId":"7e63849fdb197cd66b229a28408845908fa844a7.1490966385.git.mhagger@alum.mit.edu","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"[PATCH v2 11/20] refs: record the ref_store in ref_cache, not ref_dir","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-31T14:11:09Z","receivedAt":"2017-03-31T14:12:36Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Instead of keeping a pointer to the ref_store in every ref_dir entry,\nstore it once in `struct ref_cache`, and change `struct ref_dir` to\ninclude a pointer to its containing `ref_cache` instead. This makes it\neasier to add to the information that is accessible from a `ref_dir`\nwithout increasing the size of every `ref_dir` instance.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c |  6 +++---\n refs/ref-cache.c     | 12 +++++++-----\n refs/ref-cache.h     |  9 ++++++---\n 3 files changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex a7d912ae39..78572e55a0 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -433,7 +433,7 @@ static void add_packed_ref(struct files_ref_store *refs,\n  */\n void read_loose_refs(const char *dirname, struct ref_dir *dir)\n {\n-\tstruct files_ref_store *refs = dir->ref_store;\n+\tstruct files_ref_store *refs = dir->cache->ref_store;\n \tDIR *d;\n \tstruct dirent *de;\n \tint dirnamelen = strlen(dirname);\n@@ -469,7 +469,7 @@ void read_loose_refs(const char *dirname, struct ref_dir *dir)\n \t\t} else if (S_ISDIR(st.st_mode)) {\n \t\t\tstrbuf_addch(&refname, '/');\n \t\t\tadd_entry_to_dir(dir,\n-\t\t\t\t\t create_dir_entry(refs, refname.buf,\n+\t\t\t\t\t create_dir_entry(dir->cache, refname.buf,\n \t\t\t\t\t\t\t  refname.len, 1));\n \t\t} else {\n \t\t\tif (!refs_resolve_ref_unsafe(&refs->base,\n@@ -526,7 +526,7 @@ static struct ref_dir *get_loose_refs(struct files_ref_store *refs)\n \t\t * lazily):\n \t\t */\n \t\tadd_entry_to_dir(get_ref_dir(refs->loose->root),\n-\t\t\t\t create_dir_entry(refs, \"refs/\", 5, 1));\n+\t\t\t\t create_dir_entry(refs->loose, \"refs/\", 5, 1));\n \t}\n \treturn get_ref_dir(refs->loose->root);\n }\ndiff --git a/refs/ref-cache.c b/refs/ref-cache.c\nindex bf911028c8..96da094788 100644\n--- a/refs/ref-cache.c\n+++ b/refs/ref-cache.c\n@@ -36,7 +36,7 @@ struct ref_dir *get_ref_dir(struct ref_entry *entry)\n \t\t\tint pos = search_ref_dir(dir, \"refs/bisect/\", 12);\n \t\t\tif (pos < 0) {\n \t\t\t\tstruct ref_entry *child_entry;\n-\t\t\t\tchild_entry = create_dir_entry(dir->ref_store,\n+\t\t\t\tchild_entry = create_dir_entry(dir->cache,\n \t\t\t\t\t\t\t       \"refs/bisect/\",\n \t\t\t\t\t\t\t       12, 1);\n \t\t\t\tadd_entry_to_dir(dir, child_entry);\n@@ -67,7 +67,8 @@ struct ref_cache *create_ref_cache(struct files_ref_store *refs)\n {\n \tstruct ref_cache *ret = xcalloc(1, sizeof(*ret));\n \n-\tret->root = create_dir_entry(refs, \"\", 0, 1);\n+\tret->ref_store = refs;\n+\tret->root = create_dir_entry(ret, \"\", 0, 1);\n \treturn ret;\n }\n \n@@ -104,13 +105,14 @@ static void clear_ref_dir(struct ref_dir *dir)\n \tdir->entries = NULL;\n }\n \n-struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,\n+struct ref_entry *create_dir_entry(struct ref_cache *cache,\n \t\t\t\t   const char *dirname, size_t len,\n \t\t\t\t   int incomplete)\n {\n \tstruct ref_entry *direntry;\n+\n \tFLEX_ALLOC_MEM(direntry, name, dirname, len);\n-\tdirentry->u.subdir.ref_store = ref_store;\n+\tdirentry->u.subdir.cache = cache;\n \tdirentry->flag = REF_DIR | (incomplete ? REF_INCOMPLETE : 0);\n \treturn direntry;\n }\n@@ -181,7 +183,7 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir,\n \t\t * therefore, create an empty record for it but mark\n \t\t * the record complete.\n \t\t */\n-\t\tentry = create_dir_entry(dir->ref_store, subdirname, len, 0);\n+\t\tentry = create_dir_entry(dir->cache, subdirname, len, 0);\n \t\tadd_entry_to_dir(dir, entry);\n \t} else {\n \t\tentry = dir->entries[entry_index];\ndiff --git a/refs/ref-cache.h b/refs/ref-cache.h\nindex da5388c136..83051854ff 100644\n--- a/refs/ref-cache.h\n+++ b/refs/ref-cache.h\n@@ -3,6 +3,9 @@\n \n struct ref_cache {\n \tstruct ref_entry *root;\n+\n+\t/* A pointer to the files_ref_store whose cache this is: */\n+\tstruct files_ref_store *ref_store;\n };\n \n /*\n@@ -66,8 +69,8 @@ struct ref_dir {\n \t */\n \tint sorted;\n \n-\t/* A pointer to the files_ref_store that contains this ref_dir. */\n-\tstruct files_ref_store *ref_store;\n+\t/* The ref_cache containing this entry: */\n+\tstruct ref_cache *cache;\n \n \tstruct ref_entry **entries;\n };\n@@ -161,7 +164,7 @@ struct ref_dir *get_ref_dir(struct ref_entry *entry);\n  * dirname is the name of the directory with a trailing slash (e.g.,\n  * \"refs/heads/\") or \"\" for the top-level directory.\n  */\n-struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,\n+struct ref_entry *create_dir_entry(struct ref_cache *cache,\n \t\t\t\t   const char *dirname, size_t len,\n \t\t\t\t   int incomplete);\n \n-- \n2.11.0\n\n"},{"id":"315921","messageId":"xmqq1std1lmb.fsf@gitster.mtv.corp.google.com","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-31T16:01:16Z","receivedAt":"2017-03-31T16:01:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> This version literally only contains a few commit message changes and\n> one minor comment changes relative to v1. The code is identical. I\n> wasn't sure whether it is even worth sending this patch series to the\n> ML again; Junio, if you'd prefer I just send a link to a published\n> branch in such cases, please let me know.\n\nThe review on the list is not about letting me pick it up, but is\nabout giving reviewing contributors a chance to comment.  I think\nfor a series this important ;-) it is good that you are giving it\nmultiple exposures so that people who were offline the last time can\nhave a chance to look at it, even if the update is minimum.\n\n> This patch series is also available from my GitHub fork [2] as branch\n> \"separate-ref-cache\". These patches depend on Duy's\n> nd/files-backend-git-dir branch.\n\nI am getting the impression that the files-backend thing as well as\nthis topic are ready for 'next'.  Please stop me if I missed something\nin these topics (especially the other one) that needs updating\nbefore that happens.\n\nThanks.\n"},{"id":"315927","messageId":"CAGZ79kZ9uGjhN5n8YAdUh2xqDMxTmmfsa7rCEWMk-Bsp5uxncg@mail.gmail.com","threadId":"45565","inReplyTo":"53e0eed5e13a04f502dcaeda83b2cbb3035f1396.1490966385.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v2 01/20] get_ref_dir(): don't call read_loose_refs() for \"refs/bisect\"","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-31T17:18:26Z","receivedAt":"2017-03-31T17:18:33Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Mar 31, 2017 at 7:10 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> Since references under \"refs/bisect/\" are per-worktree, they have to\n> be sought in the worktree rather than in the main repository. But\n> since loose references are found by traversing directories, the\n> reference iterator won't even get the idea to look for a\n> \"refs/bisect/\" directory in the worktree if there is not a directory\n> with that name in the main repository. Thus `get_ref_dir()` manually\n> inserts a dir_entry for \"refs/bisect/\" whenever it reads the entry for\n> \"refs/\".\n>\n> The current code then immediately calls `read_loose_refs()` on that\n> directory. But since the dir_entry is created with its `incomplete`\n> flag set, any traversal that gets to this point will read the\n> directory automatically. So there is no need to call\n> `read_loose_refs()` explicitly; the lazy mechanism suffices.\n>\n> And in fact, the attempt to `read_loose_refs()` was broken anyway.\n> That function needs its `dirname` argument to have a trailing `/`\n> character, but the invocation here was passing it \"refs/bisect\"\n> without a trailing slash. So `read_loose_refs()` would read\n> `$GIT_DIR/refs/bisect\" correctly, but if it found an entry \"foo\" in\n> that directory, it would try to read \"$GIT_DIR/refs/bisectfoo\".\n> Normally it wouldn't find anything at that path, but the failure was\n> canceled out because `get_ref_dir()` *also* forgot to reset the\n> `REF_INCOMPLETE` bit on the dir_entry. So the read was attempted again\n> when it was accessed, via the lazy mechanism, and this time the read\n> was done correctly.\n\nWith all this retry logic going on, it is hard to add a test demonstrating\nthis is correct now. The description makes sense though.\n\nThanks,\nStefan\n"},{"id":"315987","messageId":"13eaacf0-5923-ad33-79ed-2cec47660904@alum.mit.edu","threadId":"45565","inReplyTo":"xmqq1std1lmb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-04-01T05:16:56Z","receivedAt":"2017-04-01T05:17:11Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 03/31/2017 06:01 PM, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n>> This version literally only contains a few commit message changes and\n>> one minor comment changes relative to v1. The code is identical. I\n>> wasn't sure whether it is even worth sending this patch series to the\n>> ML again; Junio, if you'd prefer I just send a link to a published\n>> branch in such cases, please let me know.\n> \n> The review on the list is not about letting me pick it up, but is\n> about giving reviewing contributors a chance to comment.  I think\n> for a series this important ;-) it is good that you are giving it\n> multiple exposures so that people who were offline the last time can\n> have a chance to look at it, even if the update is minimum.\n> \n>> This patch series is also available from my GitHub fork [2] as branch\n>> \"separate-ref-cache\". These patches depend on Duy's\n>> nd/files-backend-git-dir branch.\n> \n> I am getting the impression that the files-backend thing as well as\n> this topic are ready for 'next'.  Please stop me if I missed something\n> in these topics (especially the other one) that needs updating\n> before that happens.\n\nI don't know of any remaining problems with these two patch series\n(aside from a couple of cosmetic problems that I just pointed out about\nv7 of nd/files-backend-git-dir). I'm pretty confident about both of them.\n\nDuy, have you looked over my patch series? Since you've been working in\nthe area, your feedback would be very welcome, if you have the time for it.\n\nMichael\n\n"},{"id":"315993","messageId":"20170401062000.3j2jblntkfqd4cd4@sigill.intra.peff.net","threadId":"45565","inReplyTo":"cover.1490966385.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-01T06:20:00Z","receivedAt":"2017-04-01T06:20:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 31, 2017 at 04:10:58PM +0200, Michael Haggerty wrote:\n\n> * I added a long blurb about the removal of an internal consistency\n>   check in the commit message for\n> \n>       [18/20] commit_packed_refs(): use reference iteration\n\nThanks, the explanation there makes sense.\n\nI think this version addresses all of my comments.\n\n-Peff\n"},{"id":"316030","messageId":"9b9a6cc2-a7d4-a7b8-0483-5aefdf926ca7@ramsayjones.plus.com","threadId":"45565","inReplyTo":"xmqq1std1lmb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-04-01T21:26:59Z","receivedAt":"2017-04-01T21:27:08Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 31/03/17 17:01, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n>> This version literally only contains a few commit message changes and\n>> one minor comment changes relative to v1. The code is identical. I\n>> wasn't sure whether it is even worth sending this patch series to the\n>> ML again; Junio, if you'd prefer I just send a link to a published\n>> branch in such cases, please let me know.\n> \n> The review on the list is not about letting me pick it up, but is\n> about giving reviewing contributors a chance to comment.  I think\n> for a series this important ;-) it is good that you are giving it\n> multiple exposures so that people who were offline the last time can\n> have a chance to look at it, even if the update is minimum.\n> \n>> This patch series is also available from my GitHub fork [2] as branch\n>> \"separate-ref-cache\". These patches depend on Duy's\n>> nd/files-backend-git-dir branch.\n> \n> I am getting the impression that the files-backend thing as well as\n> this topic are ready for 'next'.  Please stop me if I missed something\n> in these topics (especially the other one) that needs updating\n> before that happens.\n\nHmm, are these branches 'tangled' with nd/prune-in-worktree?\n(I think they were at one point, but maybe not now?).\n\nI sent Duy some comments on that branch (an unused function and\na 'plain integer used as NULL pointer' warning).\n\nATB,\nRamsay Jones\n\n\n"},{"id":"316033","messageId":"xmqqh927tr6p.fsf@gitster.mtv.corp.google.com","threadId":"45565","inReplyTo":"9b9a6cc2-a7d4-a7b8-0483-5aefdf926ca7@ramsayjones.plus.com","subject":"Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-02T03:38:06Z","receivedAt":"2017-04-02T03:38:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsayjones.plus.com> writes:\n\n>> I am getting the impression that the files-backend thing as well as\n>> this topic are ready for 'next'.  Please stop me if I missed something\n>> in these topics (especially the other one) that needs updating\n>> before that happens.\n>\n> Hmm, are these branches 'tangled' with nd/prune-in-worktree?\n> (I think they were at one point, but maybe not now?).\n\nMichael's mh/separate-ref-cache builds directly on top of\nnd/files-backend-git-dir topic.\n\nnd/prune-in-worktree builds directly on top of\nnd/worktree-kill-parse-ref, which in turn builds directly on top of\nnd/files-backend-git-dir.\n\nIn that sense, Michael's series and Duy's later two series are\n\"tangled\" (i.e. shares some common commits that are still not in\n'master').  If nd/files-backend-git-dir that is shared among them is\never rebased, all of them need to be rebased on top of it\nconsistently.\n\nBut if either of nd/prune-in-worktree and nd/worktree-kill-parse-ref\nneeds to be rerolled, that can be done independently from Michael's\nseries, as long as nd/files-backend-git-dir is solid and unchanging.\n\n\n"},{"id":"316056","messageId":"fe8f84d7-7bcc-f872-4bad-32e5b957bf17@ramsayjones.plus.com","threadId":"45565","inReplyTo":"xmqqh927tr6p.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-04-02T14:48:40Z","receivedAt":"2017-04-02T14:48:48Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 02/04/17 04:38, Junio C Hamano wrote:\n> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:\n> \n>>> I am getting the impression that the files-backend thing as well as\n>>> this topic are ready for 'next'.  Please stop me if I missed something\n>>> in these topics (especially the other one) that needs updating\n>>> before that happens.\n>>\n>> Hmm, are these branches 'tangled' with nd/prune-in-worktree?\n>> (I think they were at one point, but maybe not now?).\n> \n> Michael's mh/separate-ref-cache builds directly on top of\n> nd/files-backend-git-dir topic.\n> \n> nd/prune-in-worktree builds directly on top of\n> nd/worktree-kill-parse-ref, which in turn builds directly on top of\n> nd/files-backend-git-dir.\n> \n> In that sense, Michael's series and Duy's later two series are\n> \"tangled\" (i.e. shares some common commits that are still not in\n> 'master').  If nd/files-backend-git-dir that is shared among them is\n> ever rebased, all of them need to be rebased on top of it\n> consistently.\n> \n> But if either of nd/prune-in-worktree and nd/worktree-kill-parse-ref\n> needs to be rerolled, that can be done independently from Michael's\n> series, as long as nd/files-backend-git-dir is solid and unchanging.\n\nI suspected as much (hence the 'maybe not now' comment), but I noticed\nthat all four branches were merged into 'jch'. So, it seemed possible\nto me that they were all being considered for merging into next.\n\nThanks!\n\nATB,\nRamsay Jones\n\n\n"},{"id":"316057","messageId":"xmqq4ly6u5cy.fsf@gitster.mtv.corp.google.com","threadId":"45565","inReplyTo":"fe8f84d7-7bcc-f872-4bad-32e5b957bf17@ramsayjones.plus.com","subject":"Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-02T16:44:13Z","receivedAt":"2017-04-02T16:44:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsayjones.plus.com> writes:\n\n>> In that sense, Michael's series and Duy's later two series are\n>> \"tangled\" (i.e. shares some common commits that are still not in\n>> 'master').  If nd/files-backend-git-dir that is shared among them is\n>> ever rebased, all of them need to be rebased on top of it\n>> consistently.\n>> \n>> But if either of nd/prune-in-worktree and nd/worktree-kill-parse-ref\n>> needs to be rerolled, that can be done independently from Michael's\n>> series, as long as nd/files-backend-git-dir is solid and unchanging.\n>\n> I suspected as much (hence the 'maybe not now' comment), but I noticed\n> that all four branches were merged into 'jch'. So, it seemed possible\n> to me that they were all being considered for merging into next.\n\nYes they were (and still are, but I do not expect they will be\nmoving to 'next' while I am offline ;-).  What you can do to help is\nto review and say things like \"nd/files-backend-git-dir and\nMichael's one on top of it looked good, no opinion on others\",\n\"the others have this and that issues that need a reroll\", etc.\n\nThanks.\n\n"},{"id":"316206","messageId":"CACsJy8BErEEmbq884XOvg=pQbcVhCkdmsaux58sMaNW_B0nSZg@mail.gmail.com","threadId":"45565","inReplyTo":"13eaacf0-5923-ad33-79ed-2cec47660904@alum.mit.edu","subject":"Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-05T14:03:37Z","receivedAt":"2017-04-05T14:04:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Apr 1, 2017 at 12:16 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> Duy, have you looked over my patch series? Since you've been working in\n> the area, your feedback would be very welcome, if you have the time for it.\n\nYou probably have guessed my answer based on my lack of response :)\nTomorrow is holiday though, will have a look. But I doubt I would find\nanything given that both you and Jeff are involved in this.\n\nOn Sun, Apr 2, 2017 at 4:26 AM, Ramsay Jones\n<ramsay@ramsayjones.plus.com> wrote:\n> I sent Duy some comments on that branch (an unused function and\n> a 'plain integer used as NULL pointer' warning).\n\nI still remember :) Or at least I vaguely remember something I need to\nfix from you, and made sure comments in all my re-rolls are addressed.\nnd/prune-in-worktree should be the next one, soon.\n-- \nDuy\n"},{"id":"316330","messageId":"CACsJy8CECcC1EDpr8u39Pzq75E5KXrYwm_Bz+UTXobWvgV5weA@mail.gmail.com","threadId":"45565","inReplyTo":"840b199ad58f11a690c684ad110807ed81d3b0eb.1490966385.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v2 03/20] refs_ref_iterator_begin(): new function","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-07T10:57:15Z","receivedAt":"2017-04-07T10:57:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> Extract a new function from `do_for_each_ref()`. It will be useful\n> elsewhere.\n>\n> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n> ---\n>  refs.c               | 15 +++++++++++++--\n>  refs/refs-internal.h | 11 +++++++++++\n>  2 files changed, 24 insertions(+), 2 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 0ed6c3c7a4..aeb704ab92 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1230,6 +1230,18 @@ int head_ref(each_ref_fn fn, void *cb_data)\n>         return head_ref_submodule(NULL, fn, cb_data);\n>  }\n>\n> +struct ref_iterator *refs_ref_iterator_begin(\n> +               struct ref_store *refs,\n> +               const char *prefix, int trim, int flags)\n> +{\n> +       struct ref_iterator *iter;\n> +\n> +       iter = refs->be->iterator_begin(refs, prefix, flags);\n> +       iter = prefix_ref_iterator_begin(iter, prefix, trim);\n\nOff topic. This code made me wonder if we really need the prefix\niterator if prefix is NULL. And in fact we don't since\nprefix_ref_iterator_begin() will return the old iter in that case. But\nit's probably better to move that optimization outside. I think it's\neasier to understand that way, calling prefix_ref_ will always give\nyou a new iterator. Don't call it unless you want to have it.\n\n> +/*\n> + * Return an iterator that goes over each reference in `refs` for\n> + * which the refname begins with prefix. If trim is non-zero, then\n> + * trim that many characters off the beginning of each refname. flags\n> + * can be DO_FOR_EACH_INCLUDE_BROKEN to include broken references in\n> + * the iteration.\n> + */\n\nDo we need a separate docstring here? I think we document more or less\nthe same for ref_iterator_begin_fn (except the include-broken flag).\n\n> +struct ref_iterator *refs_ref_iterator_begin(\n> +               struct ref_store *refs,\n> +               const char *prefix, int trim, int flags);\n> +\n-- \nDuy\n"},{"id":"316333","messageId":"CACsJy8CZwPN06Q4OFmbjh8iCigZbrTUGU20hmxNCDRiAbB+KVA@mail.gmail.com","threadId":"45565","inReplyTo":"22abd274bfdada94b3654a811ee209822640765f.1490966385.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v2 04/20] refs_verify_refname_available(): implement once for all backends","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-07T11:20:07Z","receivedAt":"2017-04-07T11:20:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> It turns out that we can now implement\n> `refs_verify_refname_available()` based on the other virtual\n> functions, so there is no need for it to be defined at the backend\n> level. Instead, define it once in `refs.c` and remove the\n> `files_backend` definition.\n>\n> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n> ---\n>  refs.c               | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++--\n>  refs.h               |  2 +-\n>  refs/files-backend.c | 39 +++++-------------------\n\nMuch appreciated. This will make future backends simpler to implement as well.\n\n> +       iter = refs_ref_iterator_begin(refs, dirname.buf, 0,\n> +                                      DO_FOR_EACH_INCLUDE_BROKEN);\n> +       while ((ok = ref_iterator_advance(iter)) == ITER_OK) {\n> +               if (skip &&\n> +                   string_list_has_string(skip, iter->refname))\n> +                       continue;\n> +\n> +               strbuf_addf(err, \"'%s' exists; cannot create '%s'\",\n> +                           iter->refname, refname);\n> +               ok = ref_iterator_abort(iter);\n\nSaving the return code in \"ok\" seems redundant because you don't use\nit. Or did you want to check that ok == ITER_DONE or die() too?\n\n> +               goto cleanup;\n> +       }\n> +\n> +       if (ok != ITER_DONE)\n> +               die(\"BUG: error while iterating over references\");\n> +\n> +       extra_refname = find_descendant_ref(dirname.buf, extras, skip);\n> +       if (extra_refname)\n> +               strbuf_addf(err, \"cannot process '%s' and '%s' at the same time\",\n> +                           refname, extra_refname);\n> +       else\n> +               ret = 0;\n> +\n> +cleanup:\n> +       strbuf_release(&referent);\n> +       strbuf_release(&dirname);\n> +       return ret;\n>  }\n-- \nDuy\n"},{"id":"316334","messageId":"CACsJy8Careik7fCtXo_Luu40G8YPXAgZomFKDkCOuuOfEYm7nA@mail.gmail.com","threadId":"45565","inReplyTo":"e90f9afd20cbfc07f8844ba4f81e7369b4b9bb00.1490966385.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v2 10/20] ref-cache: introduce a new type, ref_cache","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-07T11:32:22Z","receivedAt":"2017-04-07T11:32:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> +void free_ref_cache(struct ref_cache *cache)\n> +{\n> +       free_ref_entry(cache->root);\n> +       free(cache);\n> +}\n\nfree(NULL) is no-op (and safe). Maybe we should follow the same\npattern for free_ref_cache(). It's really up to you.\n-- \nDuy\n"},{"id":"316335","messageId":"CACsJy8BM1nHhbKo9ioAP2OC2UU-RMWhmJ5OsYZwWYqek++DA0Q@mail.gmail.com","threadId":"45565","inReplyTo":"7e63849fdb197cd66b229a28408845908fa844a7.1490966385.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v2 11/20] refs: record the ref_store in ref_cache, not ref_dir","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-07T11:38:04Z","receivedAt":"2017-04-07T11:38:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> Instead of keeping a pointer to the ref_store in every ref_dir entry,\n> store it once in `struct ref_cache`, and change `struct ref_dir` to\n> include a pointer to its containing `ref_cache` instead. This makes it\n> easier to add to the information that is accessible from a `ref_dir`\n> without increasing the size of every `ref_dir` instance.\n...\n> @@ -526,7 +526,7 @@ static struct ref_dir *get_loose_refs(struct files_ref_store *refs)\n>                  * lazily):\n>                  */\n>                 add_entry_to_dir(get_ref_dir(refs->loose->root),\n> -                                create_dir_entry(refs, \"refs/\", 5, 1));\n> +                                create_dir_entry(refs->loose, \"refs/\", 5, 1));\n\nThe commit message mentions nothing about this change. Is it intended?\n-- \nDuy\n"},{"id":"316336","messageId":"CACsJy8DAFUQnjkLnaXD0aM_97_E_7F_=fMeJb2Azigt2Pv0gTw@mail.gmail.com","threadId":"45565","inReplyTo":"1d131a33fd155f14908ee1d41e98be8a1309c6b9.1490966385.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v2 19/20] files_pack_refs(): use reference iteration","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-07T11:51:24Z","receivedAt":"2017-04-07T11:52:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> Use reference iteration rather than do_for_each_entry_in_dir() in the\n> definition of files_pack_refs().\n\nA \"why\" is missing here. My guess is readability/maintainability\nbecause it's easier to follow the code with iterator design, than the\ncallback one? No maintaining data in struct pack_refs_cb_data is also\nvery nice.\n-- \nDuy\n"},{"id":"316337","messageId":"CACsJy8ArrVRjmBoXtxKVtftNaSoSX_nXkuExsXwaBFkayv9kQg@mail.gmail.com","threadId":"45565","inReplyTo":"CACsJy8BErEEmbq884XOvg=pQbcVhCkdmsaux58sMaNW_B0nSZg@mail.gmail.com","subject":"Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-07T11:53:47Z","receivedAt":"2017-04-07T11:54:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 5, 2017 at 9:03 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Sat, Apr 1, 2017 at 12:16 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n>> Duy, have you looked over my patch series? Since you've been working in\n>> the area, your feedback would be very welcome, if you have the time for it.\n>\n> You probably have guessed my answer based on my lack of response :)\n> Tomorrow is holiday though, will have a look. But I doubt I would find\n> anything given that both you and Jeff are involved in this.\n\nOverall, very nice. I made a comment here and there, but they're more\nquestions for my education than actual code review ) At least there's\nanother set of eyes on this series now.\n-- \nDuy\n"},{"id":"316909","messageId":"3b508517-80fc-d759-d27f-a582a36286ba@alum.mit.edu","threadId":"45565","inReplyTo":"CACsJy8CECcC1EDpr8u39Pzq75E5KXrYwm_Bz+UTXobWvgV5weA@mail.gmail.com","subject":"Re: [PATCH v2 03/20] refs_ref_iterator_begin(): new function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-04-16T04:39:04Z","receivedAt":"2017-04-16T04:39:16Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/07/2017 12:57 PM, Duy Nguyen wrote:\n> On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n>> Extract a new function from `do_for_each_ref()`. It will be useful\n>> elsewhere.\n>>\n>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n>> ---\n>>  refs.c               | 15 +++++++++++++--\n>>  refs/refs-internal.h | 11 +++++++++++\n>>  2 files changed, 24 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/refs.c b/refs.c\n>> index 0ed6c3c7a4..aeb704ab92 100644\n>> --- a/refs.c\n>> +++ b/refs.c\n>> @@ -1230,6 +1230,18 @@ int head_ref(each_ref_fn fn, void *cb_data)\n>>         return head_ref_submodule(NULL, fn, cb_data);\n>>  }\n>>\n>> +struct ref_iterator *refs_ref_iterator_begin(\n>> +               struct ref_store *refs,\n>> +               const char *prefix, int trim, int flags)\n>> +{\n>> +       struct ref_iterator *iter;\n>> +\n>> +       iter = refs->be->iterator_begin(refs, prefix, flags);\n>> +       iter = prefix_ref_iterator_begin(iter, prefix, trim);\n> \n> Off topic. This code made me wonder if we really need the prefix\n> iterator if prefix is NULL. And in fact we don't since\n> prefix_ref_iterator_begin() will return the old iter in that case. But\n> it's probably better to move that optimization outside. I think it's\n> easier to understand that way, calling prefix_ref_ will always give\n> you a new iterator. Don't call it unless you want to have it.\n\nI like this aspect of the design because it is more powerful. Consider\nfor example that some reference backend might (e.g., a future\n`packed_ref_store`) need to use a `prefix_ref_iterator` to implement\n`iterator_begin()`, while another (e.g., one based on `ref_cache`) might\nnot. It would be easy to make `prefix_ref_iterator_begin()` check\nwhether its argument is already a `prefix_ref_iterator`, and if so, swap\nit out with a new one with different arguments (to avoid having to stack\nthem up). It would be inappropriate for the caller to make such an\noptimization, because it (a) shouldn't have to care what type of\nreference iterator it is dealing with, and (b) shouldn't have to know\nenough about `prefix_ref_iterator`s to know that the optimization makes\nsense.\n\n>> +/*\n>> + * Return an iterator that goes over each reference in `refs` for\n>> + * which the refname begins with prefix. If trim is non-zero, then\n>> + * trim that many characters off the beginning of each refname. flags\n>> + * can be DO_FOR_EACH_INCLUDE_BROKEN to include broken references in\n>> + * the iteration.\n>> + */\n> \n> Do we need a separate docstring here? I think we document more or less\n> the same for ref_iterator_begin_fn (except the include-broken flag).\n\nThere is a subtle difference: currently, `ref_iterator_begin_fn()`\ndoesn't use its full `prefix` argument as prefix, but rather\n`find_containing_dir(prefix)`. (This is basically an implementation\ndetail of `ref_cache` leaking out into the virtual function interface.)\n\n`refs_ref_iterator_begin()`, on the other hand, treats the prefix\nliterally, using `starts_with()`.\n\nI don't like this design and plan to improve it sometime, but for now\nthat's an important difference. The fix, incidentally, would motivate\nthe `prefix_ref_iterator_begin()` optimization mentioned above.\n\nMichael\n\n"},{"id":"316911","messageId":"b9383ff6-10ce-cbb5-8f55-6d5f32361c10@alum.mit.edu","threadId":"45565","inReplyTo":"CACsJy8CZwPN06Q4OFmbjh8iCigZbrTUGU20hmxNCDRiAbB+KVA@mail.gmail.com","subject":"Re: [PATCH v2 04/20] refs_verify_refname_available(): implement once for all backends","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-04-16T04:44:42Z","receivedAt":"2017-04-16T04:44:55Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/07/2017 01:20 PM, Duy Nguyen wrote:\n> On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n>> It turns out that we can now implement\n>> `refs_verify_refname_available()` based on the other virtual\n>> functions, so there is no need for it to be defined at the backend\n>> level. Instead, define it once in `refs.c` and remove the\n>> `files_backend` definition.\n>>\n>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n>> ---\n>>  refs.c               | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++--\n>>  refs.h               |  2 +-\n>>  refs/files-backend.c | 39 +++++-------------------\n> \n> Much appreciated. This will make future backends simpler to implement as well.\n> \n>> +       iter = refs_ref_iterator_begin(refs, dirname.buf, 0,\n>> +                                      DO_FOR_EACH_INCLUDE_BROKEN);\n>> +       while ((ok = ref_iterator_advance(iter)) == ITER_OK) {\n>> +               if (skip &&\n>> +                   string_list_has_string(skip, iter->refname))\n>> +                       continue;\n>> +\n>> +               strbuf_addf(err, \"'%s' exists; cannot create '%s'\",\n>> +                           iter->refname, refname);\n>> +               ok = ref_iterator_abort(iter);\n> \n> Saving the return code in \"ok\" seems redundant because you don't use\n> it. Or did you want to check that ok == ITER_DONE or die() too?\n\nTrue, setting `ok` here is redundant. I don't think there's much point\nworrying about whether `ref_iterator_abort()` fails here, since we've\nalready gotten the information that we require.\n\nI'll remove it.\n\nThanks,\nMichael\n\n"},{"id":"316913","messageId":"8208a712-e6ea-a2e6-f206-9796f056f1af@alum.mit.edu","threadId":"45565","inReplyTo":"CACsJy8Careik7fCtXo_Luu40G8YPXAgZomFKDkCOuuOfEYm7nA@mail.gmail.com","subject":"Re: [PATCH v2 10/20] ref-cache: introduce a new type, ref_cache","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-04-16T04:52:47Z","receivedAt":"2017-04-16T04:52:57Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/07/2017 01:32 PM, Duy Nguyen wrote:\n> On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n>> +void free_ref_cache(struct ref_cache *cache)\n>> +{\n>> +       free_ref_entry(cache->root);\n>> +       free(cache);\n>> +}\n> \n> free(NULL) is no-op (and safe). Maybe we should follow the same\n> pattern for free_ref_cache(). It's really up to you.\n\nIf I look at other `free_*()` functions in our codebase, it doesn't seem\nthat many of them handle NULL, and that feature wouldn't help the\ncallers of this function. So I think I'll leave it the way that it is.\n\nMichael\n\n"},{"id":"316914","messageId":"c057d1f5-e8c6-3c08-2b18-e5cf73f74e57@alum.mit.edu","threadId":"45565","inReplyTo":"CACsJy8BM1nHhbKo9ioAP2OC2UU-RMWhmJ5OsYZwWYqek++DA0Q@mail.gmail.com","subject":"Re: [PATCH v2 11/20] refs: record the ref_store in ref_cache, not ref_dir","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-04-16T05:49:47Z","receivedAt":"2017-04-16T05:50:02Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/07/2017 01:38 PM, Duy Nguyen wrote:\n> On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n>> Instead of keeping a pointer to the ref_store in every ref_dir entry,\n>> store it once in `struct ref_cache`, and change `struct ref_dir` to\n>> include a pointer to its containing `ref_cache` instead. This makes it\n>> easier to add to the information that is accessible from a `ref_dir`\n>> without increasing the size of every `ref_dir` instance.\n> ...\n>> @@ -526,7 +526,7 @@ static struct ref_dir *get_loose_refs(struct files_ref_store *refs)\n>>                  * lazily):\n>>                  */\n>>                 add_entry_to_dir(get_ref_dir(refs->loose->root),\n>> -                                create_dir_entry(refs, \"refs/\", 5, 1));\n>> +                                create_dir_entry(refs->loose, \"refs/\", 5, 1));\n> \n> The commit message mentions nothing about this change. Is it intended?\n\nThe old `create_dir_entry()` took a `files_ref_store` as its first\nparameter, because that is what needed to be stored into the old\n`dir_entry` struct. The new version takes a `ref_cache`, because that is\nwhat the new `dir_entry` struct requires. This hunk is a logical\nconsequence of that change.\n\nI'll improve the commit message to explain this better.\n\nThanks,\nMichael\n\n"},{"id":"316915","messageId":"c29c0e5f-e0eb-2be3-3ebf-0a8f838fc5b1@alum.mit.edu","threadId":"45565","inReplyTo":"CACsJy8DAFUQnjkLnaXD0aM_97_E_7F_=fMeJb2Azigt2Pv0gTw@mail.gmail.com","subject":"Re: [PATCH v2 19/20] files_pack_refs(): use reference iteration","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-04-16T06:13:58Z","receivedAt":"2017-04-16T06:14:10Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/07/2017 01:51 PM, Duy Nguyen wrote:\n> On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n>> Use reference iteration rather than do_for_each_entry_in_dir() in the\n>> definition of files_pack_refs().\n> \n> A \"why\" is missing here. My guess is readability/maintainability\n> because it's easier to follow the code with iterator design, than the\n> callback one? No maintaining data in struct pack_refs_cb_data is also\n> very nice.\n\nYou're right. I'll improve the commit message to something like\n\nfiles_pack_refs(): use reference iteration\n\nUse reference iteration rather than `do_for_each_entry_in_dir()` in the\ndefinition of `files_pack_refs()`. This makes the code shorter and\neasier to follow, because the logic can be inline rather than spread\nbetween the main function and a callback function, and it removes the\nneed to use `pack_refs_cb_data` to preserve intermediate state.\n\nThis removes the last callers of `entry_resolves_to_object()` and\n`get_loose_ref_dir()`, so delete those functions.\n\nThanks,\nMichael\n\n"},{"id":"316916","messageId":"23a5cf22-5b5d-bb2a-e0c5-50fce6abddfd@alum.mit.edu","threadId":"45565","inReplyTo":"CACsJy8ArrVRjmBoXtxKVtftNaSoSX_nXkuExsXwaBFkayv9kQg@mail.gmail.com","subject":"Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-04-16T06:15:38Z","receivedAt":"2017-04-16T06:15:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/07/2017 01:53 PM, Duy Nguyen wrote:\n> On Wed, Apr 5, 2017 at 9:03 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n>> On Sat, Apr 1, 2017 at 12:16 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n>>> Duy, have you looked over my patch series? Since you've been working in\n>>> the area, your feedback would be very welcome, if you have the time for it.\n>>\n>> You probably have guessed my answer based on my lack of response :)\n>> Tomorrow is holiday though, will have a look. But I doubt I would find\n>> anything given that both you and Jeff are involved in this.\n> \n> Overall, very nice. I made a comment here and there, but they're more\n> questions for my education than actual code review ) At least there's\n> another set of eyes on this series now.\n\nThanks for your review! I'll submit v3 shortly to address your feedback.\n\nMichael\n\n"}]}