{"thread":{"id":"46659","subject":"[PATCH v4 00/16] Fix git-gc losing objects in multi worktree","startedAt":"2017-08-23T12:37:30Z","lastAt":"2017-09-09T06:46:01Z","messageCount":40,"participants":["Nguyễn Thái Ngọc Duy","Stefan Beller","Richard Maw","Junio C Hamano","Michael J Gruber","Duy Nguyen","Michael Haggerty"],"isPatch":true,"patchVersion":4,"patchTotal":16},"messages":[{"id":"327024","messageId":"20170823123704.16518-1-pclouds@gmail.com","threadId":"46659","inReplyTo":null,"subject":"[PATCH v4 00/16] Fix git-gc losing objects in multi worktree","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:48Z","receivedAt":"2017-08-23T12:37:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\"git gc\" when used in multiple worktrees ignore some per-worktree\nreferences: object references in the index, HEAD and reflog. This\nseries fixes it by making the revision walker include these from all\nworktrees by default (and the series is basically split in three parts\nin the same order). There's a couple more cleanups in refs.c. Luckily\nit does not conflict with anything in 'pu'.\n\nCompared to v3 [1], the largest change is supporting multi worktree in\nthe reflog iterator. The merge iterator is now used (Micheal was right\nall along).\n\n[1] https://public-inbox.org/git/20170419110145.5086-1-pclouds@gmail.com/\n\nNguyễn Thái Ngọc Duy (16):\n revision.h: new flag in struct rev_info wrt. worktree-related refs\n refs.c: use is_dir_sep() in resolve_gitlink_ref()\n revision.c: refactor add_index_objects_to_pending()\n revision.c: --indexed-objects add objects from all worktrees\n refs.c: refactor get_submodule_ref_store(), share common free block\n refs: move submodule slash stripping code to get_submodule_ref_store\n refs: add refs_head_ref()\n revision.c: use refs_for_each*() instead of for_each_*_submodule()\n refs.c: move for_each_remote_ref_submodule() to submodule.c\n refs: remove dead for_each_*_submodule()\n revision.c: --all adds HEAD from all worktrees\n files-backend: make reflog iterator go through per-worktree reflog\n revision.c: --reflog add HEAD reflog from all worktrees\n rev-list: expose and document --single-worktree\n refs.c: remove fallback-to-main-store code get_submodule_ref_store()\n refs.c: reindent get_submodule_ref_store()\n\n Documentation/rev-list-options.txt | 8 ++\n Documentation/technical/api-ref-iteration.txt | 7 +-\n reachable.c | 2 +\n refs.c | 110 ++++++---------------\n refs.h | 20 +---\n refs/files-backend.c | 59 +++++++++---\n revision.c | 131 +++++++++++++++++++++-----\n revision.h | 1 +\n submodule.c | 9 ++\n t/t1407-worktree-ref-store.sh | 30 ++++++\n t/t5304-prune.sh | 37 ++++++++\n worktree.c | 22 +++++\n worktree.h | 8 ++\n 13 files changed, 308 insertions(+), 136 deletions(-)\n\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327025","messageId":"20170823123704.16518-2-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 01/16] revision.h: new flag in struct rev_info wrt. worktree-related refs","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:49Z","receivedAt":"2017-08-23T12:37:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The revision walker can walk through per-worktree refs like HEAD or\nSHA-1 references in the index. These currently are from the current\nworktree only. This new flag is added to change rev-list behavior in\nthis regard:\n\nWhen single_worktree is set, only current worktree is considered. When\nit is not set (which is the default), all worktrees are considered.\n\nThe default is chosen so because the two big components that rev-list\nworks with are object database (entirely shared between worktrees) and\nrefs (mostly shared). It makes sense that default behavior goes per-repo\ntoo instead of per-worktree.\n\nThe flag will eventually be exposed as a rev-list argument with\ndocuments. For now it stays internal until the new behavior is fully\nimplemented.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n revision.h | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/revision.h b/revision.h\nindex bc18487d6f..3a3d3e2cf8 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -96,6 +96,7 @@ struct rev_info {\n \t\t\ttopo_order:1,\n \t\t\tsimplify_merges:1,\n \t\t\tsimplify_by_decoration:1,\n+\t\t\tsingle_worktree:1,\n \t\t\ttag_objects:1,\n \t\t\ttree_objects:1,\n \t\t\tblob_objects:1,\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327026","messageId":"20170823123704.16518-3-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 02/16] refs.c: use is_dir_sep() in resolve_gitlink_ref()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:50Z","receivedAt":"2017-08-23T12:37:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The \"submodule\" argument in this function is a path, which can have\neither '/' or '\\\\' as a separator. Use is_dir_sep() to support both.\n\nNoticed-by: Johannes Sixt \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 3d549a8970..dec899a57a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1507,7 +1507,7 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n \tstruct ref_store *refs;\n \tint flags;\n \n-\twhile (len && submodule[len - 1] == '/')\n+\twhile (len && is_dir_sep(submodule[len - 1]))\n \t\tlen--;\n \n \tif (!len)\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327027","messageId":"20170823123704.16518-4-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 03/16] revision.c: refactor add_index_objects_to_pending()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:51Z","receivedAt":"2017-08-23T12:37:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The core code is factored out and take 'struct index_state *' instead so\nthat we can reuse it to add objects from index files other than .git/index\nin the next patch.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n revision.c | 18 ++++++++++++------\n 1 file changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex aa3b946a8d..6c46ad6c8a 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1262,13 +1262,13 @@ static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,\n \n }\n \n-void add_index_objects_to_pending(struct rev_info *revs, unsigned flags)\n+static void do_add_index_objects_to_pending(struct rev_info *revs,\n+\t\t\t\t\t struct index_state *istate)\n {\n \tint i;\n \n-\tread_cache();\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tstruct cache_entry *ce = active_cache[i];\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n \t\tstruct blob *blob;\n \n \t\tif (S_ISGITLINK(ce->ce_mode))\n@@ -1281,13 +1281,19 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned flags)\n \t\t\t\t\t ce->ce_mode, ce->name);\n \t}\n \n-\tif (active_cache_tree) {\n+\tif (istate->cache_tree) {\n \t\tstruct strbuf path = STRBUF_INIT;\n-\t\tadd_cache_tree(active_cache_tree, revs, &path);\n+\t\tadd_cache_tree(istate->cache_tree, revs, &path);\n \t\tstrbuf_release(&path);\n \t}\n }\n \n+void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n+{\n+\tread_cache();\n+\tdo_add_index_objects_to_pending(revs, &the_index);\n+}\n+\n static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,\n \t\t\t int exclude_parent)\n {\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327028","messageId":"20170823123704.16518-5-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 04/16] revision.c: --indexed-objects add objects from all worktrees","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:52Z","receivedAt":"2017-08-23T12:37:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is the result of single_worktree flag never being set (no way to up\nuntil now). To get objects from current index only, set single_worktree.\n\nThe other add_index_objects_to_pending's caller is mark_reachable_objects()\n(e.g. \"git prune\") which also mark objects from all indexes.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n revision.c | 21 +++++++++++++++++++++\n t/t5304-prune.sh | 9 +++++++++\n 2 files changed, 30 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex 6c46ad6c8a..f35cb49af5 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -19,6 +19,7 @@\n #include \"dir.h\"\n #include \"cache-tree.h\"\n #include \"bisect.h\"\n+#include \"worktree.h\"\n \n volatile show_early_output_fn_t show_early_output;\n \n@@ -1290,8 +1291,28 @@ static void do_add_index_objects_to_pending(struct rev_info *revs,\n \n void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n {\n+\tstruct worktree **worktrees, **p;\n+\n \tread_cache();\n \tdo_add_index_objects_to_pending(revs, &the_index);\n+\n+\tif (revs->single_worktree)\n+\t\treturn;\n+\n+\tworktrees = get_worktrees(0);\n+\tfor (p = worktrees; *p; p++) {\n+\t\tstruct worktree *wt = *p;\n+\t\tstruct index_state istate = { NULL };\n+\n+\t\tif (wt->is_current)\n+\t\t\tcontinue; /* current index already taken care of */\n+\n+\t\tif (read_index_from(&istate,\n+\t\t\t\t worktree_git_path(wt, \"index\")) > 0)\n+\t\t\tdo_add_index_objects_to_pending(revs, &istate);\n+\t\tdiscard_index(&istate);\n+\t}\n+\tfree_worktrees(worktrees);\n }\n \n static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,\ndiff --git a/t/t5304-prune.sh b/t/t5304-prune.sh\nindex 133b5842b1..cba45c7be9 100755\n--- a/t/t5304-prune.sh\n+++ b/t/t5304-prune.sh\n@@ -283,4 +283,13 @@ test_expect_success 'prune: handle alternate object database' '\n \tgit -C B prune\n '\n \n+test_expect_success 'prune: handle index in multiple worktrees' '\n+\tgit worktree add second-worktree &&\n+\techo \"new blob for second-worktree\" >second-worktree/blob &&\n+\tgit -C second-worktree add blob &&\n+\tgit prune --expire=now &&\n+\tgit -C second-worktree show :blob >actual &&\n+\ttest_cmp second-worktree/blob actual\n+'\n+\n test_done\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327029","messageId":"20170823123704.16518-6-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 05/16] refs.c: refactor get_submodule_ref_store(), share common free block","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:53Z","receivedAt":"2017-08-23T12:37:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 17 ++++++-----------\n 1 file changed, 6 insertions(+), 11 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex dec899a57a..522c4ab74f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1636,7 +1636,6 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n {\n \tstruct strbuf submodule_sb = STRBUF_INIT;\n \tstruct ref_store *refs;\n-\tint ret;\n \n \tif (!submodule || !*submodule) {\n \t\t/*\n@@ -1648,19 +1647,14 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \n \trefs = lookup_ref_store_map(&submodule_ref_stores, submodule);\n \tif (refs)\n-\t\treturn refs;\n+\t\tgoto done;\n \n \tstrbuf_addstr(&submodule_sb, submodule);\n-\tret = is_nonbare_repository_dir(&submodule_sb);\n-\tstrbuf_release(&submodule_sb);\n-\tif (!ret)\n-\t\treturn NULL;\n+\tif (!is_nonbare_repository_dir(&submodule_sb))\n+\t\tgoto done;\n \n-\tret = submodule_to_gitdir(&submodule_sb, submodule);\n-\tif (ret) {\n-\t\tstrbuf_release(&submodule_sb);\n-\t\treturn NULL;\n-\t}\n+\tif (submodule_to_gitdir(&submodule_sb, submodule))\n+\t\tgoto done;\n \n \t/* assume that add_submodule_odb() has been called */\n \trefs = ref_store_init(submodule_sb.buf,\n@@ -1668,6 +1662,7 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \tregister_ref_store_map(&submodule_ref_stores, \"submodule\",\n \t\t\t refs, submodule);\n \n+done:\n \tstrbuf_release(&submodule_sb);\n \treturn refs;\n }\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327030","messageId":"20170823123704.16518-7-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 06/16] refs: move submodule slash stripping code to get_submodule_ref_store","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:54Z","receivedAt":"2017-08-23T12:38:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is a better place that will benefit all submodule callers instead\nof just resolve_gitlink_ref()\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 33 +++++++++++++++++----------------\n 1 file changed, 17 insertions(+), 16 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 522c4ab74f..ea8e6b9f42 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1503,25 +1503,10 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n int resolve_gitlink_ref(const char *submodule, const char *refname,\n \t\t\tunsigned char *sha1)\n {\n-\tsize_t len = strlen(submodule);\n \tstruct ref_store *refs;\n \tint flags;\n \n-\twhile (len && is_dir_sep(submodule[len - 1]))\n-\t\tlen--;\n-\n-\tif (!len)\n-\t\treturn -1;\n-\n-\tif (submodule[len]) {\n-\t\t/* We need to strip off one or more trailing slashes */\n-\t\tchar *stripped = xmemdupz(submodule, len);\n-\n-\t\trefs = get_submodule_ref_store(stripped);\n-\t\tfree(stripped);\n-\t} else {\n-\t\trefs = get_submodule_ref_store(submodule);\n-\t}\n+\trefs = get_submodule_ref_store(submodule);\n \n \tif (!refs)\n \t\treturn -1;\n@@ -1636,6 +1621,16 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n {\n \tstruct strbuf submodule_sb = STRBUF_INIT;\n \tstruct ref_store *refs;\n+\tchar *to_free = NULL;\n+\tsize_t len;\n+\n+\tif (submodule) {\n+\t\tlen = strlen(submodule);\n+\t\twhile (len && is_dir_sep(submodule[len - 1]))\n+\t\t\tlen--;\n+\t\tif (!len)\n+\t\t\treturn NULL;\n+\t}\n \n \tif (!submodule || !*submodule) {\n \t\t/*\n@@ -1645,6 +1640,10 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \t\treturn get_main_ref_store();\n \t}\n \n+\tif (submodule[len])\n+\t\t/* We need to strip off one or more trailing slashes */\n+\t\tsubmodule = to_free = xmemdupz(submodule, len);\n+\n \trefs = lookup_ref_store_map(&submodule_ref_stores, submodule);\n \tif (refs)\n \t\tgoto done;\n@@ -1664,6 +1663,8 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \n done:\n \tstrbuf_release(&submodule_sb);\n+\tfree(to_free);\n+\n \treturn refs;\n }\n \n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327031","messageId":"20170823123704.16518-8-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 07/16] refs: add refs_head_ref()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:55Z","receivedAt":"2017-08-23T12:38:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 23 +++++++++++++----------\n refs.h | 2 ++\n 2 files changed, 15 insertions(+), 10 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex ea8e6b9f42..b3a0a24469 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1248,27 +1248,30 @@ int refs_rename_ref_available(struct ref_store *refs,\n \treturn ok;\n }\n \n-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n+int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n {\n \tstruct object_id oid;\n \tint flag;\n \n-\tif (submodule) {\n-\t\tif (resolve_gitlink_ref(submodule, \"HEAD\", oid.hash) == 0)\n-\t\t\treturn fn(\"HEAD\", &oid, 0, cb_data);\n-\n-\t\treturn 0;\n-\t}\n-\n-\tif (!read_ref_full(\"HEAD\", RESOLVE_REF_READING, oid.hash, &flag))\n+\tif (!refs_read_ref_full(refs, \"HEAD\", RESOLVE_REF_READING,\n+\t\t\t\toid.hash, &flag))\n \t\treturn fn(\"HEAD\", &oid, flag, cb_data);\n \n \treturn 0;\n }\n \n+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n+{\n+\tstruct ref_store *refs = get_submodule_ref_store(submodule);\n+\n+\tif (!refs)\n+\t\treturn -1;\n+\treturn refs_head_ref(refs, fn, cb_data);\n+}\n+\n int head_ref(each_ref_fn fn, void *cb_data)\n {\n-\treturn head_ref_submodule(NULL, fn, cb_data);\n+\treturn refs_head_ref(get_main_ref_store(), fn, cb_data);\n }\n \n struct ref_iterator *refs_ref_iterator_begin(\ndiff --git a/refs.h b/refs.h\nindex 6daa78eb50..8073f8ab56 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -275,6 +275,8 @@ typedef int each_ref_fn(const char *refname,\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-- \n2.11.0.157.gd943d85\n\n"},{"id":"327032","messageId":"20170823123704.16518-9-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 08/16] revision.c: use refs_for_each*() instead of for_each_*_submodule()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:56Z","receivedAt":"2017-08-23T12:38:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 9 ++++-----\n refs.h | 6 +++---\n revision.c | 48 ++++++++++++++++++++++++++++++++----------------\n 3 files changed, 39 insertions(+), 24 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex b3a0a24469..cd61509bc8 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1362,16 +1362,15 @@ int for_each_ref_in_submodule(const char *submodule, const char *prefix,\n \t\t\t\t prefix, fn, cb_data);\n }\n \n-int for_each_fullref_in_submodule(const char *submodule, const char *prefix,\n-\t\t\t\t each_ref_fn fn, void *cb_data,\n-\t\t\t\t unsigned int broken)\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 {\n \tunsigned int flag = 0;\n \n \tif (broken)\n \t\tflag = DO_FOR_EACH_INCLUDE_BROKEN;\n-\treturn do_for_each_ref(get_submodule_ref_store(submodule),\n-\t\t\t prefix, fn, 0, flag, cb_data);\n+\treturn do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);\n }\n \n int for_each_replace_ref(each_ref_fn fn, void *cb_data)\ndiff --git a/refs.h b/refs.h\nindex 8073f8ab56..a8d6f33703 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -291,6 +291,9 @@ int refs_for_each_remote_ref(struct ref_store *refs,\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@@ -306,9 +309,6 @@ 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\t\t each_ref_fn fn, void *cb_data);\n-int for_each_fullref_in_submodule(const char *submodule, const char *prefix,\n-\t\t\t\t each_ref_fn fn, void *cb_data,\n-\t\t\t\t unsigned int broken);\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,\ndiff --git a/revision.c b/revision.c\nindex f35cb49af5..8d04516266 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1188,12 +1188,19 @@ void add_ref_exclusion(struct string_list **ref_excludes_p, const char *exclude)\n \tstring_list_append(*ref_excludes_p, exclude);\n }\n \n-static void handle_refs(const char *submodule, struct rev_info *revs, unsigned flags,\n-\t\tint (*for_each)(const char *, each_ref_fn, void *))\n+static void handle_refs(struct ref_store *refs,\n+\t\t\tstruct rev_info *revs, unsigned flags,\n+\t\t\tint (*for_each)(struct ref_store *, each_ref_fn, void *))\n {\n \tstruct all_refs_cb cb;\n+\n+\tif (!refs) {\n+\t\t/* this could happen with uninitialized submodules */\n+\t\treturn;\n+\t}\n+\n \tinit_all_refs_cb(&cb, revs, flags);\n-\tfor_each(submodule, handle_one_ref, &cb);\n+\tfor_each(refs, handle_one_ref, &cb);\n }\n \n static void handle_one_reflog_commit(struct object_id *oid, void *cb_data)\n@@ -2095,23 +2102,25 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \tctx->argc -= n;\n }\n \n-static int for_each_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data, const char *term) {\n+static int for_each_bisect_ref(struct ref_store *refs, each_ref_fn fn,\n+\t\t\t void *cb_data, const char *term)\n+{\n \tstruct strbuf bisect_refs = STRBUF_INIT;\n \tint status;\n \tstrbuf_addf(&bisect_refs, \"refs/bisect/%s\", term);\n-\tstatus = for_each_fullref_in_submodule(submodule, bisect_refs.buf, fn, cb_data, 0);\n+\tstatus = refs_for_each_fullref_in(refs, bisect_refs.buf, fn, cb_data, 0);\n \tstrbuf_release(&bisect_refs);\n \treturn status;\n }\n \n-static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n+static int for_each_bad_bisect_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n {\n-\treturn for_each_bisect_ref(submodule, fn, cb_data, term_bad);\n+\treturn for_each_bisect_ref(refs, fn, cb_data, term_bad);\n }\n \n-static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n+static int for_each_good_bisect_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n {\n-\treturn for_each_bisect_ref(submodule, fn, cb_data, term_good);\n+\treturn for_each_bisect_ref(refs, fn, cb_data, term_good);\n }\n \n static int handle_revision_pseudo_opt(const char *submodule,\n@@ -2120,8 +2129,14 @@ static int handle_revision_pseudo_opt(const char *submodule,\n {\n \tconst char *arg = argv[0];\n \tconst char *optarg;\n+\tstruct ref_store *refs;\n \tint argcount;\n \n+\tif (submodule) {\n+\t\trefs = get_submodule_ref_store(submodule);\n+\t} else\n+\t\trefs = get_main_ref_store();\n+\n \t/*\n \t * NOTE!\n \t *\n@@ -2133,22 +2148,23 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \t * register it in the list at the top of handle_revision_opt.\n \t */\n \tif (!strcmp(arg, \"--all\")) {\n-\t\thandle_refs(submodule, revs, *flags, for_each_ref_submodule);\n-\t\thandle_refs(submodule, revs, *flags, head_ref_submodule);\n+\t\thandle_refs(refs, revs, *flags, refs_for_each_ref);\n+\t\thandle_refs(refs, revs, *flags, refs_head_ref);\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n \t} else if (!strcmp(arg, \"--branches\")) {\n-\t\thandle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);\n+\t\thandle_refs(refs, revs, *flags, refs_for_each_branch_ref);\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n \t} else if (!strcmp(arg, \"--bisect\")) {\n \t\tread_bisect_terms(&term_bad, &term_good);\n-\t\thandle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);\n-\t\thandle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);\n+\t\thandle_refs(refs, revs, *flags, for_each_bad_bisect_ref);\n+\t\thandle_refs(refs, revs, *flags ^ (UNINTERESTING | BOTTOM),\n+\t\t\t for_each_good_bisect_ref);\n \t\trevs->bisect = 1;\n \t} else if (!strcmp(arg, \"--tags\")) {\n-\t\thandle_refs(submodule, revs, *flags, for_each_tag_ref_submodule);\n+\t\thandle_refs(refs, revs, *flags, refs_for_each_tag_ref);\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n \t} else if (!strcmp(arg, \"--remotes\")) {\n-\t\thandle_refs(submodule, revs, *flags, for_each_remote_ref_submodule);\n+\t\thandle_refs(refs, revs, *flags, refs_for_each_remote_ref);\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n \t} else if ((argcount = parse_long_opt(\"glob\", argv, &optarg))) {\n \t\tstruct all_refs_cb cb;\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327033","messageId":"20170823123704.16518-10-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 09/16] refs.c: move for_each_remote_ref_submodule() to submodule.c","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:57Z","receivedAt":"2017-08-23T12:38:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 6 ------\n refs.h | 2 --\n submodule.c | 7 +++++++\n 3 files changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex cd61509bc8..7fa19e9309 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -368,12 +368,6 @@ int for_each_remote_ref(each_ref_fn fn, void *cb_data)\n \treturn refs_for_each_remote_ref(get_main_ref_store(), fn, cb_data);\n }\n \n-int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n-{\n-\treturn refs_for_each_remote_ref(get_submodule_ref_store(submodule),\n-\t\t\t\t\tfn, cb_data);\n-}\n-\n int head_ref_namespaced(each_ref_fn fn, void *cb_data)\n {\n \tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/refs.h b/refs.h\nindex a8d6f33703..5d25da227a 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -313,8 +313,6 @@ 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 int head_ref_namespaced(each_ref_fn fn, void *cb_data);\n int for_each_namespaced_ref(each_ref_fn fn, void *cb_data);\ndiff --git a/submodule.c b/submodule.c\nindex e072036e79..98e1f9d3c7 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -69,6 +69,13 @@ int is_staging_gitmodules_ok(const struct index_state *istate)\n \treturn 1;\n }\n \n+static int for_each_remote_ref_submodule(const char *submodule,\n+\t\t\t\t\t each_ref_fn fn, void *cb_data)\n+{\n+\treturn refs_for_each_remote_ref(get_submodule_ref_store(submodule),\n+\t\t\t\t\tfn, cb_data);\n+}\n+\n /*\n * Try to update the \"path\" entry in the \"submodule.\" section of the\n * .gitmodules file. Return 0 only if a .gitmodules file was found, a section\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327034","messageId":"20170823123704.16518-11-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 10/16] refs: remove dead for_each_*_submodule()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:58Z","receivedAt":"2017-08-23T12:38:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"These are used in revision.c. After the last patch they are replaced\nwith the refs_ version. Delete them.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/technical/api-ref-iteration.txt | 7 ++----\n refs.c | 33 ---------------------------\n refs.h | 10 --------\n 3 files changed, 2 insertions(+), 48 deletions(-)\n\ndiff --git a/Documentation/technical/api-ref-iteration.txt b/Documentation/technical/api-ref-iteration.txt\nindex 37379d8337..46c3d5c355 100644\n--- a/Documentation/technical/api-ref-iteration.txt\n+++ b/Documentation/technical/api-ref-iteration.txt\n@@ -32,11 +32,8 @@ Iteration functions\n \n * `for_each_glob_ref_in()` the previous and `for_each_ref_in()` combined.\n \n-* `head_ref_submodule()`, `for_each_ref_submodule()`,\n- `for_each_ref_in_submodule()`, `for_each_tag_ref_submodule()`,\n- `for_each_branch_ref_submodule()`, `for_each_remote_ref_submodule()`\n- do the same as the functions described above but for a specified\n- submodule.\n+* Use `refs_` API for accessing submodules. The submodule ref store could\n+ be obtained with `get_submodule_ref_store()`.\n \n * `for_each_rawref()` can be used to learn about broken ref and symref.\n \ndiff --git a/refs.c b/refs.c\nindex 7fa19e9309..8c989ffec7 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -336,12 +336,6 @@ int for_each_tag_ref(each_ref_fn fn, void *cb_data)\n \treturn refs_for_each_tag_ref(get_main_ref_store(), fn, cb_data);\n }\n \n-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n-{\n-\treturn refs_for_each_tag_ref(get_submodule_ref_store(submodule),\n-\t\t\t\t fn, cb_data);\n-}\n-\n int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n {\n \treturn refs_for_each_ref_in(refs, \"refs/heads/\", fn, cb_data);\n@@ -352,12 +346,6 @@ int for_each_branch_ref(each_ref_fn fn, void *cb_data)\n \treturn refs_for_each_branch_ref(get_main_ref_store(), fn, cb_data);\n }\n \n-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n-{\n-\treturn refs_for_each_branch_ref(get_submodule_ref_store(submodule),\n-\t\t\t\t\tfn, cb_data);\n-}\n-\n int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n {\n \treturn refs_for_each_ref_in(refs, \"refs/remotes/\", fn, cb_data);\n@@ -1254,15 +1242,6 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n \treturn 0;\n }\n \n-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n-{\n-\tstruct ref_store *refs = get_submodule_ref_store(submodule);\n-\n-\tif (!refs)\n-\t\treturn -1;\n-\treturn refs_head_ref(refs, fn, cb_data);\n-}\n-\n int head_ref(each_ref_fn fn, void *cb_data)\n {\n \treturn refs_head_ref(get_main_ref_store(), fn, cb_data);\n@@ -1323,11 +1302,6 @@ int for_each_ref(each_ref_fn fn, void *cb_data)\n \treturn refs_for_each_ref(get_main_ref_store(), fn, cb_data);\n }\n \n-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n-{\n-\treturn refs_for_each_ref(get_submodule_ref_store(submodule), 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 {\n@@ -1349,13 +1323,6 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsig\n \t\t\t prefix, fn, 0, flag, cb_data);\n }\n \n-int for_each_ref_in_submodule(const char *submodule, const char *prefix,\n-\t\t\t each_ref_fn fn, void *cb_data)\n-{\n-\treturn refs_for_each_ref_in(get_submodule_ref_store(submodule),\n-\t\t\t\t prefix, fn, 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)\ndiff --git a/refs.h b/refs.h\nindex 5d25da227a..78a26400b6 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -304,16 +304,6 @@ 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_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\t\t 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-\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-- \n2.11.0.157.gd943d85\n\n"},{"id":"327035","messageId":"20170823123704.16518-12-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 11/16] revision.c: --all adds HEAD from all worktrees","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:36:59Z","receivedAt":"2017-08-23T12:38:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Unless single_worktree is set, --all now adds HEAD from all worktrees.\n\nSince reachable.c code does not use setup_revisions(), we need to call\nother_head_refs_submodule() explicitly there to have the same effect on\n\"git prune\", so that we won't accidentally delete objects needed by some\nother HEADs.\n\nA new FIXME is added because we would need something like\n\n int refs_other_head_refs(struct ref_store *, each_ref_fn, cb_data);\n\nin addition to other_head_refs() to handle it, which might require\n\n int get_submodule_worktrees(const char *submodule, int flags);\n\nIt could be a separate topic to reduce the scope of this one.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n reachable.c | 2 ++\n revision.c | 14 ++++++++++++++\n submodule.c | 2 ++\n t/t5304-prune.sh | 12 ++++++++++++\n worktree.c | 22 ++++++++++++++++++++++\n worktree.h | 8 ++++++++\n 6 files changed, 60 insertions(+)\n\ndiff --git a/reachable.c b/reachable.c\nindex c62efbfd43..492e87b9fa 100644\n--- a/reachable.c\n+++ b/reachable.c\n@@ -9,6 +9,7 @@\n #include \"cache-tree.h\"\n #include \"progress.h\"\n #include \"list-objects.h\"\n+#include \"worktree.h\"\n \n struct connectivity_progress {\n \tstruct progress *progress;\n@@ -176,6 +177,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,\n \n \t/* detached HEAD is not included in the list above */\n \thead_ref(add_one_ref, revs);\n+\tother_head_refs(add_one_ref, revs);\n \n \t/* Add all reflog info */\n \tif (mark_reflog)\ndiff --git a/revision.c b/revision.c\nindex 8d04516266..0e98444857 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2133,6 +2133,14 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \tint argcount;\n \n \tif (submodule) {\n+\t\t/*\n+\t\t * We need some something like get_submodule_worktrees()\n+\t\t * before we can go through all worktrees of a submodule,\n+\t\t * .e.g with adding all HEADs from --all, which is not\n+\t\t * supported right now, so stick to single worktree.\n+\t\t */\n+\t\tif (!revs->single_worktree)\n+\t\t\tdie(\"BUG: --single-worktree cannot be used together with submodule\");\n \t\trefs = get_submodule_ref_store(submodule);\n \t} else\n \t\trefs = get_main_ref_store();\n@@ -2150,6 +2158,12 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \tif (!strcmp(arg, \"--all\")) {\n \t\thandle_refs(refs, revs, *flags, refs_for_each_ref);\n \t\thandle_refs(refs, revs, *flags, refs_head_ref);\n+\t\tif (!revs->single_worktree) {\n+\t\t\tstruct all_refs_cb cb;\n+\n+\t\t\tinit_all_refs_cb(&cb, revs, *flags);\n+\t\t\tother_head_refs(handle_one_ref, &cb);\n+\t\t}\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n \t} else if (!strcmp(arg, \"--branches\")) {\n \t\thandle_refs(refs, revs, *flags, refs_for_each_branch_ref);\ndiff --git a/submodule.c b/submodule.c\nindex 98e1f9d3c7..61a38adcd4 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1685,6 +1685,8 @@ static int find_first_merges(struct object_array *result, const char *path,\n \t\t\toid_to_hex(&a->object.oid));\n \tinit_revisions(&revs, NULL);\n \trev_opts.submodule = path;\n+\t/* FIXME: can't handle linked worktrees in submodules yet */\n+\trevs.single_worktree = path != NULL;\n \tsetup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts);\n \n \t/* save all revisions from the above list that contain b */\ndiff --git a/t/t5304-prune.sh b/t/t5304-prune.sh\nindex cba45c7be9..683bdb031c 100755\n--- a/t/t5304-prune.sh\n+++ b/t/t5304-prune.sh\n@@ -292,4 +292,16 @@ test_expect_success 'prune: handle index in multiple worktrees' '\n \ttest_cmp second-worktree/blob actual\n '\n \n+test_expect_success 'prune: handle HEAD in multiple worktrees' '\n+\tgit worktree add --detach third-worktree &&\n+\techo \"new blob for third-worktree\" >third-worktree/blob &&\n+\tgit -C third-worktree add blob &&\n+\tgit -C third-worktree commit -m \"third\" &&\n+\trm .git/worktrees/third-worktree/index &&\n+\ttest_must_fail git -C third-worktree show :blob &&\n+\tgit prune --expire=now &&\n+\tgit -C third-worktree show HEAD:blob >actual &&\n+\ttest_cmp third-worktree/blob actual\n+'\n+\n test_done\ndiff --git a/worktree.c b/worktree.c\nindex e28ffbeb09..389e2e952c 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -386,3 +386,25 @@ int submodule_uses_worktrees(const char *path)\n \tclosedir(dir);\n \treturn ret;\n }\n+\n+int other_head_refs(each_ref_fn fn, void *cb_data)\n+{\n+\tstruct worktree **worktrees, **p;\n+\tint ret = 0;\n+\n+\tworktrees = get_worktrees(0);\n+\tfor (p = worktrees; *p; p++) {\n+\t\tstruct worktree *wt = *p;\n+\t\tstruct ref_store *refs;\n+\n+\t\tif (wt->is_current)\n+\t\t\tcontinue;\n+\n+\t\trefs = get_worktree_ref_store(wt);\n+\t\tret = refs_head_ref(refs, fn, cb_data);\n+\t\tif (ret)\n+\t\t\tbreak;\n+\t}\n+\tfree_worktrees(worktrees);\n+\treturn ret;\n+}\ndiff --git a/worktree.h b/worktree.h\nindex 5ea5e503fb..9276c81ae7 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -1,6 +1,8 @@\n #ifndef WORKTREE_H\n #define WORKTREE_H\n \n+#include \"refs.h\"\n+\n struct worktree {\n \tchar *path;\n \tchar *id;\n@@ -70,6 +72,12 @@ extern void free_worktrees(struct worktree **);\n extern const struct worktree *find_shared_symref(const char *symref,\n \t\t\t\t\t\t const char *target);\n \n+/*\n+ * Similar to head_ref() for all HEADs _except_ one from the current\n+ * worktree, which is covered by head_ref().\n+ */\n+int other_head_refs(each_ref_fn fn, void *cb_data);\n+\n int is_worktree_being_rebased(const struct worktree *wt, const char *target);\n int is_worktree_being_bisected(const struct worktree *wt, const char *target);\n \n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327036","messageId":"20170823123704.16518-13-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 12/16] files-backend: make reflog iterator go through per-worktree reflog","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:37:00Z","receivedAt":"2017-08-23T12:38:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"refs/bisect is unfortunately per-worktree, so we need to look in\nper-worktree logs/refs/bisect in addition to per-repo logs/refs. The\ncurrent iterator only goes through per-repo logs/refs.\n\nUse merge iterator to walk two ref stores at the same time and pick\nper-worktree refs from the right iterator.\n\nPS. Note the unsorted order of for_each_reflog in the test. This is\nsupposed to be OK, for now. If we enforce order on for_each_reflog()\nthen some more work will be required.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs/files-backend.c | 59 +++++++++++++++++++++++++++++++++----------\n t/t1407-worktree-ref-store.sh | 30 ++++++++++++++++++++++\n 2 files changed, 75 insertions(+), 14 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 5cca55510b..d4d22882ef 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -106,15 +106,6 @@ static void files_reflog_path(struct files_ref_store *refs,\n \t\t\t struct strbuf *sb,\n \t\t\t const char *refname)\n {\n-\tif (!refname) {\n-\t\t/*\n-\t\t * FIXME: of course this is wrong in multi worktree\n-\t\t * setting. To be fixed real soon.\n-\t\t */\n-\t\tstrbuf_addf(sb, \"%s/logs\", refs->gitcommondir);\n-\t\treturn;\n-\t}\n-\n \tswitch (ref_type(refname)) {\n \tcase REF_TYPE_PER_WORKTREE:\n \tcase REF_TYPE_PSEUDOREF:\n@@ -2055,23 +2046,63 @@ static struct ref_iterator_vtable files_reflog_iterator_vtable = {\n \tfiles_reflog_iterator_abort\n };\n \n-static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_store)\n+static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,\n+\t\t\t\t\t\t const char *gitdir)\n {\n-\tstruct files_ref_store *refs =\n-\t\tfiles_downcast(ref_store, REF_STORE_READ,\n-\t\t\t \"reflog_iterator_begin\");\n \tstruct files_reflog_iterator *iter = xcalloc(1, sizeof(*iter));\n \tstruct ref_iterator *ref_iterator = &iter->base;\n \tstruct strbuf sb = STRBUF_INIT;\n \n \tbase_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable);\n-\tfiles_reflog_path(refs, &sb, NULL);\n+\tstrbuf_addf(&sb, \"%s/logs\", gitdir);\n \titer->dir_iterator = dir_iterator_begin(sb.buf);\n \titer->ref_store = ref_store;\n \tstrbuf_release(&sb);\n+\n \treturn ref_iterator;\n }\n \n+static enum iterator_selection reflog_iterator_select(\n+\tstruct ref_iterator *iter_worktree,\n+\tstruct ref_iterator *iter_common,\n+\tvoid *cb_data)\n+{\n+\tif (iter_worktree) {\n+\t\t/*\n+\t\t * We're a bit loose here. We probably should ignore\n+\t\t * common refs if they are accidentally added as\n+\t\t * per-worktree refs.\n+\t\t */\n+\t\treturn ITER_SELECT_0;\n+\t} else if (iter_common) {\n+\t\tif (ref_type(iter_common->refname) == REF_TYPE_NORMAL)\n+\t\t\treturn ITER_SELECT_1;\n+\n+\t\t/*\n+\t\t * The main ref store may contain main worktree's\n+\t\t * per-worktree refs, which should be ignored\n+\t\t */\n+\t\treturn ITER_SKIP_1;\n+\t} else\n+\t\treturn ITER_DONE;\n+}\n+\n+static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_store)\n+{\n+\tstruct files_ref_store *refs =\n+\t\tfiles_downcast(ref_store, REF_STORE_READ,\n+\t\t\t \"reflog_iterator_begin\");\n+\n+\tif (!strcmp(refs->gitdir, refs->gitcommondir)) {\n+\t\treturn reflog_iterator_begin(ref_store, refs->gitcommondir);\n+\t} else {\n+\t\treturn merge_ref_iterator_begin(\n+\t\t\treflog_iterator_begin(ref_store, refs->gitdir),\n+\t\t\treflog_iterator_begin(ref_store, refs->gitcommondir),\n+\t\t\treflog_iterator_select, refs);\n+\t}\n+}\n+\n /*\n * If update is a direct update of head_ref (the reference pointed to\n * by HEAD), then add an extra REF_LOG_ONLY update for HEAD.\ndiff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh\nindex 5df06f3556..8842d0329f 100755\n--- a/t/t1407-worktree-ref-store.sh\n+++ b/t/t1407-worktree-ref-store.sh\n@@ -49,4 +49,34 @@ test_expect_success 'create_symref(FOO, refs/heads/master)' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'for_each_reflog()' '\n+\techo $_z40 > .git/logs/PSEUDO-MAIN &&\n+\tmkdir -p .git/logs/refs/bisect &&\n+\techo $_z40 > .git/logs/refs/bisect/random &&\n+\n+\techo $_z40 > .git/worktrees/wt/logs/PSEUDO-WT &&\n+\tmkdir -p .git/worktrees/wt/logs/refs/bisect &&\n+\techo $_z40 > .git/worktrees/wt/logs/refs/bisect/wt-random &&\n+\n+\t$RWT for-each-reflog | cut -c 42- | sort >actual &&\n+\tcat >expected <<-\\EOF &&\n+\tHEAD 0x1\n+\tPSEUDO-WT 0x0\n+\trefs/bisect/wt-random 0x0\n+\trefs/heads/master 0x0\n+\trefs/heads/wt-master 0x0\n+\tEOF\n+\ttest_cmp expected actual &&\n+\n+\t$RMAIN for-each-reflog | cut -c 42- | sort >actual &&\n+\tcat >expected <<-\\EOF &&\n+\tHEAD 0x1\n+\tPSEUDO-MAIN 0x0\n+\trefs/bisect/random 0x0\n+\trefs/heads/master 0x0\n+\trefs/heads/wt-master 0x0\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327037","messageId":"20170823123704.16518-14-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 13/16] revision.c: --reflog add HEAD reflog from all worktrees","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:37:01Z","receivedAt":"2017-08-23T12:38:43Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Note that add_other_reflogs_to_pending() is a bit inefficient, since\nit scans reflog for all refs of each worktree, including shared refs,\nso the shared ref's reflog is scanned over and over again.\n\nWe could update refs API to pass \"per-worktree only\" flag to avoid\nthat. But long term we should be able to obtain a \"per-worktree only\"\nref store and would need to revert the changes in reflog iteration\nAPI. So let's just wait until then.\n\nadd_reflogs_to_pending() is called by reachable.c so by default \"git\nprune\" will examine reflog from all worktrees.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n revision.c | 28 +++++++++++++++++++++++++++-\n t/t5304-prune.sh | 16 ++++++++++++++++\n 2 files changed, 43 insertions(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 0e98444857..d100b3a3be 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1132,6 +1132,7 @@ struct all_refs_cb {\n \tint warned_bad_reflog;\n \tstruct rev_info *all_revs;\n \tconst char *name_for_errormsg;\n+\tstruct ref_store *refs;\n };\n \n int ref_excluded(struct string_list *ref_excludes, const char *path)\n@@ -1168,6 +1169,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,\n \tcb->all_revs = revs;\n \tcb->all_flags = flags;\n \trevs->rev_input_given = 1;\n+\tcb->refs = NULL;\n }\n \n void clear_ref_exclusion(struct string_list **ref_excludes_p)\n@@ -1236,17 +1238,41 @@ static int handle_one_reflog(const char *path, const struct object_id *oid,\n \tstruct all_refs_cb *cb = cb_data;\n \tcb->warned_bad_reflog = 0;\n \tcb->name_for_errormsg = path;\n-\tfor_each_reflog_ent(path, handle_one_reflog_ent, cb_data);\n+\trefs_for_each_reflog_ent(cb->refs, path,\n+\t\t\t\t handle_one_reflog_ent, cb_data);\n \treturn 0;\n }\n \n+static void add_other_reflogs_to_pending(struct all_refs_cb *cb)\n+{\n+\tstruct worktree **worktrees, **p;\n+\n+\tworktrees = get_worktrees(0);\n+\tfor (p = worktrees; *p; p++) {\n+\t\tstruct worktree *wt = *p;\n+\n+\t\tif (wt->is_current)\n+\t\t\tcontinue;\n+\n+\t\tcb->refs = get_worktree_ref_store(wt);\n+\t\trefs_for_each_reflog(cb->refs,\n+\t\t\t\t handle_one_reflog,\n+\t\t\t\t cb);\n+\t}\n+\tfree_worktrees(worktrees);\n+}\n+\n void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)\n {\n \tstruct all_refs_cb cb;\n \n \tcb.all_revs = revs;\n \tcb.all_flags = flags;\n+\tcb.refs = get_main_ref_store();\n \tfor_each_reflog(handle_one_reflog, &cb);\n+\n+\tif (!revs->single_worktree)\n+\t\tadd_other_reflogs_to_pending(&cb);\n }\n \n static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,\ndiff --git a/t/t5304-prune.sh b/t/t5304-prune.sh\nindex 683bdb031c..6694c19a1e 100755\n--- a/t/t5304-prune.sh\n+++ b/t/t5304-prune.sh\n@@ -304,4 +304,20 @@ test_expect_success 'prune: handle HEAD in multiple worktrees' '\n \ttest_cmp third-worktree/blob actual\n '\n \n+test_expect_success 'prune: handle HEAD reflog in multiple worktrees' '\n+\tgit config core.logAllRefUpdates true &&\n+\techo \"lost blob for third-worktree\" >expected &&\n+\t(\n+\t\tcd third-worktree &&\n+\t\tcat ../expected >blob &&\n+\t\tgit add blob &&\n+\t\tgit commit -m \"second commit in third\" &&\n+\t\tgit reset --hard HEAD^\n+\t) &&\n+\tgit prune --expire=now &&\n+\tSHA1=`git hash-object expected` &&\n+\tgit -C third-worktree show \"$SHA1\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327038","messageId":"20170823123704.16518-15-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 14/16] rev-list: expose and document --single-worktree","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:37:02Z","receivedAt":"2017-08-23T12:38:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/rev-list-options.txt | 8 ++++++++\n revision.c | 2 ++\n 2 files changed, 10 insertions(+)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex a6cf9eb380..7d860bfca1 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -184,6 +184,14 @@ explicitly.\n \tPretend as if all objects mentioned by reflogs are listed on the\n \tcommand line as ``.\n \n+--single-worktree::\n+\tBy default, all working trees will be examined by the\n+\tfollowing options when there are more than one (see\n+\tlinkgit:git-worktree[1]): `--all`, `--reflog` and\n+\t`--indexed-objects`.\n+\tThis option forces them to examine the current working tree\n+\tonly.\n+\n --ignore-missing::\n \tUpon seeing an invalid object name in the input, pretend as if\n \tthe bad input was not given.\ndiff --git a/revision.c b/revision.c\nindex d100b3a3be..6eba4131b4 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2251,6 +2251,8 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \t\t\treturn error(\"invalid argument to --no-walk\");\n \t} else if (!strcmp(arg, \"--do-walk\")) {\n \t\trevs->no_walk = 0;\n+\t} else if (!strcmp(arg, \"--single-worktree\")) {\n+\t\trevs->single_worktree = 1;\n \t} else {\n \t\treturn 0;\n \t}\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327039","messageId":"20170823123704.16518-16-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 15/16] refs.c: remove fallback-to-main-store code get_submodule_ref_store()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:37:03Z","receivedAt":"2017-08-23T12:38:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"At this state, there are three get_submodule_ref_store() callers:\n\n - for_each_remote_ref_submodule()\n - handle_revision_pseudo_opt()\n - resolve_gitlink_ref()\n\nThe first two deal explicitly with submodules (and we should never fall\nback to the main ref store as a result). They are only called from\nsubmodule.c:\n\n - find_first_merges()\n - submodule_needs_pushing()\n - push_submodule()\n\nThe last one, as its name implies, deals only with submodules too, and\nthe \"submodule\" (path) argument must be a non-NULL, non-empty string.\n\nSo, this \"if NULL or empty string\" code block should never ever\ntrigger. And it's wrong to fall back to the main ref store\nanyway. Delete it.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 8c989ffec7..a0c5078901 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1587,6 +1587,9 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \tchar *to_free = NULL;\n \tsize_t len;\n \n+\tif (!submodule)\n+\t\treturn NULL;\n+\n \tif (submodule) {\n \t\tlen = strlen(submodule);\n \t\twhile (len && is_dir_sep(submodule[len - 1]))\n@@ -1595,14 +1598,6 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \t\t\treturn NULL;\n \t}\n \n-\tif (!submodule || !*submodule) {\n-\t\t/*\n-\t\t * FIXME: This case is ideally not allowed. But that\n-\t\t * can't happen until we clean up all the callers.\n-\t\t */\n-\t\treturn get_main_ref_store();\n-\t}\n-\n \tif (submodule[len])\n \t\t/* We need to strip off one or more trailing slashes */\n \t\tsubmodule = to_free = xmemdupz(submodule, len);\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327040","messageId":"20170823123704.16518-17-pclouds@gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"[PATCH v4 16/16] refs.c: reindent get_submodule_ref_store()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-23T12:37:04Z","receivedAt":"2017-08-23T12:39:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"With the new \"if (!submodule) return NULL;\" code added in the previous\ncommit, we don't need to check if submodule is not NULL anymore.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 12 +++++-------\n 1 file changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a0c5078901..206af61d62 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1590,13 +1590,11 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \tif (!submodule)\n \t\treturn NULL;\n \n-\tif (submodule) {\n-\t\tlen = strlen(submodule);\n-\t\twhile (len && is_dir_sep(submodule[len - 1]))\n-\t\t\tlen--;\n-\t\tif (!len)\n-\t\t\treturn NULL;\n-\t}\n+\tlen = strlen(submodule);\n+\twhile (len && is_dir_sep(submodule[len - 1]))\n+\t\tlen--;\n+\tif (!len)\n+\t\treturn NULL;\n \n \tif (submodule[len])\n \t\t/* We need to strip off one or more trailing slashes */\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327068","messageId":"CAGZ79kbvKZXGy80RCU3zkdm0S3M-tGB=LAJYiY=o+YwUB9RNHA@mail.gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-3-pclouds@gmail.com","subject":"Re: [PATCH v4 02/16] refs.c: use is_dir_sep() in resolve_gitlink_ref()","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-23T19:14:15Z","receivedAt":"2017-08-23T19:14:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 23, 2017 at 5:36 AM, Nguyễn Thái Ngọc Duy wrote:\n> The \"submodule\" argument in this function is a path, which can have\n> either '/' or '\\\\' as a separator. Use is_dir_sep() to support both.\n>\n> Noticed-by: Johannes Sixt \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n\nImmediate questions that come to mind:\n* Could this go in as an independent bug fix?\n -> If so do we have any test that fails or stops failing on Windows?\n -> If not, do we need one?\n* Assuming this is not an independent bug fix:\n Why do we need this patch in this series here?\n (I thought this is about worktrees, not submodules.\n So does this fix a potential bug that will be exposed\n later in this series, but was harmless up to now?)\n\nI'll read the next patches to see if I will be enlightened.\n\nThanks,\nStefan\n\n> ---\n> refs.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 3d549a8970..dec899a57a 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1507,7 +1507,7 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n> struct ref_store *refs;\n> int flags;\n>\n> - while (len && submodule[len - 1] == '/')\n> + while (len && is_dir_sep(submodule[len - 1]))\n> len--;\n>\n> if (!len)\n> --\n> 2.11.0.157.gd943d85\n>\n"},{"id":"327069","messageId":"CAGZ79kbPTr+DgcXcMmOax6SzLad4q8VByCiO=FH5hcmN7icc3g@mail.gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-5-pclouds@gmail.com","subject":"Re: [PATCH v4 04/16] revision.c: --indexed-objects add objects from all worktrees","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-23T19:25:22Z","receivedAt":"2017-08-23T19:25:28Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 23, 2017 at 5:36 AM, Nguyễn Thái Ngọc Duy wrote:\n> This is the result of single_worktree flag never being set (no way to up\n> until now). To get objects from current index only, set single_worktree.\n>\n> The other add_index_objects_to_pending's caller is mark_reachable_objects()\n> (e.g. \"git prune\") which also mark objects from all indexes.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> revision.c | 21 +++++++++++++++++++++\n> t/t5304-prune.sh | 9 +++++++++\n> 2 files changed, 30 insertions(+)\n>\n> diff --git a/revision.c b/revision.c\n> index 6c46ad6c8a..f35cb49af5 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -19,6 +19,7 @@\n> #include \"dir.h\"\n> #include \"cache-tree.h\"\n> #include \"bisect.h\"\n> +#include \"worktree.h\"\n>\n> volatile show_early_output_fn_t show_early_output;\n>\n> @@ -1290,8 +1291,28 @@ static void do_add_index_objects_to_pending(struct rev_info *revs,\n>\n> void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n> {\n> + struct worktree **worktrees, **p;\n> +\n> read_cache();\n> do_add_index_objects_to_pending(revs, &the_index);\n> +\n> + if (revs->single_worktree)\n> + return;\n> +\n> + worktrees = get_worktrees(0);\n> + for (p = worktrees; *p; p++) {\n> + struct worktree *wt = *p;\n> + struct index_state istate = { NULL };\n> +\n> + if (wt->is_current)\n> + continue; /* current index already taken care of */\n> +\n> + if (read_index_from(&istate,\n> + worktree_git_path(wt, \"index\")) > 0)\n> + do_add_index_objects_to_pending(revs, &istate);\n> + discard_index(&istate);\n> + }\n> + free_worktrees(worktrees);\n> }\n\nThe commit (message and code) looks acceptable and easy to read.\nI wonder though if another approach would be more feasible:\n\n* factor out anything after the early return into a new function\n \"add_wt_index_objects_to_pending\", maybe?\n* introduce a new function\n for_each_wt_except_current(function, callback_cookie)\n such that this code could use this predefined iterator.\n (I have the impression I have seen this code pattern before,\n hence it may be useful to have such a foreach function; we\n have foreach functions already for various ref things, string\n list items, reflogs, and soon submodule lists)\n\n>\n> static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,\n> diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh\n> index 133b5842b1..cba45c7be9 100755\n> --- a/t/t5304-prune.sh\n> +++ b/t/t5304-prune.sh\n> @@ -283,4 +283,13 @@ test_expect_success 'prune: handle alternate object database' '\n> git -C B prune\n> '\n>\n> +test_expect_success 'prune: handle index in multiple worktrees' '\n> + git worktree add second-worktree &&\n> + echo \"new blob for second-worktree\" >second-worktree/blob &&\n> + git -C second-worktree add blob &&\n> + git prune --expire=now &&\n> + git -C second-worktree show :blob >actual &&\n> + test_cmp second-worktree/blob actual\n> +'\n> +\n> test_done\n> --\n> 2.11.0.157.gd943d85\n>\n"},{"id":"327071","messageId":"CAGZ79kZ2e5+trudmcft39UC_PrROwvrrAE0w_-a_b_3s7ZjFQQ@mail.gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-6-pclouds@gmail.com","subject":"Re: [PATCH v4 05/16] refs.c: refactor get_submodule_ref_store(), share common free block","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-23T19:34:12Z","receivedAt":"2017-08-23T19:34:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 23, 2017 at 5:36 AM, Nguyễn Thái Ngọc Duy wrote:\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> refs.c | 17 ++++++-----------\n> 1 file changed, 6 insertions(+), 11 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index dec899a57a..522c4ab74f 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1636,7 +1636,6 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n> {\n> struct strbuf submodule_sb = STRBUF_INIT;\n> struct ref_store *refs;\n\nDo we need to assign NULL here, to cover the early outs?\n\n> - int ret;\n>\n> if (!submodule || !*submodule) {\n> /*\n> @@ -1648,19 +1647,14 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n>\n> refs = lookup_ref_store_map(&submodule_ref_stores, submodule);\n\nAnswering the question from above, no we do not need to\nassign NULL as we have an unconditional assignment\nhere and any early outs are after this.\n\nThanks,\nStefan\n"},{"id":"327074","messageId":"CAGZ79kaGjE-A6e1rHAzVsg5gCL5e2Kgidt40p6VPj=hXSHc_eg@mail.gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-11-pclouds@gmail.com","subject":"Re: [PATCH v4 10/16] refs: remove dead for_each_*_submodule()","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-23T19:45:36Z","receivedAt":"2017-08-23T19:45:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 23, 2017 at 5:36 AM, Nguyễn Thái Ngọc Duy wrote:\n> These are used in revision.c. After the last patch they are replaced\n> with the refs_ version. Delete them.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n\nThe refactoring up to this patch looks good to me.\n\nThanks,\nStefan\n"},{"id":"327076","messageId":"CAGZ79kY8cW+6+60bgSAhS69nBWZbn1LA7gT3t4Ddii+MB2V_cA@mail.gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-12-pclouds@gmail.com","subject":"Re: [PATCH v4 11/16] revision.c: --all adds HEAD from all worktrees","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-23T19:54:38Z","receivedAt":"2017-08-23T19:54:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> +int other_head_refs(each_ref_fn fn, void *cb_data)\n> +{\n> + struct worktree **worktrees, **p;\n> + int ret = 0;\n> +\n> + worktrees = get_worktrees(0);\n> + for (p = worktrees; *p; p++) {\n> + struct worktree *wt = *p;\n> + struct ref_store *refs;\n> +\n> + if (wt->is_current)\n> + continue;\n\nAs said in an earlier patch (and now this pattern\ncoming up twice in this patch series alone), the lines\nof this function up to here, could become\npart of a worktree iterator function?\n\n> + refs = get_worktree_ref_store(wt);\n> + ret = refs_head_ref(refs, fn, cb_data);\n> + if (ret)\n> + break;\n\nwith these 4 lines in the callback function.\n\n> +/*\n> + * Similar to head_ref() for all HEADs _except_ one from the current\n> + * worktree, which is covered by head_ref().\n> + */\n> +int other_head_refs(each_ref_fn fn, void *cb_data);\n\nThis is already such an iterator function, just at another\nabstraction level.\n"},{"id":"327084","messageId":"CAGZ79kadXiwY9R4e0jYSanTmQJgR7axbEoDP2WcVQTxkDhq_Sg@mail.gmail.com","threadId":"46659","inReplyTo":"20170823123704.16518-15-pclouds@gmail.com","subject":"Re: [PATCH v4 14/16] rev-list: expose and document --single-worktree","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-23T20:45:51Z","receivedAt":"2017-08-23T20:46:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 23, 2017 at 5:37 AM, Nguyễn Thái Ngọc Duy wrote:\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> Documentation/rev-list-options.txt | 8 ++++++++\n> revision.c | 2 ++\n> 2 files changed, 10 insertions(+)\n>\n> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n> index a6cf9eb380..7d860bfca1 100644\n> --- a/Documentation/rev-list-options.txt\n> +++ b/Documentation/rev-list-options.txt\n> @@ -184,6 +184,14 @@ explicitly.\n> Pretend as if all objects mentioned by reflogs are listed on the\n> command line as ``.\n>\n> +--single-worktree::\n> + By default, all working trees will be examined by the\n> + following options when there are more than one (see\n> + linkgit:git-worktree[1]): `--all`, `--reflog` and\n> + `--indexed-objects`.\n> + This option forces them to examine the current working tree\n> + only.\n> +\n> --ignore-missing::\n> Upon seeing an invalid object name in the input, pretend as if\n> the bad input was not given.\n> diff --git a/revision.c b/revision.c\n> index d100b3a3be..6eba4131b4 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2251,6 +2251,8 @@ static int handle_revision_pseudo_opt(const char *submodule,\n> return error(\"invalid argument to --no-walk\");\n> } else if (!strcmp(arg, \"--do-walk\")) {\n> revs->no_walk = 0;\n> + } else if (!strcmp(arg, \"--single-worktree\")) {\n> + revs->single_worktree = 1;\n\nThis is in handle_revision_pseudo_opt, that has the note\n\n/*\n* NOTE!\n*\n* Commands like \"git shortlog\" will not accept the options below\n* unless parse_revision_opt queues them (as opposed to erroring\n* out).\n*\n* When implementing your new pseudo-option, remember to\n* register it in the list at the top of handle_revision_opt.\n*/\n\nThe registration needs to be done at around line 1700.\n\nBut come to think of it, is it really a pseudo opt?\nCould it be a \"real\" (non pseudo) opt in handle_revision_opt?\nThe reasoning (either way) would be of interest in the\ncommit message, IMHO.\n"},{"id":"327129","messageId":"20170824141348.GC2021@mimir","threadId":"46659","inReplyTo":"20170823123704.16518-13-pclouds@gmail.com","subject":"Re: [PATCH v4 12/16] files-backend: make reflog iterator go through per-worktree reflog","fromName":"Richard Maw","fromEmail":"richard.maw@gmail.com","sentAt":"2017-08-24T14:13:48Z","receivedAt":"2017-08-24T14:13:58Z","isPatch":true,"sender":{"key":"richard.maw@gmail.com","avatar":"https://gravatar.com/avatar/ee59b32f2b88ec3ea1653e30e237c5952e6eaa8602906afd161831127745ff70?d=mp&s=160"},"body":"On Wed, Aug 23, 2017 at 07:37:00PM +0700, Nguyễn Thái Ngọc Duy wrote:\n> refs/bisect is unfortunately per-worktree, so we need to look in\n> per-worktree logs/refs/bisect in addition to per-repo logs/refs. The\n> current iterator only goes through per-repo logs/refs.\n> \n> Use merge iterator to walk two ref stores at the same time and pick\n> per-worktree refs from the right iterator.\n> \n> PS. Note the unsorted order of for_each_reflog in the test. This is\n> supposed to be OK, for now. If we enforce order on for_each_reflog()\n> then some more work will be required.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> refs/files-backend.c | 59 +++++++++++++++++++++++++++++++++----------\n> t/t1407-worktree-ref-store.sh | 30 ++++++++++++++++++++++\n> 2 files changed, 75 insertions(+), 14 deletions(-)\n> \n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 5cca55510b..d4d22882ef 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -2055,23 +2046,63 @@ static struct ref_iterator_vtable files_reflog_iterator_vtable = {\n\n> +static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_store)\n> +{\n> +\tstruct files_ref_store *refs =\n> +\t\tfiles_downcast(ref_store, REF_STORE_READ,\n> +\t\t\t \"reflog_iterator_begin\");\n> +\n> +\tif (!strcmp(refs->gitdir, refs->gitcommondir)) {\n> +\t\treturn reflog_iterator_begin(ref_store, refs->gitcommondir);\n> +\t} else {\n> +\t\treturn merge_ref_iterator_begin(\n> +\t\t\treflog_iterator_begin(ref_store, refs->gitdir),\n> +\t\t\treflog_iterator_begin(ref_store, refs->gitcommondir),\n> +\t\t\treflog_iterator_select, refs);\n> +\t}\n> +}\n> +\n> /*\n> * If update is a direct update of head_ref (the reference pointed to\n> * by HEAD), then add an extra REF_LOG_ONLY update for HEAD.\n\nWhilst trying to use ref backends to implement ref namespaces\none of the issues I had was that namespaced bisect refs weren't being found.\n\nThis turned out to be for the same reason their reflogs weren't found,\nthat by only iterating through the commondir refs it was missing the gitdir ones\nand that it only worked normally because of a special case in loose_fill_ref_dir\n\n\n /*\n * Manually add refs/bisect, which, being per-worktree, might\n * not appear in the directory listing for refs/ in the main\n * repo.\n */\n if (!strcmp(dirname, \"refs/\")) {\n int pos = search_ref_dir(dir, \"refs/bisect/\", 12);\n\n if (pos < 0) {\n struct ref_entry *child_entry = create_dir_entry(\n dir->cache, \"refs/bisect/\", 12, 1);\n add_entry_to_dir(dir, child_entry);\n }\n }\n\nIf files_ref_iterator_begin was made to use a merged or overlay ref iterator too\nthen this special case could be removed and bisecting in a namespaced workspace\nwould work.\n\nI've yet to work out whether namespaced bisect refs makes sense,\nbut that's a problem for the next time I have time to work on it :).\n"},{"id":"327161","messageId":"xmqqfucgvdk8.fsf@gitster.mtv.corp.google.com","threadId":"46659","inReplyTo":"20170823123704.16518-8-pclouds@gmail.com","subject":"Re: [PATCH v4 07/16] refs: add refs_head_ref()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-24T21:52:23Z","receivedAt":"2017-08-24T21:52:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n... which does what?\n\n Unlike refs_for_each_ref() and friends, this does not iterate.\n It just uses the same function signature to make a single call\n of fn on the \"HEAD\" ref.\n\nDid I capture what it does right?\n\nThanks.\n"},{"id":"327162","messageId":"xmqqbmn4vde5.fsf@gitster.mtv.corp.google.com","threadId":"46659","inReplyTo":"20170823123704.16518-9-pclouds@gmail.com","subject":"Re: [PATCH v4 08/16] revision.c: use refs_for_each*() instead of for_each_*_submodule()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-24T21:56:02Z","receivedAt":"2017-08-24T21:56:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n\nSo... the idea is that the caller gives a ref-store and tells these\nfunctions to iterate over refs in it, and the existing submodule\nrelated callers can prepare a ref-store for the submodule---that\nway, refs.[ch] layer does not have to _care_ that the set of refs it\nwas asked to look at is for submodule processing.\n\nNice. Very nice.\n\n> @@ -2120,8 +2129,14 @@ static int handle_revision_pseudo_opt(const char *submodule,\n> {\n> \tconst char *arg = argv[0];\n> \tconst char *optarg;\n> +\tstruct ref_store *refs;\n> \tint argcount;\n> \n> +\tif (submodule) {\n> +\t\trefs = get_submodule_ref_store(submodule);\n> +\t} else\n> +\t\trefs = get_main_ref_store();\n> +\n\n"},{"id":"327181","messageId":"6b67875c-847c-8d5f-14bb-c3fb7004ebdc@grubix.eu","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"Re: [PATCH v4 00/16] Fix git-gc losing objects in multi worktree","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-08-25T11:21:06Z","receivedAt":"2017-08-25T11:21:21Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Nguyễn Thái Ngọc Duy venit, vidit, dixit 23.08.2017 14:36:\n> \"git gc\" when used in multiple worktrees ignore some per-worktree\n> references: object references in the index, HEAD and reflog. This\n> series fixes it by making the revision walker include these from all\n> worktrees by default (and the series is basically split in three parts\n> in the same order). There's a couple more cleanups in refs.c. Luckily\n> it does not conflict with anything in 'pu'.\n> \n> Compared to v3 [1], the largest change is supporting multi worktree in\n> the reflog iterator. The merge iterator is now used (Micheal was right\n> all along).\n> \n> [1] https://public-inbox.org/git/20170419110145.5086-1-pclouds@gmail.com/\n> \n> Nguyễn Thái Ngọc Duy (16):\n> revision.h: new flag in struct rev_info wrt. worktree-related refs\n> refs.c: use is_dir_sep() in resolve_gitlink_ref()\n> revision.c: refactor add_index_objects_to_pending()\n> revision.c: --indexed-objects add objects from all worktrees\n> refs.c: refactor get_submodule_ref_store(), share common free block\n> refs: move submodule slash stripping code to get_submodule_ref_store\n> refs: add refs_head_ref()\n> revision.c: use refs_for_each*() instead of for_each_*_submodule()\n> refs.c: move for_each_remote_ref_submodule() to submodule.c\n> refs: remove dead for_each_*_submodule()\n> revision.c: --all adds HEAD from all worktrees\n> files-backend: make reflog iterator go through per-worktree reflog\n> revision.c: --reflog add HEAD reflog from all worktrees\n> rev-list: expose and document --single-worktree\n> refs.c: remove fallback-to-main-store code get_submodule_ref_store()\n> refs.c: reindent get_submodule_ref_store()\n\nI probably won't be able to review this (many commits without commit\nmessage), but I'm happy to see progress on the \"--all\" and prune front\n(and will test). I suggest we think about the UI exposure a bit when it\ncomes to including all heads or naming options, though:\n\n* HEAD is \"the current head\"\n* refs/heads is where all local branch heads are\n\n* --branches is the rev-list/log option for refs/heads/*\n* --all is the rev-list/log option for refs/* plus HEAD\n* HEAD is the rev-list/log argument for HEAD\n\n* --heads is the show-ref option limiting to refs/heads/*\n* --head is the show-ref option which adds HEAD\n\n* refs/heads is the for-each-ref-pattern for refs/heads/*\n* HEAD is not the for-each-ref-pattern for HEAD\n[I'll suggest a patch to change the latter, shortly.]\n\nI would hope that the result of this series and other efforts will be:\n\n* consistent way to specify \"all local branch heads\"\n* consistent way to specify \"the head\" aka HEAD\n* consistent way to specify \"all linked worktree heads\"\n[* maybe something for submodules...]\n\nThis may require changing the misnamed show-ref option, but also\nthinking twice before changing the meaning of \"--all\" for the\nrev-list/log family: it's easy to say \"--all --linked\" or \"--all\n--heads\" to get everything plus all linked worktree heads when \"--all\"\n== \"--branches --tags HEAD\", but it's more cumbersome with a changed\n--all that is \"really everything\". I gues my suggestion would be:\n\n--all as it is now (refs/* plus HEAD)\n\n--head alternative way to say HEAD (as it is now for show-ref)\n\n--heads HEAD for all linked worktrees (incompatible change for show-ref)\n\nAnd all of them should work the same for the rev-list/log family as well\nas for-each-ref/show-ref.\n\nI thinking that changing show-ref (porcelain, not quite as commonly\nused) should do the least harm compared to all other options.\n\nMichael\n"},{"id":"327639","messageId":"CACsJy8Cd20iPo4uv5+sq0SiuPT3Y6sxEtJqDsox+3tVRsgg9UA@mail.gmail.com","threadId":"46659","inReplyTo":"6b67875c-847c-8d5f-14bb-c3fb7004ebdc@grubix.eu","subject":"Re: [PATCH v4 00/16] Fix git-gc losing objects in multi worktree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-09-06T10:53:05Z","receivedAt":"2017-09-06T10:53:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Aug 25, 2017 at 6:21 PM, Michael J Gruber wrote:\n> I suggest we think about the UI exposure a bit when it\n> comes to including all heads or naming options, though:\n>\n> * HEAD is \"the current head\"\n> * refs/heads is where all local branch heads are\n>\n> * --branches is the rev-list/log option for refs/heads/*\n> * --all is the rev-list/log option for refs/* plus HEAD\n> * HEAD is the rev-list/log argument for HEAD\n\nIt also covers object references from the index file aka --indexed-objects\n\n> * --heads is the show-ref option limiting to refs/heads/*\n> * --head is the show-ref option which adds HEAD\n>\n> * refs/heads is the for-each-ref-pattern for refs/heads/*\n> * HEAD is not the for-each-ref-pattern for HEAD\n> [I'll suggest a patch to change the latter, shortly.]\n>\n> I would hope that the result of this series and other efforts will be:\n>\n> * consistent way to specify \"all local branch heads\"\n> * consistent way to specify \"the head\" aka HEAD\n> * consistent way to specify \"all linked worktree heads\"\n> [* maybe something for submodules...]\n\nHmm.. I admit that I completely overlooked 'git show-ref'.\n\n> This may require changing the misnamed show-ref option, but also\n> thinking twice before changing the meaning of \"--all\" for the\n> rev-list/log family: it's easy to say \"--all --linked\" or \"--all\n> --heads\" to get everything plus all linked worktree heads when \"--all\"\n> == \"--branches --tags HEAD\", but it's more cumbersome with a changed\n> --all that is \"really everything\". I gues my suggestion would be:\n>\n> --all as it is now (refs/* plus HEAD)\n>\n> --head alternative way to say HEAD (as it is now for show-ref)\n>\n> --heads HEAD for all linked worktrees (incompatible change for show-ref)\n>\n> And all of them should work the same for the rev-list/log family as well\n> as for-each-ref/show-ref.\n\nHow about: show-ref learns a new option to let it list HEAD (and other\nper-worktree refs) of one/current worktree, or all worktrees. This is\nwhat the --single-worktree option is for, which is added by this\nseries (but I need to make sure if's exposed in show-ref as well). For\nshowing refs as viewed by another worktree, we could have the global\noption similar to --git-dir to select that worktree, e.g. \"git\n--work-tree-id=XXX show-ref ...\"?\n\nSince this seems a good thing to do, but not necessary to fix the\n\"prune\" bug, I'll do it separately instead of adding in this series. I\nmay need to look at \"git for-each-ref\" too for that matter.\n\n> I thinking that changing show-ref (porcelain, not quite as commonly\n> used) should do the least harm compared to all other options.\n-- \nDuy\n"},{"id":"327640","messageId":"CACsJy8A0x-PJsO+uhv8=mdfeKLEBPfT3sq3_Fi2_HffeexDeAw@mail.gmail.com","threadId":"46659","inReplyTo":"CAGZ79kbvKZXGy80RCU3zkdm0S3M-tGB=LAJYiY=o+YwUB9RNHA@mail.gmail.com","subject":"Re: [PATCH v4 02/16] refs.c: use is_dir_sep() in resolve_gitlink_ref()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-09-06T11:08:27Z","receivedAt":"2017-09-06T11:09:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Aug 24, 2017 at 2:14 AM, Stefan Beller wrote:\n> On Wed, Aug 23, 2017 at 5:36 AM, Nguyễn Thái Ngọc Duy wrote:\n>> The \"submodule\" argument in this function is a path, which can have\n>> either '/' or '\\\\' as a separator. Use is_dir_sep() to support both.\n>>\n>> Noticed-by: Johannes Sixt \n>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>\n> Immediate questions that come to mind:\n> * Could this go in as an independent bug fix?\n\nIt probably could. But it may depend on other submodule changes in this series.\n\n> -> If so do we have any test that fails or stops failing on Windows?\n\nIt was spotted during the review [1] so I guess no test fails on\nWindows (not that I would know because I can't run tests on Windows)\n\n> -> If not, do we need one?\n\nSince I don't use Windows, I don't particularly care. And I can't add\none anyway because I can't run it.\n\n[1] https://public-inbox.org/git/%3Ca74cf309-fb16-2f45-8189-d1d0c655dea4@kdbg.org%3E/\n\n> * Assuming this is not an independent bug fix:\n> Why do we need this patch in this series here?\n> (I thought this is about worktrees, not submodules.\n> So does this fix a potential bug that will be exposed\n> later in this series, but was harmless up to now?)\n\nThe series could probably be split in two. The first part is about\nusing the new refs_* API in revision.c. This helps clean up refs API a\nbit, all *_submodule() functions will be one. Not strictly needed to\nbe part of this, it's just a continuation of my previous series that\nintroduces *_refs. Since submodule code is touched, this is found.\n\nThe second part is actually fixing the prune bug.\n\nShould I split the series in two? I think I separated two parts in the\npast (maybe I misremember) at least in the description...\n-- \nDuy\n"},{"id":"327641","messageId":"CACsJy8BiRM5HE5vkt6pa+dsNmoPjEbRM+UTA3323kPe9rwu9VQ@mail.gmail.com","threadId":"46659","inReplyTo":"CAGZ79kY8cW+6+60bgSAhS69nBWZbn1LA7gT3t4Ddii+MB2V_cA@mail.gmail.com","subject":"Re: [PATCH v4 11/16] revision.c: --all adds HEAD from all worktrees","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-09-06T11:19:23Z","receivedAt":"2017-09-06T11:20:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Aug 24, 2017 at 2:54 AM, Stefan Beller wrote:\n>> +int other_head_refs(each_ref_fn fn, void *cb_data)\n>> +{\n>> + struct worktree **worktrees, **p;\n>> + int ret = 0;\n>> +\n>> + worktrees = get_worktrees(0);\n>> + for (p = worktrees; *p; p++) {\n>> + struct worktree *wt = *p;\n>> + struct ref_store *refs;\n>> +\n>> + if (wt->is_current)\n>> + continue;\n>\n> As said in an earlier patch (and now this pattern\n> coming up twice in this patch series alone), the lines\n> of this function up to here, could become\n> part of a worktree iterator function?\n\nThere are a couple \"loop through all worktrees\" code so far, but the\nfilter condition is not always this.\n\nWhile I like the idea of iterator function (especially if it does\n\"struct worktree\" memory management internally), I think it's a bit\noverkill to go for_each_worktree() with a callback function when the\nequivalent for(;;) is so short. We would need to declare struct to\npass callback data, and the reader may have to got read\nfor_each_worktree() code then come back here.\n\nSo, probably no worktree iterator (yet).\n\n>> + refs = get_worktree_ref_store(wt);\n>> + ret = refs_head_ref(refs, fn, cb_data);\n>> + if (ret)\n>> + break;\n>\n> with these 4 lines in the callback function.\n>\n>> +/*\n>> + * Similar to head_ref() for all HEADs _except_ one from the current\n>> + * worktree, which is covered by head_ref().\n>> + */\n>> +int other_head_refs(each_ref_fn fn, void *cb_data);\n>\n> This is already such an iterator function, just at another\n> abstraction level.\n\nyeah.. but we can't mix and match (or combine) ref/worktree iterator callbacks\n\n-- \nDuy\n"},{"id":"327642","messageId":"CACsJy8BdyS0TpP6o+qyfQpQ1wyfSn63P8U70EkGoV4-+EFteHw@mail.gmail.com","threadId":"46659","inReplyTo":"xmqqfucgvdk8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 07/16] refs: add refs_head_ref()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-09-06T11:23:12Z","receivedAt":"2017-09-06T11:23:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Aug 25, 2017 at 4:52 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n> ... which does what?\n>\n> Unlike refs_for_each_ref() and friends, this does not iterate.\n> It just uses the same function signature to make a single call\n> of fn on the \"HEAD\" ref.\n>\n> Did I capture what it does right?\n\nIt's basically head_ref() but can take any ref store (while head_ref()\nalways uses the main ref store). I'll update the commit message. What\nyou describe is correct, but I think I'll leave that out and assume\npeople know what head_ref() does.\n-- \nDuy\n"},{"id":"327660","messageId":"CAGZ79kagZOmTDhsHr7-t42xoecW3rXUOU7Ss5B13Jyfk6PT24Q@mail.gmail.com","threadId":"46659","inReplyTo":"CACsJy8A0x-PJsO+uhv8=mdfeKLEBPfT3sq3_Fi2_HffeexDeAw@mail.gmail.com","subject":"Re: [PATCH v4 02/16] refs.c: use is_dir_sep() in resolve_gitlink_ref()","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-09-06T17:41:31Z","receivedAt":"2017-09-06T17:41:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Sep 6, 2017 at 4:08 AM, Duy Nguyen wrote:\n> On Thu, Aug 24, 2017 at 2:14 AM, Stefan Beller wrote:\n>> On Wed, Aug 23, 2017 at 5:36 AM, Nguyễn Thái Ngọc Duy wrote:\n>>> The \"submodule\" argument in this function is a path, which can have\n>>> either '/' or '\\\\' as a separator. Use is_dir_sep() to support both.\n>>>\n>>> Noticed-by: Johannes Sixt \n>>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>>\n>> Immediate questions that come to mind:\n>> * Could this go in as an independent bug fix?\n>\n> It probably could. But it may depend on other submodule changes in this series.\n>\n>> -> If so do we have any test that fails or stops failing on Windows?\n>\n> It was spotted during the review [1] so I guess no test fails on\n> Windows (not that I would know because I can't run tests on Windows)\n>\n>> -> If not, do we need one?\n>\n> Since I don't use Windows, I don't particularly care. And I can't add\n> one anyway because I can't run it.\n>\n> [1] https://public-inbox.org/git/%3Ca74cf309-fb16-2f45-8189-d1d0c655dea4@kdbg.org%3E/\n\nIIRC I asked these questions as I was genuinely confused,\nI do not mind too much either.\n\n>\n>> * Assuming this is not an independent bug fix:\n>> Why do we need this patch in this series here?\n>> (I thought this is about worktrees, not submodules.\n>> So does this fix a potential bug that will be exposed\n>> later in this series, but was harmless up to now?)\n>\n> The series could probably be split in two. The first part is about\n> using the new refs_* API in revision.c. This helps clean up refs API a\n> bit, all *_submodule() functions will be one. Not strictly needed to\n> be part of this, it's just a continuation of my previous series that\n> introduces *_refs. Since submodule code is touched, this is found.\n>\n> The second part is actually fixing the prune bug.\n>\n> Should I split the series in two? I think I separated two parts in the\n> past (maybe I misremember) at least in the description...\n\nI had to reread the coverletter\nhttps://public-inbox.org/git/20170823123704.16518-1-pclouds@gmail.com/\nto get that part. I would not be opposed to splitting the series, but\nI'll review an unsplit series as well.\n\nThanks,\nStefan\n\n> --\n> Duy\n"},{"id":"327661","messageId":"CAGZ79kbw9_hAOTQz6hBsydcMoeK45pkjM-RoNedqzztCZSqbYQ@mail.gmail.com","threadId":"46659","inReplyTo":"CACsJy8BiRM5HE5vkt6pa+dsNmoPjEbRM+UTA3323kPe9rwu9VQ@mail.gmail.com","subject":"Re: [PATCH v4 11/16] revision.c: --all adds HEAD from all worktrees","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-09-06T17:43:03Z","receivedAt":"2017-09-06T17:43:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Sep 6, 2017 at 4:19 AM, Duy Nguyen wrote:\n>\n> So, probably no worktree iterator (yet).\n\nOk, thanks for considering it.\n"},{"id":"327800","messageId":"c620f399-9074-4a43-aed1-3995a1651114@alum.mit.edu","threadId":"46659","inReplyTo":"20170823123704.16518-7-pclouds@gmail.com","subject":"Re: [PATCH v4 06/16] refs: move submodule slash stripping code to get_submodule_ref_store","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-09-09T05:45:05Z","receivedAt":"2017-09-09T05:45:14Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/23/2017 02:36 PM, Nguyễn Thái Ngọc Duy wrote:\n> This is a better place that will benefit all submodule callers instead\n> of just resolve_gitlink_ref()\n\nThis is a nice sentiment, but I have to wonder whether we should rather\nbe requiring callers to use the \"canonical\" submodule name rather than\ncovering up their sloppiness for them (perhaps multiple times?). I\nvaguely remember intentionally limiting the number of functions that do\nthis canonicalization, and having in mind the goal that the number\nshould become smaller over time, not larger.\n\nFor example, there could be some kind of `canonicalize_submodule_name()`\nfunction that callers can use to clean up whatever submodule string they\nhave in hand, then let them use the cleaned up value from then on.\n\nI don't really know much about the callers, though, so that is more of a\nlazy philosophical speculation than a concrete suggestion.\n\nMichael\n"},{"id":"327801","messageId":"9705a733-76b4-e92c-1e4c-bb734b184529@alum.mit.edu","threadId":"46659","inReplyTo":"20170823123704.16518-11-pclouds@gmail.com","subject":"Re: [PATCH v4 10/16] refs: remove dead for_each_*_submodule()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-09-09T05:59:05Z","receivedAt":"2017-09-09T05:59:13Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/23/2017 02:36 PM, Nguyễn Thái Ngọc Duy wrote:\n> These are used in revision.c. After the last patch they are replaced\n> with the refs_ version. Delete them.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> Documentation/technical/api-ref-iteration.txt | 7 ++----\n> refs.c | 33 ---------------------------\n> refs.h | 10 --------\n> 3 files changed, 2 insertions(+), 48 deletions(-)\n\nWhat a lovely diffstat :-)\n\nMichael\n"},{"id":"327802","messageId":"ee8f9da9-3743-b8a0-d8dd-ec6a3d6f2cbf@alum.mit.edu","threadId":"46659","inReplyTo":"20170823123704.16518-12-pclouds@gmail.com","subject":"Re: [PATCH v4 11/16] revision.c: --all adds HEAD from all worktrees","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-09-09T06:04:35Z","receivedAt":"2017-09-09T06:04:46Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/23/2017 02:36 PM, Nguyễn Thái Ngọc Duy wrote:\n> [...]\n> diff --git a/revision.c b/revision.c\n> index 8d04516266..0e98444857 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2133,6 +2133,14 @@ static int handle_revision_pseudo_opt(const char *submodule,\n> \tint argcount;\n> \n> \tif (submodule) {\n> +\t\t/*\n> +\t\t * We need some something like get_submodule_worktrees()\n> +\t\t * before we can go through all worktrees of a submodule,\n> +\t\t * .e.g with adding all HEADs from --all, which is not\n> +\t\t * supported right now, so stick to single worktree.\n> +\t\t */\n> +\t\tif (!revs->single_worktree)\n> +\t\t\tdie(\"BUG: --single-worktree cannot be used together with submodule\");\n> \t\trefs = get_submodule_ref_store(submodule);\n> \t} else\n> \t\trefs = get_main_ref_store();\n\nTiny nit: s/.e.g/e.g./\n\n> [...]\n\nMichael\n"},{"id":"327803","messageId":"22ce3629-cd72-7fe0-8511-29b34e135122@alum.mit.edu","threadId":"46659","inReplyTo":"20170823123704.16518-13-pclouds@gmail.com","subject":"Re: [PATCH v4 12/16] files-backend: make reflog iterator go through per-worktree reflog","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-09-09T06:30:18Z","receivedAt":"2017-09-09T06:30:27Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/23/2017 02:37 PM, Nguyễn Thái Ngọc Duy wrote:\n> refs/bisect is unfortunately per-worktree, so we need to look in\n> per-worktree logs/refs/bisect in addition to per-repo logs/refs. The\n> current iterator only goes through per-repo logs/refs.\n> \n> Use merge iterator to walk two ref stores at the same time and pick\n> per-worktree refs from the right iterator.\n> \n> PS. Note the unsorted order of for_each_reflog in the test. This is\n> supposed to be OK, for now. If we enforce order on for_each_reflog()\n> then some more work will be required.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> refs/files-backend.c | 59 +++++++++++++++++++++++++++++++++----------\n> t/t1407-worktree-ref-store.sh | 30 ++++++++++++++++++++++\n> 2 files changed, 75 insertions(+), 14 deletions(-)\n> \n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 5cca55510b..d4d22882ef 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> [...]\n> +static enum iterator_selection reflog_iterator_select(\n> +\tstruct ref_iterator *iter_worktree,\n> +\tstruct ref_iterator *iter_common,\n> +\tvoid *cb_data)\n> +{\n> +\tif (iter_worktree) {\n> +\t\t/*\n> +\t\t * We're a bit loose here. We probably should ignore\n> +\t\t * common refs if they are accidentally added as\n> +\t\t * per-worktree refs.\n> +\t\t */\n> +\t\treturn ITER_SELECT_0;\n\nI don't understand the point of the comment. If we should ignore common\nrefs here, why not do it rather than commenting about it? Wouldn't it be\nreally easy to implement? OTOH if it's not needed, then why the comment?\n\n> [...]\n\nMichael\n"},{"id":"327804","messageId":"8e447d30-9306-9ac6-6e87-be8192a1c7f6@alum.mit.edu","threadId":"46659","inReplyTo":"20170823123704.16518-16-pclouds@gmail.com","subject":"Re: [PATCH v4 15/16] refs.c: remove fallback-to-main-store code get_submodule_ref_store()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-09-09T06:36:30Z","receivedAt":"2017-09-09T06:36:40Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/23/2017 02:37 PM, Nguyễn Thái Ngọc Duy wrote:\n> At this state, there are three get_submodule_ref_store() callers:\n> \n> - for_each_remote_ref_submodule()\n> - handle_revision_pseudo_opt()\n> - resolve_gitlink_ref()\n> \n> The first two deal explicitly with submodules (and we should never fall\n> back to the main ref store as a result). They are only called from\n> submodule.c:\n> \n> - find_first_merges()\n> - submodule_needs_pushing()\n> - push_submodule()\n> \n> The last one, as its name implies, deals only with submodules too, and\n> the \"submodule\" (path) argument must be a non-NULL, non-empty string.\n> \n> So, this \"if NULL or empty string\" code block should never ever\n> trigger. And it's wrong to fall back to the main ref store\n> anyway. Delete it.\n\nNice! Thanks for the cleanup.\n\nMichael\n"},{"id":"327805","messageId":"a445a47f-9fe9-1c56-8e14-eb5aafbdbb0f@alum.mit.edu","threadId":"46659","inReplyTo":"20170823123704.16518-17-pclouds@gmail.com","subject":"Re: [PATCH v4 16/16] refs.c: reindent get_submodule_ref_store()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-09-09T06:41:54Z","receivedAt":"2017-09-09T06:42:02Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/23/2017 02:37 PM, Nguyễn Thái Ngọc Duy wrote:\n> With the new \"if (!submodule) return NULL;\" code added in the previous\n> commit, we don't need to check if submodule is not NULL anymore.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> refs.c | 12 +++++-------\n> 1 file changed, 5 insertions(+), 7 deletions(-)\n> \n> diff --git a/refs.c b/refs.c\n> index a0c5078901..206af61d62 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1590,13 +1590,11 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n> \tif (!submodule)\n> \t\treturn NULL;\n> \n> -\tif (submodule) {\n> -\t\tlen = strlen(submodule);\n> -\t\twhile (len && is_dir_sep(submodule[len - 1]))\n> -\t\t\tlen--;\n> -\t\tif (!len)\n> -\t\t\treturn NULL;\n> -\t}\n> +\tlen = strlen(submodule);\n> +\twhile (len && is_dir_sep(submodule[len - 1]))\n> +\t\tlen--;\n> +\tif (!len)\n> +\t\treturn NULL;\n> \n> \tif (submodule[len])\n> \t\t/* We need to strip off one or more trailing slashes */\n> \n\nNow I'm confused. Is it still allowed to call\n`get_submodule_ref_store(NULL)`? If not, then the previous commit should\nprobably do\n\n\tif (!submdule)\n\t\tBUG(...);\n\nEither way, it seems like it would be natural to combine this commit\nwith the previous one.\n\nMichael\n"},{"id":"327806","messageId":"c627a63e-bb70-9533-f638-e2b9d7ba3d8b@alum.mit.edu","threadId":"46659","inReplyTo":"20170823123704.16518-1-pclouds@gmail.com","subject":"Re: [PATCH v4 00/16] Fix git-gc losing objects in multi worktree","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-09-09T06:45:54Z","receivedAt":"2017-09-09T06:46:01Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/23/2017 02:36 PM, Nguyễn Thái Ngọc Duy wrote:\n> \"git gc\" when used in multiple worktrees ignore some per-worktree\n> references: object references in the index, HEAD and reflog. This\n> series fixes it by making the revision walker include these from all\n> worktrees by default (and the series is basically split in three parts\n> in the same order). There's a couple more cleanups in refs.c. Luckily\n> it does not conflict with anything in 'pu'.\n> \n> Compared to v3 [1], the largest change is supporting multi worktree in\n> the reflog iterator. The merge iterator is now used (Micheal was right\n> all along).\n\nI read over all of the refs-related changes in this patch series. Aside\nfrom the comments that I left, they look good to me. Thanks!\n\nMichael\n"}]}