{"thread":{"id":"45084","subject":"[PATCH RFC 0/2] Kill manual ref parsing code in worktree.c","startedAt":"2017-02-08T11:32:01Z","lastAt":"2017-04-25T04:30:24Z","messageCount":51,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Stefan Beller","Michael Haggerty","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"311051","messageId":"20170208113144.8201-1-pclouds@gmail.com","threadId":"45084","inReplyTo":null,"subject":"[PATCH RFC 0/2] Kill manual ref parsing code in worktree.c","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-08T11:31:42Z","receivedAt":"2017-02-08T11:32:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"A hundred years ago I added this code because a \"standalone\" ref\nparsing code was not available from refs.c and the file was going through\nsome heavy changes that refactoring its ref parsing code out was not\nan option. I promised to kill this parse_ref() eventually. I'm\nfulfilling it today (or soon).\n\nI would really like to you double check the approach I'm using here\n(using submodule interface for accessing refs from another worktree)\nsince that may be the way forward to fix the \"gc losing objects\" in\nmulti worktrees. I've given it lots of thoughts in the last 24 hours.\nStill can't find any fundamental flaw...\n\nNguyễn Thái Ngọc Duy (2):\n refs.c: add resolve_ref_submodule()\n worktree.c: use submodule interface to access refs from another worktree\n\n branch.c | 3 +-\n refs.c | 20 +++++++++----\n refs.h | 3 ++\n worktree.c | 99 +++++++++++++++-----------------------------------------------\n worktree.h | 2 +-\n 5 files changed, 44 insertions(+), 83 deletions(-)\n\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"311052","messageId":"20170208113144.8201-2-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170208113144.8201-1-pclouds@gmail.com","subject":"[PATCH 1/2] refs.c: add resolve_ref_submodule()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-08T11:31:43Z","receivedAt":"2017-02-08T11:32:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is basically the extended version of resolve_gitlink_ref() where we\nhave access to more info from the underlying resolve_ref_recursively() call.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 20 ++++++++++++++------\n refs.h | 3 +++\n 2 files changed, 17 insertions(+), 6 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex cd36b64ed9..02e35d83f3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1325,18 +1325,18 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t\t resolve_flags, sha1, flags);\n }\n \n-int resolve_gitlink_ref(const char *submodule, const char *refname,\n-\t\t\tunsigned char *sha1)\n+const char *resolve_ref_submodule(const char *submodule, const char *refname,\n+\t\t\t\t int resolve_flags, unsigned char *sha1,\n+\t\t\t\t int *flags)\n {\n \tsize_t len = strlen(submodule);\n \tstruct ref_store *refs;\n-\tint flags;\n \n \twhile (len && submodule[len - 1] == '/')\n \t\tlen--;\n \n \tif (!len)\n-\t\treturn -1;\n+\t\treturn NULL;\n \n \tif (submodule[len]) {\n \t\t/* We need to strip off one or more trailing slashes */\n@@ -1349,9 +1349,17 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n \t}\n \n \tif (!refs)\n-\t\treturn -1;\n+\t\treturn NULL;\n+\n+\treturn resolve_ref_recursively(refs, refname, resolve_flags, sha1, flags);\n+}\n+\n+int resolve_gitlink_ref(const char *submodule, const char *refname,\n+\t\t\tunsigned char *sha1)\n+{\n+\tint flags;\n \n-\tif (!resolve_ref_recursively(refs, refname, 0, sha1, &flags) ||\n+\tif (!resolve_ref_submodule(submodule, refname, 0, sha1, &flags) ||\n \t is_null_sha1(sha1))\n \t\treturn -1;\n \treturn 0;\ndiff --git a/refs.h b/refs.h\nindex 9fbff90e79..74542468d8 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -88,6 +88,9 @@ int peel_ref(const char *refname, unsigned char *sha1);\n */\n int resolve_gitlink_ref(const char *submodule, const char *refname,\n \t\t\tunsigned char *sha1);\n+const char *resolve_ref_submodule(const char *submodule, const char *refname,\n+\t\t\t\t int resolve_flags, unsigned char *sha1,\n+\t\t\t\t int *flags);\n \n /*\n * Return true iff abbrev_name is a possible abbreviation for\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"311057","messageId":"20170208113144.8201-3-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170208113144.8201-1-pclouds@gmail.com","subject":"[PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-08T11:31:44Z","receivedAt":"2017-02-08T12:35:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The patch itself is relatively simple: manual parsing code is replaced\nwith a call to resolve_ref_submodule(). The manual parsing code must die\nbecause only refs/files-backend.c should do that. Why submodule here is\na more interesting question.\n\nFrom an outside look, any .git/worktrees/foo is seen as a \"normal\"\nrepository. You can set GIT_DIR to it and have access to everything,\neven shared things that are not literally inside that directory, like\nobject db or shared refs.\n\nOn top of that, linked worktrees point to those directories with \".git\"\nfiles. These two make a linked worktree's path \"X\" a \"submodule\" (*) (**)\nbecause X/.git is a file that points to a repository somewhere.\n\nAs such, we can just linked worktree's path as a submodule. We just need\nto make sure they are unique because they are used to lookup submodule\nrefs store.\n\nMain worktree is a a bit trickier. If we stand at a linked worktree, we\nmay still need to peek into main worktree's HEAD, for example. We can\ntreat main worktree's path as submodule as well since git_path_submodule()\ncan tolerate \".git\" dirs, in addition to \".git\" files.\n\nThe constraint is, if main worktree is X, then the git repo must be at\nX/.git. If the user separates .git repo far away and tell git to point\nto it via GIT_DIR or something else, then the \"main worktree as submodule\"\ntrick fails. Within multiple worktree context, I think we can limit\nsupport to \"standard\" layout, at least for now.\n\n(*) The differences in sharing object database and refs between\nsubmodules and linked worktrees don't really matter in this context.\n\n(**) At this point, we may want to rename refs *_submodule API to\nsomething more neutral, maybe s/_submodule/_remote/\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n branch.c | 3 +-\n worktree.c | 99 +++++++++++++++-----------------------------------------------\n worktree.h | 2 +-\n 3 files changed, 27 insertions(+), 77 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex b955d4f316..db5843718f 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -354,7 +354,8 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref)\n \tfor (i = 0; worktrees[i]; i++) {\n \t\tif (worktrees[i]->is_detached)\n \t\t\tcontinue;\n-\t\tif (strcmp(oldref, worktrees[i]->head_ref))\n+\t\tif (worktrees[i]->head_ref &&\n+\t\t strcmp(oldref, worktrees[i]->head_ref))\n \t\t\tcontinue;\n \n \t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\ndiff --git a/worktree.c b/worktree.c\nindex d633761575..25e5bc9a3e 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -19,54 +19,24 @@ void free_worktrees(struct worktree **worktrees)\n \tfree (worktrees);\n }\n \n-/*\n- * read 'path_to_ref' into 'ref'. Also if is_detached is not NULL,\n- * set is_detached to 1 (0) if the ref is detached (is not detached).\n- *\n- * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside $GIT_DIR so\n- * for linked worktrees, `resolve_ref_unsafe()` won't work (it uses\n- * git_path). Parse the ref ourselves.\n- *\n- * return -1 if the ref is not a proper ref, 0 otherwise (success)\n- */\n-static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)\n-{\n-\tif (is_detached)\n-\t\t*is_detached = 0;\n-\tif (!strbuf_readlink(ref, path_to_ref, 0)) {\n-\t\t/* HEAD is symbolic link */\n-\t\tif (!starts_with(ref->buf, \"refs/\") ||\n-\t\t\t\tcheck_refname_format(ref->buf, 0))\n-\t\t\treturn -1;\n-\t} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {\n-\t\t/* textual symref or detached */\n-\t\tif (!starts_with(ref->buf, \"ref:\")) {\n-\t\t\tif (is_detached)\n-\t\t\t\t*is_detached = 1;\n-\t\t} else {\n-\t\t\tstrbuf_remove(ref, 0, strlen(\"ref:\"));\n-\t\t\tstrbuf_trim(ref);\n-\t\t\tif (check_refname_format(ref->buf, 0))\n-\t\t\t\treturn -1;\n-\t\t}\n-\t} else\n-\t\treturn -1;\n-\treturn 0;\n-}\n-\n /**\n- * Add the head_sha1 and head_ref (if not detached) to the given worktree\n+ * Update head_sha1, head_ref and is_detached of the given worktree\n */\n-static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)\n+static void add_head_info(struct worktree *wt)\n {\n-\tif (head_ref->len) {\n-\t\tif (worktree->is_detached) {\n-\t\t\tget_sha1_hex(head_ref->buf, worktree->head_sha1);\n-\t\t} else {\n-\t\t\tresolve_ref_unsafe(head_ref->buf, 0, worktree->head_sha1, NULL);\n-\t\t\tworktree->head_ref = strbuf_detach(head_ref, NULL);\n-\t\t}\n-\t}\n+\tint flags;\n+\tconst char *target;\n+\n+\ttarget = resolve_ref_submodule(wt->path, \"HEAD\",\n+\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t wt->head_sha1, &flags);\n+\tif (!target)\n+\t\treturn;\n+\n+\tif (flags & REF_ISSYMREF)\n+\t\twt->head_ref = xstrdup(target);\n+\telse\n+\t\twt->is_detached = 1;\n }\n \n /**\n@@ -77,9 +47,7 @@ static struct worktree *get_main_worktree(void)\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n-\tstruct strbuf head_ref = STRBUF_INIT;\n \tint is_bare = 0;\n-\tint is_detached = 0;\n \n \tstrbuf_add_absolute_path(&worktree_path, get_git_common_dir());\n \tis_bare = !strbuf_strip_suffix(&worktree_path, \"/.git\");\n@@ -91,13 +59,10 @@ static struct worktree *get_main_worktree(void)\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->is_bare = is_bare;\n-\tworktree->is_detached = is_detached;\n-\tif (!parse_ref(path.buf, &head_ref, &is_detached))\n-\t\tadd_head_info(&head_ref, worktree);\n+\tadd_head_info(worktree);\n \n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n-\tstrbuf_release(&head_ref);\n \treturn worktree;\n }\n \n@@ -106,8 +71,6 @@ static struct worktree *get_linked_worktree(const char *id)\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n-\tstruct strbuf head_ref = STRBUF_INIT;\n-\tint is_detached = 0;\n \n \tif (!id)\n \t\tdie(\"Missing linked worktree name\");\n@@ -127,19 +90,14 @@ static struct worktree *get_linked_worktree(const char *id)\n \tstrbuf_reset(&path);\n \tstrbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n \n-\tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n-\t\tgoto done;\n-\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->id = xstrdup(id);\n-\tworktree->is_detached = is_detached;\n-\tadd_head_info(&head_ref, worktree);\n+\tadd_head_info(worktree);\n \n done:\n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n-\tstrbuf_release(&head_ref);\n \treturn worktree;\n }\n \n@@ -334,8 +292,6 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\t\t\t\t const char *target)\n {\n \tconst struct worktree *existing = NULL;\n-\tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf sb = STRBUF_INIT;\n \tstatic struct worktree **worktrees;\n \tint i = 0;\n \n@@ -345,6 +301,10 @@ const struct worktree *find_shared_symref(const char *symref,\n \n \tfor (i = 0; worktrees[i]; i++) {\n \t\tstruct worktree *wt = worktrees[i];\n+\t\tconst char *symref_target;\n+\t\tunsigned char sha1[20];\n+\t\tint flags;\n+\n \t\tif (wt->is_bare)\n \t\t\tcontinue;\n \n@@ -359,25 +319,14 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\t\t}\n \t\t}\n \n-\t\tstrbuf_reset(&path);\n-\t\tstrbuf_reset(&sb);\n-\t\tstrbuf_addf(&path, \"%s/%s\",\n-\t\t\t get_worktree_git_dir(wt),\n-\t\t\t symref);\n-\n-\t\tif (parse_ref(path.buf, &sb, NULL)) {\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\tif (!strcmp(sb.buf, target)) {\n+\t\tsymref_target = resolve_ref_submodule(wt->path, symref, 0,\n+\t\t\t\t\t\t sha1, &flags);\n+\t\tif ((flags & REF_ISSYMREF) && !strcmp(symref_target, target)) {\n \t\t\texisting = wt;\n \t\t\tbreak;\n \t\t}\n \t}\n \n-\tstrbuf_release(&path);\n-\tstrbuf_release(&sb);\n-\n \treturn existing;\n }\n \ndiff --git a/worktree.h b/worktree.h\nindex 6bfb985203..5ea5e503fb 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -4,7 +4,7 @@\n struct worktree {\n \tchar *path;\n \tchar *id;\n-\tchar *head_ref;\n+\tchar *head_ref;\t\t/* NULL if HEAD is broken or detached */\n \tchar *lock_reason;\t/* internal use */\n \tunsigned char head_sha1[20];\n \tint is_detached;\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"311074","messageId":"xmqqlgtgwnqa.fsf@gitster.mtv.corp.google.com","threadId":"45084","inReplyTo":"20170208113144.8201-1-pclouds@gmail.com","subject":"Re: [PATCH RFC 0/2] Kill manual ref parsing code in worktree.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-08T18:18:05Z","receivedAt":"2017-02-08T18:18:24Z","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> A hundred years ago I added this code because a \"standalone\" ref\n> parsing code was not available from refs.c and the file was going through\n> some heavy changes that refactoring its ref parsing code out was not\n> an option. I promised to kill this parse_ref() eventually. I'm\n> fulfilling it today (or soon).\n\n;-) \n\nThanks. And thanks for looping Michael in. I'd appreciate his\ninput in this area.\n\n> I would really like to you double check the approach I'm using here\n> (using submodule interface for accessing refs from another worktree)\n> since that may be the way forward to fix the \"gc losing objects\" in\n> multi worktrees. I've given it lots of thoughts in the last 24 hours.\n> Still can't find any fundamental flaw...\n\nI see that you posted a separate message outlining the idea\nyesterday and I didn't see any response (and I was sick and lacked\nenergy to think it through); I think the basic approach to use \"an\nAPI to bring set of refs hidden from your normal view\" is sensible.\nExcept for the unfortunate naming of the interface that makes it\nsound as if it is only to access submodules, but that is where the\nfeature original came from, so let's not complain too loudly ;-)\n\n> Nguyễn Thái Ngọc Duy (2):\n> refs.c: add resolve_ref_submodule()\n> worktree.c: use submodule interface to access refs from another worktree\n>\n> branch.c | 3 +-\n> refs.c | 20 +++++++++----\n> refs.h | 3 ++\n> worktree.c | 99 +++++++++++++++-----------------------------------------------\n> worktree.h | 2 +-\n> 5 files changed, 44 insertions(+), 83 deletions(-)\n"},{"id":"311080","messageId":"CAGZ79kZk1Chq1R-anz0RC+0GDEubGm5kEQOsy5bz9zwCYEi9nw@mail.gmail.com","threadId":"45084","inReplyTo":"20170208113144.8201-3-pclouds@gmail.com","subject":"Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-02-08T19:50:40Z","receivedAt":"2017-02-08T19:51:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Feb 8, 2017 at 3:31 AM, Nguyễn Thái Ngọc Duy wrote:\n> (**) At this point, we may want to rename refs *_submodule API to\n> something more neutral, maybe s/_submodule/_remote/\n\nI agree on (**), except that I am not sure if /_remote/ is a better name,\nbecause there is already a concept of a \"remote\" as well as\n\"remote-tracking\" in Git. (Usually it is not reachable on the same\nFS, but resides on another machine).\n\nSo if we were to do s/_submodule/_remote/, we'd have e.g.\n\n for_each_ref_remote\n\nwhich could mean that we do networking things to obtain the\nactual remote refs or just talk about remote tracking refs.\n\nMy gut reaction would be to s/submodule/alternative/ here,\nbut we also have a thing called alternates already.\n\nSo we're looking for a name that describes refs for both\nworktrees as well as submodules. (Not sure if we can generalize\nto also include alternates, too)\n\nAnd the one thing they share is that they have their refs\n\"not at the usual place\", e.g. not at .git/refs but rather at\n.git/{modules,worktrees}.\n\n Recently we had a tangential discussion in submodule land\n about the different places, when adding the\n \"git submodule absorbgitdirs\" command, that moves\n the submodule/.git directory into .git/modules/.\n We chose \"absorb\" here as the name, because it\n was moved into the .git/ area.\n\nSo maybe one of:\n\n s/submodule_ref/unusual_ref/\n (to emphasize it is not a regular ref inside the repo, so:)\n s/submodule_ref/irregular_ref/\n\n s/resolve_ref_submodule/resolve_ref_out_of_place/\n s/resolve_ref_submodule/resolve_ref_gitlink_pointed/\n s/resolve_ref_submodule/resolve_ref_linked_pointer/\n\nwould be my current thinking.\n\nThanks,\nStefan\n"},{"id":"311119","messageId":"xmqqvasktgmj.fsf@gitster.mtv.corp.google.com","threadId":"45084","inReplyTo":"CAGZ79kZk1Chq1R-anz0RC+0GDEubGm5kEQOsy5bz9zwCYEi9nw@mail.gmail.com","subject":"Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-08T23:19:48Z","receivedAt":"2017-02-08T23:19:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Wed, Feb 8, 2017 at 3:31 AM, Nguyễn Thái Ngọc Duy wrote:\n>> (**) At this point, we may want to rename refs *_submodule API to\n>> something more neutral, maybe s/_submodule/_remote/\n>\n> I agree on (**), except that I am not sure if /_remote/ is a better name,\n> because there is already a concept of a \"remote\" as well as\n> \"remote-tracking\" in Git. (Usually it is not reachable on the same\n> FS, but resides on another machine).\n\nI agree with you that the concept of remote is quite detached from\nthe concept of wt and submodule whose refs need to be peeked at from\nthe local repository. After all, \"remote\" tracking branches are\npart of local repository's refs.\n\n> My gut reaction would be to s/submodule/alternative/ here,\n> but we also have a thing called alternates already.\n\n... and I tend to think that is far closer a concept. You borrow\nobjects from your alternate object store, and that alternate object\nstore may have its own set of refs you would need to peek when you\nare computing reachability from refs.\n\nAlso don't we already enumerate such refs that pin objects in the\nalternate object store when doing object transfer negotiation in\norder to send \".have\" entries for their tips? What API do we use to\ndo that, I wonder.\n"},{"id":"311120","messageId":"xmqqr338tgc8.fsf@gitster.mtv.corp.google.com","threadId":"45084","inReplyTo":"20170208113144.8201-3-pclouds@gmail.com","subject":"Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-08T23:25:59Z","receivedAt":"2017-02-08T23:27:43Z","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> As such, we can just linked worktree's path as a submodule. We just need\n\nLack of verb made me read this three times. \"We can just treat\nlinked worktree's path as if it were a submodule\"?\n\nI agree with you that the \"submodule\" in the name is misleading. We\nwould want to be able to walk refs from other local repositories\n(and repository-like entities, like the .git/ thing in a liked\nworktree), like our own submodule, or the repository we borrow\nobjects from via the alternate mechanism.\n"},{"id":"311148","messageId":"19bcda56-077e-b0a8-4408-d0012754bf17@alum.mit.edu","threadId":"45084","inReplyTo":"20170208113144.8201-2-pclouds@gmail.com","subject":"Re: [PATCH 1/2] refs.c: add resolve_ref_submodule()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-02-09T05:20:55Z","receivedAt":"2017-02-09T05:26:12Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/08/2017 12:31 PM, Nguyễn Thái Ngọc Duy wrote:\n> This is basically the extended version of resolve_gitlink_ref() where we\n> have access to more info from the underlying resolve_ref_recursively() call.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> refs.c | 20 ++++++++++++++------\n> refs.h | 3 +++\n> 2 files changed, 17 insertions(+), 6 deletions(-)\n> \n> diff --git a/refs.c b/refs.c\n> index cd36b64ed9..02e35d83f3 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1325,18 +1325,18 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n> \t\t\t\t resolve_flags, sha1, flags);\n> }\n> \n> -int resolve_gitlink_ref(const char *submodule, const char *refname,\n> -\t\t\tunsigned char *sha1)\n> +const char *resolve_ref_submodule(const char *submodule, const char *refname,\n> +\t\t\t\t int resolve_flags, unsigned char *sha1,\n> +\t\t\t\t int *flags)\n> {\n> \tsize_t len = strlen(submodule);\n> \tstruct ref_store *refs;\n> -\tint flags;\n> \n> \twhile (len && submodule[len - 1] == '/')\n> \t\tlen--;\n> \n> \tif (!len)\n> -\t\treturn -1;\n> +\t\treturn NULL;\n> \n> \tif (submodule[len]) {\n> \t\t/* We need to strip off one or more trailing slashes */\n> @@ -1349,9 +1349,17 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n> \t}\n> \n> \tif (!refs)\n> -\t\treturn -1;\n> +\t\treturn NULL;\n> +\n> +\treturn resolve_ref_recursively(refs, refname, resolve_flags, sha1, flags);\n> +}\n> +\n> +int resolve_gitlink_ref(const char *submodule, const char *refname,\n> +\t\t\tunsigned char *sha1)\n> +{\n> +\tint flags;\n> \n> -\tif (!resolve_ref_recursively(refs, refname, 0, sha1, &flags) ||\n> +\tif (!resolve_ref_submodule(submodule, refname, 0, sha1, &flags) ||\n> \t is_null_sha1(sha1))\n> \t\treturn -1;\n> \treturn 0;\n> diff --git a/refs.h b/refs.h\n> index 9fbff90e79..74542468d8 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -88,6 +88,9 @@ int peel_ref(const char *refname, unsigned char *sha1);\n> */\n> int resolve_gitlink_ref(const char *submodule, const char *refname,\n> \t\t\tunsigned char *sha1);\n> +const char *resolve_ref_submodule(const char *submodule, const char *refname,\n> +\t\t\t\t int resolve_flags, unsigned char *sha1,\n> +\t\t\t\t int *flags);\n\nThis function is the analog of resolve_ref_unsafe(); i.e., it returns a\npointer to either a static buffer or a pointer into the refname\nargument. Therefore, I think it should have \"unsafe\" in its name. And/or\nmaybe there should be a safe version of the function analogous to\nresolve_refdup().\n\nMoreover, this function has inherited the code for stripping trailing\nslashes from the submodule name. I have the feeling that this is a wart,\nnot a feature, and that it would be sad to see it spread. How about\nmoving the slash-stripping code to resolve_gitlink_ref() and making\nresolve_ref_submodule() assume that its submodule name is already clean?\n\nIt would be nice to have a docstring here.\n\nI also have some higher-level concerns about the approach of this patch\nseries, which I'll write about in a comment to patch 2/2.\n\nMichael\n\n"},{"id":"311150","messageId":"37fe2024-0378-a974-a28d-18a89d3e2312@alum.mit.edu","threadId":"45084","inReplyTo":"20170208113144.8201-3-pclouds@gmail.com","subject":"Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-02-09T06:07:39Z","receivedAt":"2017-02-09T06:08:01Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/08/2017 12:31 PM, Nguyễn Thái Ngọc Duy wrote:\n> The patch itself is relatively simple: manual parsing code is replaced\n> with a call to resolve_ref_submodule(). The manual parsing code must die\n> because only refs/files-backend.c should do that. Why submodule here is\n> a more interesting question.\n> \n> From an outside look, any .git/worktrees/foo is seen as a \"normal\"\n> repository. You can set GIT_DIR to it and have access to everything,\n> even shared things that are not literally inside that directory, like\n> object db or shared refs.\n> \n> On top of that, linked worktrees point to those directories with \".git\"\n> files. These two make a linked worktree's path \"X\" a \"submodule\" (*) (**)\n> because X/.git is a file that points to a repository somewhere.\n> \n> As such, we can just linked worktree's path as a submodule. We just need\n> to make sure they are unique because they are used to lookup submodule\n> refs store.\n> \n> Main worktree is a a bit trickier. If we stand at a linked worktree, we\n> may still need to peek into main worktree's HEAD, for example. We can\n> treat main worktree's path as submodule as well since git_path_submodule()\n> can tolerate \".git\" dirs, in addition to \".git\" files.\n> \n> The constraint is, if main worktree is X, then the git repo must be at\n> X/.git. If the user separates .git repo far away and tell git to point\n> to it via GIT_DIR or something else, then the \"main worktree as submodule\"\n> trick fails. Within multiple worktree context, I think we can limit\n> support to \"standard\" layout, at least for now.\n> \n> (*) The differences in sharing object database and refs between\n> submodules and linked worktrees don't really matter in this context.\n> \n> (**) At this point, we may want to rename refs *_submodule API to\n> something more neutral, maybe s/_submodule/_remote/\n\nIt is unquestionably a good goal to avoid parsing references outside of\n`refs/files-backend.c`. But I'm not a fan of this approach.\n\nThere are two meanings of the concept of a \"ref store\", and I think this\nchange muddles them:\n\n1. The references that happen to be *physically* stored in a particular\n location, for example the `refs/bisect/*` references in a worktree.\n\n2. The references that *logically* should be considered part of a\n particular repository. This might require stitching together\n references from multiple sources, for example `HEAD` and\n `refs/bisect` from a worktree's own directory with other\n references from the main repository.\n\nEither of these concepts can be implemented via the `ref_store` abstraction.\n\nThe `ref_store` for a submodule should represent the references\nlogically visible from the submodule. The main program shouldn't care\nwhether the references are stored in a single physical location or\nspread across multiple locations (for example, if the submodule were\nitself a linked worktree).\n\nThe `ref_store` that you want here for a worktree is not the worktree's\n*logical* `ref_store`. You want the worktree's *physical* `ref_store`.\nMixing logical and physical reference stores together is a bad idea\n(even if we were willing to ignore the fact that worktrees are not\nsubmodules in the accepted sense of the word).\n\nThe point of my `submodule-hash` branch [1] was to separate these\nconcepts better by breaking the current 1:1 connection between\n`ref_store`s and submodules. This would allow `ref_store`s to be created\nfor other purposes, such as to represent worktree refs. If you want the\n*logical* `ref_store` for a submodule, you access it through the\n`submodule_ref_stores` table. If you want the *physical* `ref_store` for\na worktree, you should access it through a different table.\n\nI think the best solution would be to expose the concept of `ref_store`\nin the public refs API. Then users of submodules would essentially do\n\n struct ref_store *refs = get_submodule_refs(submodule_path);\n ... resolve_ref_recursively(refs, refname, 0, sha1, &flags) ...\n ... for_each_ref(refs, fn, cb_data) ...\n\nwhereas for a worktree you'd have to look up the `ref_store` instance\nsomewhere else (or maybe keep it as part of some worktree structure, if\nthere is one) but you would use it via the same API.\n\nMichael\n\n[1] https://github.com/mhagger/git\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> branch.c | 3 +-\n> worktree.c | 99 +++++++++++++++-----------------------------------------------\n> worktree.h | 2 +-\n> 3 files changed, 27 insertions(+), 77 deletions(-)\n> \n> diff --git a/branch.c b/branch.c\n> index b955d4f316..db5843718f 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -354,7 +354,8 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref)\n> \tfor (i = 0; worktrees[i]; i++) {\n> \t\tif (worktrees[i]->is_detached)\n> \t\t\tcontinue;\n> -\t\tif (strcmp(oldref, worktrees[i]->head_ref))\n> +\t\tif (worktrees[i]->head_ref &&\n> +\t\t strcmp(oldref, worktrees[i]->head_ref))\n> \t\t\tcontinue;\n> \n> \t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\n> diff --git a/worktree.c b/worktree.c\n> index d633761575..25e5bc9a3e 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -19,54 +19,24 @@ void free_worktrees(struct worktree **worktrees)\n> \tfree (worktrees);\n> }\n> \n> -/*\n> - * read 'path_to_ref' into 'ref'. Also if is_detached is not NULL,\n> - * set is_detached to 1 (0) if the ref is detached (is not detached).\n> - *\n> - * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside $GIT_DIR so\n> - * for linked worktrees, `resolve_ref_unsafe()` won't work (it uses\n> - * git_path). Parse the ref ourselves.\n> - *\n> - * return -1 if the ref is not a proper ref, 0 otherwise (success)\n> - */\n> -static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)\n> -{\n> -\tif (is_detached)\n> -\t\t*is_detached = 0;\n> -\tif (!strbuf_readlink(ref, path_to_ref, 0)) {\n> -\t\t/* HEAD is symbolic link */\n> -\t\tif (!starts_with(ref->buf, \"refs/\") ||\n> -\t\t\t\tcheck_refname_format(ref->buf, 0))\n> -\t\t\treturn -1;\n> -\t} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {\n> -\t\t/* textual symref or detached */\n> -\t\tif (!starts_with(ref->buf, \"ref:\")) {\n> -\t\t\tif (is_detached)\n> -\t\t\t\t*is_detached = 1;\n> -\t\t} else {\n> -\t\t\tstrbuf_remove(ref, 0, strlen(\"ref:\"));\n> -\t\t\tstrbuf_trim(ref);\n> -\t\t\tif (check_refname_format(ref->buf, 0))\n> -\t\t\t\treturn -1;\n> -\t\t}\n> -\t} else\n> -\t\treturn -1;\n> -\treturn 0;\n> -}\n> -\n> /**\n> - * Add the head_sha1 and head_ref (if not detached) to the given worktree\n> + * Update head_sha1, head_ref and is_detached of the given worktree\n> */\n> -static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)\n> +static void add_head_info(struct worktree *wt)\n> {\n> -\tif (head_ref->len) {\n> -\t\tif (worktree->is_detached) {\n> -\t\t\tget_sha1_hex(head_ref->buf, worktree->head_sha1);\n> -\t\t} else {\n> -\t\t\tresolve_ref_unsafe(head_ref->buf, 0, worktree->head_sha1, NULL);\n> -\t\t\tworktree->head_ref = strbuf_detach(head_ref, NULL);\n> -\t\t}\n> -\t}\n> +\tint flags;\n> +\tconst char *target;\n> +\n> +\ttarget = resolve_ref_submodule(wt->path, \"HEAD\",\n> +\t\t\t\t RESOLVE_REF_READING,\n> +\t\t\t\t wt->head_sha1, &flags);\n> +\tif (!target)\n> +\t\treturn;\n> +\n> +\tif (flags & REF_ISSYMREF)\n> +\t\twt->head_ref = xstrdup(target);\n> +\telse\n> +\t\twt->is_detached = 1;\n> }\n> \n> /**\n> @@ -77,9 +47,7 @@ static struct worktree *get_main_worktree(void)\n> \tstruct worktree *worktree = NULL;\n> \tstruct strbuf path = STRBUF_INIT;\n> \tstruct strbuf worktree_path = STRBUF_INIT;\n> -\tstruct strbuf head_ref = STRBUF_INIT;\n> \tint is_bare = 0;\n> -\tint is_detached = 0;\n> \n> \tstrbuf_add_absolute_path(&worktree_path, get_git_common_dir());\n> \tis_bare = !strbuf_strip_suffix(&worktree_path, \"/.git\");\n> @@ -91,13 +59,10 @@ static struct worktree *get_main_worktree(void)\n> \tworktree = xcalloc(1, sizeof(*worktree));\n> \tworktree->path = strbuf_detach(&worktree_path, NULL);\n> \tworktree->is_bare = is_bare;\n> -\tworktree->is_detached = is_detached;\n> -\tif (!parse_ref(path.buf, &head_ref, &is_detached))\n> -\t\tadd_head_info(&head_ref, worktree);\n> +\tadd_head_info(worktree);\n> \n> \tstrbuf_release(&path);\n> \tstrbuf_release(&worktree_path);\n> -\tstrbuf_release(&head_ref);\n> \treturn worktree;\n> }\n> \n> @@ -106,8 +71,6 @@ static struct worktree *get_linked_worktree(const char *id)\n> \tstruct worktree *worktree = NULL;\n> \tstruct strbuf path = STRBUF_INIT;\n> \tstruct strbuf worktree_path = STRBUF_INIT;\n> -\tstruct strbuf head_ref = STRBUF_INIT;\n> -\tint is_detached = 0;\n> \n> \tif (!id)\n> \t\tdie(\"Missing linked worktree name\");\n> @@ -127,19 +90,14 @@ static struct worktree *get_linked_worktree(const char *id)\n> \tstrbuf_reset(&path);\n> \tstrbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n> \n> -\tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n> -\t\tgoto done;\n> -\n> \tworktree = xcalloc(1, sizeof(*worktree));\n> \tworktree->path = strbuf_detach(&worktree_path, NULL);\n> \tworktree->id = xstrdup(id);\n> -\tworktree->is_detached = is_detached;\n> -\tadd_head_info(&head_ref, worktree);\n> +\tadd_head_info(worktree);\n> \n> done:\n> \tstrbuf_release(&path);\n> \tstrbuf_release(&worktree_path);\n> -\tstrbuf_release(&head_ref);\n> \treturn worktree;\n> }\n> \n> @@ -334,8 +292,6 @@ const struct worktree *find_shared_symref(const char *symref,\n> \t\t\t\t\t const char *target)\n> {\n> \tconst struct worktree *existing = NULL;\n> -\tstruct strbuf path = STRBUF_INIT;\n> -\tstruct strbuf sb = STRBUF_INIT;\n> \tstatic struct worktree **worktrees;\n> \tint i = 0;\n> \n> @@ -345,6 +301,10 @@ const struct worktree *find_shared_symref(const char *symref,\n> \n> \tfor (i = 0; worktrees[i]; i++) {\n> \t\tstruct worktree *wt = worktrees[i];\n> +\t\tconst char *symref_target;\n> +\t\tunsigned char sha1[20];\n> +\t\tint flags;\n> +\n> \t\tif (wt->is_bare)\n> \t\t\tcontinue;\n> \n> @@ -359,25 +319,14 @@ const struct worktree *find_shared_symref(const char *symref,\n> \t\t\t}\n> \t\t}\n> \n> -\t\tstrbuf_reset(&path);\n> -\t\tstrbuf_reset(&sb);\n> -\t\tstrbuf_addf(&path, \"%s/%s\",\n> -\t\t\t get_worktree_git_dir(wt),\n> -\t\t\t symref);\n> -\n> -\t\tif (parse_ref(path.buf, &sb, NULL)) {\n> -\t\t\tcontinue;\n> -\t\t}\n> -\n> -\t\tif (!strcmp(sb.buf, target)) {\n> +\t\tsymref_target = resolve_ref_submodule(wt->path, symref, 0,\n> +\t\t\t\t\t\t sha1, &flags);\n> +\t\tif ((flags & REF_ISSYMREF) && !strcmp(symref_target, target)) {\n> \t\t\texisting = wt;\n> \t\t\tbreak;\n> \t\t}\n> \t}\n> \n> -\tstrbuf_release(&path);\n> -\tstrbuf_release(&sb);\n> -\n> \treturn existing;\n> }\n> \n> diff --git a/worktree.h b/worktree.h\n> index 6bfb985203..5ea5e503fb 100644\n> --- a/worktree.h\n> +++ b/worktree.h\n> @@ -4,7 +4,7 @@\n> struct worktree {\n> \tchar *path;\n> \tchar *id;\n> -\tchar *head_ref;\n> +\tchar *head_ref;\t\t/* NULL if HEAD is broken or detached */\n> \tchar *lock_reason;\t/* internal use */\n> \tunsigned char head_sha1[20];\n> \tint is_detached;\n> \n\n"},{"id":"311151","messageId":"xmqqpoirsvin.fsf@gitster.mtv.corp.google.com","threadId":"45084","inReplyTo":"37fe2024-0378-a974-a28d-18a89d3e2312@alum.mit.edu","subject":"Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-09T06:55:44Z","receivedAt":"2017-02-09T07:04:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> There are two meanings of the concept of a \"ref store\", and I think this\n> change muddles them:\n>\n> 1. The references that happen to be *physically* stored in a particular\n> location, for example the `refs/bisect/*` references in a worktree.\n>\n> 2. The references that *logically* should be considered part of a\n> particular repository. This might require stitching together\n> references from multiple sources, for example `HEAD` and\n> `refs/bisect` from a worktree's own directory with other\n> references from the main repository.\n>\n> Either of these concepts can be implemented via the `ref_store` abstraction.\n> ...\n> The `ref_store` that you want here for a worktree is not the worktree's\n> *logical* `ref_store`. You want the worktree's *physical* `ref_store`.\n> Mixing logical and physical reference stores together is a bad idea\n> (even if we were willing to ignore the fact that worktrees are not\n> submodules in the accepted sense of the word).\n\nI am not quite sure what mental model you are suggesting as a\npreferred solution. We can\n\n - represent a set of refs stored for a particular worktree\n (i.e. HEAD, refs/bisect, and refs/worktree/, iirc), as\n bunch of ref_stores;\n\n - represent a set of refs shared across a set of worktrees that\n share the primary one, as another ref_store;\n\n - a caller who wants to get a \"logical\" view of a single worktree\n user can pick one of the first kind and union that with the\n second one, and represent the result as a (synthetic) ref_store.\n\nThe third one is \"stitching together from multiple sources\". By\n\"mixing logical and physical is a bad idea\", do you mean that the\nsame abstraction \"ref_store\" should not be used for the first two\n(which are physical) and the third one (which is logical)? Do you\nwant to call the first two \"physical_ref_store\"and the last one\n\"ref_store\" and keep them distinct?\n\nFor the purpose of anchoring objects in the object store shared by\nmultiple worktrees, we can either iterate over all the ref_stores\nof the first two kind, or iterate over all the ref_stores of the\nthird kind for all worktrees. The latter of course is less\nefficient as the enumeration\n\n\tfor worktree in all worktrees:\n\t\tfor ref in get_ref_store(worktree)\n\t\t\tmark tip of ref reachable\n\nwill work on all the shared refs multiple times, but as an\nabstraction that may be simpler. The alternative of working at the\nphysical level would be more efficient\n\n\tfor worktree in all worktrees:\n\t\tfor ref in get_ref_store_specific_to_worktree(worktree):\n\t \tmark tip of ref reachable\n\tfor ref in get_ref_store_shared_across_worktrees():\n \tmark tip of ref reachable\n\nbut this consumer now _knows_ how the logical ref_store of a\nworktree is constructed (i.e. by combining the two ref_stores),\nwhich appears as a layering violation.\n\nI am however not sure if these issues are what you are driving at,\nand what exact design you are suggesting.\n"},{"id":"311153","messageId":"3ac5517a-aa82-7078-91a5-e182e1faed1a@alum.mit.edu","threadId":"45084","inReplyTo":"xmqqpoirsvin.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-02-09T08:04:58Z","receivedAt":"2017-02-09T08:15:27Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/09/2017 07:55 AM, Junio C Hamano wrote:\n> Michael Haggerty writes:\n> \n>> There are two meanings of the concept of a \"ref store\", and I think this\n>> change muddles them:\n>>\n>> 1. The references that happen to be *physically* stored in a particular\n>> location, for example the `refs/bisect/*` references in a worktree.\n>>\n>> 2. The references that *logically* should be considered part of a\n>> particular repository. This might require stitching together\n>> references from multiple sources, for example `HEAD` and\n>> `refs/bisect` from a worktree's own directory with other\n>> references from the main repository.\n>>\n>> Either of these concepts can be implemented via the `ref_store` abstraction.\n>> ...\n>> The `ref_store` that you want here for a worktree is not the worktree's\n>> *logical* `ref_store`. You want the worktree's *physical* `ref_store`.\n>> Mixing logical and physical reference stores together is a bad idea\n>> (even if we were willing to ignore the fact that worktrees are not\n>> submodules in the accepted sense of the word).\n> \n> I am not quite sure what mental model you are suggesting as a\n> preferred solution. We can\n> \n> - represent a set of refs stored for a particular worktree\n> (i.e. HEAD, refs/bisect, and refs/worktree/, iirc), as\n> bunch of ref_stores;\n> \n> - represent a set of refs shared across a set of worktrees that\n> share the primary one, as another ref_store;\n> \n> - a caller who wants to get a \"logical\" view of a single worktree\n> user can pick one of the first kind and union that with the\n> second one, and represent the result as a (synthetic) ref_store.\n> \n> The third one is \"stitching together from multiple sources\". By\n> \"mixing logical and physical is a bad idea\", do you mean that the\n> same abstraction \"ref_store\" should not be used for the first two\n> (which are physical) and the third one (which is logical)? Do you\n> want to call the first two \"physical_ref_store\"and the last one\n> \"ref_store\" and keep them distinct?\n\nThe existing `ref_store` abstraction, I think, is capable of\nrepresenting either kind of reference store. The stitching together to\nget the \"logical\" view of a worktree should probably happen within the\nrefs code rather than forcing callers to deal with it. But yes, I think\nthat code should put together a compound `ref_store` object that\ndelegates to multiple underlying `ref_store` objects as you've described.\n\nWhich kind of `ref_store *` you have in your hand would depend on where\nyou got it. If you call the hypothetical `get_submodule_refs()`\nfunction, you would get a `ref_store *` representing the references that\nare logically visible from that submodule. There might be a separate\n`get_worktree_specific_refs()` that returns a `ref_store *` representing\nthe worktree-specific references physically stored for the worktree. But\nmaybe the latter is not even necessary; see below.\n\n> For the purpose of anchoring objects in the object store shared by\n> multiple worktrees, we can either iterate over all the ref_stores\n> of the first two kind, or iterate over all the ref_stores of the\n> third kind for all worktrees. The latter of course is less\n> efficient as the enumeration\n> \n> \tfor worktree in all worktrees:\n> \t\tfor ref in get_ref_store(worktree)\n> \t\t\tmark tip of ref reachable\n> \n> will work on all the shared refs multiple times, but as an\n> abstraction that may be simpler. The alternative of working at the\n> physical level would be more efficient\n> \n> \tfor worktree in all worktrees:\n> \t\tfor ref in get_ref_store_specific_to_worktree(worktree):\n> \t \tmark tip of ref reachable\n> \tfor ref in get_ref_store_shared_across_worktrees():\n> \tmark tip of ref reachable\n> \n> but this consumer now _knows_ how the logical ref_store of a\n> worktree is constructed (i.e. by combining the two ref_stores),\n> which appears as a layering violation.\n> \n> I am however not sure if these issues are what you are driving at,\n> and what exact design you are suggesting.\n\nReachability is a special case, because it needs all of the references\nthat refer to a particular object store, even though the reference names\nmight overlap. I personally think that reachability roots should be\nrequested via a new refs API call separate from `for_each_rawref()` (or\nwhatever is used now). Internally it would be implemented much like your\nsecond \"efficient\" algorithm, but the implementation would be within the\nrefs code, and the caller could remain ignorant of those details.\n\nExternally, it might not even want to pass the caller the real reference\nnames (I assume that callers mainly only use the reference names for\ndiagnostic messages). For example, it might want to report references\n`HEAD` and `refs/bisect/bad` in worktree `foo` under the pseudonyms\n`worktree/foo/HEAD` and `worktree/foo/refs/bisect/bad`, so that they can\nbe distinguished from any homonyms in the main repo and in other\nworktrees. If you ask for the reachability roots while in a worktree, it\nwould either automatically crawl up to the main repository and across to\nsibling worktrees to get the full set of reachability roots, or maybe it\nwould refuse to run at all (if we want to require such commands to be\nexecuted from the main repo).\n\nI don't know exactly who would be the consumers of the reachability\nroots, so maybe there are some problems with this suggestion.\n\nMichael\n\n"},{"id":"311161","messageId":"CACsJy8Diy92CNbJ1OBn893VFFrSsxBFWSyQHjt_Dzq9x7jfibQ@mail.gmail.com","threadId":"45084","inReplyTo":"37fe2024-0378-a974-a28d-18a89d3e2312@alum.mit.edu","subject":"Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-09T11:59:31Z","receivedAt":"2017-02-09T12:00:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Feb 9, 2017 at 1:07 PM, Michael Haggerty wrote:\n> It is unquestionably a good goal to avoid parsing references outside of\n> `refs/files-backend.c`. But I'm not a fan of this approach.\n\nYes. But in this context it was more of a guinea pig. I wanted\nsomething simple enough to code up show we can see what the approach\nlooked like. Good thing I did it.\n\n>\n> There are two meanings of the concept of a \"ref store\", and I think this\n> change muddles them:\n>\n> 1. The references that happen to be *physically* stored in a particular\n> location, for example the `refs/bisect/*` references in a worktree.\n>\n> 2. The references that *logically* should be considered part of a\n> particular repository. This might require stitching together\n> references from multiple sources, for example `HEAD` and\n> `refs/bisect` from a worktree's own directory with other\n> references from the main repository.\n>\n> Either of these concepts can be implemented via the `ref_store` abstraction.\n>\n> The `ref_store` for a submodule should represent the references\n> logically visible from the submodule. The main program shouldn't care\n> whether the references are stored in a single physical location or\n> spread across multiple locations (for example, if the submodule were\n> itself a linked worktree).\n>\n> The `ref_store` that you want here for a worktree is not the worktree's\n> *logical* `ref_store`. You want the worktree's *physical* `ref_store`.\n\nYep.\n\n> Mixing logical and physical reference stores together is a bad idea\n> (even if we were willing to ignore the fact that worktrees are not\n> submodules in the accepted sense of the word).\n>\n> ...\n>\n> I think the best solution would be to expose the concept of `ref_store`\n> in the public refs API. Then users of submodules would essentially do\n>\n> struct ref_store *refs = get_submodule_refs(submodule_path);\n> ... resolve_ref_recursively(refs, refname, 0, sha1, &flags) ...\n> ... for_each_ref(refs, fn, cb_data) ...\n>\n> whereas for a worktree you'd have to look up the `ref_store` instance\n> somewhere else (or maybe keep it as part of some worktree structure, if\n> there is one) but you would use it via the same API.\n\nOh I was going to reply to Stefan about his comment to my (**)\nfootnote. Something along the this line\n\n\"Ideally we would introduce a new set of api, maybe with refs_ prefix,\nthat takes a refs_store. Then submodule people can get a ref store\nsomewhere and pass to it. Worktree people get maybe some other refs\nstore for it. The \"old\" api like for_each_ref() is a thin wrapper\naround it, just like read_cache() vs read_index(&the_index). If the\n*_submodule does not see much use, we might as well kill it and use\nthe generic refs_*\".\n\nIf I didn't misunderstood anything else, then I think we're on the same page.\n\nNow I need to see if I can get there in a reasonable time frame (so I\ncan fix my \"gc in worktree\" problem properly) or I would need\nsomething temporary but not so hacky. I'll try to make this new api\nand see how it works out. If you think I should not do it right away,\nfor whatever reason, stop me now.\n-- \nDuy\n"},{"id":"311174","messageId":"4bea30e9-9724-0a71-e9da-d79ad8accd28@alum.mit.edu","threadId":"45084","inReplyTo":"CACsJy8Diy92CNbJ1OBn893VFFrSsxBFWSyQHjt_Dzq9x7jfibQ@mail.gmail.com","subject":"Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-02-09T14:03:50Z","receivedAt":"2017-02-09T14:05:28Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/09/2017 12:59 PM, Duy Nguyen wrote:\n> On Thu, Feb 9, 2017 at 1:07 PM, Michael Haggerty wrote:\n>> It is unquestionably a good goal to avoid parsing references outside of\n>> `refs/files-backend.c`. But I'm not a fan of this approach.\n> \n> Yes. But in this context it was more of a guinea pig. I wanted\n> something simple enough to code up show we can see what the approach\n> looked like. Good thing I did it.\n> \n>>\n>> There are two meanings of the concept of a \"ref store\", and I think this\n>> change muddles them:\n>>\n>> 1. The references that happen to be *physically* stored in a particular\n>> location, for example the `refs/bisect/*` references in a worktree.\n>>\n>> 2. The references that *logically* should be considered part of a\n>> particular repository. This might require stitching together\n>> references from multiple sources, for example `HEAD` and\n>> `refs/bisect` from a worktree's own directory with other\n>> references from the main repository.\n>>\n>> Either of these concepts can be implemented via the `ref_store` abstraction.\n>>\n>> The `ref_store` for a submodule should represent the references\n>> logically visible from the submodule. The main program shouldn't care\n>> whether the references are stored in a single physical location or\n>> spread across multiple locations (for example, if the submodule were\n>> itself a linked worktree).\n>>\n>> The `ref_store` that you want here for a worktree is not the worktree's\n>> *logical* `ref_store`. You want the worktree's *physical* `ref_store`.\n> \n> Yep.\n> \n>> Mixing logical and physical reference stores together is a bad idea\n>> (even if we were willing to ignore the fact that worktrees are not\n>> submodules in the accepted sense of the word).\n>>\n>> ...\n>>\n>> I think the best solution would be to expose the concept of `ref_store`\n>> in the public refs API. Then users of submodules would essentially do\n>>\n>> struct ref_store *refs = get_submodule_refs(submodule_path);\n>> ... resolve_ref_recursively(refs, refname, 0, sha1, &flags) ...\n>> ... for_each_ref(refs, fn, cb_data) ...\n>>\n>> whereas for a worktree you'd have to look up the `ref_store` instance\n>> somewhere else (or maybe keep it as part of some worktree structure, if\n>> there is one) but you would use it via the same API.\n> \n> Oh I was going to reply to Stefan about his comment to my (**)\n> footnote. Something along the this line\n> \n> \"Ideally we would introduce a new set of api, maybe with refs_ prefix,\n> that takes a refs_store. Then submodule people can get a ref store\n> somewhere and pass to it. Worktree people get maybe some other refs\n> store for it. The \"old\" api like for_each_ref() is a thin wrapper\n> around it, just like read_cache() vs read_index(&the_index). If the\n> *_submodule does not see much use, we might as well kill it and use\n> the generic refs_*\".\n> \n> If I didn't misunderstood anything else, then I think we're on the same page.\n> \n> Now I need to see if I can get there in a reasonable time frame (so I\n> can fix my \"gc in worktree\" problem properly) or I would need\n> something temporary but not so hacky. I'll try to make this new api\n> and see how it works out. If you think I should not do it right away,\n> for whatever reason, stop me now.\n\nSounds good. A couple of hints and points to remember:\n\n* I think `struct ref_store *` should remain opaque outside of the refs\n code.\n\n* The virtual function interface embodied in `struct ref_storage_be`\n isn't meant to be an external interface (i.e., don't just expose it\n and have external callers use it directly).\n\n* One important distinction between the main reference store and\n submodule reference stores is that we know the object store for\n the former but not the latter. That implies that some operations\n are, or should be, impossible for submodules (e.g., anything that\n involves looking up objects, including peeling refs). However, we\n *do* know the object store for linked worktrees. So they don't have\n to be as dumbed-down as submodule ref stores. (This might be\n irrelevant if you don't need any additional features for your\n current purposes.)\n\nAlso, I just sent my submodule-hash patch series to the mailing list in\ncase you want to build on that.\n\nMichael\n\n"},{"id":"311755","messageId":"20170216120302.5302-1-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170208113144.8201-1-pclouds@gmail.com","subject":"[PATCH v2 0/5] Kill manual ref parsing code in worktree.c","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-16T12:02:57Z","receivedAt":"2017-02-16T12:03:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"v2 still kills parse_ref(), but the series now depends on my other\nseries [1] and as a result does not make any confusing \"submodule\"\ncalls any more.\n\nIt also kills another function, introduced for multi-worktree, that\nshould not have been there to begin with. Good riddance.\n\nAgain, the naming convention with refs_ prefix for new APIs may not be\nthe best idea...\n\n[1] public-inbox.org/git/20170213152011.12050-1-pclouds@gmail.com\n\nNguyễn Thái Ngọc Duy (5):\n refs: introduce get_worktree_ref_store()\n refs.c: add refs_resolve_ref_unsafe()\n worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()\n refs: add refs_create_symref()\n refs: kill set_worktree_head_symref()\n\n branch.c | 15 ++++----\n refs.c | 58 ++++++++++++++++++++++++-----\n refs.h | 21 ++++++-----\n refs/files-backend.c | 43 +---------------------\n refs/refs-internal.h | 5 ---\n worktree.c | 102 ++++++++++++++-------------------------------------\n worktree.h | 2 +-\n 7 files changed, 98 insertions(+), 148 deletions(-)\n\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"311756","messageId":"20170216120302.5302-2-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170216120302.5302-1-pclouds@gmail.com","subject":"[PATCH v2 1/5] refs: introduce get_worktree_ref_store()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-16T12:02:58Z","receivedAt":"2017-02-16T12:03:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"files-backend at this point is still aware of the per-repo/worktree\nseparation in refs, so it can handle a linked worktree.\n\nNote: accessing a worktree of a submodule remains unaddressed. Perhaps\nafter get_worktrees() can access submodule (or rather a new function\nget_submodule_worktrees(), that lists worktrees of a submodule), we can\nupdate this function to work with submodules as well.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 27 +++++++++++++++++++++++++++\n refs.h | 2 ++\n 2 files changed, 29 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex e7206a420..ba4d9420c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -10,6 +10,7 @@\n #include \"object.h\"\n #include \"tag.h\"\n #include \"submodule.h\"\n+#include \"worktree.h\"\n \n /*\n * List of all available backends\n@@ -1486,6 +1487,32 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \treturn refs;\n }\n \n+struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n+{\n+\tstruct ref_store *refs;\n+\n+\tif (wt->is_current)\n+\t\treturn get_main_ref_store();\n+\n+\t/*\n+\t * We share the same hash map with submodules for\n+\t * now. submodule paths are always relative (to topdir) while\n+\t * worktree paths are always absolute. No chance of conflict.\n+\t */\n+\trefs = lookup_submodule_ref_store(wt->path);\n+\tif (refs)\n+\t\treturn refs;\n+\n+\tif (wt->id)\n+\t\trefs = ref_store_init(git_common_path(\"worktrees/%s\", wt->id));\n+\telse\n+\t\trefs = ref_store_init(get_git_common_dir());\n+\n+\tif (refs)\n+\t\tregister_submodule_ref_store(refs, wt->path);\n+\treturn refs;\n+}\n+\n void base_ref_store_init(struct ref_store *refs,\n \t\t\t const struct ref_storage_be *be)\n {\ndiff --git a/refs.h b/refs.h\nindex 1287ba59c..464cc384a 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -565,5 +565,7 @@ struct ref_store *get_main_ref_store(void);\n * submodule==NULL.\n */\n struct ref_store *get_submodule_ref_store(const char *submodule);\n+struct worktree;\n+struct ref_store *get_worktree_ref_store(const struct worktree *wt);\n \n #endif /* REFS_H */\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"311757","messageId":"20170216120302.5302-3-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170216120302.5302-1-pclouds@gmail.com","subject":"[PATCH v2 2/5] refs.c: add refs_resolve_ref_unsafe()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-16T12:02:59Z","receivedAt":"2017-02-16T12:04:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Surprise surprise. This is just rename and introduce\nresolve_ref_recursively() to the public.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 9 +++++----\n refs.h | 6 ++++++\n refs/files-backend.c | 2 +-\n refs/refs-internal.h | 5 -----\n 4 files changed, 12 insertions(+), 10 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex ba4d9420c..e0e191107 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1226,7 +1226,7 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)\n }\n \n /* This function needs to return a meaningful errno on failure */\n-const char *resolve_ref_recursively(struct ref_store *refs,\n+const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t const char *refname,\n \t\t\t\t int resolve_flags,\n \t\t\t\t unsigned char *sha1, int *flags)\n@@ -1313,8 +1313,9 @@ int refs_init_db(struct strbuf *err)\n const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t unsigned char *sha1, int *flags)\n {\n-\treturn resolve_ref_recursively(get_main_ref_store(), refname,\n-\t\t\t\t resolve_flags, sha1, flags);\n+\treturn refs_resolve_ref_unsafe(get_main_ref_store(),\n+\t\t\t\t refname, resolve_flags,\n+\t\t\t\t sha1, flags);\n }\n \n int resolve_gitlink_ref(const char *submodule, const char *refname,\n@@ -1343,7 +1344,7 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n \tif (!refs)\n \t\treturn -1;\n \n-\tif (!resolve_ref_recursively(refs, refname, 0, sha1, &flags) ||\n+\tif (!refs_resolve_ref_unsafe(refs, refname, 0, sha1, &flags) ||\n \t is_null_sha1(sha1))\n \t\treturn -1;\n \treturn 0;\ndiff --git a/refs.h b/refs.h\nindex 464cc384a..10c2cfc00 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -568,4 +568,10 @@ struct ref_store *get_submodule_ref_store(const char *submodule);\n struct worktree;\n struct ref_store *get_worktree_ref_store(const struct worktree *wt);\n \n+const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n+\t\t\t\t const char *refname,\n+\t\t\t\t int resolve_flags,\n+\t\t\t\t unsigned char *sha1,\n+\t\t\t\t int *flags);\n+\n #endif /* REFS_H */\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 685ea5c14..f3be620ab 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1276,7 +1276,7 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)\n \t\t\t\t\t create_dir_entry(refs, refname.buf,\n \t\t\t\t\t\t\t refname.len, 1));\n \t\t} else {\n-\t\t\tif (!resolve_ref_recursively(&refs->base,\n+\t\t\tif (!refs_resolve_ref_unsafe(&refs->base,\n \t\t\t\t\t\t refname.buf,\n \t\t\t\t\t\t RESOLVE_REF_READING,\n \t\t\t\t\t\t sha1, &flag)) {\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex cb6882779..6b29dc3b1 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -634,11 +634,6 @@ struct ref_store {\n void base_ref_store_init(struct ref_store *refs,\n \t\t\t const struct ref_storage_be *be);\n \n-const char *resolve_ref_recursively(struct ref_store *refs,\n-\t\t\t\t const char *refname,\n-\t\t\t\t int resolve_flags,\n-\t\t\t\t unsigned char *sha1, int *flags);\n-\n static inline int is_per_worktree_ref(const char *refname)\n {\n \treturn !starts_with(refname, \"refs/\") ||\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"311758","messageId":"20170216120302.5302-4-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170216120302.5302-1-pclouds@gmail.com","subject":"[PATCH v2 3/5] worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-16T12:03:00Z","receivedAt":"2017-02-16T12:04:12Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The manual parsing code is replaced with a call to refs_resolve_ref_unsafe().\nThe manual parsing code must die because only refs/files-backend.c\nshould do that.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n branch.c | 3 +-\n worktree.c | 102 ++++++++++++++++---------------------------------------------\n worktree.h | 2 +-\n 3 files changed, 30 insertions(+), 77 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex c431cbf6a..fcb4a765c 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -354,7 +354,8 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref)\n \tfor (i = 0; worktrees[i]; i++) {\n \t\tif (worktrees[i]->is_detached)\n \t\t\tcontinue;\n-\t\tif (strcmp(oldref, worktrees[i]->head_ref))\n+\t\tif (worktrees[i]->head_ref &&\n+\t\t strcmp(oldref, worktrees[i]->head_ref))\n \t\t\tcontinue;\n \n \t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\ndiff --git a/worktree.c b/worktree.c\nindex eb6121263..f8f520036 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -19,54 +19,25 @@ void free_worktrees(struct worktree **worktrees)\n \tfree (worktrees);\n }\n \n-/*\n- * read 'path_to_ref' into 'ref'. Also if is_detached is not NULL,\n- * set is_detached to 1 (0) if the ref is detached (is not detached).\n- *\n- * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside $GIT_DIR so\n- * for linked worktrees, `resolve_ref_unsafe()` won't work (it uses\n- * git_path). Parse the ref ourselves.\n- *\n- * return -1 if the ref is not a proper ref, 0 otherwise (success)\n- */\n-static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)\n-{\n-\tif (is_detached)\n-\t\t*is_detached = 0;\n-\tif (!strbuf_readlink(ref, path_to_ref, 0)) {\n-\t\t/* HEAD is symbolic link */\n-\t\tif (!starts_with(ref->buf, \"refs/\") ||\n-\t\t\t\tcheck_refname_format(ref->buf, 0))\n-\t\t\treturn -1;\n-\t} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {\n-\t\t/* textual symref or detached */\n-\t\tif (!starts_with(ref->buf, \"ref:\")) {\n-\t\t\tif (is_detached)\n-\t\t\t\t*is_detached = 1;\n-\t\t} else {\n-\t\t\tstrbuf_remove(ref, 0, strlen(\"ref:\"));\n-\t\t\tstrbuf_trim(ref);\n-\t\t\tif (check_refname_format(ref->buf, 0))\n-\t\t\t\treturn -1;\n-\t\t}\n-\t} else\n-\t\treturn -1;\n-\treturn 0;\n-}\n-\n /**\n- * Add the head_sha1 and head_ref (if not detached) to the given worktree\n+ * Update head_sha1, head_ref and is_detached of the given worktree\n */\n-static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)\n+static void add_head_info(struct worktree *wt)\n {\n-\tif (head_ref->len) {\n-\t\tif (worktree->is_detached) {\n-\t\t\tget_sha1_hex(head_ref->buf, worktree->head_sha1);\n-\t\t} else {\n-\t\t\tresolve_ref_unsafe(head_ref->buf, 0, worktree->head_sha1, NULL);\n-\t\t\tworktree->head_ref = strbuf_detach(head_ref, NULL);\n-\t\t}\n-\t}\n+\tint flags;\n+\tconst char *target;\n+\n+\ttarget = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n+\t\t\t\t\t \"HEAD\",\n+\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t wt->head_sha1, &flags);\n+\tif (!target)\n+\t\treturn;\n+\n+\tif (flags & REF_ISSYMREF)\n+\t\twt->head_ref = xstrdup(target);\n+\telse\n+\t\twt->is_detached = 1;\n }\n \n /**\n@@ -77,9 +48,7 @@ static struct worktree *get_main_worktree(void)\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n-\tstruct strbuf head_ref = STRBUF_INIT;\n \tint is_bare = 0;\n-\tint is_detached = 0;\n \n \tstrbuf_add_absolute_path(&worktree_path, get_git_common_dir());\n \tis_bare = !strbuf_strip_suffix(&worktree_path, \"/.git\");\n@@ -91,13 +60,10 @@ static struct worktree *get_main_worktree(void)\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->is_bare = is_bare;\n-\tworktree->is_detached = is_detached;\n-\tif (!parse_ref(path.buf, &head_ref, &is_detached))\n-\t\tadd_head_info(&head_ref, worktree);\n+\tadd_head_info(worktree);\n \n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n-\tstrbuf_release(&head_ref);\n \treturn worktree;\n }\n \n@@ -106,8 +72,6 @@ static struct worktree *get_linked_worktree(const char *id)\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n-\tstruct strbuf head_ref = STRBUF_INIT;\n-\tint is_detached = 0;\n \n \tif (!id)\n \t\tdie(\"Missing linked worktree name\");\n@@ -127,19 +91,14 @@ static struct worktree *get_linked_worktree(const char *id)\n \tstrbuf_reset(&path);\n \tstrbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n \n-\tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n-\t\tgoto done;\n-\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->id = xstrdup(id);\n-\tworktree->is_detached = is_detached;\n-\tadd_head_info(&head_ref, worktree);\n+\tadd_head_info(worktree);\n \n done:\n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n-\tstrbuf_release(&head_ref);\n \treturn worktree;\n }\n \n@@ -334,8 +293,6 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\t\t\t\t const char *target)\n {\n \tconst struct worktree *existing = NULL;\n-\tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf sb = STRBUF_INIT;\n \tstatic struct worktree **worktrees;\n \tint i = 0;\n \n@@ -345,6 +302,11 @@ const struct worktree *find_shared_symref(const char *symref,\n \n \tfor (i = 0; worktrees[i]; i++) {\n \t\tstruct worktree *wt = worktrees[i];\n+\t\tconst char *symref_target;\n+\t\tunsigned char sha1[20];\n+\t\tstruct ref_store *refs;\n+\t\tint flags;\n+\n \t\tif (wt->is_bare)\n \t\t\tcontinue;\n \n@@ -359,24 +321,14 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\t\t}\n \t\t}\n \n-\t\tstrbuf_reset(&path);\n-\t\tstrbuf_reset(&sb);\n-\t\tstrbuf_addf(&path, \"%s/%s\",\n-\t\t\t get_worktree_git_dir(wt),\n-\t\t\t symref);\n-\n-\t\tif (parse_ref(path.buf, &sb, NULL)) {\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\tif (!strcmp(sb.buf, target)) {\n+\t\trefs = get_worktree_ref_store(wt);\n+\t\tsymref_target = refs_resolve_ref_unsafe(refs, symref, 0,\n+\t\t\t\t\t\t\tsha1, &flags);\n+\t\tif ((flags & REF_ISSYMREF) && !strcmp(symref_target, target)) {\n \t\t\texisting = wt;\n \t\t\tbreak;\n \t\t}\n \t}\n \n-\tstrbuf_release(&path);\n-\tstrbuf_release(&sb);\n-\n \treturn existing;\n }\ndiff --git a/worktree.h b/worktree.h\nindex d59ce1fee..961252e71 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -4,7 +4,7 @@\n struct worktree {\n \tchar *path;\n \tchar *id;\n-\tchar *head_ref;\n+\tchar *head_ref;\t\t/* NULL if HEAD is broken or detached */\n \tchar *lock_reason;\t/* internal use */\n \tunsigned char head_sha1[20];\n \tint is_detached;\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"311759","messageId":"20170216120302.5302-5-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170216120302.5302-1-pclouds@gmail.com","subject":"[PATCH v2 4/5] refs: add refs_create_symref()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-16T12:03:01Z","receivedAt":"2017-02-16T12:04:14Z","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 | 22 +++++++++++++++++-----\n refs.h | 4 ++++\n 2 files changed, 21 insertions(+), 5 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex e0e191107..23e0a8eda 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1535,13 +1535,25 @@ int peel_ref(const char *refname, unsigned char *sha1)\n \treturn refs->be->peel_ref(refs, refname, sha1);\n }\n \n-int create_symref(const char *ref_target, const char *refs_heads_master,\n+int refs_create_symref(struct ref_store *refs,\n+\t\t const char *ref_target,\n+\t\t const char *refs_heads_master,\n+\t\t const char *logmsg)\n+{\n+\treturn refs->be->create_symref(refs,\n+\t\t\t\t ref_target,\n+\t\t\t\t refs_heads_master,\n+\t\t\t\t logmsg);\n+}\n+\n+int create_symref(const char *ref_target,\n+\t\t const char *refs_heads_master,\n \t\t const char *logmsg)\n {\n-\tstruct ref_store *refs = get_main_ref_store();\n-\n-\treturn refs->be->create_symref(refs, ref_target, refs_heads_master,\n-\t\t\t\t logmsg);\n+\treturn refs_create_symref(get_main_ref_store(),\n+\t\t\t\t ref_target,\n+\t\t\t\t refs_heads_master,\n+\t\t\t\t logmsg);\n }\n \n int ref_transaction_commit(struct ref_transaction *transaction,\ndiff --git a/refs.h b/refs.h\nindex 10c2cfc00..694769963 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -573,5 +573,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t int resolve_flags,\n \t\t\t\t unsigned char *sha1,\n \t\t\t\t int *flags);\n+int refs_create_symref(struct ref_store *refs,\n+\t\t const char *refname,\n+\t\t const char *target,\n+\t\t const char *logmsg);\n \n #endif /* REFS_H */\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"311760","messageId":"20170216120302.5302-6-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170216120302.5302-1-pclouds@gmail.com","subject":"[PATCH v2 5/5] refs: kill set_worktree_head_symref()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-16T12:03:02Z","receivedAt":"2017-02-16T12:04:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"70999e9cec (branch -m: update all per-worktree HEADs - 2016-03-27)\nadded this function in order to update HEADs of all relevant\nworktrees, when a branch is renamed.\n\nIt, as a public ref api, kind of breaks abstraction when it uses\ninternal functions of files backend. With the introduction of\nrefs_create_symref(), we can move back pretty close to the code before\n70999e9cec, where create_symref() was used for updating HEAD.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n branch.c | 12 ++++++------\n refs.h | 9 ---------\n refs/files-backend.c | 41 -----------------------------------------\n 3 files changed, 6 insertions(+), 56 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex fcb4a765c..ad0cc0489 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -352,18 +352,18 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref)\n \tint i;\n \n \tfor (i = 0; worktrees[i]; i++) {\n+\t\tstruct ref_store *refs;\n+\n \t\tif (worktrees[i]->is_detached)\n \t\t\tcontinue;\n \t\tif (worktrees[i]->head_ref &&\n \t\t strcmp(oldref, worktrees[i]->head_ref))\n \t\t\tcontinue;\n \n-\t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\n-\t\t\t\t\t newref)) {\n-\t\t\tret = -1;\n-\t\t\terror(_(\"HEAD of working tree %s is not updated\"),\n-\t\t\t worktrees[i]->path);\n-\t\t}\n+\t\trefs = get_worktree_ref_store(worktrees[i]);\n+\t\tif (refs_create_symref(refs, \"HEAD\", newref, NULL))\n+\t\t\tret = error(_(\"HEAD of working tree %s is not updated\"),\n+\t\t\t\t worktrees[i]->path);\n \t}\n \n \tfree_worktrees(worktrees);\ndiff --git a/refs.h b/refs.h\nindex 694769963..bce77891a 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -325,15 +325,6 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg);\n \n int create_symref(const char *refname, const char *target, const char *logmsg);\n \n-/*\n- * Update HEAD of the specified gitdir.\n- * Similar to create_symref(\"relative-git-dir/HEAD\", target, NULL), but\n- * this can update the main working tree's HEAD regardless of where\n- * $GIT_DIR points to.\n- * Return 0 if successful, non-zero otherwise.\n- * */\n-int set_worktree_head_symref(const char *gitdir, const char *target);\n-\n enum action_on_err {\n \tUPDATE_REFS_MSG_ON_ERR,\n \tUPDATE_REFS_DIE_ON_ERR,\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex f3be620ab..ba56e46d4 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3108,47 +3108,6 @@ static int files_create_symref(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-int set_worktree_head_symref(const char *gitdir, const char *target)\n-{\n-\t/*\n-\t * FIXME: this obviously will not work well for future refs\n-\t * backends. This function needs to die.\n-\t */\n-\tstruct files_ref_store *refs =\n-\t\tfiles_downcast(get_main_ref_store(), \"set_head_symref\");\n-\tstatic struct lock_file head_lock;\n-\tstruct ref_lock *lock;\n-\tstruct strbuf head_path = STRBUF_INIT;\n-\tconst char *head_rel;\n-\tint ret;\n-\n-\tstrbuf_addf(&head_path, \"%s/HEAD\", absolute_path(gitdir));\n-\tif (hold_lock_file_for_update(&head_lock, head_path.buf,\n-\t\t\t\t LOCK_NO_DEREF) < 0) {\n-\t\tstruct strbuf err = STRBUF_INIT;\n-\t\tunable_to_lock_message(head_path.buf, errno, &err);\n-\t\terror(\"%s\", err.buf);\n-\t\tstrbuf_release(&err);\n-\t\tstrbuf_release(&head_path);\n-\t\treturn -1;\n-\t}\n-\n-\t/* head_rel will be \"HEAD\" for the main tree, \"worktrees/wt/HEAD\" for\n-\t linked trees */\n-\thead_rel = remove_leading_path(head_path.buf,\n-\t\t\t\t absolute_path(get_git_common_dir()));\n-\t/* to make use of create_symref_locked(), initialize ref_lock */\n-\tlock = xcalloc(1, sizeof(struct ref_lock));\n-\tlock->lk = &head_lock;\n-\tlock->ref_name = xstrdup(head_rel);\n-\n-\tret = create_symref_locked(refs, lock, head_rel, target, NULL);\n-\n-\tunlock_ref(lock); /* will free lock */\n-\tstrbuf_release(&head_path);\n-\treturn ret;\n-}\n-\n static int files_reflog_exists(struct ref_store *ref_store,\n \t\t\t const char *refname)\n {\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"314470","messageId":"20170318100206.5980-1-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170216120302.5302-1-pclouds@gmail.com","subject":"[PATCH v3 0/4] Kill manual ref parsing code in worktree.c","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-18T10:02:02Z","receivedAt":"2017-03-18T10:09:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"v3 is a rebased version on latest nd/files-backend-git-dir [1]. Since\nthat series added a bunch of new refs_* functions, v2's 02/05 and\n04/05 are removed. The new 01/04 could be an indepedent fix, but at\ntest-ref-store.c requires it, so I put it here. More tests are added\nnow that we have test-ref-store.c (yay!)\n\n[1] http://public-inbox.org/git/%3C20170318020337.22767-1-pclouds@gmail.com%3E/\n\nNguyễn Thái Ngọc Duy (4):\n environment.c: fix potential segfault by get_git_common_dir()\n refs: introduce get_worktree_ref_store()\n worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()\n refs: kill set_worktree_head_symref()\n\n branch.c | 15 ++---\n environment.c | 2 +\n refs.c | 32 +++++++++++\n refs.h | 12 +---\n refs/files-backend.c | 44 --------------\n t/helper/test-ref-store.c | 19 ++++++\n t/t1407-worktree-ref-store.sh (new +x) | 52 +++++++++++++++++\n worktree.c | 102 +++++++++------------------------\n worktree.h | 2 +-\n 9 files changed, 143 insertions(+), 137 deletions(-)\n create mode 100755 t/t1407-worktree-ref-store.sh\n\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"314486","messageId":"20170318100206.5980-5-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170318100206.5980-1-pclouds@gmail.com","subject":"[PATCH v3 4/4] refs: kill set_worktree_head_symref()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-18T10:02:06Z","receivedAt":"2017-03-18T10:34:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"70999e9cec (branch -m: update all per-worktree HEADs - 2016-03-27)\nadded this function in order to update HEADs of all relevant\nworktrees, when a branch is renamed.\n\nIt, as a public ref api, kind of breaks abstraction when it uses\ninternal functions of files backend. With the introduction of\nrefs_create_symref(), we can move back pretty close to the code before\n70999e9cec, where create_symref() was used for updating HEAD.\n---\n branch.c | 12 ++++++------\n refs.h | 10 ----------\n refs/files-backend.c | 44 -------------------------------------------\n t/t1407-worktree-ref-store.sh | 12 ++++++++++++\n 4 files changed, 18 insertions(+), 60 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 0b949b7fb2..69d5eea84b 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -353,18 +353,18 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n \tint i;\n \n \tfor (i = 0; worktrees[i]; i++) {\n+\t\tstruct ref_store *refs;\n+\n \t\tif (worktrees[i]->is_detached)\n \t\t\tcontinue;\n \t\tif (worktrees[i]->head_ref &&\n \t\t strcmp(oldref, worktrees[i]->head_ref))\n \t\t\tcontinue;\n \n-\t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\n-\t\t\t\t\t newref, logmsg)) {\n-\t\t\tret = -1;\n-\t\t\terror(_(\"HEAD of working tree %s is not updated\"),\n-\t\t\t worktrees[i]->path);\n-\t\t}\n+\t\trefs = get_worktree_ref_store(worktrees[i]);\n+\t\tif (refs_create_symref(refs, \"HEAD\", newref, logmsg))\n+\t\t\tret = error(_(\"HEAD of working tree %s is not updated\"),\n+\t\t\t\t worktrees[i]->path);\n \t}\n \n \tfree_worktrees(worktrees);\ndiff --git a/refs.h b/refs.h\nindex 6df69a2adb..447381d378 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -402,16 +402,6 @@ int refs_create_symref(struct ref_store *refs, const char *refname,\n \t\t const char *target, const char *logmsg);\n int create_symref(const char *refname, const char *target, const char *logmsg);\n \n-/*\n- * Update HEAD of the specified gitdir.\n- * Similar to create_symref(\"relative-git-dir/HEAD\", target, NULL), but\n- * this can update the main working tree's HEAD regardless of where\n- * $GIT_DIR points to.\n- * Return 0 if successful, non-zero otherwise.\n- * */\n-int set_worktree_head_symref(const char *gitdir, const char *target,\n-\t\t\t const char *logmsg);\n-\n enum action_on_err {\n \tUPDATE_REFS_MSG_ON_ERR,\n \tUPDATE_REFS_DIE_ON_ERR,\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 4242486118..62d8e0713a 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3163,50 +3163,6 @@ static int files_create_symref(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-int set_worktree_head_symref(const char *gitdir, const char *target, const char *logmsg)\n-{\n-\t/*\n-\t * FIXME: this obviously will not work well for future refs\n-\t * backends. This function needs to die.\n-\t */\n-\tstruct files_ref_store *refs =\n-\t\tfiles_downcast(get_main_ref_store(),\n-\t\t\t REF_STORE_WRITE,\n-\t\t\t \"set_head_symref\");\n-\n-\tstatic struct lock_file head_lock;\n-\tstruct ref_lock *lock;\n-\tstruct strbuf head_path = STRBUF_INIT;\n-\tconst char *head_rel;\n-\tint ret;\n-\n-\tstrbuf_addf(&head_path, \"%s/HEAD\", absolute_path(gitdir));\n-\tif (hold_lock_file_for_update(&head_lock, head_path.buf,\n-\t\t\t\t LOCK_NO_DEREF) < 0) {\n-\t\tstruct strbuf err = STRBUF_INIT;\n-\t\tunable_to_lock_message(head_path.buf, errno, &err);\n-\t\terror(\"%s\", err.buf);\n-\t\tstrbuf_release(&err);\n-\t\tstrbuf_release(&head_path);\n-\t\treturn -1;\n-\t}\n-\n-\t/* head_rel will be \"HEAD\" for the main tree, \"worktrees/wt/HEAD\" for\n-\t linked trees */\n-\thead_rel = remove_leading_path(head_path.buf,\n-\t\t\t\t absolute_path(get_git_common_dir()));\n-\t/* to make use of create_symref_locked(), initialize ref_lock */\n-\tlock = xcalloc(1, sizeof(struct ref_lock));\n-\tlock->lk = &head_lock;\n-\tlock->ref_name = xstrdup(head_rel);\n-\n-\tret = create_symref_locked(refs, lock, head_rel, target, logmsg);\n-\n-\tunlock_ref(lock); /* will free lock */\n-\tstrbuf_release(&head_path);\n-\treturn ret;\n-}\n-\n static int files_reflog_exists(struct ref_store *ref_store,\n \t\t\t const char *refname)\n {\ndiff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh\nindex 04d1e9d177..5df06f3556 100755\n--- a/t/t1407-worktree-ref-store.sh\n+++ b/t/t1407-worktree-ref-store.sh\n@@ -37,4 +37,16 @@ test_expect_success 'resolve_ref()' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'create_symref(FOO, refs/heads/master)' '\n+\t$RWT create-symref FOO refs/heads/master nothing &&\n+\techo refs/heads/master >expected &&\n+\tgit -C wt symbolic-ref FOO >actual &&\n+\ttest_cmp expected actual &&\n+\n+\t$RMAIN create-symref FOO refs/heads/wt-master nothing &&\n+\techo refs/heads/wt-master >expected &&\n+\tgit symbolic-ref FOO >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"314493","messageId":"20170318100206.5980-4-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170318100206.5980-1-pclouds@gmail.com","subject":"[PATCH v3 3/4] worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-18T10:02:05Z","receivedAt":"2017-03-18T11:08:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The manual parsing code is replaced with a call to refs_resolve_ref_unsafe().\nThe manual parsing code must die because only refs/files-backend.c\nshould do that.\n---\n branch.c | 3 +-\n t/helper/test-ref-store.c | 19 ++++++\n t/t1407-worktree-ref-store.sh (new +x) | 40 +++++++++++++\n worktree.c | 102 +++++++++------------------------\n worktree.h | 2 +-\n 5 files changed, 89 insertions(+), 77 deletions(-)\n create mode 100755 t/t1407-worktree-ref-store.sh\n\ndiff --git a/branch.c b/branch.c\nindex 5c12036b02..0b949b7fb2 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -355,7 +355,8 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n \tfor (i = 0; worktrees[i]; i++) {\n \t\tif (worktrees[i]->is_detached)\n \t\t\tcontinue;\n-\t\tif (strcmp(oldref, worktrees[i]->head_ref))\n+\t\tif (worktrees[i]->head_ref &&\n+\t\t strcmp(oldref, worktrees[i]->head_ref))\n \t\t\tcontinue;\n \n \t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\ndiff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c\nindex 57849ee59a..3c926ed85e 100644\n--- a/t/helper/test-ref-store.c\n+++ b/t/helper/test-ref-store.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"refs.h\"\n+#include \"worktree.h\"\n \n static const char *notnull(const char *arg, const char *name)\n {\n@@ -32,6 +33,23 @@ static const char **get_store(const char **argv, struct ref_store **refs)\n \t\tstrbuf_release(&sb);\n \n \t\t*refs = get_submodule_ref_store(gitdir);\n+\t} else if (skip_prefix(argv[0], \"worktree:\", &gitdir)) {\n+\t\tstruct worktree **p, **worktrees = get_worktrees(0);\n+\n+\t\tfor (p = worktrees; *p; p++) {\n+\t\t\tstruct worktree *wt = *p;\n+\n+\t\t\tif (!wt->id) {\n+\t\t\t\t/* special case for main worktree */\n+\t\t\t\tif (!strcmp(gitdir, \"main\"))\n+\t\t\t\t\tbreak;\n+\t\t\t} else if (!strcmp(gitdir, wt->id))\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif (!*p)\n+\t\t\tdie(\"no such worktree: %s\", gitdir);\n+\n+\t\t*refs = get_worktree_ref_store(*p);\n \t} else\n \t\tdie(\"unknown backend %s\", argv[0]);\n \n@@ -261,6 +279,7 @@ int cmd_main(int argc, const char **argv)\n \tconst char *func;\n \tstruct command *cmd;\n \n+\tsetup_git_directory();\n \targv = get_store(argv + 1, &refs);\n \n \tfunc = *argv++;\ndiff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh\nnew file mode 100755\nindex 0000000000..04d1e9d177\n--- /dev/null\n+++ b/t/t1407-worktree-ref-store.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description='test worktree ref store api'\n+\n+. ./test-lib.sh\n+\n+RWT=\"test-ref-store worktree:wt\"\n+RMAIN=\"test-ref-store worktree:main\"\n+\n+test_expect_success 'setup' '\n+\ttest_commit first &&\n+\tgit worktree add -b wt-master wt &&\n+\t(\n+\t\tcd wt &&\n+\t\ttest_commit second\n+\t)\n+'\n+\n+test_expect_success 'resolve_ref()' '\n+\tSHA1=`git rev-parse master` &&\n+\techo \"$SHA1 refs/heads/master 0x0\" >expected &&\n+\t$RWT resolve-ref refs/heads/master 0 >actual &&\n+\ttest_cmp expected actual &&\n+\t$RMAIN resolve-ref refs/heads/master 0 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'resolve_ref()' '\n+\tSHA1=`git -C wt rev-parse HEAD` &&\n+\techo \"$SHA1 refs/heads/wt-master 0x1\" >expected &&\n+\t$RWT resolve-ref HEAD 0 >actual &&\n+\ttest_cmp expected actual &&\n+\n+\tSHA1=`git rev-parse HEAD` &&\n+\techo \"$SHA1 refs/heads/master 0x1\" >expected &&\n+\t$RMAIN resolve-ref HEAD 0 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\ndiff --git a/worktree.c b/worktree.c\nindex fa7bc67a50..c1ec334b06 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -19,54 +19,25 @@ void free_worktrees(struct worktree **worktrees)\n \tfree (worktrees);\n }\n \n-/*\n- * read 'path_to_ref' into 'ref'. Also if is_detached is not NULL,\n- * set is_detached to 1 (0) if the ref is detached (is not detached).\n- *\n- * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside $GIT_DIR so\n- * for linked worktrees, `resolve_ref_unsafe()` won't work (it uses\n- * git_path). Parse the ref ourselves.\n- *\n- * return -1 if the ref is not a proper ref, 0 otherwise (success)\n- */\n-static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)\n-{\n-\tif (is_detached)\n-\t\t*is_detached = 0;\n-\tif (!strbuf_readlink(ref, path_to_ref, 0)) {\n-\t\t/* HEAD is symbolic link */\n-\t\tif (!starts_with(ref->buf, \"refs/\") ||\n-\t\t\t\tcheck_refname_format(ref->buf, 0))\n-\t\t\treturn -1;\n-\t} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {\n-\t\t/* textual symref or detached */\n-\t\tif (!starts_with(ref->buf, \"ref:\")) {\n-\t\t\tif (is_detached)\n-\t\t\t\t*is_detached = 1;\n-\t\t} else {\n-\t\t\tstrbuf_remove(ref, 0, strlen(\"ref:\"));\n-\t\t\tstrbuf_trim(ref);\n-\t\t\tif (check_refname_format(ref->buf, 0))\n-\t\t\t\treturn -1;\n-\t\t}\n-\t} else\n-\t\treturn -1;\n-\treturn 0;\n-}\n-\n /**\n- * Add the head_sha1 and head_ref (if not detached) to the given worktree\n+ * Update head_sha1, head_ref and is_detached of the given worktree\n */\n-static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)\n+static void add_head_info(struct worktree *wt)\n {\n-\tif (head_ref->len) {\n-\t\tif (worktree->is_detached) {\n-\t\t\tget_sha1_hex(head_ref->buf, worktree->head_sha1);\n-\t\t} else {\n-\t\t\tresolve_ref_unsafe(head_ref->buf, 0, worktree->head_sha1, NULL);\n-\t\t\tworktree->head_ref = strbuf_detach(head_ref, NULL);\n-\t\t}\n-\t}\n+\tint flags;\n+\tconst char *target;\n+\n+\ttarget = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n+\t\t\t\t\t \"HEAD\",\n+\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t wt->head_sha1, &flags);\n+\tif (!target)\n+\t\treturn;\n+\n+\tif (flags & REF_ISSYMREF)\n+\t\twt->head_ref = xstrdup(target);\n+\telse\n+\t\twt->is_detached = 1;\n }\n \n /**\n@@ -77,9 +48,7 @@ static struct worktree *get_main_worktree(void)\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n-\tstruct strbuf head_ref = STRBUF_INIT;\n \tint is_bare = 0;\n-\tint is_detached = 0;\n \n \tstrbuf_add_absolute_path(&worktree_path, get_git_common_dir());\n \tis_bare = !strbuf_strip_suffix(&worktree_path, \"/.git\");\n@@ -91,13 +60,10 @@ static struct worktree *get_main_worktree(void)\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->is_bare = is_bare;\n-\tworktree->is_detached = is_detached;\n-\tif (!parse_ref(path.buf, &head_ref, &is_detached))\n-\t\tadd_head_info(&head_ref, worktree);\n+\tadd_head_info(worktree);\n \n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n-\tstrbuf_release(&head_ref);\n \treturn worktree;\n }\n \n@@ -106,8 +72,6 @@ static struct worktree *get_linked_worktree(const char *id)\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n-\tstruct strbuf head_ref = STRBUF_INIT;\n-\tint is_detached = 0;\n \n \tif (!id)\n \t\tdie(\"Missing linked worktree name\");\n@@ -127,19 +91,14 @@ static struct worktree *get_linked_worktree(const char *id)\n \tstrbuf_reset(&path);\n \tstrbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n \n-\tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n-\t\tgoto done;\n-\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->id = xstrdup(id);\n-\tworktree->is_detached = is_detached;\n-\tadd_head_info(&head_ref, worktree);\n+\tadd_head_info(worktree);\n \n done:\n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n-\tstrbuf_release(&head_ref);\n \treturn worktree;\n }\n \n@@ -334,8 +293,6 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\t\t\t\t const char *target)\n {\n \tconst struct worktree *existing = NULL;\n-\tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf sb = STRBUF_INIT;\n \tstatic struct worktree **worktrees;\n \tint i = 0;\n \n@@ -345,6 +302,11 @@ const struct worktree *find_shared_symref(const char *symref,\n \n \tfor (i = 0; worktrees[i]; i++) {\n \t\tstruct worktree *wt = worktrees[i];\n+\t\tconst char *symref_target;\n+\t\tunsigned char sha1[20];\n+\t\tstruct ref_store *refs;\n+\t\tint flags;\n+\n \t\tif (wt->is_bare)\n \t\t\tcontinue;\n \n@@ -359,25 +321,15 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\t\t}\n \t\t}\n \n-\t\tstrbuf_reset(&path);\n-\t\tstrbuf_reset(&sb);\n-\t\tstrbuf_addf(&path, \"%s/%s\",\n-\t\t\t get_worktree_git_dir(wt),\n-\t\t\t symref);\n-\n-\t\tif (parse_ref(path.buf, &sb, NULL)) {\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\tif (!strcmp(sb.buf, target)) {\n+\t\trefs = get_worktree_ref_store(wt);\n+\t\tsymref_target = refs_resolve_ref_unsafe(refs, symref, 0,\n+\t\t\t\t\t\t\tsha1, &flags);\n+\t\tif ((flags & REF_ISSYMREF) && !strcmp(symref_target, target)) {\n \t\t\texisting = wt;\n \t\t\tbreak;\n \t\t}\n \t}\n \n-\tstrbuf_release(&path);\n-\tstrbuf_release(&sb);\n-\n \treturn existing;\n }\n \ndiff --git a/worktree.h b/worktree.h\nindex 6bfb985203..5ea5e503fb 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -4,7 +4,7 @@\n struct worktree {\n \tchar *path;\n \tchar *id;\n-\tchar *head_ref;\n+\tchar *head_ref;\t\t/* NULL if HEAD is broken or detached */\n \tchar *lock_reason;\t/* internal use */\n \tunsigned char head_sha1[20];\n \tint is_detached;\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"314494","messageId":"20170318100206.5980-3-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170318100206.5980-1-pclouds@gmail.com","subject":"[PATCH v3 2/4] refs: introduce get_worktree_ref_store()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-18T10:02:04Z","receivedAt":"2017-03-18T11:10:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"files-backend at this point is still aware of the per-repo/worktree\nseparation in refs, so it can handle a linked worktree.\n\nSome refs operations are known not working when current files-backend is\nused in a linked worktree (e.g. reflog). Tests will be written when\nrefs_* functions start to be called with worktree backend to verify that\nthey work as expected.\n\nNote: accessing a worktree of a submodule remains unaddressed. Perhaps\nafter get_worktrees() can access submodule (or rather a new function\nget_submodule_worktrees(), that lists worktrees of a submodule), we can\nupdate this function to work with submodules as well.\n---\n refs.c | 32 ++++++++++++++++++++++++++++++++\n refs.h | 2 ++\n 2 files changed, 34 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 77a39f8b17..6695140cfe 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -10,6 +10,7 @@\n #include \"object.h\"\n #include \"tag.h\"\n #include \"submodule.h\"\n+#include \"worktree.h\"\n \n /*\n * List of all available backends\n@@ -1593,6 +1594,37 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \treturn refs;\n }\n \n+struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n+{\n+\tstruct ref_store *refs;\n+\tunsigned int refs_all_capabilities =\n+\t\tREF_STORE_READ | REF_STORE_WRITE |\n+\t\tREF_STORE_ODB | REF_STORE_MAIN;\n+\n+\tif (wt->is_current)\n+\t\treturn get_main_ref_store();\n+\n+\t/*\n+\t * We share the same hash map with submodules for\n+\t * now. submodule paths are always relative (to topdir) while\n+\t * worktree paths are always absolute. No chance of conflict.\n+\t */\n+\trefs = lookup_submodule_ref_store(wt->path);\n+\tif (refs)\n+\t\treturn refs;\n+\n+\tif (wt->id)\n+\t\trefs = ref_store_init(git_common_path(\"worktrees/%s\", wt->id),\n+\t\t\t\t refs_all_capabilities);\n+\telse\n+\t\trefs = ref_store_init(get_git_common_dir(),\n+\t\t\t\t refs_all_capabilities);\n+\n+\tif (refs)\n+\t\tregister_submodule_ref_store(refs, wt->path);\n+\treturn refs;\n+}\n+\n void base_ref_store_init(struct ref_store *refs,\n \t\t\t const struct ref_storage_be *be)\n {\ndiff --git a/refs.h b/refs.h\nindex 49e97d7d5f..6df69a2adb 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -5,6 +5,7 @@ struct object_id;\n struct ref_store;\n struct strbuf;\n struct string_list;\n+struct worktree;\n \n /*\n * Resolve a reference, recursively following symbolic refererences.\n@@ -655,5 +656,6 @@ struct ref_store *get_main_ref_store(void);\n * submodule==NULL.\n */\n struct ref_store *get_submodule_ref_store(const char *submodule);\n+struct ref_store *get_worktree_ref_store(const struct worktree *wt);\n \n #endif /* REFS_H */\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"314497","messageId":"20170318100206.5980-2-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170318100206.5980-1-pclouds@gmail.com","subject":"[PATCH v3 1/4] environment.c: fix potential segfault by get_git_common_dir()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-18T10:02:03Z","receivedAt":"2017-03-18T11:50:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"setup_git_env() must be called before this function to initialize\ngit_common_dir so that it returns a non NULL string. And it must return\na non NULL string or segfault can happen because all callers expect so.\n\nNormally if somebody has called get_git_dir(), or set_git_dir() then\nsetup_git_env() is already called. But if you do setup_git_directory()\nat top dir (which skips set_git_dir) and never call get_git_dir, you'll\nget NULL here.\n\ntest-ref-store.c will hit this problem because it's very lightweight,\njust enough initialization to exercise refs code, and get_git_dir() will\nnever be called until get_worktrees() is, which uses get_git_common_dir().\n---\n environment.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/environment.c b/environment.c\nindex 42dc3106d2..2986ee7200 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -214,6 +214,8 @@ const char *get_git_dir(void)\n \n const char *get_git_common_dir(void)\n {\n+\tif (!git_dir)\n+\t\tsetup_git_env();\n \treturn git_common_dir;\n }\n \n-- \n2.11.0.157.gd943d85\n\n"},{"id":"314512","messageId":"xmqqd1de4gqi.fsf@gitster.mtv.corp.google.com","threadId":"45084","inReplyTo":"20170318100206.5980-1-pclouds@gmail.com","subject":"Re: [PATCH v3 0/4] Kill manual ref parsing code in worktree.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-18T17:49:57Z","receivedAt":"2017-03-18T17:50:22Z","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> v3 is a rebased version on latest nd/files-backend-git-dir [1]. Since\n> that series added a bunch of new refs_* functions, v2's 02/05 and\n> 04/05 are removed. The new 01/04 could be an indepedent fix, but at\n> test-ref-store.c requires it, so I put it here. More tests are added\n> now that we have test-ref-store.c (yay!)\n\nI think the corresponding ones were queued on a separate topic\nnd/worktree-kill-parse-ref that is based on nd/files-backend-git-dir\nbut I do not mind making these 4 patches just part of the latter\ntopic.\n\nWill send comments on individual patches separately.\n\nThanks. \n\n>\n> [1] http://public-inbox.org/git/%3C20170318020337.22767-1-pclouds@gmail.com%3E/\n>\n> Nguyễn Thái Ngọc Duy (4):\n> environment.c: fix potential segfault by get_git_common_dir()\n> refs: introduce get_worktree_ref_store()\n> worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()\n> refs: kill set_worktree_head_symref()\n>\n> branch.c | 15 ++---\n> environment.c | 2 +\n> refs.c | 32 +++++++++++\n> refs.h | 12 +---\n> refs/files-backend.c | 44 --------------\n> t/helper/test-ref-store.c | 19 ++++++\n> t/t1407-worktree-ref-store.sh (new +x) | 52 +++++++++++++++++\n> worktree.c | 102 +++++++++------------------------\n> worktree.h | 2 +-\n> 9 files changed, 143 insertions(+), 137 deletions(-)\n> create mode 100755 t/t1407-worktree-ref-store.sh\n"},{"id":"314515","messageId":"xmqq8to24gif.fsf@gitster.mtv.corp.google.com","threadId":"45084","inReplyTo":"20170318100206.5980-2-pclouds@gmail.com","subject":"Re: [PATCH v3 1/4] environment.c: fix potential segfault by get_git_common_dir()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-18T17:54:48Z","receivedAt":"2017-03-18T17:58:54Z","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> setup_git_env() must be called before this function to initialize\n> git_common_dir so that it returns a non NULL string. And it must return\n> a non NULL string or segfault can happen because all callers expect so.\n>\n> Normally if somebody has called get_git_dir(), or set_git_dir() then\n> setup_git_env() is already called. But if you do setup_git_directory()\n> at top dir (which skips set_git_dir) and never call get_git_dir, you'll\n> get NULL here.\n\nHmph, and the solution for the problem not being \"so let's make sure\nget_git_dir() is called even when the command is started at the top\ndirectory\" is because...?\n\n> test-ref-store.c will hit this problem because it's very lightweight,\n> just enough initialization to exercise refs code, and get_git_dir() will\n> never be called until get_worktrees() is, which uses get_git_common_dir().\n> ---\n\nMissing sign-off.\n\n\n\n> environment.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/environment.c b/environment.c\n> index 42dc3106d2..2986ee7200 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -214,6 +214,8 @@ const char *get_git_dir(void)\n> \n> const char *get_git_common_dir(void)\n> {\n> +\tif (!git_dir)\n> +\t\tsetup_git_env();\n> \treturn git_common_dir;\n> }\n"},{"id":"314587","messageId":"CACsJy8B=NKAERaGWCH-kvU0WE7j8KSXz74U3N+fHmkhqXWTBeQ@mail.gmail.com","threadId":"45084","inReplyTo":"xmqq8to24gif.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 1/4] environment.c: fix potential segfault by get_git_common_dir()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-19T09:34:12Z","receivedAt":"2017-03-19T09:34:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Mar 19, 2017 at 12:54 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> setup_git_env() must be called before this function to initialize\n>> git_common_dir so that it returns a non NULL string. And it must return\n>> a non NULL string or segfault can happen because all callers expect so.\n>>\n>> Normally if somebody has called get_git_dir(), or set_git_dir() then\n>> setup_git_env() is already called. But if you do setup_git_directory()\n>> at top dir (which skips set_git_dir) and never call get_git_dir, you'll\n>> get NULL here.\n>\n> Hmph, and the solution for the problem not being \"so let's make sure\n> get_git_dir() is called even when the command is started at the top\n> directory\" is because...?\n\n-EHARDTOPARSE. There's a hidden dependency between get_git_dir() and\nget_git_common_dir() which is not good. If we lazily call\nset_git_env(), make sure we do it lazily but consistently at all\nrelevant function calls (i.e. including get_git_common_dir).\n\nAlternatively (I was thinking of this but didn't really follow up\nbecause this was side issue) we should make sure setup_git_env() is\nalways called at the end of setup_git_dir...() and remove the laziness\nin get_git_dir(). This may be more in line of recent attempts to catch\nrepo access without calling setup_git_directory..() first. But sadly I\nhaven't read Jeff's series, so I can't say whether it's true.\n-- \nDuy\n"},{"id":"314632","messageId":"b841a674-7b2b-420d-7faa-e5b836fb534e@alum.mit.edu","threadId":"45084","inReplyTo":"20170318100206.5980-3-pclouds@gmail.com","subject":"Re: [PATCH v3 2/4] refs: introduce get_worktree_ref_store()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-20T06:59:30Z","receivedAt":"2017-03-20T07:02:02Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 03/18/2017 11:02 AM, Nguyễn Thái Ngọc Duy wrote:\n> files-backend at this point is still aware of the per-repo/worktree\n> separation in refs, so it can handle a linked worktree.\n> \n> Some refs operations are known not working when current files-backend is\n> used in a linked worktree (e.g. reflog). Tests will be written when\n> refs_* functions start to be called with worktree backend to verify that\n> they work as expected.\n> \n> Note: accessing a worktree of a submodule remains unaddressed. Perhaps\n> after get_worktrees() can access submodule (or rather a new function\n> get_submodule_worktrees(), that lists worktrees of a submodule), we can\n> update this function to work with submodules as well.\n> ---\n> refs.c | 32 ++++++++++++++++++++++++++++++++\n> refs.h | 2 ++\n> 2 files changed, 34 insertions(+)\n> \n> diff --git a/refs.c b/refs.c\n> index 77a39f8b17..6695140cfe 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -10,6 +10,7 @@\n> #include \"object.h\"\n> #include \"tag.h\"\n> #include \"submodule.h\"\n> +#include \"worktree.h\"\n> \n> /*\n> * List of all available backends\n> @@ -1593,6 +1594,37 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n> \treturn refs;\n> }\n> \n> +struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n> +{\n> +\tstruct ref_store *refs;\n> +\tunsigned int refs_all_capabilities =\n> +\t\tREF_STORE_READ | REF_STORE_WRITE |\n> +\t\tREF_STORE_ODB | REF_STORE_MAIN;\n> +\n> +\tif (wt->is_current)\n> +\t\treturn get_main_ref_store();\n> +\n> +\t/*\n> +\t * We share the same hash map with submodules for\n> +\t * now. submodule paths are always relative (to topdir) while\n> +\t * worktree paths are always absolute. No chance of conflict.\n> +\t */\n> +\trefs = lookup_submodule_ref_store(wt->path);\n> +\tif (refs)\n> +\t\treturn refs;\n> +\n> +\tif (wt->id)\n> +\t\trefs = ref_store_init(git_common_path(\"worktrees/%s\", wt->id),\n> +\t\t\t\t refs_all_capabilities);\n> +\telse\n> +\t\trefs = ref_store_init(get_git_common_dir(),\n> +\t\t\t\t refs_all_capabilities);\n> +\n> +\tif (refs)\n> +\t\tregister_submodule_ref_store(refs, wt->path);\n> +\treturn refs;\n> +}\n> +\n\nI guess I can hold my nose and accept storing worktree and submodule\n`ref_store`s together in a single hashmap, but not if the code continues\nto use \"submodule\" in the names, because that will be a continuing\nsource of confusion to readers. For example,\n\n* lookup_submodule_ref_store()\n* register_submodule_ref_store()\n* submodule_hash_entry\n* submodule_hash_cmp()\n* alloc_submodule_hash_entry()\n* submodule_ref_stores\n\nAny docstrings would also have to be updated.\n\nAs a new naming scheme, maybe use s/submodule/other/, for example\n`lookup_other_ref_store()` etc.\n\n> [...]\n\nMichael\n\n"},{"id":"314642","messageId":"CACsJy8B85TH0DOViEmfh8mOz_u7rVotKJa-HYk6TJ81kvV4xPg@mail.gmail.com","threadId":"45084","inReplyTo":"b841a674-7b2b-420d-7faa-e5b836fb534e@alum.mit.edu","subject":"Re: [PATCH v3 2/4] refs: introduce get_worktree_ref_store()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-20T12:01:55Z","receivedAt":"2017-03-20T12:31:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Mar 20, 2017 at 1:59 PM, Michael Haggerty wrote:\n> I guess I can hold my nose and accept storing worktree and submodule\n> `ref_store`s together in a single hashmap\n\nRelease your nose, I'll add a new hashmap :) But while we're here,\nwhat are your thoughts about moving submodule-specific to submodule.c?\nThat includes the hashmap, get_submodule_ref_store() and all other\nsubmodule stuff, to submodule.c where submodule-specific stuff stays.\nThe same for worktree stuff, to worktree.c. That keeps refs.c to core\nrefs business. And the hashmap can be used for more than just refs.\nFor example, submodule has configs, and worktree also has bunch of\nother stuff that I would like to just cache and not readdir() and\nparse every time.\n-- \nDuy\n"},{"id":"314651","messageId":"0d437f43-30d6-3920-5298-0c426b098c1a@alum.mit.edu","threadId":"45084","inReplyTo":"CACsJy8B85TH0DOViEmfh8mOz_u7rVotKJa-HYk6TJ81kvV4xPg@mail.gmail.com","subject":"Re: [PATCH v3 2/4] refs: introduce get_worktree_ref_store()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-03-20T14:25:24Z","receivedAt":"2017-03-20T14:25:41Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 03/20/2017 01:01 PM, Duy Nguyen wrote:\n> On Mon, Mar 20, 2017 at 1:59 PM, Michael Haggerty wrote:\n>> I guess I can hold my nose and accept storing worktree and submodule\n>> `ref_store`s together in a single hashmap\n> \n> Release your nose, I'll add a new hashmap :)\n\nThanks :-)\n\n> But while we're here,\n> what are your thoughts about moving submodule-specific to submodule.c?\n> That includes the hashmap, get_submodule_ref_store() and all other\n> submodule stuff, to submodule.c where submodule-specific stuff stays.\n> The same for worktree stuff, to worktree.c. That keeps refs.c to core\n> refs business. And the hashmap can be used for more than just refs.\n> For example, submodule has configs, and worktree also has bunch of\n> other stuff that I would like to just cache and not readdir() and\n> parse every time.\n\nI can't think of any objections, though there might be some devils in\nthe details. I assume you mean that the submodule-specific hashmap\nwouldn't be `{name -> struct ref_store}` anymore, but rather `{name ->\nstruct submodule}`, and `struct submodule` would hold the associated\n`ref_store`? That sounds very reasonable.\n\nInstead of moving all of the `for_each_*_submodule()` functions over, I\nencourage you to consider getting rid of them entirely and let the\nend-users call the `refs_for_each_*()` versions of the functions. Again,\nI'm not sure that there won't be friction in doing so, but it seems like\nit's worth a try.\n\nMichael\n\n"},{"id":"315453","messageId":"CACsJy8BRUXx7WcCTrgDEv++Lj+d=fWbLyysMCW3Rw_Z+QRcS_w@mail.gmail.com","threadId":"45084","inReplyTo":"0d437f43-30d6-3920-5298-0c426b098c1a@alum.mit.edu","subject":"Re: [PATCH v3 2/4] refs: introduce get_worktree_ref_store()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-26T08:26:19Z","receivedAt":"2017-03-26T08:27:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Mar 20, 2017 at 9:25 PM, Michael Haggerty wrote:\n> Instead of moving all of the `for_each_*_submodule()` functions over, I\n> encourage you to consider getting rid of them entirely and let the\n> end-users call the `refs_for_each_*()` versions of the functions. Again,\n> I'm not sure that there won't be friction in doing so, but it seems like\n> it's worth a try.\n\nThey are getting rid of. If you look at pu (or nd/prune-in-worktree\nactually) there's only head_ref_submodule() and\nfor_each_remote_ref_submodule() left. head_ref_submodule() has no\ncaller but is still there, I'll need to kill it. Killing the latter\ncan be done separately since the callers in submodule.c need some\nupdate.\n-- \nDuy\n"},{"id":"316145","messageId":"20170404102123.25315-2-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170404102123.25315-1-pclouds@gmail.com","subject":"[PATCH v4 1/5] environment.c: fix potential segfault by get_git_common_dir()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-04T10:21:19Z","receivedAt":"2017-04-04T10:21:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"setup_git_env() must be called before this function to initialize\ngit_common_dir so that it returns a non NULL string. And it must return\na non NULL string or segfault can happen because all callers expect so.\n\nIt does not do so explicitly though and depends on get_git_dir() being\ncalled first (which will guarantee setup_git_env()). Avoid this\ndependency and call setup_git_env() by itself.\n\ntest-ref-store.c will hit this problem because it's very lightweight,\njust enough initialization to exercise refs code, and get_git_dir() will\nnever be called until get_worktrees() is, which uses get_git_common_dir\nand hits a segfault.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n environment.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/environment.c b/environment.c\nindex 42dc3106d2..2986ee7200 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -214,6 +214,8 @@ const char *get_git_dir(void)\n \n const char *get_git_common_dir(void)\n {\n+\tif (!git_dir)\n+\t\tsetup_git_env();\n \treturn git_common_dir;\n }\n \n-- \n2.11.0.157.gd943d85\n\n"},{"id":"316146","messageId":"20170404102123.25315-3-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170404102123.25315-1-pclouds@gmail.com","subject":"[PATCH v4 2/5] refs.c: make submodule ref store hashmap generic","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-04T10:21:20Z","receivedAt":"2017-04-04T10:22:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This removes the \"submodule\" from submodule_hash_entry and other\nfunction names. The goal is to reuse the same code and data structure\nfor other ref store types. The first one is worktree ref stores.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 64 ++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 32 insertions(+), 32 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex ec1f563824..875e30a0b8 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1450,32 +1450,32 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n \treturn 0;\n }\n \n-struct submodule_hash_entry\n+struct ref_store_hash_entry\n {\n \tstruct hashmap_entry ent; /* must be the first member! */\n \n \tstruct ref_store *refs;\n \n-\t/* NUL-terminated name of submodule: */\n-\tchar submodule[FLEX_ARRAY];\n+\t/* NUL-terminated identifier of the ref store: */\n+\tchar name[FLEX_ARRAY];\n };\n \n-static int submodule_hash_cmp(const void *entry, const void *entry_or_key,\n+static int ref_store_hash_cmp(const void *entry, const void *entry_or_key,\n \t\t\t const void *keydata)\n {\n-\tconst struct submodule_hash_entry *e1 = entry, *e2 = entry_or_key;\n-\tconst char *submodule = keydata ? keydata : e2->submodule;\n+\tconst struct ref_store_hash_entry *e1 = entry, *e2 = entry_or_key;\n+\tconst char *name = keydata ? keydata : e2->name;\n \n-\treturn strcmp(e1->submodule, submodule);\n+\treturn strcmp(e1->name, name);\n }\n \n-static struct submodule_hash_entry *alloc_submodule_hash_entry(\n-\t\tconst char *submodule, struct ref_store *refs)\n+static struct ref_store_hash_entry *alloc_ref_store_hash_entry(\n+\t\tconst char *name, struct ref_store *refs)\n {\n-\tstruct submodule_hash_entry *entry;\n+\tstruct ref_store_hash_entry *entry;\n \n-\tFLEX_ALLOC_STR(entry, submodule, submodule);\n-\thashmap_entry_init(entry, strhash(submodule));\n+\tFLEX_ALLOC_STR(entry, name, name);\n+\thashmap_entry_init(entry, strhash(name));\n \tentry->refs = refs;\n \treturn entry;\n }\n@@ -1487,19 +1487,19 @@ static struct ref_store *main_ref_store;\n static struct hashmap submodule_ref_stores;\n \n /*\n- * Return the ref_store instance for the specified submodule. If that\n- * ref_store hasn't been initialized yet, return NULL.\n+ * Look up a ref store by name. If that ref_store hasn't been\n+ * registered yet, return NULL.\n */\n-static struct ref_store *lookup_submodule_ref_store(const char *submodule)\n+static struct ref_store *lookup_ref_store_map(struct hashmap *map,\n+\t\t\t\t\t const char *name)\n {\n-\tstruct submodule_hash_entry *entry;\n+\tstruct ref_store_hash_entry *entry;\n \n-\tif (!submodule_ref_stores.tablesize)\n+\tif (!map->tablesize)\n \t\t/* It's initialized on demand in register_ref_store(). */\n \t\treturn NULL;\n \n-\tentry = hashmap_get_from_hash(&submodule_ref_stores,\n-\t\t\t\t strhash(submodule), submodule);\n+\tentry = hashmap_get_from_hash(map, strhash(name), name);\n \treturn entry ? entry->refs : NULL;\n }\n \n@@ -1535,20 +1535,19 @@ struct ref_store *get_main_ref_store(void)\n }\n \n /*\n- * Register the specified ref_store to be the one that should be used\n- * for submodule. It is a fatal error to call this function twice for\n- * the same submodule.\n+ * Associate a ref store with a name. It is a fatal error to call this\n+ * function twice for the same name.\n */\n-static void register_submodule_ref_store(struct ref_store *refs,\n-\t\t\t\t\t const char *submodule)\n+static void register_ref_store_map(struct hashmap *map,\n+\t\t\t\t const char *type,\n+\t\t\t\t struct ref_store *refs,\n+\t\t\t\t const char *name)\n {\n-\tif (!submodule_ref_stores.tablesize)\n-\t\thashmap_init(&submodule_ref_stores, submodule_hash_cmp, 0);\n+\tif (!map->tablesize)\n+\t\thashmap_init(map, ref_store_hash_cmp, 0);\n \n-\tif (hashmap_put(&submodule_ref_stores,\n-\t\t\talloc_submodule_hash_entry(submodule, refs)))\n-\t\tdie(\"BUG: ref_store for submodule '%s' initialized twice\",\n-\t\t submodule);\n+\tif (hashmap_put(map, alloc_ref_store_hash_entry(name, refs)))\n+\t\tdie(\"BUG: %s ref_store '%s' initialized twice\", type, name);\n }\n \n struct ref_store *get_submodule_ref_store(const char *submodule)\n@@ -1565,7 +1564,7 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \t\treturn get_main_ref_store();\n \t}\n \n-\trefs = lookup_submodule_ref_store(submodule);\n+\trefs = lookup_ref_store_map(&submodule_ref_stores, submodule);\n \tif (refs)\n \t\treturn refs;\n \n@@ -1584,7 +1583,8 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \t/* pretend that add_submodule_odb() has been called */\n \trefs = ref_store_init(submodule_sb.buf,\n \t\t\t REF_STORE_READ | REF_STORE_ODB);\n-\tregister_submodule_ref_store(refs, submodule);\n+\tregister_ref_store_map(&submodule_ref_stores, \"submodule\",\n+\t\t\t refs, submodule);\n \n \tstrbuf_release(&submodule_sb);\n \treturn refs;\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"316147","messageId":"20170404102123.25315-4-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170404102123.25315-1-pclouds@gmail.com","subject":"[PATCH v4 3/5] refs: introduce get_worktree_ref_store()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-04T10:21:21Z","receivedAt":"2017-04-04T10:22:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"files-backend at this point is still aware of the per-repo/worktree\nseparation in refs, so it can handle a linked worktree.\n\nSome refs operations are known not working when current files-backend is\nused in a linked worktree (e.g. reflog). Tests will be written when\nrefs_* functions start to be called with worktree backend to verify that\nthey work as expected.\n\nNote: accessing a worktree of a submodule remains unaddressed. Perhaps\nafter get_worktrees() can access submodule (or rather a new function\nget_submodule_worktrees(), that lists worktrees of a submodule), we can\nupdate this function to work with submodules as well.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 31 +++++++++++++++++++++++++++++++\n refs.h | 2 ++\n 2 files changed, 33 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 875e30a0b8..a4083caf6a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -10,6 +10,7 @@\n #include \"object.h\"\n #include \"tag.h\"\n #include \"submodule.h\"\n+#include \"worktree.h\"\n \n /*\n * List of all available backends\n@@ -1486,6 +1487,9 @@ static struct ref_store *main_ref_store;\n /* A hashmap of ref_stores, stored by submodule name: */\n static struct hashmap submodule_ref_stores;\n \n+/* A hashmap of ref_stores, stored by worktree id: */\n+static struct hashmap worktree_ref_stores;\n+\n /*\n * Look up a ref store by name. If that ref_store hasn't been\n * registered yet, return NULL.\n@@ -1590,6 +1594,33 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \treturn refs;\n }\n \n+struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n+{\n+\tstruct ref_store *refs;\n+\tunsigned int refs_all_capabilities =\n+\t\tREF_STORE_READ | REF_STORE_WRITE |\n+\t\tREF_STORE_ODB | REF_STORE_MAIN;\n+\n+\tif (wt->is_current)\n+\t\treturn get_main_ref_store();\n+\n+\trefs = lookup_ref_store_map(&worktree_ref_stores, wt->id);\n+\tif (refs)\n+\t\treturn refs;\n+\n+\tif (wt->id)\n+\t\trefs = ref_store_init(git_common_path(\"worktrees/%s\", wt->id),\n+\t\t\t\t refs_all_capabilities);\n+\telse\n+\t\trefs = ref_store_init(get_git_common_dir(),\n+\t\t\t\t refs_all_capabilities);\n+\n+\tif (refs)\n+\t\tregister_ref_store_map(&worktree_ref_stores, \"worktree\",\n+\t\t\t\t refs, wt->id);\n+\treturn refs;\n+}\n+\n void base_ref_store_init(struct ref_store *refs,\n \t\t\t const struct ref_storage_be *be)\n {\ndiff --git a/refs.h b/refs.h\nindex 49e97d7d5f..6df69a2adb 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -5,6 +5,7 @@ struct object_id;\n struct ref_store;\n struct strbuf;\n struct string_list;\n+struct worktree;\n \n /*\n * Resolve a reference, recursively following symbolic refererences.\n@@ -655,5 +656,6 @@ struct ref_store *get_main_ref_store(void);\n * submodule==NULL.\n */\n struct ref_store *get_submodule_ref_store(const char *submodule);\n+struct ref_store *get_worktree_ref_store(const struct worktree *wt);\n \n #endif /* REFS_H */\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"316148","messageId":"20170404102123.25315-5-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170404102123.25315-1-pclouds@gmail.com","subject":"[PATCH v4 4/5] worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-04T10:21:22Z","receivedAt":"2017-04-04T10:22:17Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The manual parsing code is replaced with a call to refs_resolve_ref_unsafe().\nThe manual parsing code must die because only refs/files-backend.c\nshould do that.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n branch.c | 3 +-\n t/helper/test-ref-store.c | 18 ++++++\n t/t1407-worktree-ref-store.sh (new +x) | 40 +++++++++++++\n worktree.c | 102 +++++++++------------------------\n worktree.h | 2 +-\n 5 files changed, 88 insertions(+), 77 deletions(-)\n create mode 100755 t/t1407-worktree-ref-store.sh\n\ndiff --git a/branch.c b/branch.c\nindex 5c12036b02..0b949b7fb2 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -355,7 +355,8 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n \tfor (i = 0; worktrees[i]; i++) {\n \t\tif (worktrees[i]->is_detached)\n \t\t\tcontinue;\n-\t\tif (strcmp(oldref, worktrees[i]->head_ref))\n+\t\tif (worktrees[i]->head_ref &&\n+\t\t strcmp(oldref, worktrees[i]->head_ref))\n \t\t\tcontinue;\n \n \t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\ndiff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c\nindex 2d84c45ffe..4a487c014e 100644\n--- a/t/helper/test-ref-store.c\n+++ b/t/helper/test-ref-store.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"refs.h\"\n+#include \"worktree.h\"\n \n static const char *notnull(const char *arg, const char *name)\n {\n@@ -32,6 +33,23 @@ static const char **get_store(const char **argv, struct ref_store **refs)\n \t\tstrbuf_release(&sb);\n \n \t\t*refs = get_submodule_ref_store(gitdir);\n+\t} else if (skip_prefix(argv[0], \"worktree:\", &gitdir)) {\n+\t\tstruct worktree **p, **worktrees = get_worktrees(0);\n+\n+\t\tfor (p = worktrees; *p; p++) {\n+\t\t\tstruct worktree *wt = *p;\n+\n+\t\t\tif (!wt->id) {\n+\t\t\t\t/* special case for main worktree */\n+\t\t\t\tif (!strcmp(gitdir, \"main\"))\n+\t\t\t\t\tbreak;\n+\t\t\t} else if (!strcmp(gitdir, wt->id))\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif (!*p)\n+\t\t\tdie(\"no such worktree: %s\", gitdir);\n+\n+\t\t*refs = get_worktree_ref_store(*p);\n \t} else\n \t\tdie(\"unknown backend %s\", argv[0]);\n \ndiff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh\nnew file mode 100755\nindex 0000000000..04d1e9d177\n--- /dev/null\n+++ b/t/t1407-worktree-ref-store.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description='test worktree ref store api'\n+\n+. ./test-lib.sh\n+\n+RWT=\"test-ref-store worktree:wt\"\n+RMAIN=\"test-ref-store worktree:main\"\n+\n+test_expect_success 'setup' '\n+\ttest_commit first &&\n+\tgit worktree add -b wt-master wt &&\n+\t(\n+\t\tcd wt &&\n+\t\ttest_commit second\n+\t)\n+'\n+\n+test_expect_success 'resolve_ref()' '\n+\tSHA1=`git rev-parse master` &&\n+\techo \"$SHA1 refs/heads/master 0x0\" >expected &&\n+\t$RWT resolve-ref refs/heads/master 0 >actual &&\n+\ttest_cmp expected actual &&\n+\t$RMAIN resolve-ref refs/heads/master 0 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'resolve_ref()' '\n+\tSHA1=`git -C wt rev-parse HEAD` &&\n+\techo \"$SHA1 refs/heads/wt-master 0x1\" >expected &&\n+\t$RWT resolve-ref HEAD 0 >actual &&\n+\ttest_cmp expected actual &&\n+\n+\tSHA1=`git rev-parse HEAD` &&\n+\techo \"$SHA1 refs/heads/master 0x1\" >expected &&\n+\t$RMAIN resolve-ref HEAD 0 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\ndiff --git a/worktree.c b/worktree.c\nindex fa7bc67a50..c1ec334b06 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -19,54 +19,25 @@ void free_worktrees(struct worktree **worktrees)\n \tfree (worktrees);\n }\n \n-/*\n- * read 'path_to_ref' into 'ref'. Also if is_detached is not NULL,\n- * set is_detached to 1 (0) if the ref is detached (is not detached).\n- *\n- * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside $GIT_DIR so\n- * for linked worktrees, `resolve_ref_unsafe()` won't work (it uses\n- * git_path). Parse the ref ourselves.\n- *\n- * return -1 if the ref is not a proper ref, 0 otherwise (success)\n- */\n-static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)\n-{\n-\tif (is_detached)\n-\t\t*is_detached = 0;\n-\tif (!strbuf_readlink(ref, path_to_ref, 0)) {\n-\t\t/* HEAD is symbolic link */\n-\t\tif (!starts_with(ref->buf, \"refs/\") ||\n-\t\t\t\tcheck_refname_format(ref->buf, 0))\n-\t\t\treturn -1;\n-\t} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {\n-\t\t/* textual symref or detached */\n-\t\tif (!starts_with(ref->buf, \"ref:\")) {\n-\t\t\tif (is_detached)\n-\t\t\t\t*is_detached = 1;\n-\t\t} else {\n-\t\t\tstrbuf_remove(ref, 0, strlen(\"ref:\"));\n-\t\t\tstrbuf_trim(ref);\n-\t\t\tif (check_refname_format(ref->buf, 0))\n-\t\t\t\treturn -1;\n-\t\t}\n-\t} else\n-\t\treturn -1;\n-\treturn 0;\n-}\n-\n /**\n- * Add the head_sha1 and head_ref (if not detached) to the given worktree\n+ * Update head_sha1, head_ref and is_detached of the given worktree\n */\n-static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)\n+static void add_head_info(struct worktree *wt)\n {\n-\tif (head_ref->len) {\n-\t\tif (worktree->is_detached) {\n-\t\t\tget_sha1_hex(head_ref->buf, worktree->head_sha1);\n-\t\t} else {\n-\t\t\tresolve_ref_unsafe(head_ref->buf, 0, worktree->head_sha1, NULL);\n-\t\t\tworktree->head_ref = strbuf_detach(head_ref, NULL);\n-\t\t}\n-\t}\n+\tint flags;\n+\tconst char *target;\n+\n+\ttarget = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n+\t\t\t\t\t \"HEAD\",\n+\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t wt->head_sha1, &flags);\n+\tif (!target)\n+\t\treturn;\n+\n+\tif (flags & REF_ISSYMREF)\n+\t\twt->head_ref = xstrdup(target);\n+\telse\n+\t\twt->is_detached = 1;\n }\n \n /**\n@@ -77,9 +48,7 @@ static struct worktree *get_main_worktree(void)\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n-\tstruct strbuf head_ref = STRBUF_INIT;\n \tint is_bare = 0;\n-\tint is_detached = 0;\n \n \tstrbuf_add_absolute_path(&worktree_path, get_git_common_dir());\n \tis_bare = !strbuf_strip_suffix(&worktree_path, \"/.git\");\n@@ -91,13 +60,10 @@ static struct worktree *get_main_worktree(void)\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->is_bare = is_bare;\n-\tworktree->is_detached = is_detached;\n-\tif (!parse_ref(path.buf, &head_ref, &is_detached))\n-\t\tadd_head_info(&head_ref, worktree);\n+\tadd_head_info(worktree);\n \n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n-\tstrbuf_release(&head_ref);\n \treturn worktree;\n }\n \n@@ -106,8 +72,6 @@ static struct worktree *get_linked_worktree(const char *id)\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n-\tstruct strbuf head_ref = STRBUF_INIT;\n-\tint is_detached = 0;\n \n \tif (!id)\n \t\tdie(\"Missing linked worktree name\");\n@@ -127,19 +91,14 @@ static struct worktree *get_linked_worktree(const char *id)\n \tstrbuf_reset(&path);\n \tstrbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n \n-\tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n-\t\tgoto done;\n-\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->id = xstrdup(id);\n-\tworktree->is_detached = is_detached;\n-\tadd_head_info(&head_ref, worktree);\n+\tadd_head_info(worktree);\n \n done:\n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n-\tstrbuf_release(&head_ref);\n \treturn worktree;\n }\n \n@@ -334,8 +293,6 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\t\t\t\t const char *target)\n {\n \tconst struct worktree *existing = NULL;\n-\tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf sb = STRBUF_INIT;\n \tstatic struct worktree **worktrees;\n \tint i = 0;\n \n@@ -345,6 +302,11 @@ const struct worktree *find_shared_symref(const char *symref,\n \n \tfor (i = 0; worktrees[i]; i++) {\n \t\tstruct worktree *wt = worktrees[i];\n+\t\tconst char *symref_target;\n+\t\tunsigned char sha1[20];\n+\t\tstruct ref_store *refs;\n+\t\tint flags;\n+\n \t\tif (wt->is_bare)\n \t\t\tcontinue;\n \n@@ -359,25 +321,15 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\t\t}\n \t\t}\n \n-\t\tstrbuf_reset(&path);\n-\t\tstrbuf_reset(&sb);\n-\t\tstrbuf_addf(&path, \"%s/%s\",\n-\t\t\t get_worktree_git_dir(wt),\n-\t\t\t symref);\n-\n-\t\tif (parse_ref(path.buf, &sb, NULL)) {\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\tif (!strcmp(sb.buf, target)) {\n+\t\trefs = get_worktree_ref_store(wt);\n+\t\tsymref_target = refs_resolve_ref_unsafe(refs, symref, 0,\n+\t\t\t\t\t\t\tsha1, &flags);\n+\t\tif ((flags & REF_ISSYMREF) && !strcmp(symref_target, target)) {\n \t\t\texisting = wt;\n \t\t\tbreak;\n \t\t}\n \t}\n \n-\tstrbuf_release(&path);\n-\tstrbuf_release(&sb);\n-\n \treturn existing;\n }\n \ndiff --git a/worktree.h b/worktree.h\nindex 6bfb985203..5ea5e503fb 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -4,7 +4,7 @@\n struct worktree {\n \tchar *path;\n \tchar *id;\n-\tchar *head_ref;\n+\tchar *head_ref;\t\t/* NULL if HEAD is broken or detached */\n \tchar *lock_reason;\t/* internal use */\n \tunsigned char head_sha1[20];\n \tint is_detached;\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"316149","messageId":"20170404102123.25315-1-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170318100206.5980-1-pclouds@gmail.com","subject":"[PATCH v4 0/5] Kill manual ref parsing code in worktree.c","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-04T10:21:18Z","receivedAt":"2017-04-04T10:22:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"v4 adds a new patch, 2/5, which makes the hashmap related functions in\nrefs.c generic, so I could add a new map for worktree ref stores and\nnot abuse submodule hashmap.\n\nI mentioned about moving these hashmap back to submodule.c and\nworktree.c where it can map more than just ref stores (in worktree\ncase, we can cache 'struct worktree', for example). But I'm not doing\nit now to keep things small.\n\nThe commit message in 1/5 is rephrased a bit, hopefully clearer.\n\nNguyễn Thái Ngọc Duy (5):\n environment.c: fix potential segfault by get_git_common_dir()\n refs.c: make submodule ref store hashmap generic\n refs: introduce get_worktree_ref_store()\n worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()\n refs: kill set_worktree_head_symref()\n\n branch.c | 15 ++---\n environment.c | 2 +\n refs.c | 95 +++++++++++++++++++-----------\n refs.h | 12 +---\n refs/files-backend.c | 44 --------------\n t/helper/test-ref-store.c | 18 ++++++\n t/t1407-worktree-ref-store.sh (new +x) | 52 +++++++++++++++++\n worktree.c | 102 +++++++++------------------------\n worktree.h | 2 +-\n 9 files changed, 173 insertions(+), 169 deletions(-)\n create mode 100755 t/t1407-worktree-ref-store.sh\n\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"316150","messageId":"20170404102123.25315-6-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170404102123.25315-1-pclouds@gmail.com","subject":"[PATCH v4 5/5] refs: kill set_worktree_head_symref()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-04T10:21:23Z","receivedAt":"2017-04-04T10:22:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"70999e9cec (branch -m: update all per-worktree HEADs - 2016-03-27)\nadded this function in order to update HEADs of all relevant\nworktrees, when a branch is renamed.\n\nIt, as a public ref api, kind of breaks abstraction when it uses\ninternal functions of files backend. With the introduction of\nrefs_create_symref(), we can move back pretty close to the code before\n70999e9cec, where create_symref() was used for updating HEAD.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n branch.c | 12 ++++++------\n refs.h | 10 ----------\n refs/files-backend.c | 44 -------------------------------------------\n t/t1407-worktree-ref-store.sh | 12 ++++++++++++\n 4 files changed, 18 insertions(+), 60 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 0b949b7fb2..69d5eea84b 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -353,18 +353,18 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n \tint i;\n \n \tfor (i = 0; worktrees[i]; i++) {\n+\t\tstruct ref_store *refs;\n+\n \t\tif (worktrees[i]->is_detached)\n \t\t\tcontinue;\n \t\tif (worktrees[i]->head_ref &&\n \t\t strcmp(oldref, worktrees[i]->head_ref))\n \t\t\tcontinue;\n \n-\t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\n-\t\t\t\t\t newref, logmsg)) {\n-\t\t\tret = -1;\n-\t\t\terror(_(\"HEAD of working tree %s is not updated\"),\n-\t\t\t worktrees[i]->path);\n-\t\t}\n+\t\trefs = get_worktree_ref_store(worktrees[i]);\n+\t\tif (refs_create_symref(refs, \"HEAD\", newref, logmsg))\n+\t\t\tret = error(_(\"HEAD of working tree %s is not updated\"),\n+\t\t\t\t worktrees[i]->path);\n \t}\n \n \tfree_worktrees(worktrees);\ndiff --git a/refs.h b/refs.h\nindex 6df69a2adb..447381d378 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -402,16 +402,6 @@ int refs_create_symref(struct ref_store *refs, const char *refname,\n \t\t const char *target, const char *logmsg);\n int create_symref(const char *refname, const char *target, const char *logmsg);\n \n-/*\n- * Update HEAD of the specified gitdir.\n- * Similar to create_symref(\"relative-git-dir/HEAD\", target, NULL), but\n- * this can update the main working tree's HEAD regardless of where\n- * $GIT_DIR points to.\n- * Return 0 if successful, non-zero otherwise.\n- * */\n-int set_worktree_head_symref(const char *gitdir, const char *target,\n-\t\t\t const char *logmsg);\n-\n enum action_on_err {\n \tUPDATE_REFS_MSG_ON_ERR,\n \tUPDATE_REFS_DIE_ON_ERR,\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex a5b405436f..e428d1abff 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3161,50 +3161,6 @@ static int files_create_symref(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-int set_worktree_head_symref(const char *gitdir, const char *target, const char *logmsg)\n-{\n-\t/*\n-\t * FIXME: this obviously will not work well for future refs\n-\t * backends. This function needs to die.\n-\t */\n-\tstruct files_ref_store *refs =\n-\t\tfiles_downcast(get_main_ref_store(),\n-\t\t\t REF_STORE_WRITE,\n-\t\t\t \"set_head_symref\");\n-\n-\tstatic struct lock_file head_lock;\n-\tstruct ref_lock *lock;\n-\tstruct strbuf head_path = STRBUF_INIT;\n-\tconst char *head_rel;\n-\tint ret;\n-\n-\tstrbuf_addf(&head_path, \"%s/HEAD\", absolute_path(gitdir));\n-\tif (hold_lock_file_for_update(&head_lock, head_path.buf,\n-\t\t\t\t LOCK_NO_DEREF) < 0) {\n-\t\tstruct strbuf err = STRBUF_INIT;\n-\t\tunable_to_lock_message(head_path.buf, errno, &err);\n-\t\terror(\"%s\", err.buf);\n-\t\tstrbuf_release(&err);\n-\t\tstrbuf_release(&head_path);\n-\t\treturn -1;\n-\t}\n-\n-\t/* head_rel will be \"HEAD\" for the main tree, \"worktrees/wt/HEAD\" for\n-\t linked trees */\n-\thead_rel = remove_leading_path(head_path.buf,\n-\t\t\t\t absolute_path(get_git_common_dir()));\n-\t/* to make use of create_symref_locked(), initialize ref_lock */\n-\tlock = xcalloc(1, sizeof(struct ref_lock));\n-\tlock->lk = &head_lock;\n-\tlock->ref_name = xstrdup(head_rel);\n-\n-\tret = create_symref_locked(refs, lock, head_rel, target, logmsg);\n-\n-\tunlock_ref(lock); /* will free lock */\n-\tstrbuf_release(&head_path);\n-\treturn ret;\n-}\n-\n static int files_reflog_exists(struct ref_store *ref_store,\n \t\t\t const char *refname)\n {\ndiff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh\nindex 04d1e9d177..5df06f3556 100755\n--- a/t/t1407-worktree-ref-store.sh\n+++ b/t/t1407-worktree-ref-store.sh\n@@ -37,4 +37,16 @@ test_expect_success 'resolve_ref()' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'create_symref(FOO, refs/heads/master)' '\n+\t$RWT create-symref FOO refs/heads/master nothing &&\n+\techo refs/heads/master >expected &&\n+\tgit -C wt symbolic-ref FOO >actual &&\n+\ttest_cmp expected actual &&\n+\n+\t$RMAIN create-symref FOO refs/heads/wt-master nothing &&\n+\techo refs/heads/wt-master >expected &&\n+\tgit symbolic-ref FOO >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"316805","messageId":"xmqqk26nojfm.fsf@gitster.mtv.corp.google.com","threadId":"45084","inReplyTo":"20170404102123.25315-1-pclouds@gmail.com","subject":"Re: [PATCH v4 0/5] Kill manual ref parsing code in worktree.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-14T01:40:45Z","receivedAt":"2017-04-14T01:40:54Z","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> v4 adds a new patch, 2/5, which makes the hashmap related functions in\n> refs.c generic, so I could add a new map for worktree ref stores and\n> not abuse submodule hashmap.\n>\n> I mentioned about moving these hashmap back to submodule.c and\n> worktree.c where it can map more than just ref stores (in worktree\n> case, we can cache 'struct worktree', for example). But I'm not doing\n> it now to keep things small.\n>\n> The commit message in 1/5 is rephrased a bit, hopefully clearer.\n\nMichael, does this look good to replace what has been queued?\n\nI did not spot any discussion on this topic after v4 when I skimmed\nthe backlog messages, so I am hoping that I can pick this up without\nhaving to worry about seeing another reroll immediately after that\n(in which case I'd have been better off if I tended other topics in\nflight in the meantime).\n\nThanks, both.\n\n\n"},{"id":"316806","messageId":"xmqqd1cfoifl.fsf@gitster.mtv.corp.google.com","threadId":"45084","inReplyTo":"xmqqk26nojfm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 0/5] Kill manual ref parsing code in worktree.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-14T02:02:22Z","receivedAt":"2017-04-14T02:02:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> v4 adds a new patch, 2/5, which makes the hashmap related functions in\n>> refs.c generic, so I could add a new map for worktree ref stores and\n>> not abuse submodule hashmap.\n>>\n>> I mentioned about moving these hashmap back to submodule.c and\n>> worktree.c where it can map more than just ref stores (in worktree\n>> case, we can cache 'struct worktree', for example). But I'm not doing\n>> it now to keep things small.\n>>\n>> The commit message in 1/5 is rephrased a bit, hopefully clearer.\n>\n> Michael, does this look good to replace what has been queued?\n>\n> I did not spot any discussion on this topic after v4 when I skimmed\n> the backlog messages, so I am hoping that I can pick this up without\n> having to worry about seeing another reroll immediately after that\n> (in which case I'd have been better off if I tended other topics in\n> flight in the meantime).\n>\n> Thanks, both.\n\nOops, I shouldn't have done that. When applied on top of the\nfiles-backend thing (or have you updated that one and is my tree\nlacking it???), this breaks quite a few tests.\n\nt0001#41 dumps core from \"git worktree add ../linked-worktree\" for\nexample.\n\n\n\n"},{"id":"316812","messageId":"20170414124512.GA20466@ash","threadId":"45084","inReplyTo":"xmqqd1cfoifl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 0/5] Kill manual ref parsing code in worktree.c","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-14T12:45:12Z","receivedAt":"2017-04-14T12:45:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Apr 13, 2017 at 07:02:22PM -0700, Junio C Hamano wrote:\n> Oops, I shouldn't have done that. When applied on top of the\n> files-backend thing (or have you updated that one and is my tree\n> lacking it???), this breaks quite a few tests.\n> \n> t0001#41 dumps core from \"git worktree add ../linked-worktree\" for\n> example.\n\nThis is embarassing. I worked on and off on this series over a long\nperiod of time. I guess at the end I thought everything was ok \"since\nthe last time\" and just sent it away without realizing I hadn't\nactually run the test suite, because it does fail here, now.\n\nThis fixup patch should make the test suite pass. I'm sorry for\nwasting your time on this.\n\n-- 8< --\nFrom 09c6d432490b8e9de99eca09cdbdaebf53bbad2c Mon Sep 17 00:00:00 2001\nFrom: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=\n \nDate: Fri, 14 Apr 2017 19:39:53 +0700\nSubject: [PATCH] fixup! refs: introduce get_worktree_ref_store()\n\n---\n refs.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a4083caf6a..05045a64e1 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1600,11 +1600,13 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n \tunsigned int refs_all_capabilities =\n \t\tREF_STORE_READ | REF_STORE_WRITE |\n \t\tREF_STORE_ODB | REF_STORE_MAIN;\n+\tconst char *id;\n \n \tif (wt->is_current)\n \t\treturn get_main_ref_store();\n \n-\trefs = lookup_ref_store_map(&worktree_ref_stores, wt->id);\n+\tid = wt->id ? wt->id : \"/\";\n+\trefs = lookup_ref_store_map(&worktree_ref_stores, id);\n \tif (refs)\n \t\treturn refs;\n \n@@ -1617,7 +1619,7 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n \n \tif (refs)\n \t\tregister_ref_store_map(&worktree_ref_stores, \"worktree\",\n-\t\t\t\t refs, wt->id);\n+\t\t\t\t refs, id);\n \treturn refs;\n }\n \n-- \n2.11.0.157.gd943d85\n\n-- 8< --\n"},{"id":"316975","messageId":"xmqq37d7lsr0.fsf@gitster.mtv.corp.google.com","threadId":"45084","inReplyTo":"20170414124512.GA20466@ash","subject":"Re: [PATCH v4 0/5] Kill manual ref parsing code in worktree.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-17T01:36:51Z","receivedAt":"2017-04-17T01:37:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> This is embarassing. I worked on and off on this series over a long\n> period of time. I guess at the end I thought everything was ok \"since\n> the last time\" and just sent it away without realizing I hadn't\n> actually run the test suite, because it does fail here, now.\n>\n> This fixup patch should make the test suite pass. I'm sorry for\n> wasting your time on this.\n\nThanks. Will squash it in.\n"},{"id":"317530","messageId":"75cc95f4-d3e9-d483-ce9a-74cc1a356477@alum.mit.edu","threadId":"45084","inReplyTo":"20170404102123.25315-4-pclouds@gmail.com","subject":"Re: [PATCH v4 3/5] refs: introduce get_worktree_ref_store()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-04-22T04:52:34Z","receivedAt":"2017-04-22T04:52:46Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/04/2017 12:21 PM, Nguyễn Thái Ngọc Duy wrote:\n> files-backend at this point is still aware of the per-repo/worktree\n> separation in refs, so it can handle a linked worktree.\n> \n> Some refs operations are known not working when current files-backend is\n> used in a linked worktree (e.g. reflog). Tests will be written when\n> refs_* functions start to be called with worktree backend to verify that\n> they work as expected.\n> \n> Note: accessing a worktree of a submodule remains unaddressed. Perhaps\n> after get_worktrees() can access submodule (or rather a new function\n> get_submodule_worktrees(), that lists worktrees of a submodule), we can\n> update this function to work with submodules as well.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> refs.c | 31 +++++++++++++++++++++++++++++++\n> refs.h | 2 ++\n> 2 files changed, 33 insertions(+)\n> \n> diff --git a/refs.c b/refs.c\n> index 875e30a0b8..a4083caf6a 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -10,6 +10,7 @@\n> #include \"object.h\"\n> #include \"tag.h\"\n> #include \"submodule.h\"\n> +#include \"worktree.h\"\n> \n> /*\n> * List of all available backends\n> @@ -1486,6 +1487,9 @@ static struct ref_store *main_ref_store;\n> /* A hashmap of ref_stores, stored by submodule name: */\n> static struct hashmap submodule_ref_stores;\n> \n> +/* A hashmap of ref_stores, stored by worktree id: */\n> +static struct hashmap worktree_ref_stores;\n> +\n> /*\n> * Look up a ref store by name. If that ref_store hasn't been\n> * registered yet, return NULL.\n> @@ -1590,6 +1594,33 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n> \treturn refs;\n> }\n> \n> +struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n> +{\n> +\tstruct ref_store *refs;\n> +\tunsigned int refs_all_capabilities =\n> +\t\tREF_STORE_READ | REF_STORE_WRITE |\n> +\t\tREF_STORE_ODB | REF_STORE_MAIN;\n\nThis constant appears another place, too. It might make sense to define\na constant `REF_STORE_ALL_CAPABILITIES` in `refs-internal.h` alongside\nthe individual bit values.\n\nIf you prefer not to, please at least declare this variable `const` to\nspare the reader the trouble of looking to see whether it is modified\nbefore it is used.\n\nOtherwise, looks fine to me.\n\n> [...]\n\nMichael\n\n"},{"id":"317531","messageId":"93a5e70b-9c37-6c77-7c42-be253d27f277@alum.mit.edu","threadId":"45084","inReplyTo":"xmqqk26nojfm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 0/5] Kill manual ref parsing code in worktree.c","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-04-22T05:06:25Z","receivedAt":"2017-04-22T05:06:40Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/14/2017 03:40 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n> \n>> v4 adds a new patch, 2/5, which makes the hashmap related functions in\n>> refs.c generic, so I could add a new map for worktree ref stores and\n>> not abuse submodule hashmap.\n>>\n>> I mentioned about moving these hashmap back to submodule.c and\n>> worktree.c where it can map more than just ref stores (in worktree\n>> case, we can cache 'struct worktree', for example). But I'm not doing\n>> it now to keep things small.\n>>\n>> The commit message in 1/5 is rephrased a bit, hopefully clearer.\n> \n> Michael, does this look good to replace what has been queued?\n\nI finally reviewed this patch series. The refs-related changes look\nfine, and the submodule-related changes (which I'm not so familiar with)\nlooks plausible.\n\nIt's a nice cleanup :-)\n\nMichael\n\n"},{"id":"317655","messageId":"20170424100124.24637-2-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170424100124.24637-1-pclouds@gmail.com","subject":"[PATCH v5 1/6] environment.c: fix potential segfault by get_git_common_dir()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-24T10:01:19Z","receivedAt":"2017-04-24T10:01:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"setup_git_env() must be called before this function to initialize\ngit_common_dir so that it returns a non NULL string. And it must return\na non NULL string or segfault can happen because all callers expect so.\n\nIt does not do so explicitly though and depends on get_git_dir() being\ncalled first (which will guarantee setup_git_env()). Avoid this\ndependency and call setup_git_env() by itself.\n\ntest-ref-store.c will hit this problem because it's very lightweight,\njust enough initialization to exercise refs code, and get_git_dir() will\nnever be called until get_worktrees() is, which uses get_git_common_dir\nand hits a segfault.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Junio C Hamano \n---\n environment.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/environment.c b/environment.c\nindex 42dc3106d2..2986ee7200 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -214,6 +214,8 @@ const char *get_git_dir(void)\n \n const char *get_git_common_dir(void)\n {\n+\tif (!git_dir)\n+\t\tsetup_git_env();\n \treturn git_common_dir;\n }\n \n-- \n2.11.0.157.gd943d85\n\n"},{"id":"317656","messageId":"20170424100124.24637-3-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170424100124.24637-1-pclouds@gmail.com","subject":"[PATCH v5 2/6] refs.c: make submodule ref store hashmap generic","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-24T10:01:20Z","receivedAt":"2017-04-24T10:02:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This removes the \"submodule\" from submodule_hash_entry and other\nfunction names. The goal is to reuse the same code and data structure\nfor other ref store types. The first one is worktree ref stores.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Junio C Hamano \n---\n refs.c | 64 ++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 32 insertions(+), 32 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex bad05ba861..96eba92475 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1450,32 +1450,32 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n \treturn 0;\n }\n \n-struct submodule_hash_entry\n+struct ref_store_hash_entry\n {\n \tstruct hashmap_entry ent; /* must be the first member! */\n \n \tstruct ref_store *refs;\n \n-\t/* NUL-terminated name of submodule: */\n-\tchar submodule[FLEX_ARRAY];\n+\t/* NUL-terminated identifier of the ref store: */\n+\tchar name[FLEX_ARRAY];\n };\n \n-static int submodule_hash_cmp(const void *entry, const void *entry_or_key,\n+static int ref_store_hash_cmp(const void *entry, const void *entry_or_key,\n \t\t\t const void *keydata)\n {\n-\tconst struct submodule_hash_entry *e1 = entry, *e2 = entry_or_key;\n-\tconst char *submodule = keydata ? keydata : e2->submodule;\n+\tconst struct ref_store_hash_entry *e1 = entry, *e2 = entry_or_key;\n+\tconst char *name = keydata ? keydata : e2->name;\n \n-\treturn strcmp(e1->submodule, submodule);\n+\treturn strcmp(e1->name, name);\n }\n \n-static struct submodule_hash_entry *alloc_submodule_hash_entry(\n-\t\tconst char *submodule, struct ref_store *refs)\n+static struct ref_store_hash_entry *alloc_ref_store_hash_entry(\n+\t\tconst char *name, struct ref_store *refs)\n {\n-\tstruct submodule_hash_entry *entry;\n+\tstruct ref_store_hash_entry *entry;\n \n-\tFLEX_ALLOC_STR(entry, submodule, submodule);\n-\thashmap_entry_init(entry, strhash(submodule));\n+\tFLEX_ALLOC_STR(entry, name, name);\n+\thashmap_entry_init(entry, strhash(name));\n \tentry->refs = refs;\n \treturn entry;\n }\n@@ -1487,19 +1487,19 @@ static struct ref_store *main_ref_store;\n static struct hashmap submodule_ref_stores;\n \n /*\n- * Return the ref_store instance for the specified submodule. If that\n- * ref_store hasn't been initialized yet, return NULL.\n+ * Look up a ref store by name. If that ref_store hasn't been\n+ * registered yet, return NULL.\n */\n-static struct ref_store *lookup_submodule_ref_store(const char *submodule)\n+static struct ref_store *lookup_ref_store_map(struct hashmap *map,\n+\t\t\t\t\t const char *name)\n {\n-\tstruct submodule_hash_entry *entry;\n+\tstruct ref_store_hash_entry *entry;\n \n-\tif (!submodule_ref_stores.tablesize)\n+\tif (!map->tablesize)\n \t\t/* It's initialized on demand in register_ref_store(). */\n \t\treturn NULL;\n \n-\tentry = hashmap_get_from_hash(&submodule_ref_stores,\n-\t\t\t\t strhash(submodule), submodule);\n+\tentry = hashmap_get_from_hash(map, strhash(name), name);\n \treturn entry ? entry->refs : NULL;\n }\n \n@@ -1535,20 +1535,19 @@ struct ref_store *get_main_ref_store(void)\n }\n \n /*\n- * Register the specified ref_store to be the one that should be used\n- * for submodule. It is a fatal error to call this function twice for\n- * the same submodule.\n+ * Associate a ref store with a name. It is a fatal error to call this\n+ * function twice for the same name.\n */\n-static void register_submodule_ref_store(struct ref_store *refs,\n-\t\t\t\t\t const char *submodule)\n+static void register_ref_store_map(struct hashmap *map,\n+\t\t\t\t const char *type,\n+\t\t\t\t struct ref_store *refs,\n+\t\t\t\t const char *name)\n {\n-\tif (!submodule_ref_stores.tablesize)\n-\t\thashmap_init(&submodule_ref_stores, submodule_hash_cmp, 0);\n+\tif (!map->tablesize)\n+\t\thashmap_init(map, ref_store_hash_cmp, 0);\n \n-\tif (hashmap_put(&submodule_ref_stores,\n-\t\t\talloc_submodule_hash_entry(submodule, refs)))\n-\t\tdie(\"BUG: ref_store for submodule '%s' initialized twice\",\n-\t\t submodule);\n+\tif (hashmap_put(map, alloc_ref_store_hash_entry(name, refs)))\n+\t\tdie(\"BUG: %s ref_store '%s' initialized twice\", type, name);\n }\n \n struct ref_store *get_submodule_ref_store(const char *submodule)\n@@ -1565,7 +1564,7 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \t\treturn get_main_ref_store();\n \t}\n \n-\trefs = lookup_submodule_ref_store(submodule);\n+\trefs = lookup_ref_store_map(&submodule_ref_stores, submodule);\n \tif (refs)\n \t\treturn refs;\n \n@@ -1584,7 +1583,8 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \t/* assume that add_submodule_odb() has been called */\n \trefs = ref_store_init(submodule_sb.buf,\n \t\t\t REF_STORE_READ | REF_STORE_ODB);\n-\tregister_submodule_ref_store(refs, submodule);\n+\tregister_ref_store_map(&submodule_ref_stores, \"submodule\",\n+\t\t\t refs, submodule);\n \n \tstrbuf_release(&submodule_sb);\n \treturn refs;\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"317657","messageId":"20170424100124.24637-5-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170424100124.24637-1-pclouds@gmail.com","subject":"[PATCH v5 4/6] refs: introduce get_worktree_ref_store()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-24T10:01:22Z","receivedAt":"2017-04-24T10:02:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"files-backend at this point is still aware of the per-repo/worktree\nseparation in refs, so it can handle a linked worktree.\n\nSome refs operations are known not working when current files-backend is\nused in a linked worktree (e.g. reflog). Tests will be written when\nrefs_* functions start to be called with worktree backend to verify that\nthey work as expected.\n\nNote: accessing a worktree of a submodule remains unaddressed. Perhaps\nafter get_worktrees() can access submodule (or rather a new function\nget_submodule_worktrees(), that lists worktrees of a submodule), we can\nupdate this function to work with submodules as well.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 30 ++++++++++++++++++++++++++++++\n refs.h | 2 ++\n 2 files changed, 32 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 6e6f0ec5be..7972720256 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -10,6 +10,7 @@\n #include \"object.h\"\n #include \"tag.h\"\n #include \"submodule.h\"\n+#include \"worktree.h\"\n \n /*\n * List of all available backends\n@@ -1486,6 +1487,9 @@ static struct ref_store *main_ref_store;\n /* A hashmap of ref_stores, stored by submodule name: */\n static struct hashmap submodule_ref_stores;\n \n+/* A hashmap of ref_stores, stored by worktree id: */\n+static struct hashmap worktree_ref_stores;\n+\n /*\n * Look up a ref store by name. If that ref_store hasn't been\n * registered yet, return NULL.\n@@ -1586,6 +1590,32 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n \treturn refs;\n }\n \n+struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n+{\n+\tstruct ref_store *refs;\n+\tconst char *id;\n+\n+\tif (wt->is_current)\n+\t\treturn get_main_ref_store();\n+\n+\tid = wt->id ? wt->id : \"/\";\n+\trefs = lookup_ref_store_map(&worktree_ref_stores, id);\n+\tif (refs)\n+\t\treturn refs;\n+\n+\tif (wt->id)\n+\t\trefs = ref_store_init(git_common_path(\"worktrees/%s\", wt->id),\n+\t\t\t\t REF_STORE_ALL_CAPS);\n+\telse\n+\t\trefs = ref_store_init(get_git_common_dir(),\n+\t\t\t\t REF_STORE_ALL_CAPS);\n+\n+\tif (refs)\n+\t\tregister_ref_store_map(&worktree_ref_stores, \"worktree\",\n+\t\t\t\t refs, id);\n+\treturn refs;\n+}\n+\n void base_ref_store_init(struct ref_store *refs,\n \t\t\t const struct ref_storage_be *be)\n {\ndiff --git a/refs.h b/refs.h\nindex 49e97d7d5f..6df69a2adb 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -5,6 +5,7 @@ struct object_id;\n struct ref_store;\n struct strbuf;\n struct string_list;\n+struct worktree;\n \n /*\n * Resolve a reference, recursively following symbolic refererences.\n@@ -655,5 +656,6 @@ struct ref_store *get_main_ref_store(void);\n * submodule==NULL.\n */\n struct ref_store *get_submodule_ref_store(const char *submodule);\n+struct ref_store *get_worktree_ref_store(const struct worktree *wt);\n \n #endif /* REFS_H */\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"317658","messageId":"20170424100124.24637-6-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170424100124.24637-1-pclouds@gmail.com","subject":"[PATCH v5 5/6] worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-24T10:01:23Z","receivedAt":"2017-04-24T10:02:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The manual parsing code is replaced with a call to refs_resolve_ref_unsafe().\nThe manual parsing code must die because only refs/files-backend.c\nshould do that.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Junio C Hamano \n---\n branch.c | 3 +-\n t/helper/test-ref-store.c | 18 ++++++\n t/t1407-worktree-ref-store.sh (new +x) | 40 +++++++++++++\n worktree.c | 102 +++++++++------------------------\n worktree.h | 2 +-\n 5 files changed, 88 insertions(+), 77 deletions(-)\n create mode 100755 t/t1407-worktree-ref-store.sh\n\ndiff --git a/branch.c b/branch.c\nindex 5c12036b02..0b949b7fb2 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -355,7 +355,8 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n \tfor (i = 0; worktrees[i]; i++) {\n \t\tif (worktrees[i]->is_detached)\n \t\t\tcontinue;\n-\t\tif (strcmp(oldref, worktrees[i]->head_ref))\n+\t\tif (worktrees[i]->head_ref &&\n+\t\t strcmp(oldref, worktrees[i]->head_ref))\n \t\t\tcontinue;\n \n \t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\ndiff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c\nindex 2d84c45ffe..4a487c014e 100644\n--- a/t/helper/test-ref-store.c\n+++ b/t/helper/test-ref-store.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"refs.h\"\n+#include \"worktree.h\"\n \n static const char *notnull(const char *arg, const char *name)\n {\n@@ -32,6 +33,23 @@ static const char **get_store(const char **argv, struct ref_store **refs)\n \t\tstrbuf_release(&sb);\n \n \t\t*refs = get_submodule_ref_store(gitdir);\n+\t} else if (skip_prefix(argv[0], \"worktree:\", &gitdir)) {\n+\t\tstruct worktree **p, **worktrees = get_worktrees(0);\n+\n+\t\tfor (p = worktrees; *p; p++) {\n+\t\t\tstruct worktree *wt = *p;\n+\n+\t\t\tif (!wt->id) {\n+\t\t\t\t/* special case for main worktree */\n+\t\t\t\tif (!strcmp(gitdir, \"main\"))\n+\t\t\t\t\tbreak;\n+\t\t\t} else if (!strcmp(gitdir, wt->id))\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif (!*p)\n+\t\t\tdie(\"no such worktree: %s\", gitdir);\n+\n+\t\t*refs = get_worktree_ref_store(*p);\n \t} else\n \t\tdie(\"unknown backend %s\", argv[0]);\n \ndiff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh\nnew file mode 100755\nindex 0000000000..04d1e9d177\n--- /dev/null\n+++ b/t/t1407-worktree-ref-store.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description='test worktree ref store api'\n+\n+. ./test-lib.sh\n+\n+RWT=\"test-ref-store worktree:wt\"\n+RMAIN=\"test-ref-store worktree:main\"\n+\n+test_expect_success 'setup' '\n+\ttest_commit first &&\n+\tgit worktree add -b wt-master wt &&\n+\t(\n+\t\tcd wt &&\n+\t\ttest_commit second\n+\t)\n+'\n+\n+test_expect_success 'resolve_ref()' '\n+\tSHA1=`git rev-parse master` &&\n+\techo \"$SHA1 refs/heads/master 0x0\" >expected &&\n+\t$RWT resolve-ref refs/heads/master 0 >actual &&\n+\ttest_cmp expected actual &&\n+\t$RMAIN resolve-ref refs/heads/master 0 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'resolve_ref()' '\n+\tSHA1=`git -C wt rev-parse HEAD` &&\n+\techo \"$SHA1 refs/heads/wt-master 0x1\" >expected &&\n+\t$RWT resolve-ref HEAD 0 >actual &&\n+\ttest_cmp expected actual &&\n+\n+\tSHA1=`git rev-parse HEAD` &&\n+\techo \"$SHA1 refs/heads/master 0x1\" >expected &&\n+\t$RMAIN resolve-ref HEAD 0 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\ndiff --git a/worktree.c b/worktree.c\nindex fa7bc67a50..c1ec334b06 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -19,54 +19,25 @@ void free_worktrees(struct worktree **worktrees)\n \tfree (worktrees);\n }\n \n-/*\n- * read 'path_to_ref' into 'ref'. Also if is_detached is not NULL,\n- * set is_detached to 1 (0) if the ref is detached (is not detached).\n- *\n- * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside $GIT_DIR so\n- * for linked worktrees, `resolve_ref_unsafe()` won't work (it uses\n- * git_path). Parse the ref ourselves.\n- *\n- * return -1 if the ref is not a proper ref, 0 otherwise (success)\n- */\n-static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)\n-{\n-\tif (is_detached)\n-\t\t*is_detached = 0;\n-\tif (!strbuf_readlink(ref, path_to_ref, 0)) {\n-\t\t/* HEAD is symbolic link */\n-\t\tif (!starts_with(ref->buf, \"refs/\") ||\n-\t\t\t\tcheck_refname_format(ref->buf, 0))\n-\t\t\treturn -1;\n-\t} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {\n-\t\t/* textual symref or detached */\n-\t\tif (!starts_with(ref->buf, \"ref:\")) {\n-\t\t\tif (is_detached)\n-\t\t\t\t*is_detached = 1;\n-\t\t} else {\n-\t\t\tstrbuf_remove(ref, 0, strlen(\"ref:\"));\n-\t\t\tstrbuf_trim(ref);\n-\t\t\tif (check_refname_format(ref->buf, 0))\n-\t\t\t\treturn -1;\n-\t\t}\n-\t} else\n-\t\treturn -1;\n-\treturn 0;\n-}\n-\n /**\n- * Add the head_sha1 and head_ref (if not detached) to the given worktree\n+ * Update head_sha1, head_ref and is_detached of the given worktree\n */\n-static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)\n+static void add_head_info(struct worktree *wt)\n {\n-\tif (head_ref->len) {\n-\t\tif (worktree->is_detached) {\n-\t\t\tget_sha1_hex(head_ref->buf, worktree->head_sha1);\n-\t\t} else {\n-\t\t\tresolve_ref_unsafe(head_ref->buf, 0, worktree->head_sha1, NULL);\n-\t\t\tworktree->head_ref = strbuf_detach(head_ref, NULL);\n-\t\t}\n-\t}\n+\tint flags;\n+\tconst char *target;\n+\n+\ttarget = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n+\t\t\t\t\t \"HEAD\",\n+\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t wt->head_sha1, &flags);\n+\tif (!target)\n+\t\treturn;\n+\n+\tif (flags & REF_ISSYMREF)\n+\t\twt->head_ref = xstrdup(target);\n+\telse\n+\t\twt->is_detached = 1;\n }\n \n /**\n@@ -77,9 +48,7 @@ static struct worktree *get_main_worktree(void)\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n-\tstruct strbuf head_ref = STRBUF_INIT;\n \tint is_bare = 0;\n-\tint is_detached = 0;\n \n \tstrbuf_add_absolute_path(&worktree_path, get_git_common_dir());\n \tis_bare = !strbuf_strip_suffix(&worktree_path, \"/.git\");\n@@ -91,13 +60,10 @@ static struct worktree *get_main_worktree(void)\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->is_bare = is_bare;\n-\tworktree->is_detached = is_detached;\n-\tif (!parse_ref(path.buf, &head_ref, &is_detached))\n-\t\tadd_head_info(&head_ref, worktree);\n+\tadd_head_info(worktree);\n \n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n-\tstrbuf_release(&head_ref);\n \treturn worktree;\n }\n \n@@ -106,8 +72,6 @@ static struct worktree *get_linked_worktree(const char *id)\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n-\tstruct strbuf head_ref = STRBUF_INIT;\n-\tint is_detached = 0;\n \n \tif (!id)\n \t\tdie(\"Missing linked worktree name\");\n@@ -127,19 +91,14 @@ static struct worktree *get_linked_worktree(const char *id)\n \tstrbuf_reset(&path);\n \tstrbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n \n-\tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n-\t\tgoto done;\n-\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->id = xstrdup(id);\n-\tworktree->is_detached = is_detached;\n-\tadd_head_info(&head_ref, worktree);\n+\tadd_head_info(worktree);\n \n done:\n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n-\tstrbuf_release(&head_ref);\n \treturn worktree;\n }\n \n@@ -334,8 +293,6 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\t\t\t\t const char *target)\n {\n \tconst struct worktree *existing = NULL;\n-\tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf sb = STRBUF_INIT;\n \tstatic struct worktree **worktrees;\n \tint i = 0;\n \n@@ -345,6 +302,11 @@ const struct worktree *find_shared_symref(const char *symref,\n \n \tfor (i = 0; worktrees[i]; i++) {\n \t\tstruct worktree *wt = worktrees[i];\n+\t\tconst char *symref_target;\n+\t\tunsigned char sha1[20];\n+\t\tstruct ref_store *refs;\n+\t\tint flags;\n+\n \t\tif (wt->is_bare)\n \t\t\tcontinue;\n \n@@ -359,25 +321,15 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\t\t}\n \t\t}\n \n-\t\tstrbuf_reset(&path);\n-\t\tstrbuf_reset(&sb);\n-\t\tstrbuf_addf(&path, \"%s/%s\",\n-\t\t\t get_worktree_git_dir(wt),\n-\t\t\t symref);\n-\n-\t\tif (parse_ref(path.buf, &sb, NULL)) {\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\tif (!strcmp(sb.buf, target)) {\n+\t\trefs = get_worktree_ref_store(wt);\n+\t\tsymref_target = refs_resolve_ref_unsafe(refs, symref, 0,\n+\t\t\t\t\t\t\tsha1, &flags);\n+\t\tif ((flags & REF_ISSYMREF) && !strcmp(symref_target, target)) {\n \t\t\texisting = wt;\n \t\t\tbreak;\n \t\t}\n \t}\n \n-\tstrbuf_release(&path);\n-\tstrbuf_release(&sb);\n-\n \treturn existing;\n }\n \ndiff --git a/worktree.h b/worktree.h\nindex 6bfb985203..5ea5e503fb 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -4,7 +4,7 @@\n struct worktree {\n \tchar *path;\n \tchar *id;\n-\tchar *head_ref;\n+\tchar *head_ref;\t\t/* NULL if HEAD is broken or detached */\n \tchar *lock_reason;\t/* internal use */\n \tunsigned char head_sha1[20];\n \tint is_detached;\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"317659","messageId":"20170424100124.24637-4-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170424100124.24637-1-pclouds@gmail.com","subject":"[PATCH v5 3/6] refs: add REFS_STORE_ALL_CAPS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-24T10:01:21Z","receivedAt":"2017-04-24T10:02:27Z","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/refs-internal.h | 4 ++++\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 96eba92475..6e6f0ec5be 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1526,11 +1526,7 @@ struct ref_store *get_main_ref_store(void)\n \tif (main_ref_store)\n \t\treturn main_ref_store;\n \n-\tmain_ref_store = ref_store_init(get_git_dir(),\n-\t\t\t\t\t(REF_STORE_READ |\n-\t\t\t\t\t REF_STORE_WRITE |\n-\t\t\t\t\t REF_STORE_ODB |\n-\t\t\t\t\t REF_STORE_MAIN));\n+\tmain_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);\n \treturn main_ref_store;\n }\n \ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 690498698e..b26f7e41ce 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -467,6 +467,10 @@ struct ref_store;\n #define REF_STORE_WRITE\t\t(1 << 1) /* can perform update operations */\n #define REF_STORE_ODB\t\t(1 << 2) /* has access to object database */\n #define REF_STORE_MAIN\t\t(1 << 3)\n+#define REF_STORE_ALL_CAPS\t(REF_STORE_READ | \\\n+\t\t\t\t REF_STORE_WRITE | \\\n+\t\t\t\t REF_STORE_ODB | \\\n+\t\t\t\t REF_STORE_MAIN)\n \n /*\n * Initialize the ref_store for the specified gitdir. These functions\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"317660","messageId":"20170424100124.24637-7-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170424100124.24637-1-pclouds@gmail.com","subject":"[PATCH v5 6/6] refs: kill set_worktree_head_symref()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-24T10:01:24Z","receivedAt":"2017-04-24T10:02:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"70999e9cec (branch -m: update all per-worktree HEADs - 2016-03-27)\nadded this function in order to update HEADs of all relevant\nworktrees, when a branch is renamed.\n\nIt, as a public ref api, kind of breaks abstraction when it uses\ninternal functions of files backend. With the introduction of\nrefs_create_symref(), we can move back pretty close to the code before\n70999e9cec, where create_symref() was used for updating HEAD.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Junio C Hamano \n---\n branch.c | 12 ++++++------\n refs.h | 10 ----------\n refs/files-backend.c | 44 -------------------------------------------\n t/t1407-worktree-ref-store.sh | 12 ++++++++++++\n 4 files changed, 18 insertions(+), 60 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 0b949b7fb2..69d5eea84b 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -353,18 +353,18 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n \tint i;\n \n \tfor (i = 0; worktrees[i]; i++) {\n+\t\tstruct ref_store *refs;\n+\n \t\tif (worktrees[i]->is_detached)\n \t\t\tcontinue;\n \t\tif (worktrees[i]->head_ref &&\n \t\t strcmp(oldref, worktrees[i]->head_ref))\n \t\t\tcontinue;\n \n-\t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\n-\t\t\t\t\t newref, logmsg)) {\n-\t\t\tret = -1;\n-\t\t\terror(_(\"HEAD of working tree %s is not updated\"),\n-\t\t\t worktrees[i]->path);\n-\t\t}\n+\t\trefs = get_worktree_ref_store(worktrees[i]);\n+\t\tif (refs_create_symref(refs, \"HEAD\", newref, logmsg))\n+\t\t\tret = error(_(\"HEAD of working tree %s is not updated\"),\n+\t\t\t\t worktrees[i]->path);\n \t}\n \n \tfree_worktrees(worktrees);\ndiff --git a/refs.h b/refs.h\nindex 6df69a2adb..447381d378 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -402,16 +402,6 @@ int refs_create_symref(struct ref_store *refs, const char *refname,\n \t\t const char *target, const char *logmsg);\n int create_symref(const char *refname, const char *target, const char *logmsg);\n \n-/*\n- * Update HEAD of the specified gitdir.\n- * Similar to create_symref(\"relative-git-dir/HEAD\", target, NULL), but\n- * this can update the main working tree's HEAD regardless of where\n- * $GIT_DIR points to.\n- * Return 0 if successful, non-zero otherwise.\n- * */\n-int set_worktree_head_symref(const char *gitdir, const char *target,\n-\t\t\t const char *logmsg);\n-\n enum action_on_err {\n \tUPDATE_REFS_MSG_ON_ERR,\n \tUPDATE_REFS_DIE_ON_ERR,\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 4d705b4037..4149943a6e 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3160,50 +3160,6 @@ static int files_create_symref(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-int set_worktree_head_symref(const char *gitdir, const char *target, const char *logmsg)\n-{\n-\t/*\n-\t * FIXME: this obviously will not work well for future refs\n-\t * backends. This function needs to die.\n-\t */\n-\tstruct files_ref_store *refs =\n-\t\tfiles_downcast(get_main_ref_store(),\n-\t\t\t REF_STORE_WRITE,\n-\t\t\t \"set_head_symref\");\n-\n-\tstatic struct lock_file head_lock;\n-\tstruct ref_lock *lock;\n-\tstruct strbuf head_path = STRBUF_INIT;\n-\tconst char *head_rel;\n-\tint ret;\n-\n-\tstrbuf_addf(&head_path, \"%s/HEAD\", absolute_path(gitdir));\n-\tif (hold_lock_file_for_update(&head_lock, head_path.buf,\n-\t\t\t\t LOCK_NO_DEREF) < 0) {\n-\t\tstruct strbuf err = STRBUF_INIT;\n-\t\tunable_to_lock_message(head_path.buf, errno, &err);\n-\t\terror(\"%s\", err.buf);\n-\t\tstrbuf_release(&err);\n-\t\tstrbuf_release(&head_path);\n-\t\treturn -1;\n-\t}\n-\n-\t/* head_rel will be \"HEAD\" for the main tree, \"worktrees/wt/HEAD\" for\n-\t linked trees */\n-\thead_rel = remove_leading_path(head_path.buf,\n-\t\t\t\t absolute_path(get_git_common_dir()));\n-\t/* to make use of create_symref_locked(), initialize ref_lock */\n-\tlock = xcalloc(1, sizeof(struct ref_lock));\n-\tlock->lk = &head_lock;\n-\tlock->ref_name = xstrdup(head_rel);\n-\n-\tret = create_symref_locked(refs, lock, head_rel, target, logmsg);\n-\n-\tunlock_ref(lock); /* will free lock */\n-\tstrbuf_release(&head_path);\n-\treturn ret;\n-}\n-\n static int files_reflog_exists(struct ref_store *ref_store,\n \t\t\t const char *refname)\n {\ndiff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh\nindex 04d1e9d177..5df06f3556 100755\n--- a/t/t1407-worktree-ref-store.sh\n+++ b/t/t1407-worktree-ref-store.sh\n@@ -37,4 +37,16 @@ test_expect_success 'resolve_ref()' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'create_symref(FOO, refs/heads/master)' '\n+\t$RWT create-symref FOO refs/heads/master nothing &&\n+\techo refs/heads/master >expected &&\n+\tgit -C wt symbolic-ref FOO >actual &&\n+\ttest_cmp expected actual &&\n+\n+\t$RMAIN create-symref FOO refs/heads/wt-master nothing &&\n+\techo refs/heads/wt-master >expected &&\n+\tgit symbolic-ref FOO >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"317661","messageId":"20170424100124.24637-1-pclouds@gmail.com","threadId":"45084","inReplyTo":"20170404102123.25315-1-pclouds@gmail.com","subject":"[PATCH v5 0/6] Kill manual ref parsing code in worktree.c","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-24T10:01:18Z","receivedAt":"2017-04-24T10:03:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"v5 chanes are mostly cosmetic:\n\ndiff --git a/refs.c b/refs.c\nindex 5d31fb6bcf..7972720256 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1530,11 +1530,7 @@ struct ref_store *get_main_ref_store(void)\n \tif (main_ref_store)\n \t\treturn main_ref_store;\n \n-\tmain_ref_store = ref_store_init(get_git_dir(),\n-\t\t\t\t\t(REF_STORE_READ |\n-\t\t\t\t\t REF_STORE_WRITE |\n-\t\t\t\t\t REF_STORE_ODB |\n-\t\t\t\t\t REF_STORE_MAIN));\n+\tmain_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);\n \treturn main_ref_store;\n }\n \n@@ -1597,9 +1593,6 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n {\n \tstruct ref_store *refs;\n-\tunsigned int refs_all_capabilities =\n-\t\tREF_STORE_READ | REF_STORE_WRITE |\n-\t\tREF_STORE_ODB | REF_STORE_MAIN;\n \tconst char *id;\n \n \tif (wt->is_current)\n@@ -1612,10 +1605,10 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n \n \tif (wt->id)\n \t\trefs = ref_store_init(git_common_path(\"worktrees/%s\", wt->id),\n-\t\t\t\t refs_all_capabilities);\n+\t\t\t\t REF_STORE_ALL_CAPS);\n \telse\n \t\trefs = ref_store_init(get_git_common_dir(),\n-\t\t\t\t refs_all_capabilities);\n+\t\t\t\t REF_STORE_ALL_CAPS);\n \n \tif (refs)\n \t\tregister_ref_store_map(&worktree_ref_stores, \"worktree\",\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 690498698e..b26f7e41ce 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -467,6 +467,10 @@ struct ref_store;\n #define REF_STORE_WRITE\t\t(1 << 1) /* can perform update operations */\n #define REF_STORE_ODB\t\t(1 << 2) /* has access to object database */\n #define REF_STORE_MAIN\t\t(1 << 3)\n+#define REF_STORE_ALL_CAPS\t(REF_STORE_READ | \\\n+\t\t\t\t REF_STORE_WRITE | \\\n+\t\t\t\t REF_STORE_ODB | \\\n+\t\t\t\t REF_STORE_MAIN)\n \n /*\n * Initialize the ref_store for the specified gitdir. These functions\n\nNguyễn Thái Ngọc Duy (6):\n environment.c: fix potential segfault by get_git_common_dir()\n refs.c: make submodule ref store hashmap generic\n refs: add REFS_STORE_ALL_CAPS\n refs: introduce get_worktree_ref_store()\n worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()\n refs: kill set_worktree_head_symref()\n\n branch.c | 15 ++---\n environment.c | 2 +\n refs.c | 100 ++++++++++++++++++++------------\n refs.h | 12 +---\n refs/files-backend.c | 44 --------------\n refs/refs-internal.h | 4 ++\n t/helper/test-ref-store.c | 18 ++++++\n t/t1407-worktree-ref-store.sh (new +x) | 52 +++++++++++++++++\n worktree.c | 102 +++++++++------------------------\n worktree.h | 2 +-\n 10 files changed, 177 insertions(+), 174 deletions(-)\n create mode 100755 t/t1407-worktree-ref-store.sh\n\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"317770","messageId":"xmqqbmrlw1lz.fsf@gitster.mtv.corp.google.com","threadId":"45084","inReplyTo":"20170424100124.24637-1-pclouds@gmail.com","subject":"Re: [PATCH v5 0/6] Kill manual ref parsing code in worktree.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-25T04:30:16Z","receivedAt":"2017-04-25T04:30:24Z","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> v5 chanes are mostly cosmetic:\n\nGood changes. Will re-queue.\n\nThanks.\n\n>\n> diff --git a/refs.c b/refs.c\n> index 5d31fb6bcf..7972720256 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1530,11 +1530,7 @@ struct ref_store *get_main_ref_store(void)\n> \tif (main_ref_store)\n> \t\treturn main_ref_store;\n> \n> -\tmain_ref_store = ref_store_init(get_git_dir(),\n> -\t\t\t\t\t(REF_STORE_READ |\n> -\t\t\t\t\t REF_STORE_WRITE |\n> -\t\t\t\t\t REF_STORE_ODB |\n> -\t\t\t\t\t REF_STORE_MAIN));\n> +\tmain_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);\n> \treturn main_ref_store;\n> }\n> \n> @@ -1597,9 +1593,6 @@ struct ref_store *get_submodule_ref_store(const char *submodule)\n> struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n> {\n> \tstruct ref_store *refs;\n> -\tunsigned int refs_all_capabilities =\n> -\t\tREF_STORE_READ | REF_STORE_WRITE |\n> -\t\tREF_STORE_ODB | REF_STORE_MAIN;\n> \tconst char *id;\n> \n> \tif (wt->is_current)\n> @@ -1612,10 +1605,10 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n> \n> \tif (wt->id)\n> \t\trefs = ref_store_init(git_common_path(\"worktrees/%s\", wt->id),\n> -\t\t\t\t refs_all_capabilities);\n> +\t\t\t\t REF_STORE_ALL_CAPS);\n> \telse\n> \t\trefs = ref_store_init(get_git_common_dir(),\n> -\t\t\t\t refs_all_capabilities);\n> +\t\t\t\t REF_STORE_ALL_CAPS);\n> \n> \tif (refs)\n> \t\tregister_ref_store_map(&worktree_ref_stores, \"worktree\",\n> diff --git a/refs/refs-internal.h b/refs/refs-internal.h\n> index 690498698e..b26f7e41ce 100644\n> --- a/refs/refs-internal.h\n> +++ b/refs/refs-internal.h\n> @@ -467,6 +467,10 @@ struct ref_store;\n> #define REF_STORE_WRITE\t\t(1 << 1) /* can perform update operations */\n> #define REF_STORE_ODB\t\t(1 << 2) /* has access to object database */\n> #define REF_STORE_MAIN\t\t(1 << 3)\n> +#define REF_STORE_ALL_CAPS\t(REF_STORE_READ | \\\n> +\t\t\t\t REF_STORE_WRITE | \\\n> +\t\t\t\t REF_STORE_ODB | \\\n> +\t\t\t\t REF_STORE_MAIN)\n> \n> /*\n> * Initialize the ref_store for the specified gitdir. These functions\n>\n> Nguyễn Thái Ngọc Duy (6):\n> environment.c: fix potential segfault by get_git_common_dir()\n> refs.c: make submodule ref store hashmap generic\n> refs: add REFS_STORE_ALL_CAPS\n> refs: introduce get_worktree_ref_store()\n> worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()\n> refs: kill set_worktree_head_symref()\n>\n> branch.c | 15 ++---\n> environment.c | 2 +\n> refs.c | 100 ++++++++++++++++++++------------\n> refs.h | 12 +---\n> refs/files-backend.c | 44 --------------\n> refs/refs-internal.h | 4 ++\n> t/helper/test-ref-store.c | 18 ++++++\n> t/t1407-worktree-ref-store.sh (new +x) | 52 +++++++++++++++++\n> worktree.c | 102 +++++++++------------------------\n> worktree.h | 2 +-\n> 10 files changed, 177 insertions(+), 174 deletions(-)\n> create mode 100755 t/t1407-worktree-ref-store.sh\n"}]}