{"thread":{"id":"48962","subject":"[RFC PATCH 0/3] Migrate the refs API to take the repository argument","startedAt":"2018-07-27T00:37:18Z","lastAt":"2018-07-31T16:17:47Z","messageCount":14,"participants":["Stefan Beller","Duy Nguyen","Brandon Williams","Jonathan Tan"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"353680","messageId":"20180727003640.16659-1-sbeller@google.com","threadId":"48962","inReplyTo":null,"subject":"[RFC PATCH 0/3] Migrate the refs API to take the repository argument","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-27T00:36:37Z","receivedAt":"2018-07-27T00:37:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The second patch is the real API proposal.\nUnlike the lookup_* series, which caused a lot of integration pain to Junio,\nI plan to structure this in a different way, by having multiple steps:\n\n (1) in this (later to be non-RFC) series, add the new API that passes thru\n     the repository; for now do not replace refs_store argument by\n     struct repository.\n (2) the last patch is a demo of converting one of the callers over\n     to the new API; this would need to be done for all of them\n     \n (3) After some time do a cleanup series to remove callers of the\n     old API fromly introduced series that are currently in flight.\n (4) Remove the old API.\n\n (5) Introduce the final API removing the refs_store\n (6) convert all callers to the final API, using this same dual step approach\n (7) remove this API\n \nSteps 1,2 will be done in this series (2 is done only as demo here\nfor one function, but the non-RFC would do it all)\n\nSteps 3,4 would be done once there are no more series in flight using\nthe old API.\n\nBefore continuing on step (2), I would want to ask for your thoughts\nof (1).\n\nAlso note that after step (1) before (4) refs.h looks messy as well as\nbetween (5) and (7).\n\nThanks,\nStefan\n\n\nStefan Beller (3):\n  refs.c: migrate internal ref iteration to pass thru repository\n    argument\n  refs: introduce new API, wrap old API shallowly around new API\n  replace: migrate to for_each_replace_repo_ref\n\n builtin/replace.c    |   9 +-\n refs.c               | 187 ++++++++++++++--------\n refs.h               | 362 ++++++++++++++++++++++++++++++++++++++-----\n refs/iterator.c      |   6 +-\n refs/refs-internal.h |   5 +-\n replace-object.c     |   7 +-\n 6 files changed, 464 insertions(+), 112 deletions(-)\n\n-- \n2.18.0.345.g5c9ce644c3-goog\n\n"},{"id":"353681","messageId":"20180727003640.16659-2-sbeller@google.com","threadId":"48962","inReplyTo":"20180727003640.16659-1-sbeller@google.com","subject":"[PATCH 1/3] refs.c: migrate internal ref iteration to pass thru repository argument","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-27T00:36:38Z","receivedAt":"2018-07-27T00:37:20Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In 60ce76d3581 (refs: add repository argument to for_each_replace_ref,\n2018-04-11) and 0d296c57aec (refs: allow for_each_replace_ref to handle\narbitrary repositories, 2018-04-11), for_each_replace_ref learned how\nto iterate over refs by a given arbitrary repository.\nNew attempts in the object store conversion have shown that it is useful\nto have the repository handle available that the refs iteration is\ncurrently iterating over.\n\nTo achieve this goal we will need to add a repository argument to\neach_ref_fn in refs.h. However as many callers rely on the signature\nsuch a patch would be too large.\n\nSo convert the internals of the ref subsystem first to pass through a\nrepository argument without exposing the change to the user. Assume\nthe_repository for the passed through repository, although it is not\nused anywhere yet.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c               | 39 +++++++++++++++++++++++++++++++++++++--\n refs.h               | 10 ++++++++++\n refs/iterator.c      |  6 +++---\n refs/refs-internal.h |  5 +++--\n 4 files changed, 53 insertions(+), 7 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex fcfd3171e83..2513f77acb3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1390,17 +1390,50 @@ struct ref_iterator *refs_ref_iterator_begin(\n  * non-zero value, stop the iteration and return that value;\n  * otherwise, return 0.\n  */\n+static int do_for_each_repo_ref(struct repository *r, const char *prefix,\n+\t\t\t\teach_repo_ref_fn fn, int trim, int flags,\n+\t\t\t\tvoid *cb_data)\n+{\n+\tstruct ref_iterator *iter;\n+\tstruct ref_store *refs = get_main_ref_store(r);\n+\n+\tif (!refs)\n+\t\treturn 0;\n+\n+\titer = refs_ref_iterator_begin(refs, prefix, trim, flags);\n+\n+\treturn do_for_each_repo_ref_iterator(r, iter, fn, cb_data);\n+}\n+\n+struct do_for_each_ref_help {\n+\teach_ref_fn *fn;\n+\tvoid *cb_data;\n+};\n+\n+static int do_for_each_ref_helper(struct repository *r,\n+\t\t\t\t  const char *refname,\n+\t\t\t\t  const struct object_id *oid,\n+\t\t\t\t  int flags,\n+\t\t\t\t  void *cb_data)\n+{\n+\tstruct do_for_each_ref_help *hp = cb_data;\n+\n+\treturn hp->fn(refname, oid, flags, hp->cb_data);\n+}\n+\n static int do_for_each_ref(struct ref_store *refs, 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+\tstruct do_for_each_ref_help hp = { fn, cb_data };\n \n \tif (!refs)\n \t\treturn 0;\n \n \titer = refs_ref_iterator_begin(refs, prefix, trim, flags);\n \n-\treturn do_for_each_ref_iterator(iter, fn, cb_data);\n+\treturn do_for_each_repo_ref_iterator(the_repository, iter,\n+\t\t\t\t\tdo_for_each_ref_helper, &hp);\n }\n \n int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n@@ -2029,10 +2062,12 @@ int refs_verify_refname_available(struct ref_store *refs,\n int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n {\n \tstruct ref_iterator *iter;\n+\tstruct do_for_each_ref_help hp = { fn, cb_data };\n \n \titer = refs->be->reflog_iterator_begin(refs);\n \n-\treturn do_for_each_ref_iterator(iter, fn, cb_data);\n+\treturn do_for_each_repo_ref_iterator(the_repository, iter,\n+\t\t\t\t\t     do_for_each_ref_helper, &hp);\n }\n \n int for_each_reflog(each_ref_fn fn, void *cb_data)\ndiff --git a/refs.h b/refs.h\nindex cc2fb4c68c0..80eec8bbc68 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -274,6 +274,16 @@ struct ref_transaction;\n typedef int each_ref_fn(const char *refname,\n \t\t\tconst struct object_id *oid, int flags, void *cb_data);\n \n+/*\n+ * The same as each_ref_fn, but also with a repository argument that\n+ * contains the repository associated with the callback.\n+ */\n+typedef int each_repo_ref_fn(struct repository *r,\n+\t\t\t     const char *refname,\n+\t\t\t     const struct object_id *oid,\n+\t\t\t     int flags,\n+\t\t\t     void *cb_data);\n+\n /*\n  * The following functions invoke the specified callback function for\n  * each reference indicated.  If the function ever returns a nonzero\ndiff --git a/refs/iterator.c b/refs/iterator.c\nindex 2ac91ac3401..629e00a122a 100644\n--- a/refs/iterator.c\n+++ b/refs/iterator.c\n@@ -407,15 +407,15 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,\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+int do_for_each_repo_ref_iterator(struct repository *r, struct ref_iterator *iter,\n+\t\t\t\t  each_repo_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\tretval = fn(r, 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,\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex dd834314bd8..5c7414bf099 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -473,8 +473,9 @@ extern struct ref_iterator *current_ref_iter;\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+int do_for_each_repo_ref_iterator(struct repository *r,\n+\t\t\t\t  struct ref_iterator *iter,\n+\t\t\t\t  each_repo_ref_fn fn, void *cb_data);\n \n /*\n  * Only include per-worktree refs in a do_for_each_ref*() iteration.\n-- \n2.18.0.345.g5c9ce644c3-goog\n\n"},{"id":"353682","messageId":"20180727003640.16659-3-sbeller@google.com","threadId":"48962","inReplyTo":"20180727003640.16659-1-sbeller@google.com","subject":"[PATCH 2/3] refs: introduce new API, wrap old API shallowly around new API","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-27T00:36:39Z","receivedAt":"2018-07-27T00:37:22Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Currently the refs API takes a 'ref_store' as an argument to specify\nwhich ref store to iterate over; however it is more useful to specify\nthe repository instead (or later a specific worktree of a repository).\n\nIntroduce a new API, that takes a repository struct instead of a ref store;\nthe repository argument is also passed through to the callback, which is\nof type 'each_repo_ref_fn' that is introduced in a previous patch and is\nan extension of the 'each_ref_fn' type with the additional repository\nargument.\n\nWe wrap the old API as in a very shallow way around the new API,\nby wrapping the callback and the callback data into a new callback\nto translate between the 'each_ref_fn' and 'each_repo_ref_fn' type.\n\nThe wrapping implementation could be done either in refs.c or as presented\nin this patch as a 'static inline' in the header file itself. This has the\nadvantage that the line of the old API is changed (and not just its\nimplementation in refs.c), such that it will show up in git-blame.\n\nThe new API is not perfect yet, as some of them take both a 'repository'\nand 'ref_store' argument. This is done for an easy migration:\nIf the ref_store argument is non-NULL, prefer it over the repository\nto compute which refs to iterate over. That way we can ensure that this\nstep of API migration doesn't confuse which ref store to work on.\n\nOnce all callers have migrated to this newly introduced API, we can\nget rid of the old API; a second migration step in the future will remove\nthe then useless ref_store argument\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c | 158 +++++++++++++++-----------\n refs.h | 352 +++++++++++++++++++++++++++++++++++++++++++++++++++------\n 2 files changed, 407 insertions(+), 103 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 2513f77acb3..27e3772fca9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -217,7 +217,7 @@ char *resolve_refdup(const char *refname, int resolve_flags,\n /* The argument to filter_refs */\n struct ref_filter {\n \tconst char *pattern;\n-\teach_ref_fn *fn;\n+\teach_repo_ref_fn *fn;\n \tvoid *cb_data;\n };\n \n@@ -289,14 +289,15 @@ int ref_filter_match(const char *refname,\n \treturn 1;\n }\n \n-static int filter_refs(const char *refname, const struct object_id *oid,\n-\t\t\t   int flags, void *data)\n+static int filter_refs(struct repository *r,\n+\t\t       const char *refname, const struct object_id *oid,\n+\t\t       int flags, void *data)\n {\n \tstruct ref_filter *filter = (struct ref_filter *)data;\n \n \tif (wildmatch(filter->pattern, refname, 0))\n \t\treturn 0;\n-\treturn filter->fn(refname, oid, flags, filter->cb_data);\n+\treturn filter->fn(r, refname, oid, flags, filter->cb_data);\n }\n \n enum peel_status peel_object(const struct object_id *name, struct object_id *oid)\n@@ -371,46 +372,50 @@ void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct string_li\n \tfor_each_rawref(warn_if_dangling_symref, &data);\n }\n \n-int refs_for_each_tag_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n+int refs_for_each_tag_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn refs_for_each_ref_in(refs, \"refs/tags/\", fn, cb_data);\n+\treturn refs_for_each_repo_ref_in(r, NULL, \"refs/tags/\", fn, cb_data);\n }\n \n-int for_each_tag_ref(each_ref_fn fn, void *cb_data)\n+int for_each_tag_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn refs_for_each_tag_ref(get_main_ref_store(the_repository), fn, cb_data);\n+\treturn refs_for_each_tag_repo_ref(r, fn, cb_data);\n }\n \n-int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n+int refs_for_each_branch_repo_ref(struct repository *r, struct ref_store *refs,\n+\t\t\t\t  each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn refs_for_each_ref_in(refs, \"refs/heads/\", fn, cb_data);\n+\treturn refs_for_each_repo_ref_in(r, refs, \"refs/heads/\", fn, cb_data);\n }\n \n-int for_each_branch_ref(each_ref_fn fn, void *cb_data)\n+int for_each_branch_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn refs_for_each_branch_ref(get_main_ref_store(the_repository), fn, cb_data);\n+\treturn refs_for_each_branch_repo_ref(r, NULL, fn, cb_data);\n }\n \n-int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n+int refs_for_each_remote_repo_ref(struct repository *r, struct ref_store *refs,\n+\t\t\t\t  each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn refs_for_each_ref_in(refs, \"refs/remotes/\", fn, cb_data);\n+\treturn refs_for_each_repo_ref_in(r, refs, \"refs/remotes/\", fn, cb_data);\n }\n \n-int for_each_remote_ref(each_ref_fn fn, void *cb_data)\n+int for_each_remote_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn refs_for_each_remote_ref(get_main_ref_store(the_repository), fn, cb_data);\n+\treturn refs_for_each_remote_repo_ref(the_repository, NULL, fn, cb_data);\n }\n \n-int head_ref_namespaced(each_ref_fn fn, void *cb_data)\n+int head_repo_ref_namespaced(struct repository *r, each_repo_ref_fn fn, void *cb_data)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tint ret = 0;\n \tstruct object_id oid;\n \tint flag;\n+\tstruct ref_store *refs = get_main_ref_store(r);\n \n \tstrbuf_addf(&buf, \"%sHEAD\", get_git_namespace());\n-\tif (!read_ref_full(buf.buf, RESOLVE_REF_READING, &oid, &flag))\n-\t\tret = fn(buf.buf, &oid, flag, cb_data);\n+\n+\tif (!refs_read_ref_full(refs, buf.buf, RESOLVE_REF_READING, &oid, &flag))\n+\t\tret = fn(r, buf.buf, &oid, flag, cb_data);\n \tstrbuf_release(&buf);\n \n \treturn ret;\n@@ -437,8 +442,8 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,\n \tstrbuf_release(&normalized_pattern);\n }\n \n-int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n-\tconst char *prefix, void *cb_data)\n+int for_each_glob_repo_ref_in(struct repository *r, each_repo_ref_fn fn,\n+\tconst char *pattern, const char *prefix, void *cb_data)\n {\n \tstruct strbuf real_pattern = STRBUF_INIT;\n \tstruct ref_filter filter;\n@@ -460,15 +465,16 @@ int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n \tfilter.pattern = real_pattern.buf;\n \tfilter.fn = fn;\n \tfilter.cb_data = cb_data;\n-\tret = for_each_ref(filter_refs, &filter);\n+\tret = for_each_repo_ref(r, filter_refs, &filter);\n \n \tstrbuf_release(&real_pattern);\n \treturn ret;\n }\n \n-int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data)\n+int for_each_glob_repo_ref(struct repository *r, each_repo_ref_fn fn,\n+\t\t\t   const char *pattern, void *cb_data)\n {\n-\treturn for_each_glob_ref_in(fn, pattern, NULL, cb_data);\n+\treturn for_each_glob_repo_ref_in(r, fn, pattern, NULL, cb_data);\n }\n \n const char *prettify_refname(const char *name)\n@@ -1337,21 +1343,25 @@ int refs_rename_ref_available(struct ref_store *refs,\n \treturn ok;\n }\n \n-int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n+int refs_head_repo_ref(struct repository *r, struct ref_store *refs,\n+\t\t       each_repo_ref_fn fn, void *cb_data)\n {\n \tstruct object_id oid;\n \tint flag;\n \n+\tif (!refs)\n+\t\trefs = get_main_ref_store(r);\n+\n \tif (!refs_read_ref_full(refs, \"HEAD\", RESOLVE_REF_READING,\n \t\t\t\t&oid, &flag))\n-\t\treturn fn(\"HEAD\", &oid, flag, cb_data);\n+\t\treturn fn(r, \"HEAD\", &oid, flag, cb_data);\n \n \treturn 0;\n }\n \n-int head_ref(each_ref_fn fn, void *cb_data)\n+int head_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn refs_head_ref(get_main_ref_store(the_repository), fn, cb_data);\n+\treturn refs_head_repo_ref(r, NULL, fn, cb_data);\n }\n \n struct ref_iterator *refs_ref_iterator_begin(\n@@ -1390,12 +1400,15 @@ struct ref_iterator *refs_ref_iterator_begin(\n  * non-zero value, stop the iteration and return that value;\n  * otherwise, return 0.\n  */\n-static int do_for_each_repo_ref(struct repository *r, const char *prefix,\n+static int do_for_each_repo_ref(struct repository *r, struct ref_store *refs,\n+\t\t\t\tconst char *prefix,\n \t\t\t\teach_repo_ref_fn fn, int trim, int flags,\n \t\t\t\tvoid *cb_data)\n {\n \tstruct ref_iterator *iter;\n-\tstruct ref_store *refs = get_main_ref_store(r);\n+\n+\tif (!refs)\n+\t\trefs = get_main_ref_store(r);\n \n \tif (!refs)\n \t\treturn 0;\n@@ -1405,6 +1418,15 @@ static int do_for_each_repo_ref(struct repository *r, const char *prefix,\n \treturn do_for_each_repo_ref_iterator(r, iter, fn, cb_data);\n }\n \n+int each_ref_fn_repository_wrapped(struct repository *r,\n+\t\t\t\t   const char *refname,\n+\t\t\t\t   const struct object_id *oid,\n+\t\t\t\t   int flags, void *cb_data)\n+{\n+\tstruct each_ref_fn_repository_wrapper *cb = cb_data;\n+\treturn cb->fn(refname, oid, flags, cb->cb);\n+}\n+\n struct do_for_each_ref_help {\n \teach_ref_fn *fn;\n \tvoid *cb_data;\n@@ -1436,76 +1458,78 @@ static int do_for_each_ref(struct ref_store *refs, const char *prefix,\n \t\t\t\t\tdo_for_each_ref_helper, &hp);\n }\n \n-int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n+int refs_for_each_repo_ref(struct repository *r, struct ref_store *refs,\n+\t\t\t   each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(refs, \"\", fn, 0, 0, cb_data);\n+\treturn do_for_each_repo_ref(r, refs, \"\", fn, 0, 0, cb_data);\n }\n \n-int for_each_ref(each_ref_fn fn, void *cb_data)\n+int for_each_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn refs_for_each_ref(get_main_ref_store(the_repository), fn, cb_data);\n+\treturn refs_for_each_repo_ref(r, NULL, fn, cb_data);\n }\n \n-int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,\n-\t\t\t each_ref_fn fn, void *cb_data)\n+int refs_for_each_repo_ref_in(struct repository *r, struct ref_store *refs,\n+\t\t\t      const char *prefix, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(refs, prefix, fn, strlen(prefix), 0, cb_data);\n+\treturn do_for_each_repo_ref(r, refs, prefix, fn, strlen(prefix), 0, cb_data);\n }\n \n-int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)\n+int for_each_repo_ref_in(struct repository *r, const char *prefix, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn refs_for_each_ref_in(get_main_ref_store(the_repository), prefix, fn, cb_data);\n+\treturn refs_for_each_repo_ref_in(r, NULL, prefix, fn, cb_data);\n }\n \n-int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)\n+int refs_for_each_full_repo_ref_in(struct repository *r, struct ref_store *refs,\n+\t\t\t\t   const char *prefix,\n+\t\t\t\t   each_repo_ref_fn fn, void *cb_data,\n+\t\t\t\t   unsigned int broken)\n {\n \tunsigned int flag = 0;\n \n \tif (broken)\n \t\tflag = DO_FOR_EACH_INCLUDE_BROKEN;\n-\treturn do_for_each_ref(get_main_ref_store(the_repository),\n-\t\t\t       prefix, fn, 0, flag, cb_data);\n+\treturn do_for_each_repo_ref(r, refs, prefix, fn, 0, flag, cb_data);\n }\n \n-int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,\n-\t\t\t     each_ref_fn fn, void *cb_data,\n-\t\t\t     unsigned int broken)\n+int for_each_full_repo_ref_in(struct repository *r, const char *prefix,\n+\t\t\t      each_repo_ref_fn fn, void *cb_data,\n+\t\t\t      unsigned int broken)\n {\n \tunsigned int flag = 0;\n \n \tif (broken)\n \t\tflag = DO_FOR_EACH_INCLUDE_BROKEN;\n-\treturn do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);\n+\treturn do_for_each_repo_ref(r, NULL, prefix, fn, 0, flag, cb_data);\n }\n \n-int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)\n+int for_each_replace_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(get_main_ref_store(r),\n-\t\t\t       git_replace_ref_base, fn,\n-\t\t\t       strlen(git_replace_ref_base),\n-\t\t\t       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\n+\treturn do_for_each_repo_ref(r, NULL, git_replace_ref_base, fn,\n+\t\t\t\t    strlen(git_replace_ref_base),\n+\t\t\t\t    DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\n }\n \n-int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)\n+int for_each_namespaced_repo_ref(struct repository *r,\n+\t\t\t\t each_repo_ref_fn fn, void *cb_data)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tint ret;\n \tstrbuf_addf(&buf, \"%srefs/\", get_git_namespace());\n-\tret = do_for_each_ref(get_main_ref_store(the_repository),\n-\t\t\t      buf.buf, fn, 0, 0, cb_data);\n+\tret = do_for_each_repo_ref(r, NULL, buf.buf, fn, 0, 0, cb_data);\n \tstrbuf_release(&buf);\n \treturn ret;\n }\n \n-int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n+int refs_for_each_raw_repo_ref(struct repository *r, struct ref_store *refs, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(refs, \"\", fn, 0,\n-\t\t\t       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\n+\treturn do_for_each_repo_ref(r, refs, \"\", fn, 0,\n+\t\t\t\t    DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\n }\n \n-int for_each_rawref(each_ref_fn fn, void *cb_data)\n+int for_each_raw_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)\n {\n-\treturn refs_for_each_rawref(get_main_ref_store(the_repository), fn, cb_data);\n+\treturn refs_for_each_raw_repo_ref(r, NULL, fn, cb_data);\n }\n \n int refs_read_raw_ref(struct ref_store *ref_store,\n@@ -2059,20 +2083,18 @@ int refs_verify_refname_available(struct ref_store *refs,\n \treturn ret;\n }\n \n-int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n+int refs_for_each_repo_reflog(struct repository *r, struct ref_store *refs,\n+\t\t\t      each_repo_ref_fn fn, void *cb_data)\n {\n \tstruct ref_iterator *iter;\n-\tstruct do_for_each_ref_help hp = { fn, cb_data };\n \n-\titer = refs->be->reflog_iterator_begin(refs);\n+\tif (!refs)\n+\t\trefs = get_main_ref_store(r);\n \n-\treturn do_for_each_repo_ref_iterator(the_repository, iter,\n-\t\t\t\t\t     do_for_each_ref_helper, &hp);\n-}\n+\titer = refs->be->reflog_iterator_begin(refs);\n \n-int for_each_reflog(each_ref_fn fn, void *cb_data)\n-{\n-\treturn refs_for_each_reflog(get_main_ref_store(the_repository), fn, cb_data);\n+\treturn do_for_each_repo_ref_iterator(r, iter,\n+\t\t\t\t\t     fn, cb_data);\n }\n \n int refs_for_each_reflog_ent_reverse(struct ref_store *refs,\ndiff --git a/refs.h b/refs.h\nindex 80eec8bbc68..ba50fb9152d 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -284,6 +284,16 @@ typedef int each_repo_ref_fn(struct repository *r,\n \t\t\t     int flags,\n \t\t\t     void *cb_data);\n \n+struct each_ref_fn_repository_wrapper {\n+\teach_ref_fn *fn;\n+\tvoid *cb;\n+};\n+\n+int each_ref_fn_repository_wrapped(struct repository *r,\n+\t\t\t\t   const char *refname,\n+\t\t\t\t   const struct object_id *oid,\n+\t\t\t\t   int flags, void *cb_data);\n+\n /*\n  * The following functions invoke the specified callback function for\n  * each reference indicated.  If the function ever returns a nonzero\n@@ -293,41 +303,292 @@ typedef int each_repo_ref_fn(struct repository *r,\n  * modifies the reference also returns a nonzero value to immediately\n  * stop the iteration. Returned references are sorted.\n  */\n-int refs_head_ref(struct ref_store *refs,\n-\t\t  each_ref_fn fn, void *cb_data);\n-int refs_for_each_ref(struct ref_store *refs,\n-\t\t      each_ref_fn fn, void *cb_data);\n-int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,\n-\t\t\t each_ref_fn fn, void *cb_data);\n-int refs_for_each_tag_ref(struct ref_store *refs,\n-\t\t\t  each_ref_fn fn, void *cb_data);\n-int refs_for_each_branch_ref(struct ref_store *refs,\n-\t\t\t     each_ref_fn fn, void *cb_data);\n-int refs_for_each_remote_ref(struct ref_store *refs,\n-\t\t\t     each_ref_fn fn, void *cb_data);\n-\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 refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,\n-\t\t\t     each_ref_fn fn, void *cb_data,\n-\t\t\t     unsigned int broken);\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(struct repository *r, 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-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+int refs_head_repo_ref(struct repository *r, struct ref_store *refs,\n+\t\t       each_repo_ref_fn fn, void *cb_data);\n+static inline int refs_head_ref(struct ref_store *refs,\n+\t\t  each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn refs_head_repo_ref(the_repository, refs,\n+\t\t\t\t  each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int refs_for_each_repo_ref(struct repository *r, struct ref_store *refs,\n+\t\t      each_repo_ref_fn fn, void *cb_data);\n+static inline int refs_for_each_ref(struct ref_store *refs,\n+\t\t      each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn refs_for_each_repo_ref(the_repository, refs,\n+\t\t\t\t      each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int refs_for_each_repo_ref_in(struct repository *r, struct ref_store *refs,\n+\t\t\t const char *prefix, each_repo_ref_fn fn, void *cb_data);\n+static inline int refs_for_each_ref_in(struct ref_store *refs,\n+\t\t\t\t       const char *prefix,\n+\t\t\t\t       each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn refs_for_each_repo_ref_in(the_repository, refs, prefix,\n+\t\t\t\t\t each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int refs_for_each_tag_repo_ref(struct repository *r,\n+\t\t\t       each_repo_ref_fn fn, void *cb_data);\n+static inline int refs_for_each_tag_ref(struct ref_store *refs,\n+\t\t\t\t each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn refs_for_each_tag_repo_ref(the_repository,\n+\t\t\t\t\t  each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int refs_for_each_branch_repo_ref(struct repository *r, struct ref_store *refs,\n+\t\t\t\t  each_repo_ref_fn fn,\n+\t\t\t\t  void *cb_data);\n+static inline int refs_for_each_branch_ref(struct ref_store *refs,\n+\t\t\t\t\t   each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn refs_for_each_branch_repo_ref(the_repository, refs,\n+\t\t\t\t\t     each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int refs_for_each_remote_repo_ref(struct repository *r, struct ref_store *refs,\n+\t\t\t\t  each_repo_ref_fn fn,\n+\t\t\t\t  void *cb_data);\n+static inline int refs_for_each_remote_ref(struct ref_store *refs,\n+\t\t\t\t\t   each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn refs_for_each_remote_repo_ref(the_repository, refs,\n+\t\t\t\t\t     each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int head_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data);\n+static inline int head_ref(each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn head_repo_ref(the_repository,\n+\t\t\t     each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int for_each_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data);\n+static inline int for_each_ref(each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_repo_ref(the_repository,\n+\t\t\t\t each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int for_each_repo_ref_in(struct repository *r, const char *prefix,\n+\t\t\t each_repo_ref_fn fn, void *cb_data);\n+static inline int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_repo_ref_in(the_repository, prefix,\n+\t\t\t\t    each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int refs_for_each_full_repo_ref_in(struct repository *r, struct ref_store *refs,\n+\t\t\t\t   const char *prefix,\n+\t\t\t\t   each_repo_ref_fn fn, void *cb_data,\n+\t\t\t\t   unsigned int broken);\n+static inline int refs_for_each_fullref_in(struct ref_store *refs,\n+\t\t\t\t\t   const char *prefix, each_ref_fn fn,\n+\t\t\t\t\t   void *cb_data, unsigned int broken)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn refs_for_each_full_repo_ref_in(the_repository, refs, prefix,\n+\t\t\t\t\t      each_ref_fn_repository_wrapped,\n+\t\t\t\t\t      &cb, broken);\n+}\n+\n+int for_each_full_repo_ref_in(struct repository *r, const char *prefix,\n+\t\t\t      each_repo_ref_fn fn, void *cb_data,\n+\t\t\t      unsigned int broken);\n+static inline int for_each_fullref_in(const char *prefix, each_ref_fn fn,\n+\t\t\t\t      void *cb_data, unsigned int broken)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_full_repo_ref_in(the_repository, prefix,\n+\t\t\t\t\t      each_ref_fn_repository_wrapped,\n+\t\t\t\t\t      &cb, broken);\n+}\n+\n+int for_each_tag_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data);\n+static inline int for_each_tag_ref(each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_tag_repo_ref(the_repository,\n+\t\t\t\t     each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int for_each_branch_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data);\n+static inline int for_each_branch_ref(each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_branch_repo_ref(the_repository,\n+\t\t\teach_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int for_each_remote_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb);\n+static inline int for_each_remote_ref(each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_remote_repo_ref(the_repository,\n+\t\t\t\t\teach_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int for_each_replace_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data);\n+static inline int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_replace_repo_ref(r, each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int for_each_glob_repo_ref(struct repository *r, each_repo_ref_fn fn,\n+\t\t\t   const char *pattern, void *cb_data);\n+static inline int for_each_glob_ref(each_ref_fn fn, const char *pattern,\n+\t\t\t\t    void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_glob_repo_ref(the_repository,\n+\t\t\t\t      each_ref_fn_repository_wrapped,\n+\t\t\t\t      pattern, &cb);\n+}\n+\n+int for_each_glob_repo_ref_in(struct repository *r,\n+\t\t\t      each_repo_ref_fn fn, const char *pattern,\n+\t\t\t      const char *prefix, void *cb_data);\n+static inline 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+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_glob_repo_ref_in(the_repository,\n+\t\t\t\t\t each_ref_fn_repository_wrapped,\n+\t\t\t\t\t pattern, prefix, &cb);\n+}\n+\n+int head_repo_ref_namespaced(struct repository *r, each_repo_ref_fn fn, void *cb_data);\n+static inline int head_ref_namespaced(each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn head_repo_ref_namespaced(the_repository,\n+\t\t\t\t\teach_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int for_each_namespaced_repo_ref(struct repository *r, each_repo_ref_fn fn,\n+\t\t\t\t void *cb_data);\n+static inline int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_namespaced_repo_ref(the_repository,\n+\t\t\t\t\t    each_ref_fn_repository_wrapped, &cb);\n+}\n \n /* can be used to learn about broken ref and symref */\n-int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data);\n-int for_each_rawref(each_ref_fn fn, void *cb_data);\n+int refs_for_each_raw_repo_ref(struct repository *r, struct ref_store *refs,\n+\t\t\t       each_repo_ref_fn fn, void *cb_data);\n+static inline int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn refs_for_each_raw_repo_ref(the_repository, refs,\n+\t\t\t\t\t  each_ref_fn_repository_wrapped, &cb);\n+}\n+\n+int for_each_raw_repo_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data);\n+static inline int for_each_rawref(each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn for_each_raw_repo_ref(the_repository,\n+\t\t\t\t     each_ref_fn_repository_wrapped, &cb);\n+}\n \n /*\n  * Normalizes partial refs to their fully qualified form.\n@@ -442,8 +703,29 @@ 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. Reflog file order is unspecified.\n  */\n-int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data);\n-int for_each_reflog(each_ref_fn fn, void *cb_data);\n+int refs_for_each_repo_reflog(struct repository *r, struct ref_store *refs,\n+\t\t\t each_repo_ref_fn fn, void *cb_data);\n+static inline int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn,\n+\t\t\t\t       void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn refs_for_each_repo_reflog(the_repository, refs,\n+\t\t\t\t\t each_ref_fn_repository_wrapped, &cb);\n+}\n+static inline int for_each_reflog(each_ref_fn fn, void *cb_data)\n+{\n+\t/*\n+\t * NEEDSWORK: remove this function when there are no\n+\t * series in flight using this function.\n+\t */\n+\tstruct each_ref_fn_repository_wrapper cb = {fn, cb_data};\n+\treturn refs_for_each_repo_reflog(the_repository, NULL,\n+\t\t\t\t\t each_ref_fn_repository_wrapped, &cb);\n+}\n \n #define REFNAME_ALLOW_ONELEVEL 1\n #define REFNAME_REFSPEC_PATTERN 2\n-- \n2.18.0.345.g5c9ce644c3-goog\n\n"},{"id":"353683","messageId":"20180727003640.16659-4-sbeller@google.com","threadId":"48962","inReplyTo":"20180727003640.16659-1-sbeller@google.com","subject":"[PATCH 3/3] replace: migrate to for_each_replace_repo_ref","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-27T00:36:40Z","receivedAt":"2018-07-27T00:37:25Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"By upgrading the replace mechanism works well for all repositories\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/replace.c | 9 +++++----\n replace-object.c  | 7 ++++---\n 2 files changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex ef22d724bbc..fd8a935eb77 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -39,7 +39,8 @@ struct show_data {\n \tenum replace_format format;\n };\n \n-static int show_reference(const char *refname, const struct object_id *oid,\n+static int show_reference(struct repository *r, const char *refname,\n+\t\t\t  const struct object_id *oid,\n \t\t\t  int flag, void *cb_data)\n {\n \tstruct show_data *data = cb_data;\n@@ -56,9 +57,9 @@ static int show_reference(const char *refname, const struct object_id *oid,\n \t\t\tif (get_oid(refname, &object))\n \t\t\t\treturn error(\"Failed to resolve '%s' as a valid ref.\", refname);\n \n-\t\t\tobj_type = oid_object_info(the_repository, &object,\n+\t\t\tobj_type = oid_object_info(r, &object,\n \t\t\t\t\t\t   NULL);\n-\t\t\trepl_type = oid_object_info(the_repository, oid, NULL);\n+\t\t\trepl_type = oid_object_info(r, oid, NULL);\n \n \t\t\tprintf(\"%s (%s) -> %s (%s)\\n\", refname, type_name(obj_type),\n \t\t\t       oid_to_hex(oid), type_name(repl_type));\n@@ -87,7 +88,7 @@ static int list_replace_refs(const char *pattern, const char *format)\n \t\t\t     \"valid formats are 'short', 'medium' and 'long'\\n\",\n \t\t\t     format);\n \n-\tfor_each_replace_ref(the_repository, show_reference, (void *)&data);\n+\tfor_each_replace_repo_ref(the_repository, show_reference, (void *)&data);\n \n \treturn 0;\n }\ndiff --git a/replace-object.c b/replace-object.c\nindex 801b5c16789..c0457b8048c 100644\n--- a/replace-object.c\n+++ b/replace-object.c\n@@ -6,7 +6,8 @@\n #include \"repository.h\"\n #include \"commit.h\"\n \n-static int register_replace_ref(const char *refname,\n+static int register_replace_ref(struct repository *r,\n+\t\t\t\tconst char *refname,\n \t\t\t\tconst struct object_id *oid,\n \t\t\t\tint flag, void *cb_data)\n {\n@@ -25,7 +26,7 @@ static int register_replace_ref(const char *refname,\n \toidcpy(&repl_obj->replacement, oid);\n \n \t/* Register new object */\n-\tif (oidmap_put(the_repository->objects->replace_map, repl_obj))\n+\tif (oidmap_put(r->objects->replace_map, repl_obj))\n \t\tdie(\"duplicate replace ref: %s\", refname);\n \n \treturn 0;\n@@ -40,7 +41,7 @@ static void prepare_replace_object(struct repository *r)\n \t\txmalloc(sizeof(*r->objects->replace_map));\n \toidmap_init(r->objects->replace_map, 0);\n \n-\tfor_each_replace_ref(r, register_replace_ref, NULL);\n+\tfor_each_replace_repo_ref(r, register_replace_ref, NULL);\n }\n \n /* We allow \"recursive\" replacement. Only within reason, though */\n-- \n2.18.0.345.g5c9ce644c3-goog\n\n"},{"id":"353725","messageId":"CACsJy8Ae3sZvOQ3irQM+hv0fCRchGi8995kvLZBadbaphRo-3A@mail.gmail.com","threadId":"48962","inReplyTo":"20180727003640.16659-3-sbeller@google.com","subject":"Re: [PATCH 2/3] refs: introduce new API, wrap old API shallowly around new API","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-07-27T16:07:50Z","receivedAt":"2018-07-27T16:08:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jul 27, 2018 at 2:40 AM Stefan Beller <sbeller@google.com> wrote:\n>\n> Currently the refs API takes a 'ref_store' as an argument to specify\n> which ref store to iterate over; however it is more useful to specify\n> the repository instead (or later a specific worktree of a repository).\n\nThere is no 'later'. worktrees.c already passes a worktree specific\nref store. If you make this move you have to also design a way to give\na specific ref store now.\n\nFrankly I still dislike the decision to pass repo everywhere,\nespecially when refs code already has a nice ref-store abstraction.\nSome people frown upon back pointers. But I think adding a back\npointer in ref-store, pointing back to the repository is the right\nmove.\n-- \nDuy\n"},{"id":"353730","messageId":"20180727171941.GA109508@google.com","threadId":"48962","inReplyTo":"CACsJy8Ae3sZvOQ3irQM+hv0fCRchGi8995kvLZBadbaphRo-3A@mail.gmail.com","subject":"Re: [PATCH 2/3] refs: introduce new API, wrap old API shallowly around new API","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-07-27T17:19:41Z","receivedAt":"2018-07-27T17:19:45Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 07/27, Duy Nguyen wrote:\n> On Fri, Jul 27, 2018 at 2:40 AM Stefan Beller <sbeller@google.com> wrote:\n> >\n> > Currently the refs API takes a 'ref_store' as an argument to specify\n> > which ref store to iterate over; however it is more useful to specify\n> > the repository instead (or later a specific worktree of a repository).\n> \n> There is no 'later'. worktrees.c already passes a worktree specific\n> ref store. If you make this move you have to also design a way to give\n> a specific ref store now.\n> \n> Frankly I still dislike the decision to pass repo everywhere,\n> especially when refs code already has a nice ref-store abstraction.\n> Some people frown upon back pointers. But I think adding a back\n> pointer in ref-store, pointing back to the repository is the right\n> move.\n\nI don't quite understand why the refs code would need a whole repository\nand not just the ref-store it self.  I thought the refs code was self\ncontained enough that all its state was based on the passed in\nref-store.  If its not, then we've done a terrible job at avoiding\nlayering violations (well actually we're really really bad at this in\ngeneral, and I *think* we're trying to make this better though the\nobject store/index refactoring).\n\nIf anything I would expect that the actual ref-store code would remain\nuntouched by any refactoring and that instead the higher-level API that\nhasn't already been converted to explicitly use a ref-store (and instead\njust calls the underlying impl with get_main_ref_store()).  Am I missing\nsomething here?\n\n-- \nBrandon Williams\n"},{"id":"353733","messageId":"CAGZ79kZfhSwtNgNk-GRDb6f4Uq7y6fi21HVO7xHv1YiuQoaSvA@mail.gmail.com","threadId":"48962","inReplyTo":"20180727171941.GA109508@google.com","subject":"Re: [PATCH 2/3] refs: introduce new API, wrap old API shallowly around new API","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-27T17:30:58Z","receivedAt":"2018-07-27T17:31:11Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jul 27, 2018 at 10:19 AM Brandon Williams <bmwill@google.com> wrote:\n>\n> On 07/27, Duy Nguyen wrote:\n> > On Fri, Jul 27, 2018 at 2:40 AM Stefan Beller <sbeller@google.com> wrote:\n> > >\n> > > Currently the refs API takes a 'ref_store' as an argument to specify\n> > > which ref store to iterate over; however it is more useful to specify\n> > > the repository instead (or later a specific worktree of a repository).\n> >\n> > There is no 'later'. worktrees.c already passes a worktree specific\n> > ref store. If you make this move you have to also design a way to give\n> > a specific ref store now.\n> >\n> > Frankly I still dislike the decision to pass repo everywhere,\n> > especially when refs code already has a nice ref-store abstraction.\n> > Some people frown upon back pointers. But I think adding a back\n> > pointer in ref-store, pointing back to the repository is the right\n> > move.\n>\n> I don't quite understand why the refs code would need a whole repository\n> and not just the ref-store it self.  I thought the refs code was self\n> contained enough that all its state was based on the passed in\n> ref-store.  If its not, then we've done a terrible job at avoiding\n> layering violations (well actually we're really really bad at this in\n> general, and I *think* we're trying to make this better though the\n> object store/index refactoring).\n>\n> If anything I would expect that the actual ref-store code would remain\n> untouched by any refactoring and that instead the higher-level API that\n> hasn't already been converted to explicitly use a ref-store (and instead\n> just calls the underlying impl with get_main_ref_store()).  Am I missing\n> something here?\n\nThen I think we might want to go with the original in Stolees proposal\nhttps://github.com/gitgitgadget/git/pull/11/commits/300db80140dacc927db0d46c804ca0ef4dcc1be1\nbut there the call to for_each_replace_ref just looks ugly, as it takes the\nrepository as both the repository where to obtain the ref store from\nas well as the back pointer.\n\nI anticipate that we need to have a lot of back pointers to the repository\nin question, hence I think we should have the repository pointer promoted\nto not just a back pointer.\n"},{"id":"353739","messageId":"CACsJy8Cx7u5YtK6sPJ=HbAOUBXCrP7VOgMyoQ58SB6q_s4N7Gg@mail.gmail.com","threadId":"48962","inReplyTo":"CAGZ79kZfhSwtNgNk-GRDb6f4Uq7y6fi21HVO7xHv1YiuQoaSvA@mail.gmail.com","subject":"Re: [PATCH 2/3] refs: introduce new API, wrap old API shallowly around new API","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-07-27T18:04:56Z","receivedAt":"2018-07-27T18:05:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jul 27, 2018 at 7:31 PM Stefan Beller <sbeller@google.com> wrote:\n>\n> On Fri, Jul 27, 2018 at 10:19 AM Brandon Williams <bmwill@google.com> wrote:\n> >\n> > On 07/27, Duy Nguyen wrote:\n> > > On Fri, Jul 27, 2018 at 2:40 AM Stefan Beller <sbeller@google.com> wrote:\n> > > >\n> > > > Currently the refs API takes a 'ref_store' as an argument to specify\n> > > > which ref store to iterate over; however it is more useful to specify\n> > > > the repository instead (or later a specific worktree of a repository).\n> > >\n> > > There is no 'later'. worktrees.c already passes a worktree specific\n> > > ref store. If you make this move you have to also design a way to give\n> > > a specific ref store now.\n> > >\n> > > Frankly I still dislike the decision to pass repo everywhere,\n> > > especially when refs code already has a nice ref-store abstraction.\n> > > Some people frown upon back pointers. But I think adding a back\n> > > pointer in ref-store, pointing back to the repository is the right\n> > > move.\n> >\n> > I don't quite understand why the refs code would need a whole repository\n> > and not just the ref-store it self.  I thought the refs code was self\n> > contained enough that all its state was based on the passed in\n> > ref-store.  If its not, then we've done a terrible job at avoiding\n> > layering violations (well actually we're really really bad at this in\n> > general, and I *think* we're trying to make this better though the\n> > object store/index refactoring).\n> >\n> > If anything I would expect that the actual ref-store code would remain\n> > untouched by any refactoring and that instead the higher-level API that\n> > hasn't already been converted to explicitly use a ref-store (and instead\n> > just calls the underlying impl with get_main_ref_store()).  Am I missing\n> > something here?\n>\n> Then I think we might want to go with the original in Stolees proposal\n> https://github.com/gitgitgadget/git/pull/11/commits/300db80140dacc927db0d46c804ca0ef4dcc1be1\n> but there the call to for_each_replace_ref just looks ugly, as it takes the\n> repository as both the repository where to obtain the ref store from\n> as well as the back pointer.\n>\n> I anticipate that we need to have a lot of back pointers to the repository\n> in question, hence I think we should have the repository pointer promoted\n> to not just a back pointer.\n\nI will probably need more time to study that commit and maybe the mail\narchive for the history of this series. But if I remember correctly\nsome of these for_each_ api is quite a pain (perhaps it's the for_each\nversion of reflog?) and it's probably better to redesign it (again\ntalking without real understanding of the problem).\n-- \nDuy\n"},{"id":"353941","messageId":"20180730194731.220191-1-sbeller@google.com","threadId":"48962","inReplyTo":"CACsJy8Cx7u5YtK6sPJ=HbAOUBXCrP7VOgMyoQ58SB6q_s4N7Gg@mail.gmail.com","subject":"[PATCH 0/2] Cleanup refs API [WAS: Re: [PATCH 2/3] refs: introduce new API, wrap old API shallowly around new API]","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-30T19:47:29Z","receivedAt":"2018-07-30T19:47:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> > I anticipate that we need to have a lot of back pointers to the repository\n> > in question, hence I think we should have the repository pointer promoted\n> > to not just a back pointer.\n>\n> I will probably need more time to study that commit and maybe the mail\n> archive for the history of this series. But if I remember correctly\n> some of these for_each_ api is quite a pain (perhaps it's the for_each\n> version of reflog?) and it's probably better to redesign it (again\n> talking without real understanding of the problem).\n\nI stepped back a bit and reconsidered the point made above, and I do not\nthink that the repository argument is any special. If you need a repository\n(for e.g. lookup_commit or friends), you'll have to pass it through the\ncallback cookie, whether directly or as part of a struct tailored to\nyour purpose.\n\nInstead we should strive to make the refs API smaller and cleaner,\nomitting the repository argument at all, and instead should be focussing\non a ref_store argument instead.\n\nThis series applies on master; when we decide to go this direction\nwe can drop origin/sb/refs-in-repo.\n\nThanks,\nStefan\n\nDerrick Stolee (1):\n  replace-objects: use arbitrary repositories\n\nStefan Beller (1):\n  refs: switch for_each_replace_ref back to use a ref_store\n\n builtin/replace.c | 4 +---\n refs.c            | 4 ++--\n refs.h            | 2 +-\n replace-object.c  | 5 +++--\n 4 files changed, 7 insertions(+), 8 deletions(-)\n\n-- \n2.18.0.132.g195c49a2227\n\n"},{"id":"353942","messageId":"20180730194731.220191-2-sbeller@google.com","threadId":"48962","inReplyTo":"20180730194731.220191-1-sbeller@google.com","subject":"[PATCH 1/2] replace-objects: use arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-30T19:47:30Z","receivedAt":"2018-07-30T19:47:52Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Derrick Stolee <dstolee@microsoft.com>\n\nThis is the smallest possible change that makes prepare_replace_objects\nwork properly with arbitrary repositories. By supplying the repository\nas the cb_data, we do not need to modify any code in the ref iterator\nlogic. We will likely want to do a full replacement of the ref iterator\nlogic to provide a repository struct as a concrete parameter.\n\n[sb: original commit message left as-is. I disagree with it.\nWe want to keep the ref store API clean and focussed on struct\nref_store. There is no need to treat a repository any special\nfor pass-through by the callback cookie. So instead let's just\npass the repository as a cb cookie and cleanup the API in follow\nup patches]\n\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n replace-object.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/replace-object.c b/replace-object.c\nindex 801b5c16789..e99fcd1ff6e 100644\n--- a/replace-object.c\n+++ b/replace-object.c\n@@ -14,6 +14,7 @@ static int register_replace_ref(const char *refname,\n \tconst char *slash = strrchr(refname, '/');\n \tconst char *hash = slash ? slash + 1 : refname;\n \tstruct replace_object *repl_obj = xmalloc(sizeof(*repl_obj));\n+\tstruct repository *r = (struct repository *)cb_data;\n \n \tif (get_oid_hex(hash, &repl_obj->original.oid)) {\n \t\tfree(repl_obj);\n@@ -25,7 +26,7 @@ static int register_replace_ref(const char *refname,\n \toidcpy(&repl_obj->replacement, oid);\n \n \t/* Register new object */\n-\tif (oidmap_put(the_repository->objects->replace_map, repl_obj))\n+\tif (oidmap_put(r->objects->replace_map, repl_obj))\n \t\tdie(\"duplicate replace ref: %s\", refname);\n \n \treturn 0;\n@@ -40,7 +41,7 @@ static void prepare_replace_object(struct repository *r)\n \t\txmalloc(sizeof(*r->objects->replace_map));\n \toidmap_init(r->objects->replace_map, 0);\n \n-\tfor_each_replace_ref(r, register_replace_ref, NULL);\n+\tfor_each_replace_ref(r, register_replace_ref, r);\n }\n \n /* We allow \"recursive\" replacement. Only within reason, though */\n-- \n2.18.0.132.g195c49a2227\n\n"},{"id":"353943","messageId":"20180730194731.220191-3-sbeller@google.com","threadId":"48962","inReplyTo":"20180730194731.220191-1-sbeller@google.com","subject":"[PATCH 2/2] refs: switch for_each_replace_ref back to use a ref_store","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-30T19:47:31Z","receivedAt":"2018-07-30T19:47:53Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This effectively reverts commit 0d296c57ae (refs: allow for_each_replace_ref\nto handle arbitrary repositories, 2018-04-11) and 60ce76d3581 (refs: add\nrepository argument to for_each_replace_ref, 2018-04-11).\n\nThe repository argument is not any special from the ref-store's point\nof life.  If you need a repository (for e.g. lookup_commit or friends),\nyou'll have to pass it through the callback cookie, whether directly or\nas part of a struct tailored to your purpose.\n\nSo let's go back to the clean API, just requiring a ref_store as an\nargument.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/replace.c | 2 +-\n refs.c            | 4 ++--\n refs.h            | 2 +-\n replace-object.c  | 2 +-\n 4 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex deabda21012..52dc371eafc 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -87,7 +87,7 @@ static int list_replace_refs(const char *pattern, const char *format)\n \t\t\t     \"valid formats are 'short', 'medium' and 'long'\\n\",\n \t\t\t     format);\n \n-\tfor_each_replace_ref(the_repository, show_reference, (void *)&data);\n+\tfor_each_replace_ref(show_reference, (void *)&data);\n \n \treturn 0;\n }\ndiff --git a/refs.c b/refs.c\nindex 08fb5a99148..2d713499125 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1441,9 +1441,9 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,\n \treturn do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);\n }\n \n-int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)\n+int for_each_replace_ref(each_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(get_main_ref_store(r),\n+\treturn do_for_each_ref(get_main_ref_store(the_repository),\n \t\t\t       git_replace_ref_base, fn,\n \t\t\t       strlen(git_replace_ref_base),\n \t\t\t       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\ndiff --git a/refs.h b/refs.h\nindex cc2fb4c68c0..48d5ffd2082 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -307,7 +307,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,\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(struct repository *r, 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);\ndiff --git a/replace-object.c b/replace-object.c\nindex e99fcd1ff6e..ee3374ab59b 100644\n--- a/replace-object.c\n+++ b/replace-object.c\n@@ -41,7 +41,7 @@ static void prepare_replace_object(struct repository *r)\n \t\txmalloc(sizeof(*r->objects->replace_map));\n \toidmap_init(r->objects->replace_map, 0);\n \n-\tfor_each_replace_ref(r, register_replace_ref, r);\n+\tfor_each_replace_ref(register_replace_ref, r);\n }\n \n /* We allow \"recursive\" replacement. Only within reason, though */\n-- \n2.18.0.132.g195c49a2227\n\n"},{"id":"353979","messageId":"20180731001858.122968-1-jonathantanmy@google.com","threadId":"48962","inReplyTo":"20180730194731.220191-3-sbeller@google.com","subject":"Re: [PATCH 2/2] refs: switch for_each_replace_ref back to use a ref_store","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-07-31T00:18:58Z","receivedAt":"2018-07-31T00:19:04Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> So let's go back to the clean API, just requiring a ref_store as an\n> argument.\n\nHere, you say that we want ref_store as an argument...\n\n> -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)\n> +int for_each_replace_ref(each_ref_fn fn, void *cb_data)\n>  {\n> -\treturn do_for_each_ref(get_main_ref_store(r),\n> +\treturn do_for_each_ref(get_main_ref_store(the_repository),\n>  \t\t\t       git_replace_ref_base, fn,\n>  \t\t\t       strlen(git_replace_ref_base),\n>  \t\t\t       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\n\n...but there is no ref_store as an argument here - instead, the\nrepository argument is deleted with no replacement. I presume you meant\nto replace it with a ref_store instead? (This will also fix the issue\nthat for_each_replace_ref only works on the_repository.)\n\nTaking a step back, was there anything that prompted these patches?\nMaybe at least the 2nd one should wait until we have a situation that\nwarrants it (for example, if we want to for_each_replace_ref(), but we\nonly have a ref_store, not a repository).\n"},{"id":"353989","messageId":"CAGZ79kbwt2RGo2Z2ARSzfHOZdL_VWF1sR+=EE=QWx1ibLL+KwQ@mail.gmail.com","threadId":"48962","inReplyTo":"20180731001858.122968-1-jonathantanmy@google.com","subject":"Re: [PATCH 2/2] refs: switch for_each_replace_ref back to use a ref_store","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-31T00:41:31Z","receivedAt":"2018-07-31T00:41:45Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Jul 30, 2018 at 5:19 PM Jonathan Tan <jonathantanmy@google.com> wrote:\n>\n> > So let's go back to the clean API, just requiring a ref_store as an\n> > argument.\n>\n> Here, you say that we want ref_store as an argument...\n\nI do.\n\n>\n> > -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)\n> > +int for_each_replace_ref(each_ref_fn fn, void *cb_data)\n> >  {\n> > -     return do_for_each_ref(get_main_ref_store(r),\n> > +     return do_for_each_ref(get_main_ref_store(the_repository),\n> >                              git_replace_ref_base, fn,\n> >                              strlen(git_replace_ref_base),\n> >                              DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\n>\n> ...but there is no ref_store as an argument here - instead, the\n> repository argument is deleted with no replacement. I presume you meant\n> to replace it with a ref_store instead? (This will also fix the issue\n> that for_each_replace_ref only works on the_repository.)\n\nYes, I would want to pass in a ref_store and use that as the first argument\nin do_for_each_ref for now.\n\nThat would reduce the API uncleanliness to have to pass the repository twice.\n\n> Taking a step back, was there anything that prompted these patches?\n\nI am flailing around on how to approach the ref store and the repository:\n* I dislike having to pass a repository 'r' twice. (current situation after\n  patch 1. That patch itself is part of Stolees larger series to address\n  commit graphs and replace refs, so we will have that one way or another)\n* So I sent out some RFC patches to have the_repository in the ref store\n  and pass the repo through to all the call backs to make it easy for\n  users inside the callback to do basic things like looking up commits.\n* both Duy (on list) and Brandon (privately) expressed their dislike for\n  having the refs API bloated with the repository, as the repository is\n  not needed per se in the ref store.\n* After some reflection I agreed with their concerns, which let me\n  to re-examine the refs API: all but a few select functions take a\n  ref_store as the first argument (or imply to work on the ref store\n  in the_repository, then neither a repo nor a ref store argument is\n  there)\n* I want to bring back the cleanliness of the API, which is to take a\n  ref store when needed instead of the repository, which is rather\n  bloated.\n\n> Maybe at least the 2nd one should wait until we have a situation that\n> warrants it (for example, if we want to for_each_replace_ref(), but we\n> only have a ref_store, not a repository).\n\nokay, then let's drop this series for now and I'll re-examine what is\nneeded to have submodule handling in-core.\n\nThanks,\nStefan\n"},{"id":"354056","messageId":"CACsJy8DwaLCxY-ryV+=OwRytzwwQZCfVmfXo0z91z9YRMMT0VA@mail.gmail.com","threadId":"48962","inReplyTo":"CAGZ79kbwt2RGo2Z2ARSzfHOZdL_VWF1sR+=EE=QWx1ibLL+KwQ@mail.gmail.com","subject":"Re: [PATCH 2/2] refs: switch for_each_replace_ref back to use a ref_store","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-07-31T16:17:18Z","receivedAt":"2018-07-31T16:17:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jul 31, 2018 at 2:41 AM Stefan Beller <sbeller@google.com> wrote:\n> > Taking a step back, was there anything that prompted these patches?\n>\n> I am flailing around on how to approach the ref store and the repository:\n> * I dislike having to pass a repository 'r' twice. (current situation after\n>   patch 1. That patch itself is part of Stolees larger series to address\n>   commit graphs and replace refs, so we will have that one way or another)\n> * So I sent out some RFC patches to have the_repository in the ref store\n>   and pass the repo through to all the call backs to make it easy for\n>   users inside the callback to do basic things like looking up commits.\n> * both Duy (on list) and Brandon (privately) expressed their dislike for\n>   having the refs API bloated with the repository, as the repository is\n>   not needed per se in the ref store.\n> * After some reflection I agreed with their concerns, which let me\n>   to re-examine the refs API: all but a few select functions take a\n>   ref_store as the first argument (or imply to work on the ref store\n>   in the_repository, then neither a repo nor a ref store argument is\n>   there)\n\nSince I'm the one who added the refs_* variants (which take ref_store\nas the first argument). There's one thing that I should have done but\ndid not: making each_ref_fn takes the ref store.\n\nIf a callback is given a refname and wants to do something about it\n(other that just printing it), chances are you need the same ref-store\nthat triggers the callback and you should not need to pass a separate\nref-store around by yourself because you would have the same \"passing\ntwice\" problem that you disliked. This is more obvious with\nrefs_for_each_reflog() because you will very likely want to parse the\nref from the callback.\n\nThen, even ref store code needs access to object database and I don't\nthink we want to pass a pair of \"struct repository *\", \"struct\nref_store *\" in every API. We know the ref store has to be associated\nwith one repository and we do save that information (notice that\nref_store_init_fn takes gitdir and the \"files\" backend does save it).\nOnce refs code is adapted to struct repository, I think it will take a\n'struct repository *' instead of the gitdir  string and store the\npointer to the repository too for internal use.\n\nThen if a ref callback needs access to the same repository, we could\njust provide this repo via refs api. Since callbacks should already\nhave access to the ref store (preferably without having to carrying it\nvia cb_data), it has access to the repository as well and you don't\nneed to explicitly pass the repository.\n\n> * I want to bring back the cleanliness of the API, which is to take a\n>   ref store when needed instead of the repository, which is rather\n>   bloated.\n-- \nDuy;\n"}]}