{"thread":{"id":"39833","subject":"[PATCH 13/16] worktree: make setup of new HEAD distinct from worktree population","startedAt":"2015-07-11T00:05:30Z","lastAt":"2015-07-15T09:59:33Z","messageCount":31,"participants":["Eric Sunshine","Duy Nguyen","Junio C Hamano","Michael J Gruber"],"isPatch":true,"patchVersion":1,"patchTotal":16},"messages":[{"id":"266023","messageId":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":null,"subject":"[PATCH 00/16] worktree: use \"git reset --hard\" to populate worktree","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:30Z","receivedAt":"2015-07-11T00:05:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This is a follow-on series to [1], which migrated \"git checkout --to\"\nfunctionality to \"git worktree add\". That series continued using \"git\ncheckout\" for the initial population of the new worktree, which required\ngit-checkout to have too intimate knowledge that it was operating in a\nnewly created worktree.\n\nThis series eliminates git-checkout from the picture by instead\nemploying \"git reset --hard\"[2] to populate the new worktree initially.\n\nIt is built atop 1eb07d8 (worktree: add: auto-vivify new branch when\n is omitted, 2015-07-06), currently in 'next', which is\nes/worktree-add except for the final patch (which retires\n--ignore-other-worktrees) since the intention[3] was to drop that patch.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/273415\n[2]: http://thread.gmane.org/gmane.comp.version-control.git/273415/focus=273454\n[3]: http://thread.gmane.org/gmane.comp.version-control.git/273415/focus=273585\n\nEric Sunshine (16):\n checkout: avoid resolving HEAD unnecessarily\n checkout: name check_linked_checkouts() more meaningfully\n checkout: improve die_if_checked_out() robustness\n checkout: die_if_checked_out: simplify strbuf management\n checkout: generalize die_if_checked_out() branch name argument\n branch: publish die_if_checked_out()\n worktree: simplify new branch (-b/-B) option checking\n worktree: introduce options container\n worktree: make --detach mutually exclusive with -b/-B\n worktree: make branch creation distinct from worktree population\n worktree: add_worktree: construct worktree-population command locally\n worktree: detect branch symref/detach and error conditions locally\n worktree: make setup of new HEAD distinct from worktree population\n worktree: avoid resolving HEAD unnecessarily\n worktree: populate via \"git reset --hard\" rather than \"git checkout\"\n checkout: drop intimate knowledge of new worktree initial population\n\n branch.c | 65 +++++++++++++++++++++++++++++++\n branch.h | 7 ++++\n builtin/checkout.c | 82 +++------------------------------------\n builtin/worktree.c | 110 +++++++++++++++++++++++++++++++++++------------------\n 4 files changed, 151 insertions(+), 113 deletions(-)\n\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266028","messageId":"1436573146-3893-2-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 01/16] checkout: avoid resolving HEAD unnecessarily","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:31Z","receivedAt":"2015-07-11T00:05:31Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"When --ignore-other-worktree is specified, we unconditionally skip the\ncheck to see if the requested branch is already checked out in a linked\nworktree. Since we know that we will be skipping that check, there is no\nneed to resolve HEAD in order to detect other conditions under which we\nmay skip the check.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/checkout.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 5754554..75f90a9 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1145,13 +1145,13 @@ static int checkout_branch(struct checkout_opts *opts,\n \t\tdie(_(\"Cannot switch branch to a non-commit '%s'\"),\n \t\t new->name);\n \n-\tif (new->path && !opts->force_detach && !opts->new_branch) {\n+\tif (new->path && !opts->force_detach && !opts->new_branch &&\n+\t !opts->ignore_other_worktrees) {\n \t\tunsigned char sha1[20];\n \t\tint flag;\n \t\tchar *head_ref = resolve_refdup(\"HEAD\", 0, sha1, &flag);\n \t\tif (head_ref &&\n-\t\t (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) &&\n-\t\t !opts->ignore_other_worktrees)\n+\t\t (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))\n \t\t\tcheck_linked_checkouts(new);\n \t\tfree(head_ref);\n \t}\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266018","messageId":"1436573146-3893-3-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 02/16] checkout: name check_linked_checkouts() more meaningfully","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:32Z","receivedAt":"2015-07-11T00:05:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"check_linked_checkouts() doesn't just \"check\" linked checkouts for\n\"something\"; specifically, it aborts the operation if the branch about\nto be checked out is already checked out elsewhere. Therefore, rename it\nto die_if_checked_out() to give a better indication of its function.\nThe more meaningful name will be particularly important when this\nfunction is later published for use by other callers.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/checkout.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 75f90a9..e75fb5e 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -910,7 +910,7 @@ done:\n \tstrbuf_release(&gitdir);\n }\n \n-static void check_linked_checkouts(struct branch_info *new)\n+static void die_if_checked_out(struct branch_info *new)\n {\n \tstruct strbuf path = STRBUF_INIT;\n \tDIR *dir;\n@@ -1152,7 +1152,7 @@ static int checkout_branch(struct checkout_opts *opts,\n \t\tchar *head_ref = resolve_refdup(\"HEAD\", 0, sha1, &flag);\n \t\tif (head_ref &&\n \t\t (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))\n-\t\t\tcheck_linked_checkouts(new);\n+\t\t\tdie_if_checked_out(new);\n \t\tfree(head_ref);\n \t}\n \n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266029","messageId":"1436573146-3893-4-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 03/16] checkout: improve die_if_checked_out() robustness","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:33Z","receivedAt":"2015-07-11T00:05:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"die_if_checked_out() is intended to check if the branch about to be\nchecked out is already checked out either in the main worktree or in a\nlinked worktree. However, if .git/worktrees directory does not exist,\nthen it never bothers checking the main worktree, even though the\nspecified branch might indeed be checked out there, which is fragile\nbehavior.\n\nThis hasn't been a problem in practice since the current implementation\nof \"git worktree add\" (and, earlier, \"git checkout --to\") always creates\n.git/worktrees before die_if_checked_out() is called by the child \"git\ncheckout\" invocation which populates the new worktree.\n\nHowever, git-worktree will eventually be retrofitted to populate the new\nworktree via \"git reset --hard\" rather than \"git checkout\", and it will\nwant to use die_if_checked_out() to perform this check as well. But,\nthe reliance upon order of operations (creating .git/worktrees before\nchecking if a branch is already checked out) is fragile, and, as a\ngeneral function, callers should not be expected to abide by this\nundocumented and unwarranted restriction. Therefore, make\ndie_if_checked_out() more robust by checking the main worktree whether\n.git/worktrees exists or not.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/checkout.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex e75fb5e..fc8bd79 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -916,12 +916,6 @@ static void die_if_checked_out(struct branch_info *new)\n \tDIR *dir;\n \tstruct dirent *d;\n \n-\tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n-\tif ((dir = opendir(path.buf)) == NULL) {\n-\t\tstrbuf_release(&path);\n-\t\treturn;\n-\t}\n-\n \t/*\n \t * $GIT_COMMON_DIR/HEAD is practically outside\n \t * $GIT_DIR so resolve_ref_unsafe() won't work (it\n@@ -929,6 +923,12 @@ static void die_if_checked_out(struct branch_info *new)\n \t */\n \tcheck_linked_checkout(new, NULL);\n \n+\tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n+\tif ((dir = opendir(path.buf)) == NULL) {\n+\t\tstrbuf_release(&path);\n+\t\treturn;\n+\t}\n+\n \twhile ((d = readdir(dir)) != NULL) {\n \t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n \t\t\tcontinue;\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266031","messageId":"1436573146-3893-5-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 04/16] checkout: die_if_checked_out: simplify strbuf management","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:34Z","receivedAt":"2015-07-11T00:05:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"There is no reason to keep the strbuf active long after its last use.\nBy releasing it as early as possible, resource management is simplified\nand there is less worry about future changes resulting in a leak.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/checkout.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex fc8bd79..ee33a20 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -924,17 +924,16 @@ static void die_if_checked_out(struct branch_info *new)\n \tcheck_linked_checkout(new, NULL);\n \n \tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n-\tif ((dir = opendir(path.buf)) == NULL) {\n-\t\tstrbuf_release(&path);\n+\tdir = opendir(path.buf);\n+\tstrbuf_release(&path);\n+\tif (!dir)\n \t\treturn;\n-\t}\n \n \twhile ((d = readdir(dir)) != NULL) {\n \t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n \t\t\tcontinue;\n \t\tcheck_linked_checkout(new, d->d_name);\n \t}\n-\tstrbuf_release(&path);\n \tclosedir(dir);\n }\n \n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266024","messageId":"1436573146-3893-6-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 05/16] checkout: generalize die_if_checked_out() branch name argument","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:35Z","receivedAt":"2015-07-11T00:05:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The plan is to publish die_if_checked_out() so that callers other than\ngit-checkout can take advantage of it, however, those callers won't have\naccess to git-checkout's \"struct branch_info\". Therefore, change it to\naccept the full name of the branch as a simple string instead.\n\nWhile here, also give the argument a more meaningful name (\"branch\"\ninstead of \"new\").\n\nSigned-off-by: Eric Sunshine \n---\n builtin/checkout.c | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex ee33a20..673daa0 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -873,7 +873,7 @@ static const char *unique_tracking_name(const char *name, unsigned char *sha1)\n \treturn NULL;\n }\n \n-static void check_linked_checkout(struct branch_info *new, const char *id)\n+static void check_linked_checkout(const char *branch, const char *id)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n@@ -893,7 +893,7 @@ static void check_linked_checkout(struct branch_info *new, const char *id)\n \tend = start;\n \twhile (*end && !isspace(*end))\n \t\tend++;\n-\tif (strncmp(start, new->path, end - start) || new->path[end - start] != '\\0')\n+\tif (strncmp(start, branch, end - start) || branch[end - start] != '\\0')\n \t\tgoto done;\n \tif (id) {\n \t\tstrbuf_reset(&path);\n@@ -903,14 +903,15 @@ static void check_linked_checkout(struct branch_info *new, const char *id)\n \t\tstrbuf_rtrim(&gitdir);\n \t} else\n \t\tstrbuf_addstr(&gitdir, get_git_common_dir());\n-\tdie(_(\"'%s' is already checked out at '%s'\"), new->name, gitdir.buf);\n+\tskip_prefix(branch, \"refs/heads/\", &branch);\n+\tdie(_(\"'%s' is already checked out at '%s'\"), branch, gitdir.buf);\n done:\n \tstrbuf_release(&path);\n \tstrbuf_release(&sb);\n \tstrbuf_release(&gitdir);\n }\n \n-static void die_if_checked_out(struct branch_info *new)\n+static void die_if_checked_out(const char *branch)\n {\n \tstruct strbuf path = STRBUF_INIT;\n \tDIR *dir;\n@@ -921,7 +922,7 @@ static void die_if_checked_out(struct branch_info *new)\n \t * $GIT_DIR so resolve_ref_unsafe() won't work (it\n \t * uses git_path). Parse the ref ourselves.\n \t */\n-\tcheck_linked_checkout(new, NULL);\n+\tcheck_linked_checkout(branch, NULL);\n \n \tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n \tdir = opendir(path.buf);\n@@ -932,7 +933,7 @@ static void die_if_checked_out(struct branch_info *new)\n \twhile ((d = readdir(dir)) != NULL) {\n \t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n \t\t\tcontinue;\n-\t\tcheck_linked_checkout(new, d->d_name);\n+\t\tcheck_linked_checkout(branch, d->d_name);\n \t}\n \tclosedir(dir);\n }\n@@ -1151,7 +1152,7 @@ static int checkout_branch(struct checkout_opts *opts,\n \t\tchar *head_ref = resolve_refdup(\"HEAD\", 0, sha1, &flag);\n \t\tif (head_ref &&\n \t\t (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))\n-\t\t\tdie_if_checked_out(new);\n+\t\t\tdie_if_checked_out(new->path);\n \t\tfree(head_ref);\n \t}\n \n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266027","messageId":"1436573146-3893-7-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 06/16] branch: publish die_if_checked_out()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:36Z","receivedAt":"2015-07-11T00:05:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"git-worktree will eventually be retrofitted to populate the new worktree\nvia \"git reset --hard\" rather than \"git checkout\", at which time it will\nneed to be able to detect if the branch is already checked out\nelsewhere, rather than relying upon git-branch to make this\ndetermination, so publish die_if_checked_out().\n\nSigned-off-by: Eric Sunshine \n---\n branch.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n branch.h | 7 ++++++\n builtin/checkout.c | 65 ------------------------------------------------------\n 3 files changed, 72 insertions(+), 65 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 4bab55a..7b8b9a3 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -309,3 +309,68 @@ void remove_branch_state(void)\n \tunlink(git_path(\"MERGE_MODE\"));\n \tunlink(git_path(\"SQUASH_MSG\"));\n }\n+\n+static void check_linked_checkout(const char *branch, const char *id)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf gitdir = STRBUF_INIT;\n+\tconst char *start, *end;\n+\n+\tif (id)\n+\t\tstrbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n+\telse\n+\t\tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n+\n+\tif (strbuf_read_file(&sb, path.buf, 0) < 0 ||\n+\t !skip_prefix(sb.buf, \"ref:\", &start))\n+\t\tgoto done;\n+\twhile (isspace(*start))\n+\t\tstart++;\n+\tend = start;\n+\twhile (*end && !isspace(*end))\n+\t\tend++;\n+\tif (strncmp(start, branch, end - start) || branch[end - start] != '\\0')\n+\t\tgoto done;\n+\tif (id) {\n+\t\tstrbuf_reset(&path);\n+\t\tstrbuf_addf(&path, \"%s/worktrees/%s/gitdir\", get_git_common_dir(), id);\n+\t\tif (strbuf_read_file(&gitdir, path.buf, 0) <= 0)\n+\t\t\tgoto done;\n+\t\tstrbuf_rtrim(&gitdir);\n+\t} else\n+\t\tstrbuf_addstr(&gitdir, get_git_common_dir());\n+\tskip_prefix(branch, \"refs/heads/\", &branch);\n+\tdie(_(\"'%s' is already checked out at '%s'\"), branch, gitdir.buf);\n+done:\n+\tstrbuf_release(&path);\n+\tstrbuf_release(&sb);\n+\tstrbuf_release(&gitdir);\n+}\n+\n+void die_if_checked_out(const char *branch)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tDIR *dir;\n+\tstruct dirent *d;\n+\n+\t/*\n+\t * $GIT_COMMON_DIR/HEAD is practically outside\n+\t * $GIT_DIR so resolve_ref_unsafe() won't work (it\n+\t * uses git_path). Parse the ref ourselves.\n+\t */\n+\tcheck_linked_checkout(branch, NULL);\n+\n+\tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n+\tdir = opendir(path.buf);\n+\tstrbuf_release(&path);\n+\tif (!dir)\n+\t\treturn;\n+\n+\twhile ((d = readdir(dir)) != NULL) {\n+\t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n+\t\t\tcontinue;\n+\t\tcheck_linked_checkout(branch, d->d_name);\n+\t}\n+\tclosedir(dir);\n+}\ndiff --git a/branch.h b/branch.h\nindex 64173ab..58aa45f 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -52,4 +52,11 @@ extern void install_branch_config(int flag, const char *local, const char *origi\n */\n extern int read_branch_desc(struct strbuf *, const char *branch_name);\n \n+/*\n+ * Check if a branch is checked out in the main worktree or any linked\n+ * worktree and die (with a message describing its checkout location) if\n+ * it is.\n+ */\n+extern void die_if_checked_out(const char *branch);\n+\n #endif\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 673daa0..4ae895c 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -873,71 +873,6 @@ static const char *unique_tracking_name(const char *name, unsigned char *sha1)\n \treturn NULL;\n }\n \n-static void check_linked_checkout(const char *branch, const char *id)\n-{\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf gitdir = STRBUF_INIT;\n-\tconst char *start, *end;\n-\n-\tif (id)\n-\t\tstrbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n-\telse\n-\t\tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n-\n-\tif (strbuf_read_file(&sb, path.buf, 0) < 0 ||\n-\t !skip_prefix(sb.buf, \"ref:\", &start))\n-\t\tgoto done;\n-\twhile (isspace(*start))\n-\t\tstart++;\n-\tend = start;\n-\twhile (*end && !isspace(*end))\n-\t\tend++;\n-\tif (strncmp(start, branch, end - start) || branch[end - start] != '\\0')\n-\t\tgoto done;\n-\tif (id) {\n-\t\tstrbuf_reset(&path);\n-\t\tstrbuf_addf(&path, \"%s/worktrees/%s/gitdir\", get_git_common_dir(), id);\n-\t\tif (strbuf_read_file(&gitdir, path.buf, 0) <= 0)\n-\t\t\tgoto done;\n-\t\tstrbuf_rtrim(&gitdir);\n-\t} else\n-\t\tstrbuf_addstr(&gitdir, get_git_common_dir());\n-\tskip_prefix(branch, \"refs/heads/\", &branch);\n-\tdie(_(\"'%s' is already checked out at '%s'\"), branch, gitdir.buf);\n-done:\n-\tstrbuf_release(&path);\n-\tstrbuf_release(&sb);\n-\tstrbuf_release(&gitdir);\n-}\n-\n-static void die_if_checked_out(const char *branch)\n-{\n-\tstruct strbuf path = STRBUF_INIT;\n-\tDIR *dir;\n-\tstruct dirent *d;\n-\n-\t/*\n-\t * $GIT_COMMON_DIR/HEAD is practically outside\n-\t * $GIT_DIR so resolve_ref_unsafe() won't work (it\n-\t * uses git_path). Parse the ref ourselves.\n-\t */\n-\tcheck_linked_checkout(branch, NULL);\n-\n-\tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n-\tdir = opendir(path.buf);\n-\tstrbuf_release(&path);\n-\tif (!dir)\n-\t\treturn;\n-\n-\twhile ((d = readdir(dir)) != NULL) {\n-\t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n-\t\t\tcontinue;\n-\t\tcheck_linked_checkout(branch, d->d_name);\n-\t}\n-\tclosedir(dir);\n-}\n-\n static int parse_branchname_arg(int argc, const char **argv,\n \t\t\t\tint dwim_new_local_branch_ok,\n \t\t\t\tstruct branch_info *new,\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266015","messageId":"1436573146-3893-8-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 07/16] worktree: simplify new branch (-b/-B) option checking","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:37Z","receivedAt":"2015-07-11T00:05:37Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Make 'new_branch' be the name of the new branch for both forced and\nnon-forced cases; and add boolean 'force_new_branch' to indicate forced\nbranch creation. This will simplify logic later on when git-worktree\nhandles branch creation locally rather than delegating it to\ngit-checkout as part of the worktree population phase.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/worktree.c | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 69248ba..c267410 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -272,7 +272,7 @@ static int add_worktree(const char *path, const char **child_argv)\n \n static int add(int ac, const char **av, const char *prefix)\n {\n-\tint force = 0, detach = 0;\n+\tint force = 0, detach = 0, force_new_branch;\n \tconst char *new_branch = NULL, *new_branch_force = NULL;\n \tconst char *path, *branch;\n \tstruct argv_array cmd = ARGV_ARRAY_INIT;\n@@ -295,7 +295,11 @@ static int add(int ac, const char **av, const char *prefix)\n \tpath = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0];\n \tbranch = ac < 2 ? \"HEAD\" : av[1];\n \n-\tif (ac < 2 && !new_branch && !new_branch_force) {\n+\tforce_new_branch = !!new_branch_force;\n+\tif (force_new_branch)\n+\t\tnew_branch = new_branch_force;\n+\n+\tif (ac < 2 && !new_branch) {\n \t\tint n;\n \t\tconst char *s = worktree_basename(path, &n);\n \t\tnew_branch = xstrndup(s, n);\n@@ -305,9 +309,8 @@ static int add(int ac, const char **av, const char *prefix)\n \tif (force)\n \t\targv_array_push(&cmd, \"--ignore-other-worktrees\");\n \tif (new_branch)\n-\t\targv_array_pushl(&cmd, \"-b\", new_branch, NULL);\n-\tif (new_branch_force)\n-\t\targv_array_pushl(&cmd, \"-B\", new_branch_force, NULL);\n+\t\targv_array_pushl(&cmd, force_new_branch ? \"-B\" : \"-b\",\n+\t\t\t\t new_branch, NULL);\n \tif (detach)\n \t\targv_array_push(&cmd, \"--detach\");\n \targv_array_push(&cmd, branch);\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266016","messageId":"1436573146-3893-9-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 08/16] worktree: introduce options container","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:38Z","receivedAt":"2015-07-11T00:05:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"add_worktree() will eventually need to deal with some options itself, so\nintroduce a structure into which options can be conveniently bundled,\nand pass it along to add_worktree().\n\nSigned-off-by: Eric Sunshine \n---\n builtin/worktree.c | 45 +++++++++++++++++++++++++++------------------\n 1 file changed, 27 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex c267410..253382a 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -12,6 +12,13 @@ static const char * const worktree_usage[] = {\n \tNULL\n };\n \n+struct add_opts {\n+\tint force;\n+\tint detach;\n+\tconst char *new_branch;\n+\tint force_new_branch;\n+};\n+\n static int show_only;\n static int verbose;\n static unsigned long expire;\n@@ -171,7 +178,8 @@ static const char *worktree_basename(const char *path, int *olen)\n \treturn name;\n }\n \n-static int add_worktree(const char *path, const char **child_argv)\n+static int add_worktree(const char *path, const char **child_argv,\n+\t\t\tconst struct add_opts *opts)\n {\n \tstruct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -272,22 +280,23 @@ static int add_worktree(const char *path, const char **child_argv)\n \n static int add(int ac, const char **av, const char *prefix)\n {\n-\tint force = 0, detach = 0, force_new_branch;\n-\tconst char *new_branch = NULL, *new_branch_force = NULL;\n+\tstruct add_opts opts;\n+\tconst char *new_branch_force = NULL;\n \tconst char *path, *branch;\n \tstruct argv_array cmd = ARGV_ARRAY_INIT;\n \tstruct option options[] = {\n-\t\tOPT__FORCE(&force, N_(\"checkout even if already checked out in other worktree\")),\n-\t\tOPT_STRING('b', NULL, &new_branch, N_(\"branch\"),\n+\t\tOPT__FORCE(&opts.force, N_(\"checkout even if already checked out in other worktree\")),\n+\t\tOPT_STRING('b', NULL, &opts.new_branch, N_(\"branch\"),\n \t\t\t N_(\"create a new branch\")),\n \t\tOPT_STRING('B', NULL, &new_branch_force, N_(\"branch\"),\n \t\t\t N_(\"create or reset a branch\")),\n-\t\tOPT_BOOL(0, \"detach\", &detach, N_(\"detach HEAD at named commit\")),\n+\t\tOPT_BOOL(0, \"detach\", &opts.detach, N_(\"detach HEAD at named commit\")),\n \t\tOPT_END()\n \t};\n \n+\tmemset(&opts, 0, sizeof(opts));\n \tac = parse_options(ac, av, prefix, options, worktree_usage, 0);\n-\tif (new_branch && new_branch_force)\n+\tif (opts.new_branch && new_branch_force)\n \t\tdie(_(\"-b and -B are mutually exclusive\"));\n \tif (ac < 1 || ac > 2)\n \t\tusage_with_options(worktree_usage, options);\n@@ -295,27 +304,27 @@ static int add(int ac, const char **av, const char *prefix)\n \tpath = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0];\n \tbranch = ac < 2 ? \"HEAD\" : av[1];\n \n-\tforce_new_branch = !!new_branch_force;\n-\tif (force_new_branch)\n-\t\tnew_branch = new_branch_force;\n+\topts.force_new_branch = !!new_branch_force;\n+\tif (opts.force_new_branch)\n+\t\topts.new_branch = new_branch_force;\n \n-\tif (ac < 2 && !new_branch) {\n+\tif (ac < 2 && !opts.new_branch) {\n \t\tint n;\n \t\tconst char *s = worktree_basename(path, &n);\n-\t\tnew_branch = xstrndup(s, n);\n+\t\topts.new_branch = xstrndup(s, n);\n \t}\n \n \targv_array_push(&cmd, \"checkout\");\n-\tif (force)\n+\tif (opts.force)\n \t\targv_array_push(&cmd, \"--ignore-other-worktrees\");\n-\tif (new_branch)\n-\t\targv_array_pushl(&cmd, force_new_branch ? \"-B\" : \"-b\",\n-\t\t\t\t new_branch, NULL);\n-\tif (detach)\n+\tif (opts.new_branch)\n+\t\targv_array_pushl(&cmd, opts.force_new_branch ? \"-B\" : \"-b\",\n+\t\t\t\t opts.new_branch, NULL);\n+\tif (opts.detach)\n \t\targv_array_push(&cmd, \"--detach\");\n \targv_array_push(&cmd, branch);\n \n-\treturn add_worktree(path, cmd.argv);\n+\treturn add_worktree(path, cmd.argv, &opts);\n }\n \n int cmd_worktree(int ac, const char **av, const char *prefix)\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266025","messageId":"1436573146-3893-10-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 09/16] worktree: make --detach mutually exclusive with -b/-B","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:39Z","receivedAt":"2015-07-11T00:05:39Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Be consistent with git-checkout which disallows this (not particularly\nmeaningful) combination.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/worktree.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 253382a..cd06bf5 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -296,8 +296,8 @@ static int add(int ac, const char **av, const char *prefix)\n \n \tmemset(&opts, 0, sizeof(opts));\n \tac = parse_options(ac, av, prefix, options, worktree_usage, 0);\n-\tif (opts.new_branch && new_branch_force)\n-\t\tdie(_(\"-b and -B are mutually exclusive\"));\n+\tif (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)\n+\t\tdie(_(\"-b, -B, and --detach are mutually exclusive\"));\n \tif (ac < 1 || ac > 2)\n \t\tusage_with_options(worktree_usage, options);\n \n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266021","messageId":"1436573146-3893-11-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 10/16] worktree: make branch creation distinct from worktree population","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:40Z","receivedAt":"2015-07-11T00:05:40Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The plan is eventually to populate the new worktree via \"git reset\n--hard\" rather than \"git checkout\". Thus, rather than piggybacking on\ngit-checkout's -b/-B ability to create a new branch at checkout time,\ngit-worktree will need to do so itself.\n\nSigned-off-by: Eric Sunshine \n---\n\nI considered calling branch-related API, such as create_branch(),\ndirectly, however, git-branch provides extra value which may be useful\nin the future (such as when --track and --orphan get added to\ngit-worktree), so it seemed wise to re-use git-branch's implementation\nrather than duplicating the functionality. If this proves the wrong\nchoice, then the series can either be re-rolled or follow-on patches can\naddress the issue.\n\n builtin/worktree.c | 17 ++++++++++++++---\n 1 file changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex cd06bf5..9be1c74 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -314,12 +314,23 @@ static int add(int ac, const char **av, const char *prefix)\n \t\topts.new_branch = xstrndup(s, n);\n \t}\n \n+\tif (opts.new_branch) {\n+\t\tstruct child_process cp;\n+\t\tmemset(&cp, 0, sizeof(cp));\n+\t\tcp.git_cmd = 1;\n+\t\targv_array_push(&cp.args, \"branch\");\n+\t\tif (opts.force_new_branch)\n+\t\t\targv_array_push(&cp.args, \"--force\");\n+\t\targv_array_push(&cp.args, opts.new_branch);\n+\t\targv_array_push(&cp.args, branch);\n+\t\tif (run_command(&cp))\n+\t\t\treturn -1;\n+\t\tbranch = opts.new_branch;\n+\t}\n+\n \targv_array_push(&cmd, \"checkout\");\n \tif (opts.force)\n \t\targv_array_push(&cmd, \"--ignore-other-worktrees\");\n-\tif (opts.new_branch)\n-\t\targv_array_pushl(&cmd, opts.force_new_branch ? \"-B\" : \"-b\",\n-\t\t\t\t opts.new_branch, NULL);\n \tif (opts.detach)\n \t\targv_array_push(&cmd, \"--detach\");\n \targv_array_push(&cmd, branch);\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266032","messageId":"1436573146-3893-12-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 11/16] worktree: add_worktree: construct worktree-population command locally","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:41Z","receivedAt":"2015-07-11T00:05:41Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The caller of add_worktree() provides it with a command to invoke to\npopulate the new worktree. This was a useful abstraction during the\nconversion of \"git checkout --to\" functionality to \"git worktree add\"\nsince git-checkout and git-worktree constructed the population command\ndifferently. However, now that \"git checkout --to\" has been retired, and\nadd_worktree() has access to the options given to \"worktree add\", this\nextra indirection is no longer useful, and makes the code a bit more\nconvoluted than it needs to be.\n\nMoreover, the eventual goal is for git-worktree to populate the new\nworktree via \"git reset --hard\" rather than \"git checkout\", and to do\nso, it may need to invoke other commands in addition to the\nworktree-population command, so it will be doing command construction\nitself anyhow. Therefore, relocate construction of the\nworktree-population command from add() to add_worktree().\n\nSigned-off-by: Eric Sunshine \n---\n builtin/worktree.c | 19 ++++++++-----------\n 1 file changed, 8 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 9be1c74..e04a6d1 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -178,7 +178,7 @@ static const char *worktree_basename(const char *path, int *olen)\n \treturn name;\n }\n \n-static int add_worktree(const char *path, const char **child_argv,\n+static int add_worktree(const char *path, const char *refname,\n \t\t\tconst struct add_opts *opts)\n {\n \tstruct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;\n@@ -260,7 +260,12 @@ static int add_worktree(const char *path, const char **child_argv,\n \tsetenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);\n \tmemset(&cp, 0, sizeof(cp));\n \tcp.git_cmd = 1;\n-\tcp.argv = child_argv;\n+\targv_array_push(&cp.args, \"checkout\");\n+\tif (opts->force)\n+\t\targv_array_push(&cp.args, \"--ignore-other-worktrees\");\n+\tif (opts->detach)\n+\t\targv_array_push(&cp.args, \"--detach\");\n+\targv_array_push(&cp.args, refname);\n \tret = run_command(&cp);\n \tif (!ret) {\n \t\tis_junk = 0;\n@@ -283,7 +288,6 @@ static int add(int ac, const char **av, const char *prefix)\n \tstruct add_opts opts;\n \tconst char *new_branch_force = NULL;\n \tconst char *path, *branch;\n-\tstruct argv_array cmd = ARGV_ARRAY_INIT;\n \tstruct option options[] = {\n \t\tOPT__FORCE(&opts.force, N_(\"checkout even if already checked out in other worktree\")),\n \t\tOPT_STRING('b', NULL, &opts.new_branch, N_(\"branch\"),\n@@ -328,14 +332,7 @@ static int add(int ac, const char **av, const char *prefix)\n \t\tbranch = opts.new_branch;\n \t}\n \n-\targv_array_push(&cmd, \"checkout\");\n-\tif (opts.force)\n-\t\targv_array_push(&cmd, \"--ignore-other-worktrees\");\n-\tif (opts.detach)\n-\t\targv_array_push(&cmd, \"--detach\");\n-\targv_array_push(&cmd, branch);\n-\n-\treturn add_worktree(path, cmd.argv, &opts);\n+\treturn add_worktree(path, branch, &opts);\n }\n \n int cmd_worktree(int ac, const char **av, const char *prefix)\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266019","messageId":"1436573146-3893-13-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 12/16] worktree: detect branch symref/detach and error conditions locally","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:42Z","receivedAt":"2015-07-11T00:05:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The eventual goal is for git-worktree to populate the new worktree via\n\"git reset --hard\" rather than \"git checkout\". As a consequence,\ngit-worktree will no longer be able to rely upon git-branch to determine\nthe state of the worktree (detached or branch symref), or to check for\nerror conditions, such as the requested branch already checked out\nelsewhere, or an invalid reference. Therefore, imbue git-worktree with\nthe intelligence to determine a branch symref or detached state locally,\nand to perform error checking on its own.\n\nSigned-off-by: Eric Sunshine \n---\n\nThis patch is somewhat RFC since I spent a lot of time browsing the\nbranch- and ref-related APIs figuring out how to determine if the\nprovided refname was a branch name or some other (detached) reference.\nI'm not at all sure that my use of strbuf_check_branch_ref(),\nref_exists(), and lookup_commit_reference_by_name() is the best\napproach, or even appropriate, although it seems to work as desired.\n\n builtin/worktree.c | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex e04a6d1..babdef1 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -3,6 +3,8 @@\n #include \"dir.h\"\n #include \"parse-options.h\"\n #include \"argv-array.h\"\n+#include \"branch.h\"\n+#include \"refs.h\"\n #include \"run-command.h\"\n #include \"sigchain.h\"\n \n@@ -187,11 +189,23 @@ static int add_worktree(const char *path, const char *refname,\n \tstruct stat st;\n \tstruct child_process cp;\n \tint counter = 0, len, ret;\n+\tstruct strbuf symref = STRBUF_INIT;\n+\tstruct commit *commit = NULL;\n \tunsigned char rev[20];\n \n \tif (file_exists(path) && !is_empty_dir(path))\n \t\tdie(_(\"'%s' already exists\"), path);\n \n+\tif (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n+\t ref_exists(symref.buf)) {\n+\t\tif (!opts->force)\n+\t\t\tdie_if_checked_out(symref.buf);\n+\t} else {\n+\t\tcommit = lookup_commit_reference_by_name(refname);\n+\t\tif (!commit)\n+\t\t\tdie(_(\"invalid reference: %s\"), refname);\n+\t}\n+\n \tname = worktree_basename(path, &len);\n \tstrbuf_addstr(&sb_repo,\n \t\t git_path(\"worktrees/%.*s\", (int)(path + len - name), name));\n@@ -278,6 +292,7 @@ static int add_worktree(const char *path, const char *refname,\n \tstrbuf_addf(&sb, \"%s/locked\", sb_repo.buf);\n \tunlink_or_warn(sb.buf);\n \tstrbuf_release(&sb);\n+\tstrbuf_release(&symref);\n \tstrbuf_release(&sb_repo);\n \tstrbuf_release(&sb_git);\n \treturn ret;\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266014","messageId":"1436573146-3893-14-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 13/16] worktree: make setup of new HEAD distinct from worktree population","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:43Z","receivedAt":"2015-07-11T00:05:43Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The eventual goal is for git-worktree to populate the new worktree via\n\"git reset --hard\" rather than \"git checkout\". As a consequence,\ngit-worktree will no longer be able to delegate to git-branch the\nsetting of the new worktree's HEAD to the desired branch (or commit, if\ndetached). Therefore, make git-worktree responsible for setting up HEAD\nas either a symbolic reference if associated with a branch, or detached\nif not.\n\nSigned-off-by: Eric Sunshine \n---\n\nAs with the patch which separates branch creation from worktree\npopulation (by using git-branch instead of \"git checkout -b\"), this\npatch invokes git-symbolic-ref and git-update-ref rather than using the\nlower-level C API, due to the added value of those commands. If this\nproves the wrong approach, then the series can be re-rolled or follow-on\npatches can instead use the C API.\n\n builtin/worktree.c | 19 ++++++++++++++-----\n 1 file changed, 14 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex babdef1..94c1701 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -274,12 +274,20 @@ static int add_worktree(const char *path, const char *refname,\n \tsetenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);\n \tmemset(&cp, 0, sizeof(cp));\n \tcp.git_cmd = 1;\n+\n+\tif (commit)\n+\t\targv_array_pushl(&cp.args, \"update-ref\", \"HEAD\",\n+\t\t\t\t sha1_to_hex(commit->object.sha1), NULL);\n+\telse\n+\t\targv_array_pushl(&cp.args, \"symbolic-ref\", \"HEAD\",\n+\t\t\t\t symref.buf, NULL);\n+\tret = run_command(&cp);\n+\tif (ret)\n+\t\tgoto done;\n+\n+\tcp.argv = NULL;\n+\targv_array_clear(&cp.args);\n \targv_array_push(&cp.args, \"checkout\");\n-\tif (opts->force)\n-\t\targv_array_push(&cp.args, \"--ignore-other-worktrees\");\n-\tif (opts->detach)\n-\t\targv_array_push(&cp.args, \"--detach\");\n-\targv_array_push(&cp.args, refname);\n \tret = run_command(&cp);\n \tif (!ret) {\n \t\tis_junk = 0;\n@@ -288,6 +296,7 @@ static int add_worktree(const char *path, const char *refname,\n \t\tjunk_work_tree = NULL;\n \t\tjunk_git_dir = NULL;\n \t}\n+done:\n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s/locked\", sb_repo.buf);\n \tunlink_or_warn(sb.buf);\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266033","messageId":"1436573146-3893-15-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 14/16] worktree: avoid resolving HEAD unnecessarily","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:44Z","receivedAt":"2015-07-11T00:05:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Now that git-worktree sets HEAD explicitly to its final value via either\ngit-symbolic-ref or git-update-ref, rather than relying upon\ngit-checkout to do so, the \"hack\" for pacifying is_git_directory() with\na temporary HEAD, though still necessary, can be simplified.\n\nSince the real HEAD is now populated with its proper final value, the\nvalue of the temporary HEAD truly no longer matters, and any value which\nlooks like an object ID is good enough to satisfy is_git_directory().\nTherefore, just set the temporary HEAD to a literal value rather than\ngoing through the effort of resolving the current branch's HEAD.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/worktree.c | 17 +++++------------\n 1 file changed, 5 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 94c1701..9101a3c 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -191,7 +191,6 @@ static int add_worktree(const char *path, const char *refname,\n \tint counter = 0, len, ret;\n \tstruct strbuf symref = STRBUF_INIT;\n \tstruct commit *commit = NULL;\n-\tunsigned char rev[20];\n \n \tif (file_exists(path) && !is_empty_dir(path))\n \t\tdie(_(\"'%s' already exists\"), path);\n@@ -249,20 +248,14 @@ static int add_worktree(const char *path, const char *refname,\n \t\t real_path(get_git_common_dir()), name);\n \t/*\n \t * This is to keep resolve_ref() happy. We need a valid HEAD\n-\t * or is_git_directory() will reject the directory. Moreover, HEAD\n-\t * in the new worktree must resolve to the same value as HEAD in\n-\t * the current tree since the command invoked to populate the new\n-\t * worktree will be handed the branch/ref specified by the user.\n-\t * For instance, if the user asks for the new worktree to be based\n-\t * at HEAD~5, then the resolved HEAD~5 in the new worktree must\n-\t * match the resolved HEAD~5 in the current tree in order to match\n-\t * the user's expectation.\n+\t * or is_git_directory() will reject the directory. Any value which\n+\t * looks like an object ID will do since it will be immediately\n+\t * replaced by the symbolic-ref or update-ref invocation in the new\n+\t * worktree.\n \t */\n-\tif (!resolve_ref_unsafe(\"HEAD\", 0, rev, NULL))\n-\t\tdie(_(\"unable to resolve HEAD\"));\n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s/HEAD\", sb_repo.buf);\n-\twrite_file(sb.buf, 1, \"%s\\n\", sha1_to_hex(rev));\n+\twrite_file(sb.buf, 1, \"0000000000000000000000000000000000000000\\n\");\n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s/commondir\", sb_repo.buf);\n \twrite_file(sb.buf, 1, \"../..\\n\");\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266030","messageId":"1436573146-3893-16-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 15/16] worktree: populate via \"git reset --hard\" rather than \"git checkout\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:45Z","receivedAt":"2015-07-11T00:05:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Now that git-worktree handles all functionality (--force, --detach,\n-b/-B) previously delegated to git-checkout, actual population of the\nnew worktree can be accomplished more directly and more lightweight with\n\"git reset --hard\" rather than \"git checkout\".\n\nSigned-off-by: Eric Sunshine \n---\n builtin/worktree.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 9101a3c..51c57bc 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -262,7 +262,6 @@ static int add_worktree(const char *path, const char *refname,\n \n \tfprintf_ln(stderr, _(\"Enter %s (identifier %s)\"), path, name);\n \n-\tsetenv(\"GIT_CHECKOUT_NEW_WORKTREE\", \"1\", 1);\n \tsetenv(GIT_DIR_ENVIRONMENT, sb_git.buf, 1);\n \tsetenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);\n \tmemset(&cp, 0, sizeof(cp));\n@@ -280,7 +279,7 @@ static int add_worktree(const char *path, const char *refname,\n \n \tcp.argv = NULL;\n \targv_array_clear(&cp.args);\n-\targv_array_push(&cp.args, \"checkout\");\n+\targv_array_pushl(&cp.args, \"reset\", \"--hard\", NULL);\n \tret = run_command(&cp);\n \tif (!ret) {\n \t\tis_junk = 0;\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266022","messageId":"1436573146-3893-17-git-send-email-sunshine@sunshineco.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 16/16] checkout: drop intimate knowledge of new worktree initial population","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-11T00:05:46Z","receivedAt":"2015-07-11T00:05:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Now that git-worktree no longer relies upon git-checkout to perform\ninitial population of the new worktree, git-checkout no longer needs\nintimate knowledge that it may be working on a newly created worktree.\nTherefore, drop 'new_worktree_mode' and the private\nGIT_CHECKOUT_NEW_WORKTREE environment variable by which git-worktree\ncommunicated to git-checkout that it was being invoked to populate a new\nworktree.\n\nThis reverts the remaining changes to checkout.c by 529fef2 (checkout:\nsupport checking out into a new working directory, 2014-11-30).\n\nSigned-off-by: Eric Sunshine \n---\n builtin/checkout.c | 9 ++-------\n 1 file changed, 2 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 4ae895c..02d78ba 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -48,8 +48,6 @@ struct checkout_opts {\n \tconst char *prefix;\n \tstruct pathspec pathspec;\n \tstruct tree *source_tree;\n-\n-\tint new_worktree_mode;\n };\n \n static int post_checkout_hook(struct commit *old, struct commit *new,\n@@ -491,7 +489,7 @@ static int merge_working_tree(const struct checkout_opts *opts,\n \t\t\ttopts.dir->flags |= DIR_SHOW_IGNORED;\n \t\t\tsetup_standard_excludes(topts.dir);\n \t\t}\n-\t\ttree = parse_tree_indirect(old->commit && !opts->new_worktree_mode ?\n+\t\ttree = parse_tree_indirect(old->commit ?\n \t\t\t\t\t old->commit->object.sha1 :\n \t\t\t\t\t EMPTY_TREE_SHA1_BIN);\n \t\tinit_tree_desc(&trees[0], tree->buffer, tree->size);\n@@ -807,8 +805,7 @@ static int switch_branches(const struct checkout_opts *opts,\n \t\treturn ret;\n \t}\n \n-\tif (!opts->quiet && !old.path && old.commit &&\n-\t new->commit != old.commit && !opts->new_worktree_mode)\n+\tif (!opts->quiet && !old.path && old.commit && new->commit != old.commit)\n \t\torphaned_commit_warning(old.commit, new->commit);\n \n \tupdate_refs_for_switch(opts, &old, new);\n@@ -1151,8 +1148,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, checkout_usage,\n \t\t\t PARSE_OPT_KEEP_DASHDASH);\n \n-\topts.new_worktree_mode = getenv(\"GIT_CHECKOUT_NEW_WORKTREE\") != NULL;\n-\n \tif (conflict_style) {\n \t\topts.merge = 1; /* implied */\n \t\tgit_xmerge_config(\"merge.conflictstyle\", conflict_style, NULL);\n-- \n2.5.0.rc1.201.ga12d9f8\n"},{"id":"266046","messageId":"CACsJy8A8QkeFa3FyCkv8dp-J52NGGaN-AV3oG7W8tNtYqDO-cw@mail.gmail.com","threadId":"39833","inReplyTo":"1436573146-3893-11-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 10/16] worktree: make branch creation distinct from worktree population","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-12T01:20:20Z","receivedAt":"2015-07-12T01:20:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Jul 11, 2015 at 7:05 AM, Eric Sunshine wrote:\n> The plan is eventually to populate the new worktree via \"git reset\n> --hard\" rather than \"git checkout\". Thus, rather than piggybacking on\n> git-checkout's -b/-B ability to create a new branch at checkout time,\n> git-worktree will need to do so itself.\n>\n> Signed-off-by: Eric Sunshine \n> ---\n>\n> I considered calling branch-related API, such as create_branch(),\n> directly, however, git-branch provides extra value which may be useful\n> in the future (such as when --track and --orphan get added to\n> git-worktree), so it seemed wise to re-use git-branch's implementation\n> rather than duplicating the functionality. If this proves the wrong\n> choice, then the series can either be re-rolled or follow-on patches can\n> address the issue.\n\nI don't know much about ref handling code (especially after the big\ntransaction update), so i may be wrong, but do we need to invalidate\nsome caches in refs.c after this? The same for update-ref in the other\npatch. We may need to re-read the index after 'reset --hard' too if we\never need to do touch the index after that (unlikely though in the\ncase of 'worktree add')\n-- \nDuy\n"},{"id":"266048","messageId":"CAPig+cSiPeJskyanLAv71LuFoqPirUJ2FL0B0BsoT8BEVrE3LA@mail.gmail.com","threadId":"39833","inReplyTo":"CACsJy8A8QkeFa3FyCkv8dp-J52NGGaN-AV3oG7W8tNtYqDO-cw@mail.gmail.com","subject":"Re: [PATCH 10/16] worktree: make branch creation distinct from worktree population","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-12T02:36:49Z","receivedAt":"2015-07-12T02:36:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jul 11, 2015 at 9:20 PM, Duy Nguyen wrote:\n> On Sat, Jul 11, 2015 at 7:05 AM, Eric Sunshine wrote:\n>> The plan is eventually to populate the new worktree via \"git reset\n>> --hard\" rather than \"git checkout\". Thus, rather than piggybacking on\n>> git-checkout's -b/-B ability to create a new branch at checkout time,\n>> git-worktree will need to do so itself.\n>>\n>> Signed-off-by: Eric Sunshine \n>> ---\n>>\n>> I considered calling branch-related API, such as create_branch(),\n>> directly, however, git-branch provides extra value which may be useful\n>> in the future (such as when --track and --orphan get added to\n>> git-worktree), so it seemed wise to re-use git-branch's implementation\n>> rather than duplicating the functionality. If this proves the wrong\n>> choice, then the series can either be re-rolled or follow-on patches can\n>> address the issue.\n>\n> I don't know much about ref handling code (especially after the big\n> transaction update), so i may be wrong, but do we need to invalidate\n> some caches in refs.c after this? The same for update-ref in the other\n> patch. We may need to re-read the index after 'reset --hard' too if we\n> ever need to do touch the index after that (unlikely though in the\n> case of 'worktree add')\n\nI'm not sure I understand. Are you talking about this patch's\nimplementation or a possible future change which uses the C API rather\nthan git-branch?\n\nIf you're talking about this patch, then I don't think we need to do\nanything more, as the \"git branch\" and \"git reset --hard\" invocations\nare separate process invocations which shouldn't affect the current\nworktree or the current \"git worktree add\" process.\n\nIf you're talking about a future patch switching over to the C API,\nand avoiding run_command(), then perhaps (but I haven't investigated\nthat avenue thoroughly enough to answer).\n\nOr, am I totally misunderstanding you?\n"},{"id":"266049","messageId":"CACsJy8A8CeyeYZnMpbdWW-etohp3TWzGrro1khzD=8skydm6Cg@mail.gmail.com","threadId":"39833","inReplyTo":"CAPig+cSiPeJskyanLAv71LuFoqPirUJ2FL0B0BsoT8BEVrE3LA@mail.gmail.com","subject":"Re: [PATCH 10/16] worktree: make branch creation distinct from worktree population","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-12T02:48:24Z","receivedAt":"2015-07-12T02:48:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jul 12, 2015 at 9:36 AM, Eric Sunshine wrote:\n> On Sat, Jul 11, 2015 at 9:20 PM, Duy Nguyen wrote:\n>> On Sat, Jul 11, 2015 at 7:05 AM, Eric Sunshine wrote:\n>>> The plan is eventually to populate the new worktree via \"git reset\n>>> --hard\" rather than \"git checkout\". Thus, rather than piggybacking on\n>>> git-checkout's -b/-B ability to create a new branch at checkout time,\n>>> git-worktree will need to do so itself.\n>>>\n>>> Signed-off-by: Eric Sunshine \n>>> ---\n>>>\n>>> I considered calling branch-related API, such as create_branch(),\n>>> directly, however, git-branch provides extra value which may be useful\n>>> in the future (such as when --track and --orphan get added to\n>>> git-worktree), so it seemed wise to re-use git-branch's implementation\n>>> rather than duplicating the functionality. If this proves the wrong\n>>> choice, then the series can either be re-rolled or follow-on patches can\n>>> address the issue.\n>>\n>> I don't know much about ref handling code (especially after the big\n>> transaction update), so i may be wrong, but do we need to invalidate\n>> some caches in refs.c after this? The same for update-ref in the other\n>> patch. We may need to re-read the index after 'reset --hard' too if we\n>> ever need to do touch the index after that (unlikely though in the\n>> case of 'worktree add')\n>\n> I'm not sure I understand. Are you talking about this patch's\n> implementation or a possible future change which uses the C API rather\n> than git-branch?\n>\n> If you're talking about this patch, then I don't think we need to do\n> anything more, as the \"git branch\" and \"git reset --hard\" invocations\n> are separate process invocations which shouldn't affect the current\n> worktree or the current \"git worktree add\" process.\n\nThe \"shouldn't affect\" is potentially a problem.If the current\n'worktree add' process caches something (in ref handling, for example)\nthat the 'git branch' process changes, then we may need to invalidate\ncache in 'worktree add' process after run_command(). I guess it's ok\nin this case since all we do is run_command(), we do not lookup refs\nor anything else afterwards.\n-- \nDuy\n"},{"id":"266050","messageId":"CAPig+cT-1juGLZK+F1Cy+MDE8+_bPqnX6fgfpoMnd2Qypu3oZQ@mail.gmail.com","threadId":"39833","inReplyTo":"CACsJy8A8CeyeYZnMpbdWW-etohp3TWzGrro1khzD=8skydm6Cg@mail.gmail.com","subject":"Re: [PATCH 10/16] worktree: make branch creation distinct from worktree population","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-12T03:10:52Z","receivedAt":"2015-07-12T03:10:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jul 11, 2015 at 10:48 PM, Duy Nguyen wrote:\n> On Sun, Jul 12, 2015 at 9:36 AM, Eric Sunshine wrote:\n>> On Sat, Jul 11, 2015 at 9:20 PM, Duy Nguyen wrote:\n>>> On Sat, Jul 11, 2015 at 7:05 AM, Eric Sunshine wrote:\n>>>> The plan is eventually to populate the new worktree via \"git reset\n>>>> --hard\" rather than \"git checkout\". Thus, rather than piggybacking on\n>>>> git-checkout's -b/-B ability to create a new branch at checkout time,\n>>>> git-worktree will need to do so itself.\n>>>\n>>> I don't know much about ref handling code (especially after the big\n>>> transaction update), so i may be wrong, but do we need to invalidate\n>>> some caches in refs.c after this? The same for update-ref in the other\n>>> patch. We may need to re-read the index after 'reset --hard' too if we\n>>> ever need to do touch the index after that (unlikely though in the\n>>> case of 'worktree add')\n>>\n>> I'm not sure I understand. Are you talking about this patch's\n>> implementation or a possible future change which uses the C API rather\n>> than git-branch?\n>>\n>> If you're talking about this patch, then I don't think we need to do\n>> anything more, as the \"git branch\" and \"git reset --hard\" invocations\n>> are separate process invocations which shouldn't affect the current\n>> worktree or the current \"git worktree add\" process.\n>\n> The \"shouldn't affect\" is potentially a problem.If the current\n> 'worktree add' process caches something (in ref handling, for example)\n> that the 'git branch' process changes, then we may need to invalidate\n> cache in 'worktree add' process after run_command(). I guess it's ok\n> in this case since all we do is run_command(), we do not lookup refs\n> or anything else afterwards.\n\nWith this patch series applied, the code effectively does this:\n\n branch = ...\n if (create_new_branch) {\n exec \"git branch newbranch branch\"\n branch = newbranch;\n }\n if (ref_exists(branch) && !detach)\n exec \"git symbolic-ref HEAD branch\"\n else\n exec \"git update-ref HEAD $(git rev-parse branch)\"\n exec \"git reset --hard\"\n\nSo, if I understand your concern correctly, then you are worried that,\nfollowing the git-branch invocation, ref_exists() could return the\nwrong answer with a pluggable ref-backend since it might be answering\nbased upon stale information. Is that what you mean? If so, I can see\nhow that it could be an issue. (As far as I can tell, the current\nfile-based backend doesn't have a problem with this since it's hitting\nthe filesystem directly to answer the ref_exists() question.)\n"},{"id":"266051","messageId":"CAPig+cQcE0CkNCY6rbzRfjptesawPjeqGx9+rtVhCOADJzcU2Q@mail.gmail.com","threadId":"39833","inReplyTo":"CAPig+cT-1juGLZK+F1Cy+MDE8+_bPqnX6fgfpoMnd2Qypu3oZQ@mail.gmail.com","subject":"Re: [PATCH 10/16] worktree: make branch creation distinct from worktree population","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-12T03:14:08Z","receivedAt":"2015-07-12T03:14:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jul 11, 2015 at 11:10 PM, Eric Sunshine wrote:\n> On Sat, Jul 11, 2015 at 10:48 PM, Duy Nguyen wrote:\n>> The \"shouldn't affect\" is potentially a problem.If the current\n>> 'worktree add' process caches something (in ref handling, for example)\n>> that the 'git branch' process changes, then we may need to invalidate\n>> cache in 'worktree add' process after run_command(). I guess it's ok\n>> in this case since all we do is run_command(), we do not lookup refs\n>> or anything else afterwards.\n>\n> With this patch series applied, the code effectively does this:\n>\n> branch = ...\n> if (create_new_branch) {\n> exec \"git branch newbranch branch\"\n> branch = newbranch;\n> }\n> if (ref_exists(branch) && !detach)\n> exec \"git symbolic-ref HEAD branch\"\n> else\n> exec \"git update-ref HEAD $(git rev-parse branch)\"\n> exec \"git reset --hard\"\n>\n> So, if I understand your concern correctly, then you are worried that,\n> following the git-branch invocation, ref_exists() could return the\n> wrong answer with a pluggable ref-backend since it might be answering\n> based upon stale information. Is that what you mean? If so, I can see\n> how that it could be an issue. (As far as I can tell, the current\n> file-based backend doesn't have a problem with this since it's hitting\n> the filesystem directly to answer the ref_exists() question.)\n\nI meant for this final sentence to end like this:\n\n ...to answer the ref_exists() question, but it still seems\n fragile since some future change could introduce caching.\n"},{"id":"266052","messageId":"CAPig+cRV8oNw2Jp0mkn=CLTJERG+=O2pvewcjqF=xZ+dMgPnHA@mail.gmail.com","threadId":"39833","inReplyTo":"CAPig+cQcE0CkNCY6rbzRfjptesawPjeqGx9+rtVhCOADJzcU2Q@mail.gmail.com","subject":"Re: [PATCH 10/16] worktree: make branch creation distinct from worktree population","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-12T03:27:30Z","receivedAt":"2015-07-12T03:27:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jul 11, 2015 at 11:14 PM, Eric Sunshine wrote:\n> On Sat, Jul 11, 2015 at 11:10 PM, Eric Sunshine wrote:\n>> On Sat, Jul 11, 2015 at 10:48 PM, Duy Nguyen wrote:\n>>> The \"shouldn't affect\" is potentially a problem.If the current\n>>> 'worktree add' process caches something (in ref handling, for example)\n>>> that the 'git branch' process changes, then we may need to invalidate\n>>> cache in 'worktree add' process after run_command(). I guess it's ok\n>>> in this case since all we do is run_command(), we do not lookup refs\n>>> or anything else afterwards.\n>>\n>> With this patch series applied, the code effectively does this:\n>>\n>> branch = ...\n>> if (create_new_branch) {\n>> exec \"git branch newbranch branch\"\n>> branch = newbranch;\n>> }\n>> if (ref_exists(branch) && !detach)\n>> exec \"git symbolic-ref HEAD branch\"\n>> else\n>> exec \"git update-ref HEAD $(git rev-parse branch)\"\n>> exec \"git reset --hard\"\n>>\n>> So, if I understand your concern correctly, then you are worried that,\n>> following the git-branch invocation, ref_exists() could return the\n>> wrong answer with a pluggable ref-backend since it might be answering\n>> based upon stale information. Is that what you mean? If so, I can see\n>> how that it could be an issue. (As far as I can tell, the current\n>> file-based backend doesn't have a problem with this since it's hitting\n>> the filesystem directly to answer the ref_exists() question.)\n>\n> I meant for this final sentence to end like this:\n>\n> ...to answer the ref_exists() question, but it still seems\n> fragile since some future change could introduce caching.\n\nIn this case, it's easy enough to side-step the issue since there's no\nneed to call ref_exists() if the new branch was created successfully\n(since we know it exists). The logic would effectively become this:\n\n branch = ...\n if (create_new_branch) {\n exec \"git branch newbranch branch\"\n exec \"git symbolic-ref HEAD newbranch\"\n } else if (ref_exists(branch) && !detach)\n exec \"git symbolic-ref HEAD branch\"\n else\n exec \"git update-ref HEAD $(git rev-parse branch)\"\n exec \"git reset --hard\"\n"},{"id":"266058","messageId":"CACsJy8BS7Ss3jP9sq1TuD7-wH6tTYwTKBk9=YVMiJh2ePigjuQ@mail.gmail.com","threadId":"39833","inReplyTo":"CAPig+cRV8oNw2Jp0mkn=CLTJERG+=O2pvewcjqF=xZ+dMgPnHA@mail.gmail.com","subject":"Re: [PATCH 10/16] worktree: make branch creation distinct from worktree population","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-12T10:03:29Z","receivedAt":"2015-07-12T10:03:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"(resend, +everybody)\n\nOn Sun, Jul 12, 2015 at 10:27 AM, Eric Sunshine wrote:\n>>> So, if I understand your concern correctly, then you are worried that,\n>>> following the git-branch invocation, ref_exists() could return the\n>>> wrong answer with a pluggable ref-backend since it might be answering\n>>> based upon stale information. Is that what you mean? If so, I can see\n>>> how that it could be an issue. (As far as I can tell, the current\n>>> file-based backend doesn't have a problem with this since it's hitting\n>>> the filesystem directly to answer the ref_exists() question.)\n>>\n>> I meant for this final sentence to end like this:\n>>\n>> ...to answer the ref_exists() question, but it still seems\n>> fragile since some future change could introduce caching.\n>\n> In this case, it's easy enough to side-step the issue since there's no\n> need to call ref_exists() if the new branch was created successfully\n> (since we know it exists). The logic would effectively become this:\n>\n> branch = ...\n> if (create_new_branch) {\n> exec \"git branch newbranch branch\"\n> exec \"git symbolic-ref HEAD newbranch\"\n> } else if (ref_exists(branch) && !detach)\n> exec \"git symbolic-ref HEAD branch\"\n> else\n> exec \"git update-ref HEAD $(git rev-parse branch)\"\n> exec \"git reset --hard\"\n\nYeah.. Another option we can take is deal with this at run-command.c\nlevel (and outside this series) because this could affect everywhere:\nby default, invalidate all cache after running any git commands. The\ncaller can pass options to keep some cache intact if they know the\ncommand won't touch it.\n\nIf ref_exists() is the only thing we use, right now it does not use\ncache so we should be safe. If the new ref backend introduces a cache,\nthey would need to examine all callers anyway, including this one. The\ncache in refs.c seems to be for for_each_ref.. only, which we don't\ncall here.\n-- \nDuy\n"},{"id":"266068","messageId":"20150712192016.GA4410@flurp.local","threadId":"39833","inReplyTo":"CACsJy8BTTdWrCZNz=y686pgju5X8-2mPrNNQ-+z4ByeKD6O5Uw@mail.gmail.com","subject":"Re: [PATCH 10/16] worktree: make branch creation distinct from worktree population","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-12T19:20:16Z","receivedAt":"2015-07-12T19:20:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jul 12, 2015 at 04:54:02PM +0700, Duy Nguyen wrote:\n> On Sun, Jul 12, 2015 at 10:27 AM, Eric Sunshine wrot> > In this case, it's easy enough to side-step the issue since there's no\n> > need to call ref_exists() if the new branch was created successfully\n> > (since we know it exists). The logic would effectively become this:\n> >\n> > branch = ...\n> > if (create_new_branch) {\n> > exec \"git branch newbranch branch\"\n> > exec \"git symbolic-ref HEAD newbranch\"\n> > } else if (ref_exists(branch) && !detach)\n> > exec \"git symbolic-ref HEAD branch\"\n> > else\n> > exec \"git update-ref HEAD $(git rev-parse branch)\"\n> > exec \"git reset --hard\"\n> \n> Yeah.. Another option we can take is deal with this at run-command.c\n> level (and outside this series) because this could affect everywhere:\n> by default, invalidate all cache after running any git commands. The\n> caller can pass options to keep some cache intact if they know the\n> command won't touch it.\n> \n> If ref_exists() is the only thing we use, right now it does not use\n> cache so we should be safe. If the new ref backend introduces a cache,\n> they would need to examine all callers anyway, including this one. The\n> cache in refs.c seems to be for for_each_ref.. only, which we don't\n> call here.\n\nIn case I don't need to re-roll the series for some other reason, here's\na squash-in making the above adjustment to patch 12/16, which Junio can\npick up if he wants to:\n\n---- 8< ----\nFrom: Eric Sunshine \nSubject: [PATCH] fixup! worktree: detect branch symref/detach and error conditions locally\n\nCurrent logic is effectively:\n\n branch = ...\n if (create_new_branch) {\n run \"git branch newbranch branch\"\n branch = newbranch\n }\n if (!detach && ref_exists(branch)) {\n if (!force)\n die_if_already_checked_out(branch)\n run \"git symbolic-ref HEAD branch\"\n } else\n run \"git update-ref HEAD $(git rev-parse branch)\"\n run \"git reset --hard\"\n\nThe ref_exists() call following 'run \"git branch newbranch branch\"'\nworks fine since ref_exists() hits the filesystem directly to answer the\nquestion rather than relying upon potentially stale cached information.\nHowever, this is potentially fragile if someone some day implements\ncaching. Moreover, with pluggable backends on the horizon, we may not\nbe able to rely upon ref_exists() in this process accurately reflecting\nchanges made by a subprocess.\n\nIf new branch creation was successful, then we know it's a branch, and\ndon't need to ask ref_exists() about it, thus we don't need to worry\nabout ref_exists() possibly returning an answer based upon stale\ninformation, nor do we have to check if the new branch is already\nchecked out elsewhere. Therefore, rework the logic slightly to become\neffectively:\n\n branch = ...\n if (create_new_branch) {\n run \"git branch newbranch branch\"\n run \"git symbolic-ref HEAD newbranch\"\n } else if (!detach && ref_exists(branch)) {\n if (!force)\n die_if_already_checked_out(branch)\n run \"git symbolic-ref HEAD branch\"\n } else\n run \"git update-ref HEAD $(git rev-parse branch)\"\n run \"git reset --hard\"\n\nSigned-off-by: Eric Sunshine \n---\n builtin/worktree.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 51c57bc..39f87a4 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -195,8 +195,10 @@ static int add_worktree(const char *path, const char *refname,\n \tif (file_exists(path) && !is_empty_dir(path))\n \t\tdie(_(\"'%s' already exists\"), path);\n \n-\tif (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n-\t ref_exists(symref.buf)) {\n+\tif (opts->force_new_branch)\n+\t\t;\n+\telse if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n+\t\t ref_exists(symref.buf)) {\n \t\tif (!opts->force)\n \t\t\tdie_if_checked_out(symref.buf);\n \t} else {\n-- \n2.5.0.rc1.387.g8463c8d\n"},{"id":"266087","messageId":"xmqqsi8rzyzo.fsf@gitster.dls.corp.google.com","threadId":"39833","inReplyTo":"1436573146-3893-1-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 00/16] worktree: use \"git reset --hard\" to populate worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-13T18:36:11Z","receivedAt":"2015-07-13T18:36:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> This is a follow-on series to [1], which migrated \"git checkout --to\"\n> functionality to \"git worktree add\". That series continued using \"git\n> checkout\" for the initial population of the new worktree, which required\n> git-checkout to have too intimate knowledge that it was operating in a\n> newly created worktree.\n>\n> This series eliminates git-checkout from the picture by instead\n> employing \"git reset --hard\"[2] to populate the new worktree initially.\n>\n> It is built atop 1eb07d8 (worktree: add: auto-vivify new branch when\n> is omitted, 2015-07-06), currently in 'next', which is\n> es/worktree-add except for the final patch (which retires\n> --ignore-other-worktrees) since the intention[3] was to drop that patch.\n\nA few comments on things I noticed while reading (mostly coming from\nthe original before this patch series):\n\n - What does this comment apply to?\n\n /*\n * $GIT_COMMON_DIR/HEAD is practically outside\n * $GIT_DIR so resolve_ref_unsafe() won't work (it\n * uses git_path). Parse the ref ourselves.\n */\n\n It appears in front of a call to check-linked-checkout, but I\n think the comment attempts to explain why it manually decides\n what the path should be in that function, so perhaps move it to\n the callee from the caller?\n\n - check_linked_checkout() when trying to decide what branch is\n checked out assumes HEAD is always a regular file, but I do not\n think we have dropped the support of SYMLINK_HEAD yet. It needs\n to check st_mode and readlink(2), like resolve_ref_unsafe() does.\n\n - After a new skelton worktree is set up, the code runs a few\n commands to finish populating it, under a different pair of\n GIT_DIR/GIT_WORK_TREE, but the function does so with setenv(); it\n may be cleaner to use cp.env[] for it, as the process we care\n about using the updated environment is not \"worktree add\" command\n we are running ourselves, but \"update-ref/symbolic-ref\" and\n \"reset\" commands that run in the new worktree.\n\nOther than that, looks nicely done.\n\nI however have to wonder if the stress on \"reset --hard\" on log\nmessages of various commits (and in the endgame) is somewhat\nmisplaced.\n\nThe primary thing we wanted to see, which this series nicely brings\nus, is to remove \"new-worktree-mode\" hack from \"checkout\" (in other\nwords, instead of \"reset --hard\", \"checkout -f\" would also have been\na satisfactory endgame).\n\nThanks.\n"},{"id":"266111","messageId":"55A4DC1C.90908@drmicha.warpmail.net","threadId":"39833","inReplyTo":"xmqqsi8rzyzo.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 00/16] worktree: use \"git reset --hard\" to populate worktree","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-07-14T09:53:32Z","receivedAt":"2015-07-14T09:53:32Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 13.07.2015 20:36:\n> Eric Sunshine writes:\n> \n>> This is a follow-on series to [1], which migrated \"git checkout --to\"\n>> functionality to \"git worktree add\". That series continued using \"git\n>> checkout\" for the initial population of the new worktree, which required\n>> git-checkout to have too intimate knowledge that it was operating in a\n>> newly created worktree.\n>>\n>> This series eliminates git-checkout from the picture by instead\n>> employing \"git reset --hard\"[2] to populate the new worktree initially.\n>>\n>> It is built atop 1eb07d8 (worktree: add: auto-vivify new branch when\n>> is omitted, 2015-07-06), currently in 'next', which is\n>> es/worktree-add except for the final patch (which retires\n>> --ignore-other-worktrees) since the intention[3] was to drop that patch.\n> \n> A few comments on things I noticed while reading (mostly coming from\n> the original before this patch series):\n> \n> - What does this comment apply to?\n> \n> /*\n> * $GIT_COMMON_DIR/HEAD is practically outside\n> * $GIT_DIR so resolve_ref_unsafe() won't work (it\n> * uses git_path). Parse the ref ourselves.\n> */\n> \n> It appears in front of a call to check-linked-checkout, but I\n> think the comment attempts to explain why it manually decides\n> what the path should be in that function, so perhaps move it to\n> the callee from the caller?\n> \n> - check_linked_checkout() when trying to decide what branch is\n> checked out assumes HEAD is always a regular file, but I do not\n> think we have dropped the support of SYMLINK_HEAD yet. It needs\n> to check st_mode and readlink(2), like resolve_ref_unsafe() does.\n> \n> - After a new skelton worktree is set up, the code runs a few\n> commands to finish populating it, under a different pair of\n> GIT_DIR/GIT_WORK_TREE, but the function does so with setenv(); it\n> may be cleaner to use cp.env[] for it, as the process we care\n> about using the updated environment is not \"worktree add\" command\n> we are running ourselves, but \"update-ref/symbolic-ref\" and\n> \"reset\" commands that run in the new worktree.\n> \n> Other than that, looks nicely done.\n> \n> I however have to wonder if the stress on \"reset --hard\" on log\n> messages of various commits (and in the endgame) is somewhat\n> misplaced.\n> \n> The primary thing we wanted to see, which this series nicely brings\n> us, is to remove \"new-worktree-mode\" hack from \"checkout\" (in other\n> words, instead of \"reset --hard\", \"checkout -f\" would also have been\n> a satisfactory endgame).\n> \n> Thanks.\n> \n\nRelated to that, I'm interested in \"worktree list\", and I'm wondering\nhow many more worktree commands we foresee, and therefore how much\nrefactoring should be done: Currently, the parsing of the contents of\n.../worktrees/ into worktree information is done right in prune-spcefic\nfunctions. They will have to be refactored. The following questions come\nto my mind:\n\n- Is a simple funtion refactoring enough, or should we introduce a\nworktree struct (and a list of such)?\n- Should each command do its own looping, or do we want\nfor_each_worktree() with a callback?\n- Is a fixed output format for \"list\"[1] enough, or do we want something\nlike for-each-ref or log formats (GSOC project...)?\n- Finally: Who will be stepping on whose toes doing this?\n\nMichael\n\n[1] Something like:\n\n* fooworktree (master)\n barworktree (HEAD detached from deadbeef)\n\nwith \"*\" denoting the worktree you're in (if any) and (optionally?)\nadding the \"on\" info from \"git branch\" in parentheses after each\nworktree (checked out branch name, or detached info). Maybe the path, too?\n"},{"id":"266113","messageId":"CACsJy8ADt=f0X+aGLJi2ozWVMmx6aNjOoiYXw9J-Km_O=TsNkQ@mail.gmail.com","threadId":"39833","inReplyTo":"55A4DC1C.90908@drmicha.warpmail.net","subject":"Re: [PATCH 00/16] worktree: use \"git reset --hard\" to populate worktree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-14T10:09:07Z","receivedAt":"2015-07-14T10:09:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jul 14, 2015 at 4:53 PM, Michael J Gruber\n wrote:\n> Related to that, I'm interested in \"worktree list\", and I'm wondering\n> how many more worktree commands we foresee, and therefore how much\n> refactoring should be done: Currently, the parsing of the contents of\n> .../worktrees/ into worktree information is done right in prune-spcefic\n> functions. They will have to be refactored. The following questions come\n> to my mind:\n>\n> - Is a simple funtion refactoring enough, or should we introduce a\n> worktree struct (and a list of such)?\n> - Should each command do its own looping, or do we want\n> for_each_worktree() with a callback?\n\nfhe for_each_xxx sounds like a good pattern to reuse.\n\n> - Is a fixed output format for \"list\"[1] enough, or do we want something\n> like for-each-ref or log formats (GSOC project...)?\n\nWe could stick to a fixed format for \"worktree list\" then introduce\n--format later if one output does not please everybody.\n\n> - Finally: Who will be stepping on whose toes doing this?\n\nI'm happy to step aside and let people implement it. I still have\nplenty of hanging topics to finish up :(\n-- \nDuy\n"},{"id":"266131","messageId":"CAPig+cS9RVvLd8+uY1CsJzFYmLsNn9S0Z=FLQvpLQYYDX0LiBw@mail.gmail.com","threadId":"39833","inReplyTo":"55A4DC1C.90908@drmicha.warpmail.net","subject":"Re: [PATCH 00/16] worktree: use \"git reset --hard\" to populate worktree","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-14T16:40:46Z","receivedAt":"2015-07-14T16:40:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jul 14, 2015 at 5:53 AM, Michael J Gruber\n wrote:\n>> Eric Sunshine writes:\n>>> This is a follow-on series to [1], which migrated \"git checkout --to\"\n>>> functionality to \"git worktree add\". That series continued using \"git\n>>> checkout\" for the initial population of the new worktree, which required\n>>> git-checkout to have too intimate knowledge that it was operating in a\n>>> newly created worktree.\n> Related to that, I'm interested in \"worktree list\", and I'm wondering\n> how many more worktree commands we foresee[...]\n\nThe ones I and others came up with (beyond 'add' and 'prune') are\n'list', 'remove', 'mv', and 'lock' (for locking and unlocking). I\nspecifically added a BUGS section to the documentation[1] as a\nreminder.\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/273431\n\n> , and therefore how much\n> refactoring should be done: Currently, the parsing of the contents of\n> .../worktrees/ into worktree information is done right in prune-spcefic\n> functions. They will have to be refactored. The following questions come\n> to my mind:\n>\n> - Is a simple funtion refactoring enough, or should we introduce a\n> worktree struct (and a list of such)?\n> - Should each command do its own looping, or do we want\n> for_each_worktree() with a callback?\n\nfor_each_worktree() might be overkill at this time, as I think only\n'prune' and 'list' would benefit directly. 'remove', 'lock', and 'mv'\nprobably just want to lookup a particular worktree (with 'mv', when\nrenaming, also possibly looking up the destination worktree to check\nif it already exist).\n\n> - Is a fixed output format for \"list\"[1] enough, or do we want something\n> like for-each-ref or log formats (GSOC project...)?\n> - Finally: Who will be stepping on whose toes doing this?\n\nI had considered working on some of the commands as time permits, but\ndon't currently have concrete plans to do so. You're welcome to jump\nin and tackle these ideas (but perhaps let us know, so toes don't get\ntrod upon).\n\n> [1] Something like:\n>\n> * fooworktree (master)\n> barworktree (HEAD detached from deadbeef)\n>\n> with \"*\" denoting the worktree you're in (if any) and (optionally?)\n\nConsidering that \"git worktree list\" can be invoked from the main\nworktree or any linked worktree, it would be a good idea to include\nthe main worktree in the output as well.\n\n> adding the \"on\" info from \"git branch\" in parentheses after each\n> worktree (checked out branch name, or detached info). Maybe the path, too?\n\nI also had something very much like this in mind, possibly extending\nthe output either with --verbose or custom format capability (keeping\nthe GSoC project in mind), but otherwise hadn't put much thought into\nit.\n\nShowing the path seems quite important, so I'd say yes to that\nquestion, probably by default.\n"},{"id":"266146","messageId":"CAPig+cTXAdGFmq+QkNAgmHJuZgM6-Ckg_-StrTQUxmJSpX4=Qg@mail.gmail.com","threadId":"39833","inReplyTo":"xmqqsi8rzyzo.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 00/16] worktree: use \"git reset --hard\" to populate worktree","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-15T06:48:17Z","receivedAt":"2015-07-15T06:48:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 13, 2015 at 2:36 PM, Junio C Hamano wrote:\n> Eric Sunshine writes:\n>> This series eliminates git-checkout from the picture by instead\n>> employing \"git reset --hard\"[2] to populate the new worktree initially.\n>\n> A few comments on things I noticed while reading (mostly coming from\n> the original before this patch series):\n>\n> - What does this comment apply to?\n>\n> /*\n> * $GIT_COMMON_DIR/HEAD is practically outside\n> * $GIT_DIR so resolve_ref_unsafe() won't work (it\n> * uses git_path). Parse the ref ourselves.\n> */\n>\n> It appears in front of a call to check-linked-checkout, but I\n> think the comment attempts to explain why it manually decides\n> what the path should be in that function, so perhaps move it to\n> the callee from the caller?\n\nThe placement of the comment in the original code wasn't bad, but\nafter patch 3/16 moves code around, the comment does become somewhat\nconfusing, so moving it to the callee seems a reasonable idea.\n\n> - check_linked_checkout() when trying to decide what branch is\n> checked out assumes HEAD is always a regular file, but I do not\n> think we have dropped the support of SYMLINK_HEAD yet. It needs\n> to check st_mode and readlink(2), like resolve_ref_unsafe() does.\n\nHmm, I wasn't aware of SYMLINK_HEAD (and don't know if Duy was). The\nrelated code in resolve_ref_unsafe() is fairly involved, worrying\nabout race conditions and such, however, I guess\ncheck_linked_checkout()'s implementation can perhaps be simpler, as\nit's probably far less catastrophic for it to give the wrong answer\n(or just die) under such a race?\n\n> - After a new skelton worktree is set up, the code runs a few\n> commands to finish populating it, under a different pair of\n> GIT_DIR/GIT_WORK_TREE, but the function does so with setenv(); it\n> may be cleaner to use cp.env[] for it, as the process we care\n> about using the updated environment is not \"worktree add\" command\n> we are running ourselves, but \"update-ref/symbolic-ref\" and\n> \"reset\" commands that run in the new worktree.\n\nAfter sending the series, I was realized that this could be done more\ncleanly with -C, but that would have to be repeated for each command,\nso cp.env[] might indeed be a better choice.\n\n> Other than that, looks nicely done.\n>\n> I however have to wonder if the stress on \"reset --hard\" on log\n> messages of various commits (and in the endgame) is somewhat\n> misplaced.\n>\n> The primary thing we wanted to see, which this series nicely brings\n> us, is to remove \"new-worktree-mode\" hack from \"checkout\" (in other\n> words, instead of \"reset --hard\", \"checkout -f\" would also have been\n> a satisfactory endgame).\n\nI'll see if the commit messages can be reworded a bit without becoming\ntoo wordy. (\"git reset --hard\" has a nice conciseness.)\n"},{"id":"266149","messageId":"CACsJy8DF_GXgmZWJ-1iOvoy6SD6kj6u=zprbQA9k6W-Ev6V38w@mail.gmail.com","threadId":"39833","inReplyTo":"CAPig+cTXAdGFmq+QkNAgmHJuZgM6-Ckg_-StrTQUxmJSpX4=Qg@mail.gmail.com","subject":"Re: [PATCH 00/16] worktree: use \"git reset --hard\" to populate worktree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-15T09:59:33Z","receivedAt":"2015-07-15T09:59:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jul 15, 2015 at 1:48 PM, Eric Sunshine wrote:\n>> - check_linked_checkout() when trying to decide what branch is\n>> checked out assumes HEAD is always a regular file, but I do not\n>> think we have dropped the support of SYMLINK_HEAD yet. It needs\n>> to check st_mode and readlink(2), like resolve_ref_unsafe() does.\n>\n> Hmm, I wasn't aware of SYMLINK_HEAD (and don't know if Duy was). The\n\nI'm aware of it. I just didn't remember it when I wrote this code.\n\n> related code in resolve_ref_unsafe() is fairly involved, worrying\n> about race conditions and such, however, I guess\n> check_linked_checkout()'s implementation can perhaps be simpler, as\n> it's probably far less catastrophic for it to give the wrong answer\n> (or just die) under such a race?\n\nAnd if I remember correctly Mike Haggerty had a series to refactor ref\nparsing code and reuse in this place (it was my promise to do it, but\nhe took over). I think the series was halted because refs.c was going\nthrough a major update at that time. I think we could leave it as is\nfor now and completely replace it at some point in future.\n-- \nDuy\n"}]}