{"thread":{"id":"42482","subject":"[PATCH 02/13] do_for_each_ref(): move docstring to the header file","startedAt":"2016-05-30T07:55:21Z","lastAt":"2016-06-03T11:57:49Z","messageCount":27,"participants":["Michael Haggerty","Ramsay Jones","Eric Sunshine","Junio C Hamano","David Turner","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"287781","messageId":"cover.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":null,"subject":"[PATCH 00/13] Reference iterators","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:21Z","receivedAt":"2016-05-30T07:55:21Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This patch series implements a new iteration paradigm for iterating\nover references using iterators. This approach was proposed earlier as\nan RFC [1].\n\nThe justification for this change is laid out in the RFC [1] and in\nthe commit message for patch 09/13 [2]. Please refer to those, as I\nwon't repeat them here.\n\nThere are several obvious followup steps that are not included in this\npatch series (because they don't help with the initial transition to\npluggable reference backends). I've written prototypes of several of\nthese:\n\n* The iterator interface could be made public and callers could start\n using it directly.\n\n* A filter_ref_iterator could let references be filtered based on a\n callback function (this would be useful, for example, for\n for_each_glob_ref()).\n\n* A single_ref_iterator could \"iterate\" over a single reference (e.g.,\n HEAD).\n\n* A series_ref_iterator could iterate over multiple iterators, for\n callers that want to operate on, say, HEAD plus all branches.\n\n* The ref_cache could be fed from an iterator, to better decouple\n caching from reading packed and loose references, and to make it\n easy to use caching with other reference storage backends.\n\n* Per-worktree refs could be overlaid on top of shared references\n using merge_ref_iterator rather than mixing them up in the same\n ref_cache.\n\n* The dir_iterator could be used in more places; for example, when\n reading loose references from disk.\n\nTable of contents of changes:\n\n* The first eight patches are cleanups.\n\n * Patch 05/13 fixes a code path that unlinks symrefs directly\n instead of using the refs API.\n\n* Patch 09/13 is the most important part of this series. It introduces\n not only reference iterators, but also (1) a pattern that other\n iterator interfaces can follow (along with some useful constants in\n iterator.h), and (2) a pattern for building OO code with\n inheritance.\n\n* Patch 10/13 actually uses the new reference iteration mechanism.\n\n* Patch 11/13 avoids aborting reflog iterations if an unexpected file\n is found under `$GIT_DIR/logs`. This fixes a bug that could cause\n objects needed by reflogs to be pruned, breaking the reflogs.\n\n* Patch 12/13 adds an iterator interface for iterating over\n directories (using the same model as patch 09/13).\n\n* Patch 13/13 implements for_each_reflog() on top of an iterator\n interface (essentially the analogue of patch 10/13, but for\n reflogs).\n\nNote that it is not necessary to rebuild for_each_reflog_ent() on top\nof iterators at this time, because that function deals with only a\nsingle reference at a time. Therefore, composability is not important\nhere (for example, it won't have to deal with multiple refs backends\nat the same time).\n\nThis patch series applies on top of mh/split-under-lock. It is also\navailable from my GitHub repository [3] as branch \"ref-iterators\".\n\nI haven't yet completely rebased the ref-store changes (virtualization\nof the refs API) on top of all of these changes, but I will work on\nthat next.\n\nMichael\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/290409\n[2] http://mid.gmane.org/89634d216544d1102dafd5d18247bff2581d48a8.1464537050.git.mhagger@alum.mit.edu\n[3] https://github.com/mhagger/git\n\nMichael Haggerty (13):\n refs: remove unnecessary \"extern\" keywords\n do_for_each_ref(): move docstring to the header file\n refs: use name \"prefix\" consistently\n delete_refs(): add a flags argument\n remote rm: handle symbolic refs correctly\n get_ref_cache(): only create an instance if there is a submodule\n entry_resolves_to_object(): rename function from\n ref_resolves_to_object()\n ref_resolves_to_object(): new function\n refs: introduce an iterator interface\n do_for_each_ref(): reimplement using reference iteration\n for_each_reflog(): don't abort for bad references\n dir_iterator: new API for iterating over a directory tree\n for_each_reflog(): reimplement using iterators\n\n Makefile | 2 +\n builtin/fetch.c | 2 +-\n builtin/remote.c | 8 +-\n dir-iterator.c | 180 +++++++++++++++\n dir-iterator.h | 86 +++++++\n iterator.h | 81 +++++++\n refs.c | 20 ++\n refs.h | 139 +++++++-----\n refs/files-backend.c | 630 +++++++++++++++++++++++++++++++--------------------\n refs/iterator.c | 376 ++++++++++++++++++++++++++++++\n refs/refs-internal.h | 225 +++++++++++++++++-\n 11 files changed, 1427 insertions(+), 322 deletions(-)\n create mode 100644 dir-iterator.c\n create mode 100644 dir-iterator.h\n create mode 100644 iterator.h\n create mode 100644 refs/iterator.c\n\n-- \n2.8.1\n"},{"id":"287778","messageId":"fa9460fbe5c8ecc9636752919adea36acd5457ed.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 01/13] refs: remove unnecessary \"extern\" keywords","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:22Z","receivedAt":"2016-05-30T07:55:22Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"There's continuing work in this area, so clean up unneeded \"extern\"\nkeywords rather than schlepping them around. Also split up some overlong\nlines and add parameter names in a couple of places.\n\nSigned-off-by: Michael Haggerty \n---\n refs.h | 132 +++++++++++++++++++++++++++++++++++------------------------------\n 1 file changed, 72 insertions(+), 60 deletions(-)\n\ndiff --git a/refs.h b/refs.h\nindex 9230d47..21874f0 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -52,19 +52,19 @@\n #define RESOLVE_REF_NO_RECURSE 0x02\n #define RESOLVE_REF_ALLOW_BAD_NAME 0x04\n \n-extern const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n-\t\t\t\t unsigned char *sha1, int *flags);\n+const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n+\t\t\t unsigned char *sha1, int *flags);\n \n-extern char *resolve_refdup(const char *refname, int resolve_flags,\n-\t\t\t unsigned char *sha1, int *flags);\n+char *resolve_refdup(const char *refname, int resolve_flags,\n+\t\t unsigned char *sha1, int *flags);\n \n-extern int read_ref_full(const char *refname, int resolve_flags,\n-\t\t\t unsigned char *sha1, int *flags);\n-extern int read_ref(const char *refname, unsigned char *sha1);\n+int read_ref_full(const char *refname, int resolve_flags,\n+\t\t unsigned char *sha1, int *flags);\n+int read_ref(const char *refname, unsigned char *sha1);\n \n-extern int ref_exists(const char *refname);\n+int ref_exists(const char *refname);\n \n-extern int is_branch(const char *refname);\n+int is_branch(const char *refname);\n \n /*\n * If refname is a non-symbolic reference that refers to a tag object,\n@@ -74,24 +74,25 @@ extern int is_branch(const char *refname);\n * Symbolic references are considered unpeelable, even if they\n * ultimately resolve to a peelable tag.\n */\n-extern int peel_ref(const char *refname, unsigned char *sha1);\n+int peel_ref(const char *refname, unsigned char *sha1);\n \n /**\n * Resolve refname in the nested \"gitlink\" repository that is located\n * at path. If the resolution is successful, return 0 and set sha1 to\n * the name of the object; otherwise, return a non-zero value.\n */\n-extern int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sha1);\n+int resolve_gitlink_ref(const char *path, const char *refname,\n+\t\t\tunsigned char *sha1);\n \n /*\n * Return true iff abbrev_name is a possible abbreviation for\n * full_name according to the rules defined by ref_rev_parse_rules in\n * refs.c.\n */\n-extern int refname_match(const char *abbrev_name, const char *full_name);\n+int refname_match(const char *abbrev_name, const char *full_name);\n \n-extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);\n-extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);\n+int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);\n+int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);\n \n /*\n * A ref_transaction represents a collection of ref updates\n@@ -182,38 +183,45 @@ typedef int each_ref_fn(const char *refname,\n * modifies the reference also returns a nonzero value to immediately\n * stop the iteration.\n */\n-extern int head_ref(each_ref_fn fn, void *cb_data);\n-extern int for_each_ref(each_ref_fn fn, void *cb_data);\n-extern int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data);\n-extern int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken);\n-extern int for_each_tag_ref(each_ref_fn fn, void *cb_data);\n-extern int for_each_branch_ref(each_ref_fn fn, void *cb_data);\n-extern int for_each_remote_ref(each_ref_fn fn, void *cb_data);\n-extern int for_each_replace_ref(each_ref_fn fn, void *cb_data);\n-extern int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);\n-extern int for_each_glob_ref_in(each_ref_fn fn, const char *pattern, const char *prefix, void *cb_data);\n+int head_ref(each_ref_fn fn, void *cb_data);\n+int for_each_ref(each_ref_fn fn, void *cb_data);\n+int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data);\n+int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,\n+\t\t\tunsigned int broken);\n+int for_each_tag_ref(each_ref_fn fn, void *cb_data);\n+int for_each_branch_ref(each_ref_fn fn, void *cb_data);\n+int for_each_remote_ref(each_ref_fn fn, void *cb_data);\n+int for_each_replace_ref(each_ref_fn fn, void *cb_data);\n+int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);\n+int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n+\t\t\t const char *prefix, void *cb_data);\n \n-extern int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);\n-extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);\n-extern int for_each_ref_in_submodule(const char *submodule, const char *prefix,\n+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);\n+int for_each_ref_submodule(const char *submodule,\n+\t\t\t each_ref_fn fn, void *cb_data);\n+int for_each_ref_in_submodule(const char *submodule, const char *prefix,\n \t\teach_ref_fn fn, void *cb_data);\n-extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);\n-extern int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);\n-extern int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);\n+int for_each_tag_ref_submodule(const char *submodule,\n+\t\t\t each_ref_fn fn, void *cb_data);\n+int for_each_branch_ref_submodule(const char *submodule,\n+\t\t\t\t each_ref_fn fn, void *cb_data);\n+int for_each_remote_ref_submodule(const char *submodule,\n+\t\t\t\t each_ref_fn fn, void *cb_data);\n \n-extern int head_ref_namespaced(each_ref_fn fn, void *cb_data);\n-extern int for_each_namespaced_ref(each_ref_fn fn, void *cb_data);\n+int head_ref_namespaced(each_ref_fn fn, void *cb_data);\n+int for_each_namespaced_ref(each_ref_fn fn, void *cb_data);\n \n /* can be used to learn about broken ref and symref */\n-extern int for_each_rawref(each_ref_fn fn, void *cb_data);\n+int for_each_rawref(each_ref_fn fn, void *cb_data);\n \n static inline const char *has_glob_specials(const char *pattern)\n {\n \treturn strpbrk(pattern, \"?*[\");\n }\n \n-extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);\n-extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct string_list *refnames);\n+void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);\n+void warn_dangling_symrefs(FILE *fp, const char *msg_fmt,\n+\t\t\t const struct string_list *refnames);\n \n /*\n * Flags for controlling behaviour of pack_refs()\n@@ -245,13 +253,13 @@ int pack_refs(unsigned int flags);\n int safe_create_reflog(const char *refname, int force_create, struct strbuf *err);\n \n /** Reads log for the value of ref during at_time. **/\n-extern int read_ref_at(const char *refname, unsigned int flags,\n-\t\t unsigned long at_time, int cnt,\n-\t\t unsigned char *sha1, char **msg,\n-\t\t unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt);\n+int read_ref_at(const char *refname, unsigned int flags,\n+\t\tunsigned long at_time, int cnt,\n+\t\tunsigned char *sha1, char **msg,\n+\t\tunsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt);\n \n /** Check if a particular reflog exists */\n-extern int reflog_exists(const char *refname);\n+int reflog_exists(const char *refname);\n \n /*\n * Delete the specified reference. If old_sha1 is non-NULL, then\n@@ -260,21 +268,25 @@ extern int reflog_exists(const char *refname);\n * exists, regardless of its old value. It is an error for old_sha1 to\n * be NULL_SHA1. flags is passed through to ref_transaction_delete().\n */\n-extern int delete_ref(const char *refname, const unsigned char *old_sha1,\n-\t\t unsigned int flags);\n+int delete_ref(const char *refname, const unsigned char *old_sha1,\n+\t unsigned int flags);\n \n /*\n * Delete the specified references. If there are any problems, emit\n * errors but attempt to keep going (i.e., the deletes are not done in\n * an all-or-nothing transaction).\n */\n-extern int delete_refs(struct string_list *refnames);\n+int delete_refs(struct string_list *refnames);\n \n /** Delete a reflog */\n-extern int delete_reflog(const char *refname);\n+int delete_reflog(const char *refname);\n \n /* iterate over reflog entries */\n-typedef int each_reflog_ent_fn(unsigned char *osha1, unsigned char *nsha1, const char *, unsigned long, int, const char *, void *);\n+typedef int each_reflog_ent_fn(\n+\t\tunsigned char *old_sha1, unsigned char *new_sha1,\n+\t\tconst char *committer, unsigned long timestamp,\n+\t\tint tz, const char *msg, void *cb_data);\n+\n int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_data);\n int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void *cb_data);\n \n@@ -282,7 +294,7 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void\n * Calls the specified function for each reflog file until it returns nonzero,\n * and returns the value\n */\n-extern int for_each_reflog(each_ref_fn, void *);\n+int for_each_reflog(each_ref_fn fn, void *cb_data);\n \n #define REFNAME_ALLOW_ONELEVEL 1\n #define REFNAME_REFSPEC_PATTERN 2\n@@ -295,16 +307,16 @@ extern int for_each_reflog(each_ref_fn, void *);\n * allow a single \"*\" wildcard character in the refspec. No leading or\n * repeated slashes are accepted.\n */\n-extern int check_refname_format(const char *refname, int flags);\n+int check_refname_format(const char *refname, int flags);\n \n-extern const char *prettify_refname(const char *refname);\n+const char *prettify_refname(const char *refname);\n \n-extern char *shorten_unambiguous_ref(const char *refname, int strict);\n+char *shorten_unambiguous_ref(const char *refname, int strict);\n \n /** rename ref, return 0 on success **/\n-extern int rename_ref(const char *oldref, const char *newref, const char *logmsg);\n+int rename_ref(const char *oldref, const char *newref, const char *logmsg);\n \n-extern int create_symref(const char *refname, const char *target, const char *logmsg);\n+int create_symref(const char *refname, const char *target, const char *logmsg);\n \n /*\n * Update HEAD of the specified gitdir.\n@@ -313,7 +325,7 @@ extern int create_symref(const char *refname, const char *target, const char *lo\n * $GIT_DIR points to.\n * Return 0 if successful, non-zero otherwise.\n * */\n-extern int set_worktree_head_symref(const char *gitdir, const char *target);\n+int set_worktree_head_symref(const char *gitdir, const char *target);\n \n enum action_on_err {\n \tUPDATE_REFS_MSG_ON_ERR,\n@@ -463,7 +475,7 @@ int update_ref(const char *msg, const char *refname,\n \t const unsigned char *new_sha1, const unsigned char *old_sha1,\n \t unsigned int flags, enum action_on_err onerr);\n \n-extern int parse_hide_refs_config(const char *var, const char *value, const char *);\n+int parse_hide_refs_config(const char *var, const char *value, const char *);\n \n /*\n * Check whether a ref is hidden. If no namespace is set, both the first and\n@@ -473,7 +485,7 @@ extern int parse_hide_refs_config(const char *var, const char *value, const char\n * the ref is outside that namespace, the first parameter is NULL. The second\n * parameter always points to the full ref name.\n */\n-extern int ref_is_hidden(const char *, const char *);\n+int ref_is_hidden(const char *, const char *);\n \n enum ref_type {\n \tREF_TYPE_PER_WORKTREE,\n@@ -522,11 +534,11 @@ typedef void reflog_expiry_cleanup_fn(void *cb_data);\n * enum expire_reflog_flags. The three function pointers are described\n * above. On success, return zero.\n */\n-extern int reflog_expire(const char *refname, const unsigned char *sha1,\n-\t\t\t unsigned int flags,\n-\t\t\t reflog_expiry_prepare_fn prepare_fn,\n-\t\t\t reflog_expiry_should_prune_fn should_prune_fn,\n-\t\t\t reflog_expiry_cleanup_fn cleanup_fn,\n-\t\t\t void *policy_cb_data);\n+int reflog_expire(const char *refname, const unsigned char *sha1,\n+\t\t unsigned int flags,\n+\t\t reflog_expiry_prepare_fn prepare_fn,\n+\t\t reflog_expiry_should_prune_fn should_prune_fn,\n+\t\t reflog_expiry_cleanup_fn cleanup_fn,\n+\t\t void *policy_cb_data);\n \n #endif /* REFS_H */\n-- \n2.8.1\n"},{"id":"287777","messageId":"39f17243f5f85aab5eaa1f8355929027e20fcb74.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 02/13] do_for_each_ref(): move docstring to the header file","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:23Z","receivedAt":"2016-05-30T07:55:23Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Signed-off-by: Michael Haggerty \n---\n refs/files-backend.c | 9 ---------\n refs/refs-internal.h | 10 +++++++++-\n 2 files changed, 9 insertions(+), 10 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 1230dfb..68db3e8 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1878,15 +1878,6 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,\n \treturn retval;\n }\n \n-/*\n- * Call fn for each reference in the specified ref_cache for which the\n- * refname begins with base. If trim is non-zero, then trim that many\n- * characters off the beginning of each refname before passing the\n- * refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include\n- * broken references in the iteration. If fn ever returns a non-zero\n- * value, stop the iteration and return that value; otherwise, return\n- * 0.\n- */\n int do_for_each_ref(const char *submodule, const char *base,\n \t\t each_ref_fn fn, int trim, int flags, void *cb_data)\n {\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 1bb3d87..b4dd545 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -249,7 +249,15 @@ int rename_ref_available(const char *oldname, const char *newname);\n #define DO_FOR_EACH_INCLUDE_BROKEN 0x01\n \n /*\n- * The common backend for the for_each_*ref* functions\n+ * Call fn for each reference in the specified submodule for which the\n+ * refname begins with base. If trim is non-zero, then trim that many\n+ * characters off the beginning of each refname before passing the\n+ * refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include\n+ * broken references in the iteration. If fn ever returns a non-zero\n+ * value, stop the iteration and return that value; otherwise, return\n+ * 0.\n+ *\n+ * This is the common backend for the for_each_*ref* functions.\n */\n int do_for_each_ref(const char *submodule, const char *base,\n \t\t each_ref_fn fn, int trim, int flags, void *cb_data);\n-- \n2.8.1\n"},{"id":"287780","messageId":"dbe2c569a5cd2b91fd6d51f99dcbfe9385f98ad4.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 03/13] refs: use name \"prefix\" consistently","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:24Z","receivedAt":"2016-05-30T07:55:24Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"In the context of the for_each_ref() functions, call the prefix that\nreferences must start with \"prefix\". (In some places it was called\n\"base\".) This is clearer, and also prevents confusion with another\nplanned use of the word \"base\".\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 24 ++++++++++++------------\n refs/refs-internal.h | 14 +++++++-------\n 2 files changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 68db3e8..5af7441 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -542,7 +542,7 @@ static struct ref_entry *current_ref;\n typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);\n \n struct ref_entry_cb {\n-\tconst char *base;\n+\tconst char *prefix;\n \tint trim;\n \tint flags;\n \teach_ref_fn *fn;\n@@ -559,7 +559,7 @@ static int do_one_ref(struct ref_entry *entry, void *cb_data)\n \tstruct ref_entry *old_current_ref;\n \tint retval;\n \n-\tif (!starts_with(entry->name, data->base))\n+\tif (!starts_with(entry->name, data->prefix))\n \t\treturn 0;\n \n \tif (!(data->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&\n@@ -1824,12 +1824,12 @@ int peel_ref(const char *refname, unsigned char *sha1)\n \n /*\n * Call fn for each reference in the specified ref_cache, omitting\n- * references not in the containing_dir of base. fn is called for all\n- * references, including broken ones. If fn ever returns a non-zero\n+ * references not in the containing_dir of prefix. Call fn for all\n+ * references, including broken ones. If fn ever returns a non-zero\n * value, stop the iteration and return that value; otherwise, return\n * 0.\n */\n-static int do_for_each_entry(struct ref_cache *refs, const char *base,\n+static int do_for_each_entry(struct ref_cache *refs, const char *prefix,\n \t\t\t each_ref_entry_fn fn, void *cb_data)\n {\n \tstruct packed_ref_cache *packed_ref_cache;\n@@ -1846,8 +1846,8 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,\n \t * disk.\n \t */\n \tloose_dir = get_loose_refs(refs);\n-\tif (base && *base) {\n-\t\tloose_dir = find_containing_dir(loose_dir, base, 0);\n+\tif (prefix && *prefix) {\n+\t\tloose_dir = find_containing_dir(loose_dir, prefix, 0);\n \t}\n \tif (loose_dir)\n \t\tprime_ref_dir(loose_dir);\n@@ -1855,8 +1855,8 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,\n \tpacked_ref_cache = get_packed_ref_cache(refs);\n \tacquire_packed_ref_cache(packed_ref_cache);\n \tpacked_dir = get_packed_ref_dir(packed_ref_cache);\n-\tif (base && *base) {\n-\t\tpacked_dir = find_containing_dir(packed_dir, base, 0);\n+\tif (prefix && *prefix) {\n+\t\tpacked_dir = find_containing_dir(packed_dir, prefix, 0);\n \t}\n \n \tif (packed_dir && loose_dir) {\n@@ -1878,14 +1878,14 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,\n \treturn retval;\n }\n \n-int do_for_each_ref(const char *submodule, const char *base,\n+int do_for_each_ref(const char *submodule, const char *prefix,\n \t\t each_ref_fn fn, int trim, int flags, void *cb_data)\n {\n \tstruct ref_entry_cb data;\n \tstruct ref_cache *refs;\n \n \trefs = get_ref_cache(submodule);\n-\tdata.base = base;\n+\tdata.prefix = prefix;\n \tdata.trim = trim;\n \tdata.flags = flags;\n \tdata.fn = fn;\n@@ -1896,7 +1896,7 @@ int do_for_each_ref(const char *submodule, const char *base,\n \tif (ref_paranoia)\n \t\tdata.flags |= DO_FOR_EACH_INCLUDE_BROKEN;\n \n-\treturn do_for_each_entry(refs, base, do_one_ref, &data);\n+\treturn do_for_each_entry(refs, prefix, do_one_ref, &data);\n }\n \n /*\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex b4dd545..8ad02d8 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -250,16 +250,16 @@ int rename_ref_available(const char *oldname, const char *newname);\n \n /*\n * Call fn for each reference in the specified submodule for which the\n- * refname begins with base. If trim is non-zero, then trim that many\n- * characters off the beginning of each refname before passing the\n- * refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include\n- * broken references in the iteration. If fn ever returns a non-zero\n- * value, stop the iteration and return that value; otherwise, return\n- * 0.\n+ * refname begins with prefix. If trim is non-zero, then trim that\n+ * many characters off the beginning of each refname before passing\n+ * the refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to\n+ * include broken references in the iteration. If fn ever returns a\n+ * non-zero value, stop the iteration and return that value;\n+ * otherwise, return 0.\n *\n * This is the common backend for the for_each_*ref* functions.\n */\n-int do_for_each_ref(const char *submodule, const char *base,\n+int do_for_each_ref(const char *submodule, const char *prefix,\n \t\t each_ref_fn fn, int trim, int flags, void *cb_data);\n \n /*\n-- \n2.8.1\n"},{"id":"287779","messageId":"03e08d91d289ef24ea53344ce005eda0bb4a7a14.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 04/13] delete_refs(): add a flags argument","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:25Z","receivedAt":"2016-05-30T07:55:25Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This will be useful for passing REF_NODEREF through.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/fetch.c | 2 +-\n builtin/remote.c | 4 ++--\n refs.h | 5 +++--\n refs/files-backend.c | 4 ++--\n 4 files changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex f8455bd..b55c83c 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -806,7 +806,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,\n \t\tfor (ref = stale_refs; ref; ref = ref->next)\n \t\t\tstring_list_append(&refnames, ref->name);\n \n-\t\tresult = delete_refs(&refnames);\n+\t\tresult = delete_refs(&refnames, 0);\n \t\tstring_list_clear(&refnames, 0);\n \t}\n \ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex fda5c2e..1bbf9b4 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -788,7 +788,7 @@ static int rm(int argc, const char **argv)\n \tstrbuf_release(&buf);\n \n \tif (!result)\n-\t\tresult = delete_refs(&branches);\n+\t\tresult = delete_refs(&branches, 0);\n \tstring_list_clear(&branches, 0);\n \n \tif (skipped.nr) {\n@@ -1303,7 +1303,7 @@ static int prune_remote(const char *remote, int dry_run)\n \tstring_list_sort(&refs_to_prune);\n \n \tif (!dry_run)\n-\t\tresult |= delete_refs(&refs_to_prune);\n+\t\tresult |= delete_refs(&refs_to_prune, 0);\n \n \tfor_each_string_list_item(item, &states.stale) {\n \t\tconst char *refname = item->util;\ndiff --git a/refs.h b/refs.h\nindex 21874f0..6d515a4 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -274,9 +274,10 @@ int delete_ref(const char *refname, const unsigned char *old_sha1,\n /*\n * Delete the specified references. If there are any problems, emit\n * errors but attempt to keep going (i.e., the deletes are not done in\n- * an all-or-nothing transaction).\n+ * an all-or-nothing transaction). flags is passed through to\n+ * ref_transaction_delete().\n */\n-int delete_refs(struct string_list *refnames);\n+int delete_refs(struct string_list *refnames, unsigned int flags);\n \n /** Delete a reflog */\n int delete_reflog(const char *refname);\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 5af7441..a0d09f4 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2403,7 +2403,7 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n \treturn 0;\n }\n \n-int delete_refs(struct string_list *refnames)\n+int delete_refs(struct string_list *refnames, unsigned int flags)\n {\n \tstruct strbuf err = STRBUF_INIT;\n \tint i, result = 0;\n@@ -2432,7 +2432,7 @@ int delete_refs(struct string_list *refnames)\n \tfor (i = 0; i < refnames->nr; i++) {\n \t\tconst char *refname = refnames->items[i].string;\n \n-\t\tif (delete_ref(refname, NULL, 0))\n+\t\tif (delete_ref(refname, NULL, flags))\n \t\t\tresult |= error(_(\"could not remove reference %s\"), refname);\n \t}\n \n-- \n2.8.1\n"},{"id":"287790","messageId":"157904e27af9c3377b21376c301f3e16ba6a5aa2.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 05/13] remote rm: handle symbolic refs correctly","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:26Z","receivedAt":"2016-05-30T07:55:26Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"In the modern world of reference backends, it is not OK to delete a\nsymref by unlink()ing the file directly. This must be done via the refs\nAPI.\n\nWe do so by adding the symref to the list of references to delete along\nwith the non-symbolic references, then calling delete_refs() with the\nnew flags option set to REF_NODEREF.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/remote.c | 6 +-----\n 1 file changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 1bbf9b4..c4b4d67 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -539,10 +539,6 @@ static int add_branch_for_removal(const char *refname,\n \t\treturn 0;\n \t}\n \n-\t/* make sure that symrefs are deleted */\n-\tif (flags & REF_ISSYMREF)\n-\t\treturn unlink(git_path(\"%s\", refname));\n-\n \tstring_list_append(branches->branches, refname);\n \n \treturn 0;\n@@ -788,7 +784,7 @@ static int rm(int argc, const char **argv)\n \tstrbuf_release(&buf);\n \n \tif (!result)\n-\t\tresult = delete_refs(&branches, 0);\n+\t\tresult = delete_refs(&branches, REF_NODEREF);\n \tstring_list_clear(&branches, 0);\n \n \tif (skipped.nr) {\n-- \n2.8.1\n"},{"id":"287784","messageId":"3d6c93bd7907f3e6c34787a47b1d75c5cca52a1b.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 06/13] get_ref_cache(): only create an instance if there is a submodule","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:27Z","receivedAt":"2016-05-30T07:55:27Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"If there is not a nonbare repository where a submodule is supposedly\nlocated, then don't instantiate a ref_cache for it.\n\nThe analogous check can be removed from resolve_gitlink_ref().\n\nSigned-off-by: Michael Haggerty \n---\nThis doesn't actually reduce the number of ref_cache instances\ngenerated by out test suite, but it is a more logical place for\nthe check that was added in\n\n a2d5156c resolve_gitlink_ref: ignore non-repository paths\n\n refs/files-backend.c | 33 ++++++++++++++++++++++-----------\n 1 file changed, 22 insertions(+), 11 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex a0d09f4..142c977 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -954,15 +954,26 @@ static struct ref_cache *lookup_ref_cache(const char *submodule)\n \n /*\n * Return a pointer to a ref_cache for the specified submodule. For\n- * the main repository, use submodule==NULL. The returned structure\n- * will be allocated and initialized but not necessarily populated; it\n- * should not be freed.\n+ * the main repository, use submodule==NULL; such a call cannot fail.\n+ * For a submodule, the submodule must exist and be a nonbare\n+ * repository, otherwise return NULL.\n+ *\n+ * The returned structure will be allocated and initialized but not\n+ * necessarily populated; it should not be freed.\n */\n static struct ref_cache *get_ref_cache(const char *submodule)\n {\n \tstruct ref_cache *refs = lookup_ref_cache(submodule);\n-\tif (!refs)\n-\t\trefs = create_ref_cache(submodule);\n+\n+\tif (!refs) {\n+\t\tstruct strbuf submodule_sb = STRBUF_INIT;\n+\n+\t\tstrbuf_addstr(&submodule_sb, submodule);\n+\t\tif (is_nonbare_repository_dir(&submodule_sb))\n+\t\t\trefs = create_ref_cache(submodule);\n+\t\tstrbuf_release(&submodule_sb);\n+\t}\n+\n \treturn refs;\n }\n \n@@ -1341,13 +1352,10 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sh\n \t\treturn -1;\n \n \tstrbuf_add(&submodule, path, len);\n-\trefs = lookup_ref_cache(submodule.buf);\n+\trefs = get_ref_cache(submodule.buf);\n \tif (!refs) {\n-\t\tif (!is_nonbare_repository_dir(&submodule)) {\n-\t\t\tstrbuf_release(&submodule);\n-\t\t\treturn -1;\n-\t\t}\n-\t\trefs = create_ref_cache(submodule.buf);\n+\t\tstrbuf_release(&submodule);\n+\t\treturn -1;\n \t}\n \tstrbuf_release(&submodule);\n \n@@ -1885,6 +1893,9 @@ int do_for_each_ref(const char *submodule, const char *prefix,\n \tstruct ref_cache *refs;\n \n \trefs = get_ref_cache(submodule);\n+\tif (!refs)\n+\t\treturn 0;\n+\n \tdata.prefix = prefix;\n \tdata.trim = trim;\n \tdata.flags = flags;\n-- \n2.8.1\n"},{"id":"287785","messageId":"1d91b1d9f970184ac2d117c9d92379725f468d8d.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 07/13] entry_resolves_to_object(): rename function from ref_resolves_to_object()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:28Z","receivedAt":"2016-05-30T07:55:28Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Free up the old name for a more general purpose.\n\nSigned-off-by: Michael Haggerty \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 142c977..1a46f32 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -517,7 +517,7 @@ static void sort_ref_dir(struct ref_dir *dir)\n * an object in the database. Emit a warning if the referred-to\n * object does not exist.\n */\n-static int ref_resolves_to_object(struct ref_entry *entry)\n+static int entry_resolves_to_object(struct ref_entry *entry)\n {\n \tif (entry->flag & REF_ISBROKEN)\n \t\treturn 0;\n@@ -563,7 +563,7 @@ static int do_one_ref(struct ref_entry *entry, void *cb_data)\n \t\treturn 0;\n \n \tif (!(data->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&\n-\t !ref_resolves_to_object(entry))\n+\t !entry_resolves_to_object(entry))\n \t\treturn 0;\n \n \t/* Store the old value, in case this is a recursive call: */\n@@ -2228,7 +2228,7 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)\n \t\treturn 0;\n \n \t/* Do not pack symbolic or broken refs: */\n-\tif ((entry->flag & REF_ISSYMREF) || !ref_resolves_to_object(entry))\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-- \n2.8.1\n"},{"id":"287783","messageId":"d7a116ad608aa733c712870c1d6759165811756f.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 08/13] ref_resolves_to_object(): new function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:29Z","receivedAt":"2016-05-30T07:55:29Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Extract new function ref_resolves_to_object() from\nentry_resolves_to_object(). It can be used even if there is no ref_entry\nat hand.\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 33 +++++++++++++++++++++++----------\n 1 file changed, 23 insertions(+), 10 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 1a46f32..8ab4d5f 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -513,19 +513,32 @@ static void sort_ref_dir(struct ref_dir *dir)\n }\n \n /*\n- * Return true iff the reference described by entry can be resolved to\n- * an object in the database. Emit a warning if the referred-to\n- * object does not exist.\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+ * does not exist, emit a warning and return false.\n+ */\n+static int ref_resolves_to_object(const char *refname,\n+\t\t\t\t const struct object_id *oid,\n+\t\t\t\t unsigned int flags)\n+{\n+\tif (flags & REF_ISBROKEN)\n+\t\treturn 0;\n+\tif (!has_sha1_file(oid->hash)) {\n+\t\terror(\"%s does not point to a valid object!\", refname);\n+\t\treturn 0;\n+\t}\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-\tif (entry->flag & REF_ISBROKEN)\n-\t\treturn 0;\n-\tif (!has_sha1_file(entry->u.value.oid.hash)) {\n-\t\terror(\"%s does not point to a valid object!\", entry->name);\n-\t\treturn 0;\n-\t}\n-\treturn 1;\n+\treturn ref_resolves_to_object(entry->name,\n+\t\t\t\t &entry->u.value.oid, entry->flag);\n }\n \n /*\n-- \n2.8.1\n"},{"id":"287788","messageId":"89634d216544d1102dafd5d18247bff2581d48a8.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 09/13] refs: introduce an iterator interface","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:30Z","receivedAt":"2016-05-30T07:55:30Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Currently, the API for iterating over references is via a family of\nfor_each_ref()-type functions that invoke a callback function for each\nselected reference. All of these eventually call do_for_each_ref(),\nwhich knows how to do one thing: iterate in parallel through two\nref_caches, one for loose and one for packed refs, giving loose\nreferences precedence over packed refs. This is rather complicated code,\nand is quite specialized to the files backend. It also requires callers\nto encapsulate their work into a callback function, which often means\nthat they have to define and use a \"cb_data\" struct to manage their\ncontext.\n\nThe current design is already bursting at the seams, and will become\neven more awkward in the upcoming world of multiple reference storage\nbackends:\n\n* Per-worktree vs. shared references are currently handled via a kludge\n in git_path() rather than iterating over each part of the reference\n namespace separately and merging the results. This kludge will cease\n to work when we have multiple reference storage backends.\n\n* The current scheme is inflexible. What if we sometimes want to bypass\n the ref_cache, or use it only for packed or only for loose refs? What\n if we want to store symbolic refs in one type of storage backend and\n non-symbolic ones in another?\n\nIn the future, each reference backend will need to define its own way of\niterating over references. The crux of the problem with the current\ndesign is that it is impossible to compose for_each_ref()-style\niterations, because the flow of control is owned by the for_each_ref()\nfunction. There is nothing that a caller can do but iterate through all\nreferences in a single burst, so there is no way for it to interleave\nreferences from multiple backends and present the result to the rest of\nthe world as a single compound backend.\n\nThis commit introduces a new iteration primitive for references: a\nref_iterator. A ref_iterator is a polymorphic object that a reference\nstorage backend can be asked to instantiate. There are three functions\nthat can be applied to a ref_iterator:\n\n* ref_iterator_advance(): move to the next reference in the iteration\n* ref_iterator_abort(): end the iteration before it is exhausted\n* ref_iterator_peel(): peel the reference currently being looked at\n\nIterating using a ref_iterator leaves the flow of control in the hands\nof the caller, which means that ref_iterators from multiple\nsources (e.g., loose and packed refs) can be composed and presented to\nthe world as a single compound ref_iterator.\n\nIt also means that the backend code for implementing reference iteration\nwill sometimes be more complicated. For example, the\ncache_ref_iterator (which iterates over a ref_cache) can't use the C\nstack to recurse; instead, it must manage its own stack internally as\nexplicit data structures. There is also a lot of boilerplate connected\nwith object-oriented programming in C.\n\nEventually, end-user callers will be able to be written in a more\nnatural way—managing their own flow of control rather than having to\nwork via callbacks. Since there will only be a few reference backends\nbut there are many consumers of this API, this is a good tradeoff.\n\nMore importantly, we gain composability, and especially the possibility\nof writing interchangeable parts that can work with any ref_iterator.\n\nFor example, merge_ref_iterator implements a generic way of merging the\ncontents of any two ref_iterators. It is used to merge loose + packed\nrefs as part of the implementation of the files_ref_iterator. But it\nwill also be possible to use it to merge other pairs of reference\nsources (e.g., per-worktree vs. shared refs).\n\nAnother example is prefix_ref_iterator, which can be used to trim a\nprefix off the front of reference names before presenting them to the\ncaller (e.g., \"refs/heads/master\" -> \"master\").\n\nIn this patch, we introduce the iterator abstraction and many utilities,\nand implement a reference iterator for the files ref storage backend.\n(I've written several other obvious utilities, for example a generic way\nto filter references being iterated over. These will probably be useful\nin the future. But they are not needed for this patch series, so I am\nnot including them at this time.)\n\nIn a moment we will rewrite do_for_each_ref() to work via reference\niterators (allowing some special-purpose code to be discarded), and do\nsomething similar for reflogs. In future patch series, we will expose\nthe ref_iterator abstraction in the public refs API so that callers can\nuse it directly.\n\nImplementation note: I tried abstracting this a layer further to allow\ngeneric iterators (over arbitrary types of objects) and generic\nutilities like a generic merge_iterator. But the implementation in C was\nvery cumbersome, involving (in my opinion) too much boilerplate and too\nmuch unsafe casting, some of which would have had to be done on the\ncaller side. However, I did put a few iterator-related constants in a\ntop-level header file, iterator.h, as they will be useful in a moment to\nimplement iteration over directory trees and possibly other types of\niterators in the future.\n\nSigned-off-by: Michael Haggerty \n---\n Makefile | 1 +\n iterator.h | 81 ++++++++++++\n refs.h | 4 +-\n refs/files-backend.c | 282 +++++++++++++++++++++++++++++++++++++++++\n refs/iterator.c | 347 +++++++++++++++++++++++++++++++++++++++++++++++++++\n refs/refs-internal.h | 193 ++++++++++++++++++++++++++++\n 6 files changed, 907 insertions(+), 1 deletion(-)\n create mode 100644 iterator.h\n create mode 100644 refs/iterator.c\n\ndiff --git a/Makefile b/Makefile\nindex a83e322..ac8f365 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -786,6 +786,7 @@ LIB_OBJS += read-cache.o\n 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 += ref-filter.o\n LIB_OBJS += remote.o\n LIB_OBJS += replace_object.o\ndiff --git a/iterator.h b/iterator.h\nnew file mode 100644\nindex 0000000..0f6900e\n--- /dev/null\n+++ b/iterator.h\n@@ -0,0 +1,81 @@\n+#ifndef ITERATOR_H\n+#define ITERATOR_H\n+\n+/*\n+ * Generic constants related to iterators.\n+ */\n+\n+/*\n+ * The attempt to advance the iterator was successful; the iterator\n+ * reflects the new current entry.\n+ */\n+#define ITER_OK 0\n+\n+/*\n+ * The iterator is exhausted and has been freed.\n+ */\n+#define ITER_DONE -1\n+\n+/*\n+ * The iterator experienced an error. The iteration has been aborted\n+ * and the iterator has been freed.\n+ */\n+#define ITER_ERROR -2\n+\n+/*\n+ * Return values for selector functions for merge iterators. The\n+ * numerical values of these constants are important and must be\n+ * compatible with ITER_DONE and ITER_ERROR.\n+ */\n+enum iterator_selection {\n+\t/* End the iteration without an error: */\n+\tITER_SELECT_DONE = ITER_DONE,\n+\n+\t/* Report an error and abort the iteration: */\n+\tITER_SELECT_ERROR = ITER_ERROR,\n+\n+\t/*\n+\t * The next group of constants are masks that are useful\n+\t * mainly internally.\n+\t */\n+\n+\t/* The LSB selects whether iter0/iter1 is the \"current\" iterator: */\n+\tITER_CURRENT_SELECTION_MASK = 0x01,\n+\n+\t/* iter0 is the \"current\" iterator this round: */\n+\tITER_CURRENT_SELECTION_0 = 0x00,\n+\n+\t/* iter1 is the \"current\" iterator this round: */\n+\tITER_CURRENT_SELECTION_1 = 0x01,\n+\n+\t/* Yield the value from the current iterator? */\n+\tITER_YIELD_CURRENT = 0x02,\n+\n+\t/* Discard the value from the secondary iterator? */\n+\tITER_SKIP_SECONDARY = 0x04,\n+\n+\t/*\n+\t * The constants that a selector function should usually\n+\t * return.\n+\t */\n+\n+\t/* Yield the value from iter0: */\n+\tITER_SELECT_0 = ITER_CURRENT_SELECTION_0 | ITER_YIELD_CURRENT,\n+\n+\t/* Yield the value from iter0 and discard the one from iter1: */\n+\tITER_SELECT_0_SKIP_1 = ITER_SELECT_0 | ITER_SKIP_SECONDARY,\n+\n+\t/* Discard the value from iter0 without yielding anything this round: */\n+\tITER_SKIP_0 = ITER_CURRENT_SELECTION_1 | ITER_SKIP_SECONDARY,\n+\n+\t/* Yield the value from iter1: */\n+\tITER_SELECT_1 = ITER_CURRENT_SELECTION_1 | ITER_YIELD_CURRENT,\n+\n+\t/* Yield the value from iter1 and discard the one from iter0: */\n+\tITER_SELECT_1_SKIP_0 = ITER_SELECT_1 | ITER_SKIP_SECONDARY,\n+\n+\t/* Discard the value from iter1 without yielding anything this round: */\n+\tITER_SKIP_1 = ITER_CURRENT_SELECTION_0 | ITER_SKIP_SECONDARY\n+};\n+\n+#endif /* ITERATOR_H */\ndiff --git a/refs.h b/refs.h\nindex 6d515a4..442c1a5 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -141,7 +141,9 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);\n struct ref_transaction;\n \n /*\n- * Bit values set in the flags argument passed to each_ref_fn():\n+ * Bit values set in the flags argument passed to each_ref_fn() and\n+ * stored in ref_iterator::flags. Other bits are for internal use\n+ * only:\n */\n \n /* Reference is a symbolic reference. */\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 8ab4d5f..dbf1587 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 \"../iterator.h\"\n #include \"../lockfile.h\"\n #include \"../object.h\"\n #include \"../dir.h\"\n@@ -704,6 +705,154 @@ static void prime_ref_dir(struct ref_dir *dir)\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+ * 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\tsort_ref_dir(level->dir);\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+\thashcpy(peeled->hash, entry->u.value.peeled.hash);\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+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 nonmatching_ref_data {\n \tconst struct string_list *skip;\n \tconst char *conflicting_refname;\n@@ -1843,6 +1992,139 @@ int peel_ref(const char *refname, unsigned char *sha1)\n \treturn peel_object(base, sha1);\n }\n \n+struct files_ref_iterator {\n+\tstruct ref_iterator base;\n+\n+\tstruct packed_ref_cache *packed_ref_cache;\n+\tstruct ref_iterator *iter0;\n+\tunsigned int flags;\n+};\n+\n+static int files_ref_iterator_advance(struct ref_iterator *ref_iterator)\n+{\n+\tstruct files_ref_iterator *iter =\n+\t\t(struct files_ref_iterator *)ref_iterator;\n+\tint ok;\n+\n+\twhile ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) {\n+\t\tif (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&\n+\t\t !ref_resolves_to_object(iter->iter0->refname,\n+\t\t\t\t\t iter->iter0->oid,\n+\t\t\t\t\t iter->iter0->flags))\n+\t\t\tcontinue;\n+\n+\t\titer->base.refname = iter->iter0->refname;\n+\t\titer->base.oid = iter->iter0->oid;\n+\t\titer->base.flags = iter->iter0->flags;\n+\t\treturn ITER_OK;\n+\t}\n+\n+\titer->iter0 = NULL;\n+\tif (ref_iterator_abort(ref_iterator) != ITER_DONE)\n+\t\tok = ITER_ERROR;\n+\n+\treturn ok;\n+}\n+\n+static int files_ref_iterator_peel(struct ref_iterator *ref_iterator,\n+\t\t\t\t struct object_id *peeled)\n+{\n+\tstruct files_ref_iterator *iter =\n+\t\t(struct files_ref_iterator *)ref_iterator;\n+\n+\treturn ref_iterator_peel(iter->iter0, peeled);\n+}\n+\n+static int files_ref_iterator_abort(struct ref_iterator *ref_iterator)\n+{\n+\tstruct files_ref_iterator *iter =\n+\t\t(struct files_ref_iterator *)ref_iterator;\n+\tint ok = ITER_DONE;\n+\n+\tif (iter->iter0)\n+\t\tok = ref_iterator_abort(iter->iter0);\n+\n+\trelease_packed_ref_cache(iter->packed_ref_cache);\n+\tbase_ref_iterator_free(ref_iterator);\n+\treturn ok;\n+}\n+\n+struct ref_iterator_vtable files_ref_iterator_vtable = {\n+\tfiles_ref_iterator_advance,\n+\tfiles_ref_iterator_peel,\n+\tfiles_ref_iterator_abort\n+};\n+\n+struct ref_iterator *files_ref_iterator_begin(\n+\t\tconst char *submodule,\n+\t\tconst char *prefix, unsigned int flags)\n+{\n+\tstruct ref_cache *refs = get_ref_cache(submodule);\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+\n+\tif (!refs)\n+\t\treturn empty_ref_iterator_begin();\n+\n+\tif (ref_paranoia < 0)\n+\t\tref_paranoia = git_env_bool(\"GIT_REF_PARANOIA\", 0);\n+\tif (ref_paranoia)\n+\t\tflags |= DO_FOR_EACH_INCLUDE_BROKEN;\n+\n+\titer = xcalloc(1, sizeof(*iter));\n+\tref_iterator = &iter->base;\n+\tbase_ref_iterator_init(ref_iterator, &files_ref_iterator_vtable);\n+\n+\t/*\n+\t * We must make sure that all loose refs are read before\n+\t * accessing the packed-refs file; this avoids a race\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 */\n+\n+\tloose_dir = get_loose_refs(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+\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+\n+\titer->iter0 = overlay_ref_iterator_begin(packed_iter, loose_iter);\n+\titer->flags = flags;\n+\n+\treturn ref_iterator;\n+}\n+\n /*\n * Call fn for each reference in the specified ref_cache, omitting\n * references not in the containing_dir of prefix. Call fn for all\ndiff --git a/refs/iterator.c b/refs/iterator.c\nnew file mode 100644\nindex 0000000..82a6948\n--- /dev/null\n+++ b/refs/iterator.c\n@@ -0,0 +1,347 @@\n+/*\n+ * Generic reference iterator infrastructure. See refs-internal.h for\n+ * documentation about the design and use of reference iterators.\n+ */\n+\n+#include \"cache.h\"\n+#include \"refs.h\"\n+#include \"refs/refs-internal.h\"\n+#include \"iterator.h\"\n+\n+int ref_iterator_advance(struct ref_iterator *ref_iterator)\n+{\n+\treturn ref_iterator->vtable->advance(ref_iterator);\n+}\n+\n+int ref_iterator_peel(struct ref_iterator *ref_iterator,\n+\t\t struct object_id *peeled)\n+{\n+\treturn ref_iterator->vtable->peel(ref_iterator, peeled);\n+}\n+\n+int ref_iterator_abort(struct ref_iterator *ref_iterator)\n+{\n+\treturn ref_iterator->vtable->abort(ref_iterator);\n+}\n+\n+void base_ref_iterator_init(struct ref_iterator *iter,\n+\t\t\t struct ref_iterator_vtable *vtable)\n+{\n+\titer->vtable = vtable;\n+\titer->refname = NULL;\n+\titer->oid = NULL;\n+\titer->flags = 0;\n+}\n+\n+void base_ref_iterator_free(struct ref_iterator *iter)\n+{\n+\t/* Help make use-after-free bugs fail quickly: */\n+\titer->vtable = NULL;\n+\tfree(iter);\n+}\n+\n+struct empty_ref_iterator {\n+\tstruct ref_iterator base;\n+};\n+\n+static int empty_ref_iterator_advance(struct ref_iterator *ref_iterator)\n+{\n+\treturn ref_iterator_abort(ref_iterator);\n+}\n+\n+static int empty_ref_iterator_peel(struct ref_iterator *ref_iterator,\n+\t\t\t\t struct object_id *peeled)\n+{\n+\tdie(\"BUG: peel called for empty iterator\");\n+}\n+\n+static int empty_ref_iterator_abort(struct ref_iterator *ref_iterator)\n+{\n+\tbase_ref_iterator_free(ref_iterator);\n+\treturn ITER_DONE;\n+}\n+\n+static struct ref_iterator_vtable empty_ref_iterator_vtable = {\n+\tempty_ref_iterator_advance,\n+\tempty_ref_iterator_peel,\n+\tempty_ref_iterator_abort\n+};\n+\n+struct ref_iterator *empty_ref_iterator_begin(void)\n+{\n+\tstruct empty_ref_iterator *iter = xcalloc(1, sizeof(*iter));\n+\tstruct ref_iterator *ref_iterator = &iter->base;\n+\n+\tbase_ref_iterator_init(ref_iterator, &empty_ref_iterator_vtable);\n+\treturn ref_iterator;\n+}\n+\n+int is_empty_ref_iterator(struct ref_iterator *ref_iterator)\n+{\n+\treturn ref_iterator->vtable == &empty_ref_iterator_vtable;\n+}\n+\n+struct merge_ref_iterator {\n+\tstruct ref_iterator base;\n+\n+\tstruct ref_iterator *iter0, *iter1;\n+\n+\tref_iterator_select_fn *select;\n+\tvoid *cb_data;\n+\n+\t/*\n+\t * A pointer to iter0 or iter1 (whichever is supplying the\n+\t * current value), or NULL if advance has not yet been called.\n+\t */\n+\tstruct ref_iterator **current;\n+};\n+\n+static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator)\n+{\n+\tstruct merge_ref_iterator *iter =\n+\t\t(struct merge_ref_iterator *)ref_iterator;\n+\tint ok;\n+\n+\tif (!iter->current) {\n+\t\t/* Initialize: advance both iterators to their first entries */\n+\t\tif ((ok = ref_iterator_advance(iter->iter0)) != ITER_OK) {\n+\t\t\titer->iter0 = NULL;\n+\t\t\tif (ok == ITER_ERROR)\n+\t\t\t\tgoto error;\n+\t\t}\n+\t\tif ((ok = ref_iterator_advance(iter->iter1)) != ITER_OK) {\n+\t\t\titer->iter1 = NULL;\n+\t\t\tif (ok == ITER_ERROR)\n+\t\t\t\tgoto error;\n+\t\t}\n+\t} else {\n+\t\t/*\n+\t\t * Advance the current iterator past the just-used\n+\t\t * entry:\n+\t\t */\n+\t\tif ((ok = ref_iterator_advance(*iter->current)) != ITER_OK) {\n+\t\t\t*iter->current = NULL;\n+\t\t\tif (ok == ITER_ERROR)\n+\t\t\t\tgoto error;\n+\t\t}\n+\t}\n+\n+\t/* Loop until we find an entry that we can yield. */\n+\twhile (1) {\n+\t\tstruct ref_iterator **secondary;\n+\t\tenum iterator_selection selection =\n+\t\t\titer->select(iter->iter0, iter->iter1, iter->cb_data);\n+\n+\t\tif (selection == ITER_SELECT_DONE) {\n+\t\t\treturn ref_iterator_abort(ref_iterator);\n+\t\t} else if (selection == ITER_SELECT_ERROR) {\n+\t\t\tref_iterator_abort(ref_iterator);\n+\t\t\treturn ITER_ERROR;\n+\t\t}\n+\n+\t\tif ((selection & ITER_CURRENT_SELECTION_MASK) == 0) {\n+\t\t\titer->current = &iter->iter0;\n+\t\t\tsecondary = &iter->iter1;\n+\t\t} else {\n+\t\t\titer->current = &iter->iter1;\n+\t\t\tsecondary = &iter->iter0;\n+\t\t}\n+\n+\t\tif (selection & ITER_SKIP_SECONDARY) {\n+\t\t\tif ((ok = ref_iterator_advance(*secondary)) != ITER_OK) {\n+\t\t\t\t*secondary = NULL;\n+\t\t\t\tif (ok == ITER_ERROR)\n+\t\t\t\t\tgoto error;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (selection & ITER_YIELD_CURRENT) {\n+\t\t\titer->base.refname = (*iter->current)->refname;\n+\t\t\titer->base.oid = (*iter->current)->oid;\n+\t\t\titer->base.flags = (*iter->current)->flags;\n+\t\t\treturn ITER_OK;\n+\t\t}\n+\t}\n+\n+error:\n+\tref_iterator_abort(ref_iterator);\n+\treturn ITER_ERROR;\n+}\n+\n+static int merge_ref_iterator_peel(struct ref_iterator *ref_iterator,\n+\t\t\t\t struct object_id *peeled)\n+{\n+\tstruct merge_ref_iterator *iter =\n+\t\t(struct merge_ref_iterator *)ref_iterator;\n+\n+\tif (!iter->current) {\n+\t\tdie(\"BUG: peel called before advance for merge iterator\");\n+\t}\n+\treturn ref_iterator_peel(*iter->current, peeled);\n+}\n+\n+static int merge_ref_iterator_abort(struct ref_iterator *ref_iterator)\n+{\n+\tstruct merge_ref_iterator *iter =\n+\t\t(struct merge_ref_iterator *)ref_iterator;\n+\tint ok = ITER_DONE;\n+\n+\tif (iter->iter0) {\n+\t\tif (ref_iterator_abort(iter->iter0) != ITER_DONE)\n+\t\t\tok = ITER_ERROR;\n+\t}\n+\tif (iter->iter1) {\n+\t\tif (ref_iterator_abort(iter->iter1) != ITER_DONE)\n+\t\t\tok = ITER_ERROR;\n+\t}\n+\tbase_ref_iterator_free(ref_iterator);\n+\treturn ok;\n+}\n+\n+static struct ref_iterator_vtable merge_ref_iterator_vtable = {\n+\tmerge_ref_iterator_advance,\n+\tmerge_ref_iterator_peel,\n+\tmerge_ref_iterator_abort\n+};\n+\n+struct ref_iterator *merge_ref_iterator_begin(\n+\t\tstruct ref_iterator *iter0, struct ref_iterator *iter1,\n+\t\tref_iterator_select_fn *select, void *cb_data)\n+{\n+\tstruct merge_ref_iterator *iter = xcalloc(1, sizeof(*iter));\n+\tstruct ref_iterator *ref_iterator = &iter->base;\n+\n+\tbase_ref_iterator_init(ref_iterator, &merge_ref_iterator_vtable);\n+\titer->iter0 = iter0;\n+\titer->iter1 = iter1;\n+\titer->select = select;\n+\titer->cb_data = cb_data;\n+\titer->current = NULL;\n+\treturn ref_iterator;\n+}\n+\n+/*\n+ * A ref_iterator_select_fn that overlays the items from iter1 on top\n+ * of those from iter0 (like loose refs over packed refs). See\n+ * overlay_ref_iterator_begin().\n+ */\n+static enum iterator_selection overlay_iterator_select(\n+\t\tstruct ref_iterator *iter0, struct ref_iterator *iter1,\n+\t\tvoid *cb_data)\n+{\n+\tint cmp;\n+\n+\tif (!iter0)\n+\t\treturn iter1 ? ITER_SELECT_1 : ITER_SELECT_DONE;\n+\telse if (!iter1)\n+\t\treturn ITER_SELECT_0;\n+\n+\tcmp = strcmp(iter0->refname, iter1->refname);\n+\n+\tif (cmp < 0)\n+\t\treturn ITER_SELECT_0;\n+\telse if (cmp > 0)\n+\t\treturn ITER_SELECT_1;\n+\telse\n+\t\treturn ITER_SELECT_1_SKIP_0;\n+}\n+\n+struct ref_iterator *overlay_ref_iterator_begin(struct ref_iterator *iter0,\n+\t\t\t\t\t\tstruct ref_iterator *iter1)\n+{\n+\t/*\n+\t * Optimization: if one of the iterators is empty, return the\n+\t * other one rather than incurring the overhead of wrapping\n+\t * them.\n+\t */\n+\tif (is_empty_ref_iterator(iter0)) {\n+\t\tref_iterator_abort(iter0);\n+\t\treturn iter1;\n+\t} else if (is_empty_ref_iterator(iter1)) {\n+\t\tref_iterator_abort(iter1);\n+\t\treturn iter0;\n+\t}\n+\n+\treturn merge_ref_iterator_begin(iter0, iter1,\n+\t\t\t\t\toverlay_iterator_select, NULL);\n+}\n+\n+struct prefix_ref_iterator {\n+\tstruct ref_iterator base;\n+\n+\tstruct ref_iterator *iter0;\n+\tchar *prefix;\n+\tint trim;\n+};\n+\n+static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator)\n+{\n+\tstruct prefix_ref_iterator *iter =\n+\t\t(struct prefix_ref_iterator *)ref_iterator;\n+\tint ok;\n+\n+\twhile ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) {\n+\t\tif (!starts_with(iter->iter0->refname, iter->prefix))\n+\t\t\tcontinue;\n+\n+\t\titer->base.refname = iter->iter0->refname + iter->trim;\n+\t\titer->base.oid = iter->iter0->oid;\n+\t\titer->base.flags = iter->iter0->flags;\n+\t\treturn ITER_OK;\n+\t}\n+\n+\titer->iter0 = NULL;\n+\tif (ref_iterator_abort(ref_iterator) != ITER_DONE)\n+\t\treturn ITER_ERROR;\n+\treturn ok;\n+}\n+\n+static int prefix_ref_iterator_peel(struct ref_iterator *ref_iterator,\n+\t\t\t\t struct object_id *peeled)\n+{\n+\tstruct prefix_ref_iterator *iter =\n+\t\t(struct prefix_ref_iterator *)ref_iterator;\n+\n+\treturn ref_iterator_peel(iter->iter0, peeled);\n+}\n+\n+static int prefix_ref_iterator_abort(struct ref_iterator *ref_iterator)\n+{\n+\tstruct prefix_ref_iterator *iter =\n+\t\t(struct prefix_ref_iterator *)ref_iterator;\n+\tint ok = ITER_DONE;\n+\n+\tif (iter->iter0)\n+\t\tok = ref_iterator_abort(iter->iter0);\n+\tfree(iter->prefix);\n+\tbase_ref_iterator_free(ref_iterator);\n+\treturn ok;\n+}\n+\n+static struct ref_iterator_vtable prefix_ref_iterator_vtable = {\n+\tprefix_ref_iterator_advance,\n+\tprefix_ref_iterator_peel,\n+\tprefix_ref_iterator_abort\n+};\n+\n+struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,\n+\t\t\t\t\t const char *prefix,\n+\t\t\t\t\t int trim)\n+{\n+\tstruct prefix_ref_iterator *iter;\n+\tstruct ref_iterator *ref_iterator;\n+\n+\tif (!*prefix && !trim)\n+\t\treturn iter0; /* optimization: no need to wrap iterator */\n+\n+\titer = xcalloc(1, sizeof(*iter));\n+\tref_iterator = &iter->base;\n+\n+\tbase_ref_iterator_init(ref_iterator, &prefix_ref_iterator_vtable);\n+\n+\titer->iter0 = iter0;\n+\titer->prefix = xstrdup(prefix);\n+\titer->trim = trim;\n+\n+\treturn ref_iterator;\n+}\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 8ad02d8..11182b4 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -249,6 +249,199 @@ int rename_ref_available(const char *oldname, const char *newname);\n #define DO_FOR_EACH_INCLUDE_BROKEN 0x01\n \n /*\n+ * Reference iterators\n+ *\n+ * A reference iterator encapsulates the state of an in-progress\n+ * iteration over references. Create an instance of `struct\n+ * ref_iterator` via one of the functions in this module.\n+ *\n+ * A freshly-created ref_iterator doesn't yet point at a reference. To\n+ * advance the iterator, call ref_iterator_advance(). If successful,\n+ * this sets the iterator's refname, oid, and flags fields to describe\n+ * the next reference and returns ITER_OK. The data pointed at by\n+ * refname and oid belong to the iterator; if you want to retain them\n+ * after calling ref_iterator_advance() again or calling\n+ * ref_iterator_abort(), you must make a copy. When the iteration has\n+ * been exhausted, ref_iterator_advance() releases any resources\n+ * assocated with the iteration, frees the ref_iterator object, and\n+ * returns ITER_DONE. If you want to abort the iteration early, call\n+ * ref_iterator_abort(), which also frees the ref_iterator object and\n+ * any associated resources. If there was an internal error advancing\n+ * to the next entry, ref_iterator_advance() aborts the iteration,\n+ * frees the ref_iterator, and returns ITER_ERROR.\n+ *\n+ * The reference currently being looked at can be peeled by calling\n+ * ref_iterator_peel(). This function is often faster than peel_ref(),\n+ * so it should be preferred when iterating over references.\n+ *\n+ * Putting it all together, a typical iteration looks like this:\n+ *\n+ * int ok;\n+ * struct ref_iterator *iter = ...;\n+ *\n+ * while ((ok = ref_iterator_advance(iter)) == ITER_OK) {\n+ * if (want_to_stop_iteration()) {\n+ * ok = ref_iterator_abort(iter);\n+ * break;\n+ * }\n+ *\n+ * // Access information about the current reference:\n+ * if (!(iter->flags & REF_ISSYMREF))\n+ * printf(\"%s is %s\\n\", iter->refname, oid_to_hex(&iter->oid));\n+ *\n+ * // If you need to peel the reference:\n+ * ref_iterator_peel(iter, &oid);\n+ * }\n+ *\n+ * if (ok != ITER_DONE)\n+ * handle_error();\n+ */\n+struct ref_iterator {\n+\tstruct ref_iterator_vtable *vtable;\n+\tconst char *refname;\n+\tconst struct object_id *oid;\n+\tunsigned int flags;\n+};\n+\n+/*\n+ * Advance the iterator to the first or next item and return ITER_OK.\n+ * If the iteration is exhausted, free the resources associated with\n+ * the ref_iterator and return ITER_DONE. On errors, free the iterator\n+ * resources and return ITER_ERROR. It is a bug to use ref_iterator or\n+ * call this function again after it has returned false.\n+ */\n+int ref_iterator_advance(struct ref_iterator *ref_iterator);\n+\n+/*\n+ * If possible, peel the reference currently being viewed by the\n+ * iterator. Return 0 on success.\n+ */\n+int ref_iterator_peel(struct ref_iterator *ref_iterator,\n+\t\t struct object_id *peeled);\n+\n+/*\n+ * End the iteration before it has been exhausted, freeing the\n+ * reference iterator and any associated resources and returning\n+ * ITER_DONE. If the abort itself failed, return ITER_ERROR.\n+ */\n+int ref_iterator_abort(struct ref_iterator *ref_iterator);\n+\n+/*\n+ * An iterator over nothing (its first ref_iterator_advance() call\n+ * returns 0).\n+ */\n+struct ref_iterator *empty_ref_iterator_begin(void);\n+\n+/*\n+ * Return true iff ref_iterator is an empty_ref_iterator.\n+ */\n+int is_empty_ref_iterator(struct ref_iterator *ref_iterator);\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+ * return one of the constants defined in enum iterator_selection. It\n+ * must not advance either of the iterators itself.\n+ *\n+ * The function must be prepared to handle the case that iter0 and/or\n+ * iter1 is NULL, which indicates that the corresponding sub-iterator\n+ * has been exhausted. Its return value must be consistent with the\n+ * current states of the iterators; e.g., it must not return\n+ * ITER_SKIP_1 if iter1 has already been exhausted.\n+ */\n+typedef enum iterator_selection ref_iterator_select_fn(\n+\t\tstruct ref_iterator *iter0, struct ref_iterator *iter1,\n+\t\tvoid *cb_data);\n+\n+/*\n+ * Iterate over the intries from iter0 and iter1, with the values\n+ * interleaved as directed by the select function. The iterator takes\n+ * ownership of iter0 and iter1 and frees them when the iteration is\n+ * over.\n+ */\n+struct ref_iterator *merge_ref_iterator_begin(\n+\t\tstruct ref_iterator *iter0, struct ref_iterator *iter1,\n+\t\tref_iterator_select_fn *select, void *cb_data);\n+\n+/*\n+ * An iterator consisting of the union of the entries from iter0 and\n+ * iter1. If there are entries common to the two sub-iterators, use\n+ * the one from iter1. Each iterator must iterate over its entries in\n+ * strcmp() order by refname for this to work.\n+ *\n+ * The new iterator takes ownership of its arguments and frees them\n+ * when the iteration is over. As a convenience to callers, if iter0\n+ * or iter1 is_empty_ref_iterator(), then abort that one immediately\n+ * and return the other iterator directly, without wrapping it.\n+ */\n+struct ref_iterator *overlay_ref_iterator_begin(struct ref_iterator *iter0,\n+\t\t\t\t\t\tstruct ref_iterator *iter1);\n+\n+/*\n+ * Wrap iter0, only letting through the references whose names start\n+ * with prefix. If trim is set, set iter->refname to the name of the\n+ * reference with that many characters trimmed off the front;\n+ * otherwise set it to the full refname. The new iterator takes over\n+ * ownership of iter0 and frees it when iteration is over. It makes\n+ * its own copy of prefix.\n+ *\n+ * As an convenience to callers, if prefix is the empty string and\n+ * trim is zero, this function returns iter0 directly, without\n+ * wrapping it.\n+ */\n+struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,\n+\t\t\t\t\t const char *prefix,\n+\t\t\t\t\t int trim);\n+\n+/*\n+ * Iterate over the packed and loose references in the specified\n+ * submodule that are within find_containing_dir(prefix). If prefix is\n+ * NULL or the empty string, iterate over all references in the\n+ * submodule.\n+ */\n+struct ref_iterator *files_ref_iterator_begin(const char *submodule,\n+\t\t\t\t\t const char *prefix,\n+\t\t\t\t\t unsigned int flags);\n+\n+/* Internal implementation of reference iteration: */\n+\n+/*\n+ * Base class constructor for ref_iterators. Initialize the\n+ * ref_iterator part of iter, setting its vtable pointer as specified.\n+ * This is meant to be called only by the initializers of derived\n+ * classes.\n+ */\n+void base_ref_iterator_init(struct ref_iterator *iter,\n+\t\t\t struct ref_iterator_vtable *vtable);\n+\n+/*\n+ * Base class destructor for ref_iterators. Destroy the ref_iterator\n+ * part of iter and shallow-free the object. This is meant to be\n+ * called only by the destructors of derived classes.\n+ */\n+void base_ref_iterator_free(struct ref_iterator *iter);\n+\n+/* Virtual function declarations for ref_iterators: */\n+\n+typedef int ref_iterator_advance_fn(struct ref_iterator *ref_iterator);\n+\n+typedef int ref_iterator_peel_fn(struct ref_iterator *ref_iterator,\n+\t\t\t\t struct object_id *peeled);\n+\n+/*\n+ * Implementations of this function should free any resources specific\n+ * to the derived class, then call base_ref_iterator_free() to clean\n+ * up and free the ref_iterator object.\n+ */\n+typedef int ref_iterator_abort_fn(struct ref_iterator *ref_iterator);\n+\n+struct ref_iterator_vtable {\n+\tref_iterator_advance_fn *advance;\n+\tref_iterator_peel_fn *peel;\n+\tref_iterator_abort_fn *abort;\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 * many characters off the beginning of each refname before passing\n-- \n2.8.1\n"},{"id":"287782","messageId":"40ddeae3c4b40cea93d3686e627df9ff9c2df082.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 10/13] do_for_each_ref(): reimplement using reference iteration","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:31Z","receivedAt":"2016-05-30T07:55:31Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Use the reference iterator interface to implement do_for_each_ref().\nDelete a bunch of code supporting the old for_each_ref() implementation.\nAnd now that do_for_each_ref() is generic code (it is no longer tied to\nthe files backend), move it to refs.c.\n\nThe implementation is via a new function, do_for_each_ref_iterator(),\nwhich takes a reference iterator as argument and calls a callback\nfunction for each of the references in the iterator.\n\nThis change requires the current_ref performance hack for peel_ref() to\nbe implemented via ref_iterator_peel() rather than peel_entry() because\nwe don't have a ref_entry handy (it is hidden under three layers:\nfile_ref_iterator, merge_ref_iterator, and cache_ref_iterator). So:\n\n* do_for_each_ref_iterator() records the active iterator in\n current_ref_iter while it is running.\n\n* peel_ref() checks whether current_ref_iter is pointing at the\n requested reference. If so, it asks the iterator to peel the\n reference (which it can do efficiently via its \"peel\" virtual\n function). For extra safety, we do the optimization only if the\n refname *addresses* are the same, not only if the refname *strings*\n are the same, to forestall possible mixups between refnames that come\n from different ref_iterators.\n\nPlease note that this optimization of peel_ref() is only available when\niterating via do_for_each_ref_iterator() (including all of the\nfor_each_ref() functions, which call it indirectly). It would be\ncomplicated to implement a similar optimization when iterating directly\nusing a reference iterator, because multiple reference iterators can be\nin use at the same time, with interleaved calls to\nref_iterator_advance(). (In fact we do exactly that in\nmerge_ref_iterator.)\n\nBut that is not necessary. peel_ref() is only called while iterating\nover references. Callers who iterate using the for_each_ref() functions\nbenefit from the optimization described above. Callers who iterate using\nreference iterators directly have access to the ref_iterator, so they\ncan call ref_iterator_peel() themselves to get an analogous optimization\nin a more straightforward manner.\n\nIf we rewrite all callers to use the reference iteration API, then we\ncan remove the current_ref_iter hack permanently.\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 20 +++++\n refs/files-backend.c | 206 ++-------------------------------------------------\n refs/iterator.c | 29 ++++++++\n refs/refs-internal.h | 33 ++++++---\n 4 files changed, 76 insertions(+), 212 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 842c5c7..814cad3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1120,6 +1120,26 @@ int head_ref(each_ref_fn fn, void *cb_data)\n \treturn head_ref_submodule(NULL, fn, cb_data);\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+ * many characters off the beginning of each refname before passing\n+ * the refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to\n+ * include broken references in the iteration. If fn ever returns a\n+ * non-zero value, stop the iteration and return that value;\n+ * otherwise, return 0.\n+ */\n+static int do_for_each_ref(const char *submodule, const char *prefix,\n+\t\t\t each_ref_fn fn, int trim, int flags, void *cb_data)\n+{\n+\tstruct ref_iterator *iter;\n+\n+\titer = files_ref_iterator_begin(submodule, prefix, flags);\n+\titer = prefix_ref_iterator_begin(iter, prefix, trim);\n+\n+\treturn do_for_each_ref_iterator(iter, fn, cb_data);\n+}\n+\n int for_each_ref(each_ref_fn fn, void *cb_data)\n {\n \treturn do_for_each_ref(NULL, \"\", fn, 0, 0, cb_data);\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex dbf1587..c48a006 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -542,53 +542,8 @@ static int entry_resolves_to_object(struct ref_entry *entry)\n \t\t\t\t &entry->u.value.oid, entry->flag);\n }\n \n-/*\n- * current_ref is a performance hack: when iterating over references\n- * using the for_each_ref*() functions, current_ref is set to the\n- * current reference's entry before calling the callback function. If\n- * the callback function calls peel_ref(), then peel_ref() first\n- * checks whether the reference to be peeled is the current reference\n- * (it usually is) and if so, returns that reference's peeled version\n- * if it is available. This avoids a refname lookup in a common case.\n- */\n-static struct ref_entry *current_ref;\n-\n typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);\n \n-struct ref_entry_cb {\n-\tconst char *prefix;\n-\tint trim;\n-\tint flags;\n-\teach_ref_fn *fn;\n-\tvoid *cb_data;\n-};\n-\n-/*\n- * Handle one reference in a do_for_each_ref*()-style iteration,\n- * calling an each_ref_fn for each entry.\n- */\n-static int do_one_ref(struct ref_entry *entry, void *cb_data)\n-{\n-\tstruct ref_entry_cb *data = cb_data;\n-\tstruct ref_entry *old_current_ref;\n-\tint retval;\n-\n-\tif (!starts_with(entry->name, data->prefix))\n-\t\treturn 0;\n-\n-\tif (!(data->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&\n-\t !entry_resolves_to_object(entry))\n-\t\treturn 0;\n-\n-\t/* Store the old value, in case this is a recursive call: */\n-\told_current_ref = current_ref;\n-\tcurrent_ref = entry;\n-\tretval = data->fn(entry->name + data->trim, &entry->u.value.oid,\n-\t\t\t entry->flag, data->cb_data);\n-\tcurrent_ref = old_current_ref;\n-\treturn retval;\n-}\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@@ -618,78 +573,6 @@ static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,\n }\n \n /*\n- * Call fn for each reference in the union of dir1 and dir2, in order\n- * by refname. Recurse into subdirectories. If a value entry appears\n- * in both dir1 and dir2, then only process the version that is in\n- * dir2. The input dirs must already be sorted, but subdirs will be\n- * sorted as needed. fn is called for all references, including\n- * broken ones.\n- */\n-static int do_for_each_entry_in_dirs(struct ref_dir *dir1,\n-\t\t\t\t struct ref_dir *dir2,\n-\t\t\t\t each_ref_entry_fn fn, void *cb_data)\n-{\n-\tint retval;\n-\tint i1 = 0, i2 = 0;\n-\n-\tassert(dir1->sorted == dir1->nr);\n-\tassert(dir2->sorted == dir2->nr);\n-\twhile (1) {\n-\t\tstruct ref_entry *e1, *e2;\n-\t\tint cmp;\n-\t\tif (i1 == dir1->nr) {\n-\t\t\treturn do_for_each_entry_in_dir(dir2, i2, fn, cb_data);\n-\t\t}\n-\t\tif (i2 == dir2->nr) {\n-\t\t\treturn do_for_each_entry_in_dir(dir1, i1, fn, cb_data);\n-\t\t}\n-\t\te1 = dir1->entries[i1];\n-\t\te2 = dir2->entries[i2];\n-\t\tcmp = strcmp(e1->name, e2->name);\n-\t\tif (cmp == 0) {\n-\t\t\tif ((e1->flag & REF_DIR) && (e2->flag & REF_DIR)) {\n-\t\t\t\t/* Both are directories; descend them in parallel. */\n-\t\t\t\tstruct ref_dir *subdir1 = get_ref_dir(e1);\n-\t\t\t\tstruct ref_dir *subdir2 = get_ref_dir(e2);\n-\t\t\t\tsort_ref_dir(subdir1);\n-\t\t\t\tsort_ref_dir(subdir2);\n-\t\t\t\tretval = do_for_each_entry_in_dirs(\n-\t\t\t\t\t\tsubdir1, subdir2, fn, cb_data);\n-\t\t\t\ti1++;\n-\t\t\t\ti2++;\n-\t\t\t} else if (!(e1->flag & REF_DIR) && !(e2->flag & REF_DIR)) {\n-\t\t\t\t/* Both are references; ignore the one from dir1. */\n-\t\t\t\tretval = fn(e2, cb_data);\n-\t\t\t\ti1++;\n-\t\t\t\ti2++;\n-\t\t\t} else {\n-\t\t\t\tdie(\"conflict between reference and directory: %s\",\n-\t\t\t\t e1->name);\n-\t\t\t}\n-\t\t} else {\n-\t\t\tstruct ref_entry *e;\n-\t\t\tif (cmp < 0) {\n-\t\t\t\te = e1;\n-\t\t\t\ti1++;\n-\t\t\t} else {\n-\t\t\t\te = e2;\n-\t\t\t\ti2++;\n-\t\t\t}\n-\t\t\tif (e->flag & REF_DIR) {\n-\t\t\t\tstruct ref_dir *subdir = get_ref_dir(e);\n-\t\t\t\tsort_ref_dir(subdir);\n-\t\t\t\tretval = do_for_each_entry_in_dir(\n-\t\t\t\t\t\tsubdir, 0, fn, cb_data);\n-\t\t\t} else {\n-\t\t\t\tretval = fn(e, cb_data);\n-\t\t\t}\n-\t\t}\n-\t\tif (retval)\n-\t\t\treturn retval;\n-\t}\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@@ -1960,11 +1843,12 @@ int peel_ref(const char *refname, unsigned char *sha1)\n \tint flag;\n \tunsigned char base[20];\n \n-\tif (current_ref && (current_ref->name == refname\n-\t\t\t || !strcmp(current_ref->name, refname))) {\n-\t\tif (peel_entry(current_ref, 0))\n+\tif (current_ref_iter && current_ref_iter->refname == refname) {\n+\t\tstruct object_id peeled;\n+\n+\t\tif (ref_iterator_peel(current_ref_iter, &peeled))\n \t\t\treturn -1;\n-\t\thashcpy(sha1, current_ref->u.value.peeled.hash);\n+\t\thashcpy(sha1, peeled.hash);\n \t\treturn 0;\n \t}\n \n@@ -2126,86 +2010,6 @@ struct ref_iterator *files_ref_iterator_begin(\n }\n \n /*\n- * Call fn for each reference in the specified ref_cache, omitting\n- * references not in the containing_dir of prefix. Call fn for all\n- * references, including broken ones. If fn ever returns a non-zero\n- * value, stop the iteration and return that value; otherwise, return\n- * 0.\n- */\n-static int do_for_each_entry(struct ref_cache *refs, const char *prefix,\n-\t\t\t each_ref_entry_fn fn, void *cb_data)\n-{\n-\tstruct packed_ref_cache *packed_ref_cache;\n-\tstruct ref_dir *loose_dir;\n-\tstruct ref_dir *packed_dir;\n-\tint retval = 0;\n-\n-\t/*\n-\t * We must make sure that all loose refs are read before accessing the\n-\t * packed-refs file; this avoids a race condition in which loose refs\n-\t * are migrated to the packed-refs file by a simultaneous process, but\n-\t * our in-memory view is from before the migration. get_packed_ref_cache()\n-\t * takes care of making sure our view is up to date with what is on\n-\t * disk.\n-\t */\n-\tloose_dir = get_loose_refs(refs);\n-\tif (prefix && *prefix) {\n-\t\tloose_dir = find_containing_dir(loose_dir, prefix, 0);\n-\t}\n-\tif (loose_dir)\n-\t\tprime_ref_dir(loose_dir);\n-\n-\tpacked_ref_cache = get_packed_ref_cache(refs);\n-\tacquire_packed_ref_cache(packed_ref_cache);\n-\tpacked_dir = get_packed_ref_dir(packed_ref_cache);\n-\tif (prefix && *prefix) {\n-\t\tpacked_dir = find_containing_dir(packed_dir, prefix, 0);\n-\t}\n-\n-\tif (packed_dir && loose_dir) {\n-\t\tsort_ref_dir(packed_dir);\n-\t\tsort_ref_dir(loose_dir);\n-\t\tretval = do_for_each_entry_in_dirs(\n-\t\t\t\tpacked_dir, loose_dir, fn, cb_data);\n-\t} else if (packed_dir) {\n-\t\tsort_ref_dir(packed_dir);\n-\t\tretval = do_for_each_entry_in_dir(\n-\t\t\t\tpacked_dir, 0, fn, cb_data);\n-\t} else if (loose_dir) {\n-\t\tsort_ref_dir(loose_dir);\n-\t\tretval = do_for_each_entry_in_dir(\n-\t\t\t\tloose_dir, 0, fn, cb_data);\n-\t}\n-\n-\trelease_packed_ref_cache(packed_ref_cache);\n-\treturn retval;\n-}\n-\n-int do_for_each_ref(const char *submodule, const char *prefix,\n-\t\t each_ref_fn fn, int trim, int flags, void *cb_data)\n-{\n-\tstruct ref_entry_cb data;\n-\tstruct ref_cache *refs;\n-\n-\trefs = get_ref_cache(submodule);\n-\tif (!refs)\n-\t\treturn 0;\n-\n-\tdata.prefix = prefix;\n-\tdata.trim = trim;\n-\tdata.flags = flags;\n-\tdata.fn = fn;\n-\tdata.cb_data = cb_data;\n-\n-\tif (ref_paranoia < 0)\n-\t\tref_paranoia = git_env_bool(\"GIT_REF_PARANOIA\", 0);\n-\tif (ref_paranoia)\n-\t\tdata.flags |= DO_FOR_EACH_INCLUDE_BROKEN;\n-\n-\treturn do_for_each_entry(refs, prefix, do_one_ref, &data);\n-}\n-\n-/*\n * Verify that the reference locked by lock has the value old_sha1.\n * Fail if the reference doesn't exist and mustexist is set. Return 0\n * on success. On error, write an error message to err, set errno, and\ndiff --git a/refs/iterator.c b/refs/iterator.c\nindex 82a6948..c1670ab 100644\n--- a/refs/iterator.c\n+++ b/refs/iterator.c\n@@ -345,3 +345,32 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,\n \n \treturn ref_iterator;\n }\n+\n+struct ref_iterator *current_ref_iter = NULL;\n+\n+int do_for_each_ref_iterator(struct ref_iterator *iter,\n+\t\t\t each_ref_fn fn, void *cb_data)\n+{\n+\tint retval = 0, ok;\n+\tstruct ref_iterator *old_ref_iter = current_ref_iter;\n+\n+\tcurrent_ref_iter = iter;\n+\twhile ((ok = ref_iterator_advance(iter)) == ITER_OK) {\n+\t\tretval = fn(iter->refname, iter->oid, iter->flags, cb_data);\n+\t\tif (retval) {\n+\t\t\t/*\n+\t\t\t * If ref_iterator_abort() returns ITER_ERROR,\n+\t\t\t * we ignore that error in deference to the\n+\t\t\t * callback function's return value.\n+\t\t\t */\n+\t\t\tref_iterator_abort(iter);\n+\t\t\tgoto out;\n+\t\t}\n+\t}\n+\n+out:\n+\tcurrent_ref_iter = old_ref_iter;\n+\tif (ok == ITER_ERROR)\n+\t\treturn -1;\n+\treturn retval;\n+}\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 11182b4..f16a38e 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -442,18 +442,29 @@ struct ref_iterator_vtable {\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- * many characters off the beginning of each refname before passing\n- * the refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to\n- * include broken references in the iteration. If fn ever returns a\n- * non-zero value, stop the iteration and return that value;\n- * otherwise, return 0.\n- *\n- * This is the common backend for the for_each_*ref* functions.\n+ * current_ref_iter is a performance hack: when iterating over\n+ * references using the for_each_ref*() functions, current_ref_iter is\n+ * set to the reference iterator before calling the callback function.\n+ * If the callback function calls peel_ref(), then peel_ref() first\n+ * checks whether the reference to be peeled is the one referred to by\n+ * the iterator (it usually is) and if so, asks the iterator for the\n+ * peeled version of the reference if it is available. This avoids a\n+ * refname lookup in a common case. current_ref_iter is set to NULL\n+ * when the iteration is over.\n */\n-int do_for_each_ref(const char *submodule, const char *prefix,\n-\t\t each_ref_fn fn, int trim, int flags, void *cb_data);\n+extern struct ref_iterator *current_ref_iter;\n+\n+/*\n+ * The common backend for the for_each_*ref* functions. Call fn for\n+ * each reference in iter. If the iterator itself ever returns\n+ * ITER_ERROR, return -1. If fn ever returns a non-zero value, stop\n+ * the iteration and return that value. Otherwise, return 0. In any\n+ * case, free the iterator when done. This function is basically an\n+ * adapter between the callback style of reference iteration and the\n+ * iterator style.\n+ */\n+int do_for_each_ref_iterator(struct ref_iterator *iter,\n+\t\t\t each_ref_fn fn, void *cb_data);\n \n /*\n * Read the specified reference from the filesystem or packed refs\n-- \n2.8.1\n"},{"id":"287786","messageId":"6228ed402dbdf9f937cb504aff6bbf8849771c54.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 11/13] for_each_reflog(): don't abort for bad references","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:32Z","receivedAt":"2016-05-30T07:55:32Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"If there is a file under \"$GIT_DIR/logs\" with no corresponding\nreference, the old code was emitting an error message, aborting the\nreflog iteration, and returning -1. But\n\n* None of the callers was checking the exit value\n\n* The callers all want to find all legitimate reflogs (sometimes for the\n purpose of determining object reachability!) and wouldn't benefit from\n a truncated iteration anyway.\n\nSo instead, emit an error message and skip the \"broken\" reflog, but\ncontinue with the iteration.\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex c48a006..a7cc0e2 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3325,7 +3325,7 @@ static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void *cb_data\n \t\t\t\tstruct object_id oid;\n \n \t\t\t\tif (read_ref_full(name->buf, 0, oid.hash, NULL))\n-\t\t\t\t\tretval = error(\"bad ref for %s\", name->buf);\n+\t\t\t\t\terror(\"bad ref for %s\", name->buf);\n \t\t\t\telse\n \t\t\t\t\tretval = fn(name->buf, &oid, 0, cb_data);\n \t\t\t}\n-- \n2.8.1\n"},{"id":"287789","messageId":"e99655e5d919852479d223caec42e5bb294b4a2c.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 12/13] dir_iterator: new API for iterating over a directory tree","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:33Z","receivedAt":"2016-05-30T07:55:33Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The iterator interface is modeled on that for references, though no\nvtable is necessary because there is (so far?) only one type of\ndir_iterator.\n\nThere are obviously a lot of features that could easily be added to this\nclass:\n\n* Skip/include directory paths in the iteration\n* Shallow/deep iteration\n* Letting the caller decide which subdirectories to recurse into (e.g.,\n via a dir_iterator_advance_into() function)\n* Option to iterate in sorted order\n* Option to iterate over directory paths before vs. after their contents\n\nBut these are not needed for the current patch series, so I refrain.\n\nSigned-off-by: Michael Haggerty \n---\n Makefile | 1 +\n dir-iterator.c | 180 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n dir-iterator.h | 86 +++++++++++++++++++++++++++\n 3 files changed, 267 insertions(+)\n create mode 100644 dir-iterator.c\n create mode 100644 dir-iterator.h\n\ndiff --git a/Makefile b/Makefile\nindex ac8f365..b4ffc11 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -722,6 +722,7 @@ LIB_OBJS += diff-lib.o\n LIB_OBJS += diff-no-index.o\n LIB_OBJS += diff.o\n LIB_OBJS += dir.o\n+LIB_OBJS += dir-iterator.o\n LIB_OBJS += editor.o\n LIB_OBJS += entry.o\n LIB_OBJS += environment.o\ndiff --git a/dir-iterator.c b/dir-iterator.c\nnew file mode 100644\nindex 0000000..e1d60f0\n--- /dev/null\n+++ b/dir-iterator.c\n@@ -0,0 +1,180 @@\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"iterator.h\"\n+#include \"dir-iterator.h\"\n+\n+struct dir_iterator_level {\n+\tint initialized;\n+\n+\tDIR *dir;\n+\n+\t/*\n+\t * The length of the directory part of refname at this level\n+\t * (including the trailing '/'):\n+\t */\n+\tsize_t prefix_len;\n+\n+\t/*\n+\t * The last action that has been taken with the current entry\n+\t * (needed for directories, which have to be included in the\n+\t * iteration and also iterated into):\n+\t */\n+\tenum {\n+\t\tDIR_STATE_ITER,\n+\t\tDIR_STATE_RECURSE\n+\t} dir_state;\n+};\n+\n+/*\n+ * The full data structure used to manage the internal directory\n+ * iteration state. It includes members that are not part of the\n+ * public interface.\n+ */\n+struct dir_iterator_int {\n+\tstruct dir_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 directory\n+\t * that will be included in this iteration.\n+\t */\n+\tstruct dir_iterator_level *levels;\n+};\n+\n+int dir_iterator_advance(struct dir_iterator *dir_iterator)\n+{\n+\tstruct dir_iterator_int *iter =\n+\t\t(struct dir_iterator_int *)dir_iterator;\n+\n+\twhile (1) {\n+\t\tstruct dir_iterator_level *level =\n+\t\t\t&iter->levels[iter->levels_nr - 1];\n+\t\tstruct dirent *de;\n+\n+\t\tif (!level->initialized) {\n+\t\t\tif (!is_dir_sep(iter->base.path.buf[iter->base.path.len - 1]))\n+\t\t\t\tstrbuf_addch(&iter->base.path, '/');\n+\t\t\tlevel->prefix_len = iter->base.path.len;\n+\n+\t\t\t/* opendir() errors are handled below */\n+\t\t\tlevel->dir = opendir(iter->base.path.buf);\n+\n+\t\t\tlevel->initialized = 1;\n+\t\t} else if (S_ISDIR(iter->base.st.st_mode)) {\n+\t\t\tif (level->dir_state == DIR_STATE_ITER) {\n+\t\t\t\t/*\n+\t\t\t\t * The directory was just iterated\n+\t\t\t\t * over; now prepare to iterate into\n+\t\t\t\t * it.\n+\t\t\t\t */\n+\t\t\t\tlevel->dir_state = DIR_STATE_RECURSE;\n+\t\t\t\tALLOC_GROW(iter->levels, iter->levels_nr + 1,\n+\t\t\t\t\t iter->levels_alloc);\n+\t\t\t\tlevel = &iter->levels[iter->levels_nr++];\n+\t\t\t\tlevel->initialized = 0;\n+\t\t\t\tcontinue;\n+\t\t\t} else {\n+\t\t\t\t/*\n+\t\t\t\t * The directory has already been\n+\t\t\t\t * iterated over and iterated into;\n+\t\t\t\t * we're done with it.\n+\t\t\t\t */\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (!level->dir) {\n+\t\t\t/*\n+\t\t\t * This level is exhausted (or wasn't opened\n+\t\t\t * successfully); pop up a level.\n+\t\t\t */\n+\t\t\tif (--iter->levels_nr == 0) {\n+\t\t\t\treturn dir_iterator_abort(dir_iterator);\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Loop until we find an entry that we can give back\n+\t\t * to the caller:\n+\t\t */\n+\t\twhile (1) {\n+\t\t\tstrbuf_setlen(&iter->base.path, level->prefix_len);\n+\t\t\tde = readdir(level->dir);\n+\n+\t\t\tif (!de) {\n+\t\t\t\t/* This level is exhausted; pop up a level. */\n+\t\t\t\tclosedir(level->dir);\n+\t\t\t\tlevel->dir = NULL;\n+\t\t\t\tif (--iter->levels_nr == 0)\n+\t\t\t\t\treturn dir_iterator_abort(dir_iterator);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\n+\t\t\tif (is_dot_or_dotdot(de->d_name))\n+\t\t\t\tcontinue;\n+\n+\t\t\tstrbuf_addstr(&iter->base.path, de->d_name);\n+\t\t\tif (lstat(iter->base.path.buf, &iter->base.st) < 0)\n+\t\t\t\tcontinue; /* silently skip */\n+\n+\t\t\t/*\n+\t\t\t * We have to set these each time because\n+\t\t\t * the path strbuf might have been realloc()ed.\n+\t\t\t */\n+\n+\t\t\titer->base.relative_path =\n+\t\t\t\titer->base.path.buf + iter->levels[0].prefix_len;\n+\t\t\titer->base.basename =\n+\t\t\t\titer->base.path.buf + level->prefix_len;\n+\t\t\tlevel->dir_state = DIR_STATE_ITER;\n+\n+\t\t\treturn ITER_OK;\n+\t\t}\n+\t}\n+}\n+\n+int dir_iterator_abort(struct dir_iterator *dir_iterator)\n+{\n+\tstruct dir_iterator_int *iter = (struct dir_iterator_int *)dir_iterator;\n+\n+\twhile (iter->levels_nr) {\n+\t\tstruct dir_iterator_level *level =\n+\t\t\t&iter->levels[--iter->levels_nr];\n+\n+\t\tif (level->dir)\n+\t\t\tclosedir(level->dir);\n+\t}\n+\n+\tfree(iter->levels);\n+\tstrbuf_release(&iter->base.path);\n+\tfree(iter);\n+\treturn ITER_DONE;\n+}\n+\n+struct dir_iterator *dir_iterator_begin(const char *path)\n+{\n+\tstruct dir_iterator_int *iter = xcalloc(1, sizeof(*iter));\n+\tstruct dir_iterator *dir_iterator = &iter->base;\n+\n+\tif (!path || !*path)\n+\t\tdie(\"BUG: empty path passed to dir_iterator_begin()\");\n+\n+\tstrbuf_init(&iter->base.path, PATH_MAX);\n+\tstrbuf_addstr(&iter->base.path, path);\n+\n+\tALLOC_GROW(iter->levels, 10, iter->levels_alloc);\n+\n+\titer->levels_nr = 1;\n+\titer->levels[0].initialized = 0;\n+\n+\treturn dir_iterator;\n+}\ndiff --git a/dir-iterator.h b/dir-iterator.h\nnew file mode 100644\nindex 0000000..8eb1f4c\n--- /dev/null\n+++ b/dir-iterator.h\n@@ -0,0 +1,86 @@\n+#ifndef DIR_ITERATOR_H\n+#define DIR_ITERATOR_H\n+\n+/*\n+ * Iterate over a directory tree.\n+ *\n+ * Iterate over a directory tree, recursively, including paths of all\n+ * types and hidden paths. Skip \".\" and \"..\" entries and don't follow\n+ * symlinks except for the original path.\n+ *\n+ * Every time dir_iterator_advance() is called, update the members of\n+ * the dir_iterator structure to reflect the next path in the\n+ * iteration. The order that paths are iterated over within a\n+ * directory is undefined, but directory paths are always iterated\n+ * over before the subdirectory contents.\n+ *\n+ * A typical iteration looks like this:\n+ *\n+ * int ok;\n+ * struct iterator *iter = dir_iterator_begin(path);\n+ *\n+ * while ((ok = dir_iterator_advance(iter)) == ITER_OK) {\n+ * if (want_to_stop_iteration()) {\n+ * ok = dir_iterator_abort(iter);\n+ * break;\n+ * }\n+ *\n+ * // Access information about the current path:\n+ * if (S_ISDIR(iter->st.st_mode))\n+ * printf(\"%s is a directory\\n\", iter->relative_path);\n+ * }\n+ *\n+ * if (ok != ITER_DONE)\n+ * handle_error();\n+ *\n+ * Callers are allowed to modify iter->path while they are working,\n+ * but they must restore it to its original contents before calling\n+ * dir_iterator_advance() again.\n+ */\n+\n+struct dir_iterator {\n+\t/* The current path: */\n+\tstruct strbuf path;\n+\n+\t/*\n+\t * The current path relative to the starting path. This part\n+\t * of the path always uses \"/\" characters to separate path\n+\t * components:\n+\t */\n+\tconst char *relative_path;\n+\n+\t/* The current basename: */\n+\tconst char *basename;\n+\n+\t/* The result of calling lstat() on path: */\n+\tstruct stat st;\n+};\n+\n+/*\n+ * Start a directory iteration over path. Return a dir_iterator that\n+ * holds the internal state of the iteration.\n+ *\n+ * The iteration includes all paths under path, not including path\n+ * itself and not including \".\" or \"..\" entries.\n+ *\n+ * path is the starting directory. An internal copy will be made.\n+ */\n+struct dir_iterator *dir_iterator_begin(const char *path);\n+\n+/*\n+ * Advance the iterator to the first or next item and return ITER_OK.\n+ * If the iteration is exhausted, free the resources associated with\n+ * the iterator and return ITER_DONE. On error, return ITER_ERROR. It\n+ * is a bug to use iterator or call this function again after it has\n+ * returned false.\n+ */\n+int dir_iterator_advance(struct dir_iterator *iterator);\n+\n+/*\n+ * End the iteration before it has been exhausted. Free the reference\n+ * iterator and any associated resources and return ITER_DONE. Return\n+ * ITER_ERROR on error.\n+ */\n+int dir_iterator_abort(struct dir_iterator *iterator);\n+\n+#endif\n-- \n2.8.1\n"},{"id":"287787","messageId":"0e0bc1c4c503ebd0ba6978e63bbb6ac54efa8fdf.1464537050.git.mhagger@alum.mit.edu","threadId":"42482","inReplyTo":"cover.1464537050.git.mhagger@alum.mit.edu","subject":"[PATCH 13/13] for_each_reflog(): reimplement using iterators","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-30T07:55:34Z","receivedAt":"2016-05-30T07:55:34Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Allow references with reflogs to be iterated over using a ref_iterator.\nThe latter is implemented as a files_reflog_iterator, which in turn uses\ndir_iterator to read the \"logs\" directory.\n\nNote that reflog iteration doesn't correctly handle per-worktree\nreflogs (either before or after this patch).\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 113 ++++++++++++++++++++++++++++++++-------------------\n refs/refs-internal.h | 7 ++++\n 2 files changed, 78 insertions(+), 42 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex a7cc0e2..2e0d006 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2,6 +2,7 @@\n #include \"../refs.h\"\n #include \"refs-internal.h\"\n #include \"../iterator.h\"\n+#include \"../dir-iterator.h\"\n #include \"../lockfile.h\"\n #include \"../object.h\"\n #include \"../dir.h\"\n@@ -3292,60 +3293,88 @@ int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_dat\n \tstrbuf_release(&sb);\n \treturn ret;\n }\n-/*\n- * Call fn for each reflog in the namespace indicated by name. name\n- * must be empty or end with '/'. Name will be used as a scratch\n- * space, but its contents will be restored before return.\n- */\n-static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void *cb_data)\n+\n+struct files_reflog_iterator {\n+\tstruct ref_iterator base;\n+\n+\tstruct dir_iterator *dir_iterator;\n+\tstruct object_id oid;\n+};\n+\n+static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)\n {\n-\tDIR *d = opendir(git_path(\"logs/%s\", name->buf));\n-\tint retval = 0;\n-\tstruct dirent *de;\n-\tint oldlen = name->len;\n+\tstruct files_reflog_iterator *iter =\n+\t\t(struct files_reflog_iterator *)ref_iterator;\n+\tstruct dir_iterator *diter = iter->dir_iterator;\n+\tint ok;\n \n-\tif (!d)\n-\t\treturn name->len ? errno : 0;\n+\twhile ((ok = dir_iterator_advance(diter)) == ITER_OK) {\n+\t\tint flags;\n \n-\twhile ((de = readdir(d)) != NULL) {\n-\t\tstruct stat st;\n-\n-\t\tif (de->d_name[0] == '.')\n+\t\tif (!S_ISREG(diter->st.st_mode))\n+\t\t\tcontinue;\n+\t\tif (diter->basename[0] == '.')\n \t\t\tcontinue;\n-\t\tif (ends_with(de->d_name, \".lock\"))\n+\t\tif (ends_with(diter->basename, \".lock\"))\n \t\t\tcontinue;\n-\t\tstrbuf_addstr(name, de->d_name);\n-\t\tif (stat(git_path(\"logs/%s\", name->buf), &st) < 0) {\n-\t\t\t; /* silently ignore */\n-\t\t} else {\n-\t\t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\t\tstrbuf_addch(name, '/');\n-\t\t\t\tretval = do_for_each_reflog(name, fn, cb_data);\n-\t\t\t} else {\n-\t\t\t\tstruct object_id oid;\n \n-\t\t\t\tif (read_ref_full(name->buf, 0, oid.hash, NULL))\n-\t\t\t\t\terror(\"bad ref for %s\", name->buf);\n-\t\t\t\telse\n-\t\t\t\t\tretval = fn(name->buf, &oid, 0, cb_data);\n-\t\t\t}\n-\t\t\tif (retval)\n-\t\t\t\tbreak;\n+\t\tif (read_ref_full(diter->relative_path, 0,\n+\t\t\t\t iter->oid.hash, &flags)) {\n+\t\t\terror(\"bad ref for %s\", diter->path.buf);\n+\t\t\tcontinue;\n \t\t}\n-\t\tstrbuf_setlen(name, oldlen);\n+\n+\t\titer->base.refname = diter->relative_path;\n+\t\titer->base.oid = &iter->oid;\n+\t\titer->base.flags = flags;\n+\t\treturn ITER_OK;\n \t}\n-\tclosedir(d);\n-\treturn retval;\n+\n+\titer->dir_iterator = NULL;\n+\tif (ref_iterator_abort(ref_iterator) == ITER_ERROR)\n+\t\tok = ITER_ERROR;\n+\treturn ok;\n+}\n+\n+static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator,\n+\t\t\t\t struct object_id *peeled)\n+{\n+\tdie(\"BUG: ref_iterator_peel() called for reflog_iterator\");\n+}\n+\n+static int files_reflog_iterator_abort(struct ref_iterator *ref_iterator)\n+{\n+\tstruct files_reflog_iterator *iter =\n+\t\t(struct files_reflog_iterator *)ref_iterator;\n+\tint ok = ITER_DONE;\n+\n+\tif (iter->dir_iterator)\n+\t\tok = dir_iterator_abort(iter->dir_iterator);\n+\n+\tbase_ref_iterator_free(ref_iterator);\n+\treturn ok;\n+}\n+\n+struct ref_iterator_vtable files_reflog_iterator_vtable = {\n+\tfiles_reflog_iterator_advance,\n+\tfiles_reflog_iterator_peel,\n+\tfiles_reflog_iterator_abort\n+};\n+\n+struct ref_iterator *files_reflog_iterator_begin(void)\n+{\n+\tstruct files_reflog_iterator *iter = xcalloc(1, sizeof(*iter));\n+\tstruct ref_iterator *ref_iterator = &iter->base;\n+\n+\tbase_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable);\n+\titer->dir_iterator = dir_iterator_begin(git_path(\"logs\"));\n+\treturn ref_iterator;\n }\n \n int for_each_reflog(each_ref_fn fn, void *cb_data)\n {\n-\tint retval;\n-\tstruct strbuf name;\n-\tstrbuf_init(&name, PATH_MAX);\n-\tretval = do_for_each_reflog(&name, fn, cb_data);\n-\tstrbuf_release(&name);\n-\treturn retval;\n+\treturn do_for_each_ref_iterator(files_reflog_iterator_begin(),\n+\t\t\t\t\tfn, cb_data);\n }\n \n static int ref_update_reject_duplicates(struct string_list *refnames,\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex f16a38e..87ad8b1 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -403,6 +403,13 @@ struct ref_iterator *files_ref_iterator_begin(const char *submodule,\n \t\t\t\t\t const char *prefix,\n \t\t\t\t\t unsigned int flags);\n \n+/*\n+ * Iterate over the references in the main ref_store that have a\n+ * reflog. The paths within a directory are iterated over in arbitrary\n+ * order.\n+ */\n+struct ref_iterator *files_reflog_iterator_begin(void);\n+\n /* Internal implementation of reference iteration: */\n \n /*\n-- \n2.8.1\n"},{"id":"287828","messageId":"574C5AB0.4090005@ramsayjones.plus.com","threadId":"42482","inReplyTo":"89634d216544d1102dafd5d18247bff2581d48a8.1464537050.git.mhagger@alum.mit.edu","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-05-30T15:22:24Z","receivedAt":"2016-05-30T15:22:24Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 30/05/16 08:55, Michael Haggerty wrote:\n[snip]\n\n> /* Reference is a symbolic reference. */\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 8ab4d5f..dbf1587 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 \"../iterator.h\"\n> #include \"../lockfile.h\"\n> #include \"../object.h\"\n> #include \"../dir.h\"\n> @@ -704,6 +705,154 @@ static void prime_ref_dir(struct ref_dir *dir)\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> + * 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\ndo you need to sort here ...\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\tsort_ref_dir(level->dir);\n\n... given that you sort here?\n\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\nATB,\nRamsay Jones\n"},{"id":"287832","messageId":"574C70EF.5070705@ramsayjones.plus.com","threadId":"42482","inReplyTo":"574C5AB0.4090005@ramsayjones.plus.com","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-05-30T16:57:19Z","receivedAt":"2016-05-30T16:57:19Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 30/05/16 16:22, Ramsay Jones wrote:\n> \n> \n> On 30/05/16 08:55, Michael Haggerty wrote:\n> [snip]\n> \n>> /* Reference is a symbolic reference. */\n>> diff --git a/refs/files-backend.c b/refs/files-backend.c\n>> index 8ab4d5f..dbf1587 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 \"../iterator.h\"\n>> #include \"../lockfile.h\"\n>> #include \"../object.h\"\n>> #include \"../dir.h\"\n>> @@ -704,6 +705,154 @@ static void prime_ref_dir(struct ref_dir *dir)\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>> + * 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> do you need to sort here ...\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\tsort_ref_dir(level->dir);\n> \n> ... given that you sort here?\n\nI had intended to say 'or vice versa' here. When I wrote this, I had not\nfinished reading this patch (let alone the series). Now, I suspect that\nyou can simply drop this 'sort_ref_dir()' call site. Unless I've misread\nthe code, of course! ;-)\n\nATB,\nRamsay Jones\n"},{"id":"287865","messageId":"574CE601.3010701@alum.mit.edu","threadId":"42482","inReplyTo":"574C70EF.5070705@ramsayjones.plus.com","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-31T01:16:49Z","receivedAt":"2016-05-31T01:16:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/30/2016 06:57 PM, Ramsay Jones wrote:\n> \n> \n> On 30/05/16 16:22, Ramsay Jones wrote:\n>>\n>>\n>> On 30/05/16 08:55, Michael Haggerty wrote:\n>> [snip]\n>>\n>>> /* Reference is a symbolic reference. */\n>>> diff --git a/refs/files-backend.c b/refs/files-backend.c\n>>> index 8ab4d5f..dbf1587 100644\n>>> --- a/refs/files-backend.c\n>>> +++ b/refs/files-backend.c\n>>> [...]\n>>> +\t\t\tsort_ref_dir(dir);\n>>\n>> do you need to sort here ...\n>>> [...]\n>>> +\t\t\tsort_ref_dir(level->dir);\n>>\n>> ... given that you sort here?\n> \n> I had intended to say 'or vice versa' here. When I wrote this, I had not\n> finished reading this patch (let alone the series). Now, I suspect that\n> you can simply drop this 'sort_ref_dir()' call site. Unless I've misread\n> the code, of course! ;-)\n\nYes, you are right. Thanks for catching this! I'll fix it in v2.\n\nMichael\n"},{"id":"287872","messageId":"CAPig+cSjzZGUjdgkz1y7brGNb1M2gHfW0UG-wgBc00beNDQmnA@mail.gmail.com","threadId":"42482","inReplyTo":"89634d216544d1102dafd5d18247bff2581d48a8.1464537050.git.mhagger@alum.mit.edu","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-31T05:29:25Z","receivedAt":"2016-05-31T05:29:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, May 30, 2016 at 3:55 AM, Michael Haggerty wrote:\n> [...]\n> This commit introduces a new iteration primitive for references: a\n> ref_iterator. A ref_iterator is a polymorphic object that a reference\n> storage backend can be asked to instantiate. There are three functions\n> that can be applied to a ref_iterator:\n>\n> * ref_iterator_advance(): move to the next reference in the iteration\n> * ref_iterator_abort(): end the iteration before it is exhausted\n> * ref_iterator_peel(): peel the reference currently being looked at\n> [...]\n> Signed-off-by: Michael Haggerty \n> ---\n> diff --git a/refs/refs-internal.h b/refs/refs-internal.h\n> @@ -249,6 +249,199 @@ int rename_ref_available(const char *oldname, const char *newname);\n> +/*\n> + * Advance the iterator to the first or next item and return ITER_OK.\n> + * If the iteration is exhausted, free the resources associated with\n> + * the ref_iterator and return ITER_DONE. On errors, free the iterator\n> + * resources and return ITER_ERROR. It is a bug to use ref_iterator or\n> + * call this function again after it has returned false.\n> + */\n\nEither:\n\n s/false/something other than ITER_OK/\n\nor:\n\n s/false/ITER_DONE or ITER_ERROR/\n\n> +int ref_iterator_advance(struct ref_iterator *ref_iterator);\n> +\n> +/*\n> + * An iterator over nothing (its first ref_iterator_advance() call\n> + * returns 0).\n> + */\n\ns/0/ITER_DONE/\n\n> +struct ref_iterator *empty_ref_iterator_begin(void);\n> +\n> +/*\n> + * Return true iff ref_iterator is an empty_ref_iterator.\n> + */\n> +int is_empty_ref_iterator(struct ref_iterator *ref_iterator);\n\nI can see that you used this function as an optimization or\nconvenience in overlay_ref_iterator_begin(), but do you expect it to\nbe generally useful otherwise? Is it worth publishing? Do you have\nother use-cases in mind?\n\nAlso, can you explain why the merge iterator doesn't also perform the\noptimization/convenience of checking if one iterator is an empty\niterator?\n\n> +/*\n> + * Iterate over the intries from iter0 and iter1, with the values\n\ns/intries/entries/\n\n> + * interleaved as directed by the select function. The iterator takes\n> + * ownership of iter0 and iter1 and frees them when the iteration is\n> + * over.\n> + */\n> +struct ref_iterator *merge_ref_iterator_begin(\n> + struct ref_iterator *iter0, struct ref_iterator *iter1,\n> + ref_iterator_select_fn *select, void *cb_data);\n> +\n> +/*\n> + * An iterator consisting of the union of the entries from iter0 and\n> + * iter1. If there are entries common to the two sub-iterators, use\n> + * the one from iter1. Each iterator must iterate over its entries in\n> + * strcmp() order by refname for this to work.\n> + *\n> + * The new iterator takes ownership of its arguments and frees them\n> + * when the iteration is over. As a convenience to callers, if iter0\n> + * or iter1 is_empty_ref_iterator(), then abort that one immediately\n> + * and return the other iterator directly, without wrapping it.\n> + */\n> +struct ref_iterator *overlay_ref_iterator_begin(struct ref_iterator *iter0,\n> + struct ref_iterator *iter1);\n\nWhen reading about the overlay iterator (both code and documentation),\nmy expectation was that iter0 would shadow iter1, not the other way\naround as implemented here. Of course, that's entirely subjective, but\nthe generic names don't provide any useful clues as to which shadows\nwhich. Perhaps giving them more meaningful names would help.\n\n> +/*\n> + * Wrap iter0, only letting through the references whose names start\n> + * with prefix. If trim is set, set iter->refname to the name of the\n> + * reference with that many characters trimmed off the front;\n> + * otherwise set it to the full refname. The new iterator takes over\n> + * ownership of iter0 and frees it when iteration is over. It makes\n> + * its own copy of prefix.\n> + *\n> + * As an convenience to callers, if prefix is the empty string and\n> + * trim is zero, this function returns iter0 directly, without\n> + * wrapping it.\n> + */\n> +struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,\n> + const char *prefix,\n> + int trim);\n\nMinor: Similarly, when reading the code and documentation, I wondered\nwhy this was named 'iter0' when no 'iter1' was in sight. Perhaps name\nit simply 'iter'.\n"},{"id":"287875","messageId":"xmqqr3ci7o73.fsf@gitster.mtv.corp.google.com","threadId":"42482","inReplyTo":"89634d216544d1102dafd5d18247bff2581d48a8.1464537050.git.mhagger@alum.mit.edu","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-31T06:10:08Z","receivedAt":"2016-05-31T06:10:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> This commit introduces a new iteration primitive for references: a\n> ref_iterator. A ref_iterator is a polymorphic object that a reference\n> storage backend can be asked to instantiate. There are three functions\n> that can be applied to a ref_iterator:\n>\n> * ref_iterator_advance(): move to the next reference in the iteration\n> * ref_iterator_abort(): end the iteration before it is exhausted\n> * ref_iterator_peel(): peel the reference currently being looked at\n\nThis part looked somewhat strange in that it makes \"peel\" sound\nsomething very special. Even though I understand why, it made me\nfeel uneasy. I do not think of another operation like peel that may\nwant to have such a specialized helper, so I'll let it pass, but the\nprimary uneasiness I felt comes from the fact that \"iterator-peel\"\nis not an essential service of the API that needs for correctness,\nbut is a pure optimization (i.e. you could grab a ref from the\nnormal iterator call, and then ask \"please peel this ref\" to the\nusual ref API that does not know anything about iteration).\n\n> Iterating using a ref_iterator leaves the flow of control in the hands\n> of the caller, which means that ref_iterators from multiple\n> sources (e.g., loose and packed refs) can be composed and presented to\n> the world as a single compound ref_iterator.\n\nYes, this is a very good move. I am happy to see us going in this\ndirection.\n"},{"id":"287884","messageId":"574D4448.5020004@alum.mit.edu","threadId":"42482","inReplyTo":"CAPig+cSjzZGUjdgkz1y7brGNb1M2gHfW0UG-wgBc00beNDQmnA@mail.gmail.com","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-31T07:59:04Z","receivedAt":"2016-05-31T07:59:04Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/31/2016 07:29 AM, Eric Sunshine wrote:\n> On Mon, May 30, 2016 at 3:55 AM, Michael Haggerty wrote:\n>> [...]\n> [...]\n> Either:\n> \n> s/false/something other than ITER_OK/\n> \n> or:\n> \n> s/false/ITER_DONE or ITER_ERROR/\n\nThanks.\n\n>> +int ref_iterator_advance(struct ref_iterator *ref_iterator);\n>> +\n>> +/*\n>> + * An iterator over nothing (its first ref_iterator_advance() call\n>> + * returns 0).\n>> + */\n> \n> s/0/ITER_DONE/\n\nThanks. I guess you can guess what an earlier draft of this interface\nlooked like :-)\n\n>> +struct ref_iterator *empty_ref_iterator_begin(void);\n>> +\n>> +/*\n>> + * Return true iff ref_iterator is an empty_ref_iterator.\n>> + */\n>> +int is_empty_ref_iterator(struct ref_iterator *ref_iterator);\n> \n> I can see that you used this function as an optimization or\n> convenience in overlay_ref_iterator_begin(), but do you expect it to\n> be generally useful otherwise? Is it worth publishing? Do you have\n> other use-cases in mind?\n\nIt is only \"published\" within the refs module, in refs/refs-internal.h.\nThis header file is not meant to be used by code outside of the refs module.\n\nMy thinking was that it might be useful to other reference backends. The\nfunction is pretty safe for anybody to call, though I admit that it is\nnot very general.\n\nI don't have a strong feeling either way. If nobody else chimes in, I'll\nremove it from the header file as you suggested. We can always add it\nback if somebody needs it.\n\n> Also, can you explain why the merge iterator doesn't also perform the\n> optimization/convenience of checking if one iterator is an empty\n> iterator?\n\nThat's because the merge iterator doesn't know what its select function\nwill do. For example, you could imagine an \"intersect\" select function\nthat only lets through references that were in *both* sub-iterators. In\nthat case, your suggested \"optimization\" would be incorrect.\n\nIncidentally, that's also why I decided to leave the select function in\ncharge even after one or both of the sub-iterators is exhausted—because\nit lets merge_ref_iterator implement more diverse behavior.\n\n>> +/*\n>> + * Iterate over the intries from iter0 and iter1, with the values\n> \n> s/intries/entries/\n\nThanks.\n\n>> + * interleaved as directed by the select function. The iterator takes\n>> + * ownership of iter0 and iter1 and frees them when the iteration is\n>> + * over.\n>> + */\n>> +struct ref_iterator *merge_ref_iterator_begin(\n>> + struct ref_iterator *iter0, struct ref_iterator *iter1,\n>> + ref_iterator_select_fn *select, void *cb_data);\n>> +\n>> +/*\n>> + * An iterator consisting of the union of the entries from iter0 and\n>> + * iter1. If there are entries common to the two sub-iterators, use\n>> + * the one from iter1. Each iterator must iterate over its entries in\n>> + * strcmp() order by refname for this to work.\n>> + *\n>> + * The new iterator takes ownership of its arguments and frees them\n>> + * when the iteration is over. As a convenience to callers, if iter0\n>> + * or iter1 is_empty_ref_iterator(), then abort that one immediately\n>> + * and return the other iterator directly, without wrapping it.\n>> + */\n>> +struct ref_iterator *overlay_ref_iterator_begin(struct ref_iterator *iter0,\n>> + struct ref_iterator *iter1);\n> \n> When reading about the overlay iterator (both code and documentation),\n> my expectation was that iter0 would shadow iter1, not the other way\n> around as implemented here. Of course, that's entirely subjective, but\n> the generic names don't provide any useful clues as to which shadows\n> which. Perhaps giving them more meaningful names would help.\n\nThat's a good idea. I also found myself having to refer back to the\ndocumentation to remind myself which was which.\n\nHow about I rename them \"back\" and \"front\"? I will also reverse the\norder of the arguments.\n\n(But I will leave the names \"iter0\" and \"iter1\" in merge_ref_iterator,\nand also the constants like ITER_SELECT_0, because these don't\nnecessarily have the interpretation of \"back\" and \"front\".)\n\n>> +/*\n>> + * Wrap iter0, only letting through the references whose names start\n>> + * with prefix. If trim is set, set iter->refname to the name of the\n>> + * reference with that many characters trimmed off the front;\n>> + * otherwise set it to the full refname. The new iterator takes over\n>> + * ownership of iter0 and frees it when iteration is over. It makes\n>> + * its own copy of prefix.\n>> + *\n>> + * As an convenience to callers, if prefix is the empty string and\n>> + * trim is zero, this function returns iter0 directly, without\n>> + * wrapping it.\n>> + */\n>> +struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,\n>> + const char *prefix,\n>> + int trim);\n> \n> Minor: Similarly, when reading the code and documentation, I wondered\n> why this was named 'iter0' when no 'iter1' was in sight. Perhaps name\n> it simply 'iter'.\n\nI found that it got a little bit confusing, because the constructor and\nmethod implementations all use `iter` as a local variable. In particular\nin the constructor there would want to be an argument \"iter\" and also\nthe local variable \"iter\" for the iterator being constructed, so a new\nname would otherwise have to be invented for one or the other. Between\nall the \"iter\" and \"iter\" and \"iter->iter\", I found that naming the\nsub-iterator \"iter0\" made things a little bit less bewildering.\n\nIf you don't like that, we could name the embedded iterators something\nlike \"subiter\", \"subiter0\", and \"subiter1\". But the current convention\nis a bit more succinct so I slightly prefer it.\n\nThanks for all your comments!\n\nMichael\n"},{"id":"287886","messageId":"574D4F42.3090807@alum.mit.edu","threadId":"42482","inReplyTo":"xmqqr3ci7o73.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-05-31T08:45:54Z","receivedAt":"2016-05-31T08:45:54Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/31/2016 08:10 AM, Junio C Hamano wrote:\n> Michael Haggerty writes:\n> \n>> This commit introduces a new iteration primitive for references: a\n>> ref_iterator. A ref_iterator is a polymorphic object that a reference\n>> storage backend can be asked to instantiate. There are three functions\n>> that can be applied to a ref_iterator:\n>>\n>> * ref_iterator_advance(): move to the next reference in the iteration\n>> * ref_iterator_abort(): end the iteration before it is exhausted\n>> * ref_iterator_peel(): peel the reference currently being looked at\n> \n> This part looked somewhat strange in that it makes \"peel\" sound\n> something very special. Even though I understand why, it made me\n> feel uneasy. I do not think of another operation like peel that may\n> want to have such a specialized helper, so I'll let it pass, but the\n> primary uneasiness I felt comes from the fact that \"iterator-peel\"\n> is not an essential service of the API that needs for correctness,\n> but is a pure optimization (i.e. you could grab a ref from the\n> normal iterator call, and then ask \"please peel this ref\" to the\n> usual ref API that does not know anything about iteration).\n\nI agree that this is inelegant. The underlying reason is that the\niterator embodies both the iteration and also the reference currently\nbeing iterated over. So ref_iterator_advance() and ref_iterator_abort()\nare really iterator methods, whereas ref_iterator_peel() is really a\nmethod on the current reference. For that matter, the refname etc fields\nare conceptually parts of the current reference, too, not of the iteration.\n\nOne of my many coding spikes tried to separate the two concepts by\nhaving the iterator give back a separate object representing the\nreference. That object would have had a peel() method. But I wasn't\nhappy with that approach:\n\n* Anything that made it look like the reference object had a lifetime\nindependent of that of the iterator seemed like asking for trouble.\n\n* The peel() method would probably have to be a real method that knows\nwhere the reference came from (as opposed to a simple function) because\nof things like prefix_ref_iterator, where the refname stored in the\nref_iterator might have had its prefix stripped off.\n\nThe complexity level was getting beyond what I felt was tenable in C, so\nI made the compromise that you see.\n\nIf I had more time I'd do some benchmarking to see whether the \"peeling\"\noptimization is really still needed. It comes from the days when\nreferences were stored in giant lists. Now that we store refs in tries,\nand each level gets sorted as part of the iteration anyway, the lookup\nmight be fast enough that nobody would care.\n\n> [...]\n\nMichael\n"},{"id":"287955","messageId":"CAPig+cQte++ngtXdQmpjei9sr+KPKi7qs+vHgijU+Gj1eHDUXQ@mail.gmail.com","threadId":"42482","inReplyTo":"574D4448.5020004@alum.mit.edu","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-31T23:12:50Z","receivedAt":"2016-05-31T23:12:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 31, 2016 at 3:59 AM, Michael Haggerty wrote:\n> On 05/31/2016 07:29 AM, Eric Sunshine wrote:\n>> On Mon, May 30, 2016 at 3:55 AM, Michael Haggerty wrote:\n>>> +struct ref_iterator *empty_ref_iterator_begin(void);\n>>> +\n>>> +/*\n>>> + * Return true iff ref_iterator is an empty_ref_iterator.\n>>> + */\n>>> +int is_empty_ref_iterator(struct ref_iterator *ref_iterator);\n>>\n>> I can see that you used this function as an optimization or\n>> convenience in overlay_ref_iterator_begin(), but do you expect it to\n>> be generally useful otherwise? Is it worth publishing? Do you have\n>> other use-cases in mind?\n>\n> It is only \"published\" within the refs module, in refs/refs-internal.h.\n> This header file is not meant to be used by code outside of the refs module.\n\nAh, I forgot about that. In that case, it's probably less of an issue.\n\n> My thinking was that it might be useful to other reference backends. The\n> function is pretty safe for anybody to call, though I admit that it is\n> not very general.\n>\n> I don't have a strong feeling either way. If nobody else chimes in, I'll\n> remove it from the header file as you suggested. We can always add it\n> back if somebody needs it.\n\nI don't feel strongly about it either.\n\n>> Also, can you explain why the merge iterator doesn't also perform the\n>> optimization/convenience of checking if one iterator is an empty\n>> iterator?\n>\n> That's because the merge iterator doesn't know what its select function\n> will do. For example, you could imagine an \"intersect\" select function\n> that only lets through references that were in *both* sub-iterators. In\n> that case, your suggested \"optimization\" would be incorrect.\n\nMakes sense. Thanks for explaining. I wonder if this deserves a\ncomment somewhere in code or commit message to make the situation\nclear to a future developer who might think it a good idea to promote\nthe \"optimization\" to the merge iterator.\n\n>>> +/*\n>>> + * An iterator consisting of the union of the entries from iter0 and\n>>> + * iter1. If there are entries common to the two sub-iterators, use\n>>> + * the one from iter1. Each iterator must iterate over its entries in\n>>> + * strcmp() order by refname for this to work.\n>>> + *\n>>> + * The new iterator takes ownership of its arguments and frees them\n>>> + * when the iteration is over. As a convenience to callers, if iter0\n>>> + * or iter1 is_empty_ref_iterator(), then abort that one immediately\n>>> + * and return the other iterator directly, without wrapping it.\n>>> + */\n>>> +struct ref_iterator *overlay_ref_iterator_begin(struct ref_iterator *iter0,\n>>> + struct ref_iterator *iter1);\n>>\n>> When reading about the overlay iterator (both code and documentation),\n>> my expectation was that iter0 would shadow iter1, not the other way\n>> around as implemented here. Of course, that's entirely subjective, but\n>> the generic names don't provide any useful clues as to which shadows\n>> which. Perhaps giving them more meaningful names would help.\n>\n> That's a good idea. I also found myself having to refer back to the\n> documentation to remind myself which was which.\n>\n> How about I rename them \"back\" and \"front\"? I will also reverse the\n> order of the arguments.\n\nI had a hard time coming up with better names, which is why I didn't\nsuggest any in my review. The best I had was \"shadower\" and\n\"shadowee\", but they are far too similar (and long) for my tastes.\n\"back\" and \"front\" feel a bit off, but are better than anything I\nthought of.\n\nAs for the argument order, I can't explain why my expectation was that\nit would be the other way around, so I certainly don't insist that\nthey be swapped.\n\n> (But I will leave the names \"iter0\" and \"iter1\" in merge_ref_iterator,\n> and also the constants like ITER_SELECT_0, because these don't\n> necessarily have the interpretation of \"back\" and \"front\".)\n\nYes, those names are fine in the merge iterator.\n\n>>> +/*\n>>> + * Wrap iter0, only letting through the references whose names start\n>>> + * with prefix. If trim is set, set iter->refname to the name of the\n>>> + * reference with that many characters trimmed off the front;\n>>> + * otherwise set it to the full refname. The new iterator takes over\n>>> + * ownership of iter0 and frees it when iteration is over. It makes\n>>> + * its own copy of prefix.\n>>> + *\n>>> + * As an convenience to callers, if prefix is the empty string and\n>>> + * trim is zero, this function returns iter0 directly, without\n>>> + * wrapping it.\n>>> + */\n>>> +struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,\n>>> + const char *prefix,\n>>> + int trim);\n>>\n>> Minor: Similarly, when reading the code and documentation, I wondered\n>> why this was named 'iter0' when no 'iter1' was in sight. Perhaps name\n>> it simply 'iter'.\n>\n> I found that it got a little bit confusing, because the constructor and\n> method implementations all use `iter` as a local variable. In particular\n> in the constructor there would want to be an argument \"iter\" and also\n> the local variable \"iter\" for the iterator being constructed, so a new\n> name would otherwise have to be invented for one or the other. Between\n> all the \"iter\" and \"iter\" and \"iter->iter\", I found that naming the\n> sub-iterator \"iter0\" made things a little bit less bewildering.\n>\n> If you don't like that, we could name the embedded iterators something\n> like \"subiter\", \"subiter0\", and \"subiter1\". But the current convention\n> is a bit more succinct so I slightly prefer it.\n\nI'd probably have called the sub-iterator \"child\" (which is the same\nlength as \"iter0\") or \"wrapped\". If you're not interested changing the\nname in the code, perhaps in the header alone you could call it simply\n\"iter\" or omit the name altogether, but this a very minor issue and\nprobably not worth much time or effort to address.\n"},{"id":"287964","messageId":"1464739954.3988.6.camel@twopensource.com","threadId":"42482","inReplyTo":"e99655e5d919852479d223caec42e5bb294b4a2c.1464537050.git.mhagger@alum.mit.edu","subject":"Re: [PATCH 12/13] dir_iterator: new API for iterating over a directory tree","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-06-01T00:12:34Z","receivedAt":"2016-06-01T00:12:34Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2016-05-30 at 09:55 +0200, Michael Haggerty wrote:\n> +struct dir_iterator_level {\n> +\tint initialized;\n> +\n> +\tDIR *dir;\n> +\n> +\t/*\n> +\t * The length of the directory part of refname at this level\n\n\n\"refname\"? Isn't this was for directories?\n\n> +\t\t\tif (lstat(iter->base.path.buf, &iter\n->base.st) < 0)\n> +\t\t\t\tcontinue; /* silently skip */\n\nShould we warn on non-ENOENT errors here?\n\n> +/*\n> + * End the iteration before it has been exhausted. Free the\n> reference\n\ns/reference/directory/\n"},{"id":"288114","messageId":"CACsJy8AZRJ_qa8KHTt_xcX5sDmJ2rCuMd6LpW+MB0MXKErDJQw@mail.gmail.com","threadId":"42482","inReplyTo":"89634d216544d1102dafd5d18247bff2581d48a8.1464537050.git.mhagger@alum.mit.edu","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-02T10:08:17Z","receivedAt":"2016-06-02T10:08:17Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, May 30, 2016 at 2:55 PM, Michael Haggerty wrote:\n> Currently, the API for iterating over references is via a family of\n> for_each_ref()-type functions that invoke a callback function for each\n> selected reference. All of these eventually call do_for_each_ref(),\n> which knows how to do one thing: iterate in parallel through two\n> ref_caches, one for loose and one for packed refs, giving loose\n> references precedence over packed refs. This is rather complicated code,\n> and is quite specialized to the files backend. It also requires callers\n> to encapsulate their work into a callback function, which often means\n> that they have to define and use a \"cb_data\" struct to manage their\n> context.\n>\n> The current design is already bursting at the seams, and will become\n> even more awkward in the upcoming world of multiple reference storage\n> backends:\n>\n> * Per-worktree vs. shared references are currently handled via a kludge\n> in git_path() rather than iterating over each part of the reference\n> namespace separately and merging the results. This kludge will cease\n> to work when we have multiple reference storage backends.\n\nQuestion from a refs user. Right now worktree.c:get_worktrees() peeks\ndirectly to \"$GIT_DIR/worktrees/xxx/HEAD\" and parses the content\nitself, something that I promised to fix but never got around to do\nit. Will we have an iterator to go through all worktrees' HEAD, or\nwill there be an API to say \"resolve ref HEAD from worktree XYZ\"?\n-- \nDuy\n"},{"id":"288134","messageId":"57505D99.9060308@alum.mit.edu","threadId":"42482","inReplyTo":"CACsJy8AZRJ_qa8KHTt_xcX5sDmJ2rCuMd6LpW+MB0MXKErDJQw@mail.gmail.com","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-06-02T16:23:53Z","receivedAt":"2016-06-02T16:23:53Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/02/2016 12:08 PM, Duy Nguyen wrote:\n> On Mon, May 30, 2016 at 2:55 PM, Michael Haggerty wrote:\n>> Currently, the API for iterating over references is via a family of\n>> for_each_ref()-type functions that invoke a callback function for each\n>> selected reference. All of these eventually call do_for_each_ref(),\n>> which knows how to do one thing: iterate in parallel through two\n>> ref_caches, one for loose and one for packed refs, giving loose\n>> references precedence over packed refs. This is rather complicated code,\n>> and is quite specialized to the files backend. It also requires callers\n>> to encapsulate their work into a callback function, which often means\n>> that they have to define and use a \"cb_data\" struct to manage their\n>> context.\n>>\n>> The current design is already bursting at the seams, and will become\n>> even more awkward in the upcoming world of multiple reference storage\n>> backends:\n>>\n>> * Per-worktree vs. shared references are currently handled via a kludge\n>> in git_path() rather than iterating over each part of the reference\n>> namespace separately and merging the results. This kludge will cease\n>> to work when we have multiple reference storage backends.\n> \n> Question from a refs user. Right now worktree.c:get_worktrees() peeks\n> directly to \"$GIT_DIR/worktrees/xxx/HEAD\" and parses the content\n> itself, something that I promised to fix but never got around to do\n> it. Will we have an iterator to go through all worktrees' HEAD, or\n> will there be an API to say \"resolve ref HEAD from worktree XYZ\"?\n\nMy preference is that there is a way to say \"create a ref_store object\nrepresenting the loose references stored physically under\n\"$GIT_DIR/worktrees/xxx\". Then that ref_store could be asked to read its\n`HEAD` (or iterate over all of the refs under that path or whatever).\nYou could even write `HEAD` through the same ref_store.\n\nMichael\n"},{"id":"288225","messageId":"57516E84.6020007@alum.mit.edu","threadId":"42482","inReplyTo":"CAPig+cQte++ngtXdQmpjei9sr+KPKi7qs+vHgijU+Gj1eHDUXQ@mail.gmail.com","subject":"Re: [PATCH 09/13] refs: introduce an iterator interface","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-06-03T11:48:20Z","receivedAt":"2016-06-03T11:48:20Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/01/2016 01:12 AM, Eric Sunshine wrote:\n> On Tue, May 31, 2016 at 3:59 AM, Michael Haggerty wrote:\n>> On 05/31/2016 07:29 AM, Eric Sunshine wrote:\n>>> On Mon, May 30, 2016 at 3:55 AM, Michael Haggerty wrote:\n>>>> +struct ref_iterator *empty_ref_iterator_begin(void);\n>>>> +\n>>>> +/*\n>>>> + * Return true iff ref_iterator is an empty_ref_iterator.\n>>>> + */\n>>>> +int is_empty_ref_iterator(struct ref_iterator *ref_iterator);\n>>>\n>>> I can see that you used this function as an optimization or\n>>> convenience in overlay_ref_iterator_begin(), but do you expect it to\n>>> be generally useful otherwise? Is it worth publishing? Do you have\n>>> other use-cases in mind?\n>>\n>> It is only \"published\" within the refs module, in refs/refs-internal.h.\n>> This header file is not meant to be used by code outside of the refs module.\n> \n> Ah, I forgot about that. In that case, it's probably less of an issue.\n> \n>> My thinking was that it might be useful to other reference backends. The\n>> function is pretty safe for anybody to call, though I admit that it is\n>> not very general.\n>>\n>> I don't have a strong feeling either way. If nobody else chimes in, I'll\n>> remove it from the header file as you suggested. We can always add it\n>> back if somebody needs it.\n> \n> I don't feel strongly about it either.\n\nOK then, I'll leave it as-is.\n\n>>> Also, can you explain why the merge iterator doesn't also perform the\n>>> optimization/convenience of checking if one iterator is an empty\n>>> iterator?\n>>\n>> That's because the merge iterator doesn't know what its select function\n>> will do. For example, you could imagine an \"intersect\" select function\n>> that only lets through references that were in *both* sub-iterators. In\n>> that case, your suggested \"optimization\" would be incorrect.\n> \n> Makes sense. Thanks for explaining. I wonder if this deserves a\n> comment somewhere in code or commit message to make the situation\n> clear to a future developer who might think it a good idea to promote\n> the \"optimization\" to the merge iterator.\n\nGood idea. I'll add a comment.\n\n> [...]\n\nThanks,\nMichael\n"},{"id":"288227","messageId":"575170BD.2050604@alum.mit.edu","threadId":"42482","inReplyTo":"1464739954.3988.6.camel@twopensource.com","subject":"Re: [PATCH 12/13] dir_iterator: new API for iterating over a directory tree","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-06-03T11:57:49Z","receivedAt":"2016-06-03T11:57:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/01/2016 02:12 AM, David Turner wrote:\n> On Mon, 2016-05-30 at 09:55 +0200, Michael Haggerty wrote:\n>> +struct dir_iterator_level {\n>> +\tint initialized;\n>> +\n>> +\tDIR *dir;\n>> +\n>> +\t/*\n>> +\t * The length of the directory part of refname at this level\n> \n> \n> \"refname\"? Isn't this was for directories?\n\nThanks.\n\n>> +\t\t\tif (lstat(iter->base.path.buf, &iter\n> ->base.st) < 0)\n>> +\t\t\t\tcontinue; /* silently skip */\n> \n> Should we warn on non-ENOENT errors here?\n\nGood idea. I'll add that.\n\n>> +/*\n>> + * End the iteration before it has been exhausted. Free the\n>> reference\n> \n> s/reference/directory/\n\nThanks. Will fix.\n\nMichael\n"}]}