{"thread":{"id":"40380","subject":"[PATCH v8 0/4] worktree: list functions and command","startedAt":"2015-09-18T13:30:23Z","lastAt":"2015-09-24T05:00:49Z","messageCount":11,"participants":["Michael Rappazzo","Junio C Hamano","Mike Rappazzo"],"isPatch":true,"patchVersion":8,"patchTotal":4},"messages":[{"id":"270286","messageId":"1442583027-47653-1-git-send-email-rappazzo@gmail.com","threadId":"40380","inReplyTo":null,"subject":"[PATCH v8 0/4] worktree: list functions and command","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-09-18T13:30:23Z","receivedAt":"2015-09-18T13:30:23Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Changes from v7[1]:\n\n\t- Reworked history to try to maintain blame from moving functions \n\t  from branch.c\n\n\t- Removed the worktree_list struct (linked list) and instead return\n\t  return an array of worktrees.  An alternative was also proposed to\n\t  include a `next` pointer in the worktree struct.  I coded this \n\t  option separately (found in my github fork[2]), but ultimately felt\n\t  that the array was more managable from a client perspective.\n\n\t- Removed the `--skip-bare` and `--verbose` command line options for \n\t  `worktree list`.\n\n\t- The default output of the includes all of the details which were\n\t  previously only included in verbose output.  Sample output:\n\n\t\t$git worktree list\n\t\t/Users/me/code/bare-git-source         (bare)\n\t\t/Users/me/code/worktree-git-from-bare  7ecec52 [master]\n\t\t/Users/me/code/wt2                     8681989 (detached HEAD)\n\n\t- Added `--porcelain` option to the worktree list command.  \n\n\t\t$git worktree list --porcelain\n\t\tworktree /Users/mike/code/bare-git-source\n\t\tbare\n\t\t\n\t\tworktree /Users/mike/code/worktree-git-from-bare\n\t\tbranch master\n\t\t\n\t\tworktree /Users/mike/code/wt2\n\t\tdetached at 8681989\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/277335\n[2]: https://github.com/git/git/compare/master...rappazzo:worktree-list/v8/next-entry-in-worktree\n\nMichael Rappazzo (4):\n  worktree: add top-level worktree.c\n  worktree: refactor find_linked_symref function\n  worktree: add functions to get worktree details\n  worktree: add 'list' command\n\n Documentation/git-worktree.txt |  15 ++-\n Makefile                       |   1 +\n branch.c                       |  79 +-------------\n branch.h                       |   8 --\n builtin/notes.c                |   1 +\n builtin/worktree.c             |  78 ++++++++++++++\n t/t2027-worktree-list.sh       |  86 +++++++++++++++\n worktree.c                     | 238 +++++++++++++++++++++++++++++++++++++++++\n worktree.h                     |  38 +++++++\n 9 files changed, 457 insertions(+), 87 deletions(-)\n create mode 100755 t/t2027-worktree-list.sh\n create mode 100644 worktree.c\n create mode 100644 worktree.h\n\n-- \n2.5.0\n"},{"id":"270290","messageId":"1442583027-47653-2-git-send-email-rappazzo@gmail.com","threadId":"40380","inReplyTo":"1442583027-47653-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v8 1/4] worktree: add top-level worktree.c","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-09-18T13:30:24Z","receivedAt":"2015-09-18T13:30:24Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"worktree.c contains functions to work with and get information from\nworktrees.  This introduction moves functions related to worktrees\nfrom branch.c into worktree.c\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n Makefile        |  1 +\n branch.c        | 79 +-----------------------------------------------------\n branch.h        |  8 ------\n builtin/notes.c |  1 +\n worktree.c      | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n worktree.h      | 12 +++++++++\n 6 files changed, 97 insertions(+), 86 deletions(-)\n create mode 100644 worktree.c\n create mode 100644 worktree.h\n\ndiff --git a/Makefile b/Makefile\nindex 8d5df7e..f4ee2d2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -807,6 +807,7 @@ LIB_OBJS += version.o\n LIB_OBJS += versioncmp.o\n LIB_OBJS += walker.o\n LIB_OBJS += wildmatch.o\n+LIB_OBJS += worktree.o\n LIB_OBJS += wrapper.o\n LIB_OBJS += write_or_die.o\n LIB_OBJS += ws.o\ndiff --git a/branch.c b/branch.c\nindex d013374..77d7f2a 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -4,6 +4,7 @@\n #include \"refs.h\"\n #include \"remote.h\"\n #include \"commit.h\"\n+#include \"worktree.h\"\n \n struct tracking {\n \tstruct refspec spec;\n@@ -311,84 +312,6 @@ void remove_branch_state(void)\n \tunlink(git_path_squash_msg());\n }\n \n-static char *find_linked_symref(const char *symref, const char *branch,\n-\t\t\t\tconst char *id)\n-{\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf gitdir = STRBUF_INIT;\n-\tchar *existing = NULL;\n-\n-\t/*\n-\t * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside\n-\t * $GIT_DIR so resolve_ref_unsafe() won't work (it uses\n-\t * git_path). Parse the ref ourselves.\n-\t */\n-\tif (id)\n-\t\tstrbuf_addf(&path, \"%s/worktrees/%s/%s\", get_git_common_dir(), id, symref);\n-\telse\n-\t\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), symref);\n-\n-\tif (!strbuf_readlink(&sb, path.buf, 0)) {\n-\t\tif (!starts_with(sb.buf, \"refs/\") ||\n-\t\t    check_refname_format(sb.buf, 0))\n-\t\t\tgoto done;\n-\t} else if (strbuf_read_file(&sb, path.buf, 0) >= 0 &&\n-\t    starts_with(sb.buf, \"ref:\")) {\n-\t\tstrbuf_remove(&sb, 0, strlen(\"ref:\"));\n-\t\tstrbuf_trim(&sb);\n-\t} else\n-\t\tgoto done;\n-\tif (strcmp(sb.buf, branch))\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-\tstrbuf_strip_suffix(&gitdir, \".git\");\n-\n-\texisting = strbuf_detach(&gitdir, NULL);\n-done:\n-\tstrbuf_release(&path);\n-\tstrbuf_release(&sb);\n-\tstrbuf_release(&gitdir);\n-\n-\treturn existing;\n-}\n-\n-char *find_shared_symref(const char *symref, const char *target)\n-{\n-\tstruct strbuf path = STRBUF_INIT;\n-\tDIR *dir;\n-\tstruct dirent *d;\n-\tchar *existing;\n-\n-\tif ((existing = find_linked_symref(symref, target, NULL)))\n-\t\treturn existing;\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 NULL;\n-\n-\twhile ((d = readdir(dir)) != NULL) {\n-\t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n-\t\t\tcontinue;\n-\t\texisting = find_linked_symref(symref, target, d->d_name);\n-\t\tif (existing)\n-\t\t\tgoto done;\n-\t}\n-done:\n-\tclosedir(dir);\n-\n-\treturn existing;\n-}\n-\n void die_if_checked_out(const char *branch)\n {\n \tchar *existing;\ndiff --git a/branch.h b/branch.h\nindex d3446ed..58aa45f 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -59,12 +59,4 @@ extern int read_branch_desc(struct strbuf *, const char *branch_name);\n  */\n extern void die_if_checked_out(const char *branch);\n \n-/*\n- * Check if a per-worktree symref points to a ref in the main worktree\n- * or any linked worktree, and return the path to the exising worktree\n- * if it is.  Returns NULL if there is no existing ref.  The caller is\n- * responsible for freeing the returned path.\n- */\n-extern char *find_shared_symref(const char *symref, const char *target);\n-\n #endif\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 3608c64..8b30334 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -20,6 +20,7 @@\n #include \"notes-merge.h\"\n #include \"notes-utils.h\"\n #include \"branch.h\"\n+#include \"worktree.h\"\n \n static const char * const git_notes_usage[] = {\n \tN_(\"git notes [--ref <notes-ref>] [list [<object>]]\"),\ndiff --git a/worktree.c b/worktree.c\nnew file mode 100644\nindex 0000000..10e1496\n--- /dev/null\n+++ b/worktree.c\n@@ -0,0 +1,82 @@\n+#include \"cache.h\"\n+#include \"refs.h\"\n+#include \"strbuf.h\"\n+#include \"worktree.h\"\n+\n+static char *find_linked_symref(const char *symref, const char *branch,\n+\t\t\t\tconst char *id)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf gitdir = STRBUF_INIT;\n+\tchar *existing = NULL;\n+\n+\t/*\n+\t * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside\n+\t * $GIT_DIR so resolve_ref_unsafe() won't work (it uses\n+\t * git_path). Parse the ref ourselves.\n+\t */\n+\tif (id)\n+\t\tstrbuf_addf(&path, \"%s/worktrees/%s/%s\", get_git_common_dir(), id, symref);\n+\telse\n+\t\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), symref);\n+\n+\tif (!strbuf_readlink(&sb, path.buf, 0)) {\n+\t\tif (!starts_with(sb.buf, \"refs/\") ||\n+\t\t    check_refname_format(sb.buf, 0))\n+\t\t\tgoto done;\n+\t} else if (strbuf_read_file(&sb, path.buf, 0) >= 0 &&\n+\t    starts_with(sb.buf, \"ref:\")) {\n+\t\tstrbuf_remove(&sb, 0, strlen(\"ref:\"));\n+\t\tstrbuf_trim(&sb);\n+\t} else\n+\t\tgoto done;\n+\tif (strcmp(sb.buf, branch))\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+\tstrbuf_strip_suffix(&gitdir, \".git\");\n+\n+\texisting = strbuf_detach(&gitdir, NULL);\n+done:\n+\tstrbuf_release(&path);\n+\tstrbuf_release(&sb);\n+\tstrbuf_release(&gitdir);\n+\n+\treturn existing;\n+}\n+\n+char *find_shared_symref(const char *symref, const char *target)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tDIR *dir;\n+\tstruct dirent *d;\n+\tchar *existing;\n+\n+\tif ((existing = find_linked_symref(symref, target, NULL)))\n+\t\treturn existing;\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 NULL;\n+\n+\twhile ((d = readdir(dir)) != NULL) {\n+\t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n+\t\t\tcontinue;\n+\t\texisting = find_linked_symref(symref, target, d->d_name);\n+\t\tif (existing)\n+\t\t\tgoto done;\n+\t}\n+done:\n+\tclosedir(dir);\n+\n+\treturn existing;\n+}\ndiff --git a/worktree.h b/worktree.h\nnew file mode 100644\nindex 0000000..71b1409\n--- /dev/null\n+++ b/worktree.h\n@@ -0,0 +1,12 @@\n+#ifndef WORKTREE_H\n+#define WORKTREE_H\n+\n+/*\n+ * Check if a per-worktree symref points to a ref in the main worktree\n+ * or any linked worktree, and return the path to the exising worktree\n+ * if it is.  Returns NULL if there is no existing ref.  The caller is\n+ * responsible for freeing the returned path.\n+ */\n+extern char *find_shared_symref(const char *symref, const char *target);\n+\n+#endif\n-- \n2.5.0\n"},{"id":"270289","messageId":"1442583027-47653-3-git-send-email-rappazzo@gmail.com","threadId":"40380","inReplyTo":"1442583027-47653-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v8 2/4] worktree: refactor find_linked_symref function","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-09-18T13:30:25Z","receivedAt":"2015-09-18T13:30:25Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Refactoring will help transition this code to provide additional useful\nworktree functions.\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n worktree.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++----------------\n 1 file changed, 64 insertions(+), 22 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex 10e1496..5c75875 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -3,6 +3,60 @@\n #include \"strbuf.h\"\n #include \"worktree.h\"\n \n+/*\n+ * read 'path_to_ref' into 'ref'.  Also if is_detached is not NULL,\n+ * set is_detached to 1 if the ref is detatched.\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\tif (!starts_with(ref->buf, \"refs/\")\n+\t\t\t\t|| check_refname_format(ref->buf, 0))\n+\t\t\treturn -1;\n+\n+\t} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {\n+\t\tif (starts_with(ref->buf, \"ref:\")) {\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} else if (is_detached)\n+\t\t\t*is_detached = 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static char *find_main_symref(const char *symref, const char *branch)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf gitdir = STRBUF_INIT;\n+\tchar *existing = NULL;\n+\n+\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), symref);\n+\tif (parse_ref(path.buf, &sb, NULL) == -1)\n+\t\tgoto done;\n+\tif (strcmp(sb.buf, branch))\n+\t\tgoto done;\n+\tstrbuf_addstr(&gitdir, get_git_common_dir());\n+\tstrbuf_strip_suffix(&gitdir, \".git\");\n+\texisting = strbuf_detach(&gitdir, NULL);\n+done:\n+\tstrbuf_release(&path);\n+\tstrbuf_release(&sb);\n+\tstrbuf_release(&gitdir);\n+\n+\treturn existing;\n+}\n+\n static char *find_linked_symref(const char *symref, const char *branch,\n \t\t\t\tconst char *id)\n {\n@@ -11,36 +65,24 @@ static char *find_linked_symref(const char *symref, const char *branch,\n \tstruct strbuf gitdir = STRBUF_INIT;\n \tchar *existing = NULL;\n \n+\tif (!id)\n+\t\tgoto done;\n \t/*\n \t * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside\n \t * $GIT_DIR so resolve_ref_unsafe() won't work (it uses\n \t * git_path). Parse the ref ourselves.\n \t */\n-\tif (id)\n-\t\tstrbuf_addf(&path, \"%s/worktrees/%s/%s\", get_git_common_dir(), id, symref);\n-\telse\n-\t\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), symref);\n+\tstrbuf_addf(&path, \"%s/worktrees/%s/%s\", get_git_common_dir(), id, symref);\n \n-\tif (!strbuf_readlink(&sb, path.buf, 0)) {\n-\t\tif (!starts_with(sb.buf, \"refs/\") ||\n-\t\t    check_refname_format(sb.buf, 0))\n-\t\t\tgoto done;\n-\t} else if (strbuf_read_file(&sb, path.buf, 0) >= 0 &&\n-\t    starts_with(sb.buf, \"ref:\")) {\n-\t\tstrbuf_remove(&sb, 0, strlen(\"ref:\"));\n-\t\tstrbuf_trim(&sb);\n-\t} else\n+\tif (parse_ref(path.buf, &sb, NULL) == -1)\n \t\tgoto done;\n \tif (strcmp(sb.buf, branch))\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+\tstrbuf_reset(&path);\n+\tstrbuf_addf(&path, \"%s/worktrees/%s/gitdir\", get_git_common_dir(), id);\n+\tif (strbuf_read_file(&gitdir, path.buf, 0) <= 0)\n+\t\tgoto done;\n+\tstrbuf_rtrim(&gitdir);\n \tstrbuf_strip_suffix(&gitdir, \".git\");\n \n \texisting = strbuf_detach(&gitdir, NULL);\n@@ -59,7 +101,7 @@ char *find_shared_symref(const char *symref, const char *target)\n \tstruct dirent *d;\n \tchar *existing;\n \n-\tif ((existing = find_linked_symref(symref, target, NULL)))\n+\tif ((existing = find_main_symref(symref, target)))\n \t\treturn existing;\n \n \tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n-- \n2.5.0\n"},{"id":"270287","messageId":"1442583027-47653-4-git-send-email-rappazzo@gmail.com","threadId":"40380","inReplyTo":"1442583027-47653-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v8 3/4] worktree: add functions to get worktree details","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-09-18T13:30:26Z","receivedAt":"2015-09-18T13:30:26Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"The worktree structure provided for an individual worktree includes the\nabsolute path, the location of the git dir, the head ref (if not\ndetached), the head revision sha1, whether or not head is detached, and\nwhether or not the worktree is a bare repo.\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n worktree.c | 224 ++++++++++++++++++++++++++++++++++++++++++++++---------------\n worktree.h |  26 +++++++\n 2 files changed, 195 insertions(+), 55 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex 5c75875..74a2ffa 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -3,6 +3,17 @@\n #include \"strbuf.h\"\n #include \"worktree.h\"\n \n+void free_worktrees(struct worktree **worktrees)\n+{\n+\tfor (int i = 0; worktrees[i]; i++) {\n+\t\tfree(worktrees[i]->path);\n+\t\tfree(worktrees[i]->git_dir);\n+\t\tfree(worktrees[i]->head_ref);\n+\t\tfree(worktrees[i]);\n+\t}\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 if the ref is detatched.\n@@ -34,91 +45,194 @@ static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)\n \treturn 0;\n }\n \n-static char *find_main_symref(const char *symref, const char *branch)\n+/**\n+ * Add the head_sha1 and head_ref (if not detached) to the given worktree\n+ */\n+static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)\n {\n-\tstruct strbuf sb = STRBUF_INIT;\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+}\n+\n+/**\n+ * get the main worktree\n+ */\n+static struct worktree *get_main_worktree()\n+{\n+\tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf gitdir = STRBUF_INIT;\n-\tchar *existing = NULL;\n+\tstruct strbuf worktree_path = STRBUF_INIT;\n+\tstruct strbuf git_dir = STRBUF_INIT;\n+\tstruct strbuf head_ref = STRBUF_INIT;\n+\tint is_bare = 0;\n+\tint is_detached = 0;\n \n-\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), symref);\n-\tif (parse_ref(path.buf, &sb, NULL) == -1)\n-\t\tgoto done;\n-\tif (strcmp(sb.buf, branch))\n-\t\tgoto done;\n-\tstrbuf_addstr(&gitdir, get_git_common_dir());\n-\tstrbuf_strip_suffix(&gitdir, \".git\");\n-\texisting = strbuf_detach(&gitdir, NULL);\n-done:\n-\tstrbuf_release(&path);\n-\tstrbuf_release(&sb);\n-\tstrbuf_release(&gitdir);\n+\tstrbuf_addf(&git_dir, \"%s\", absolute_path(get_git_common_dir()));\n+\tstrbuf_addf(&worktree_path, \"%s\", absolute_path(get_git_common_dir()));\n+\tis_bare = !strbuf_strip_suffix(&worktree_path, \"/.git\");\n+\tif (is_bare)\n+\t\tstrbuf_strip_suffix(&worktree_path, \"/.\");\n \n-\treturn existing;\n+\tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n+\n+\tif (!parse_ref(path.buf, &head_ref, &is_detached)) {\n+\t\tworktree = xmalloc(sizeof(struct worktree));\n+\t\tworktree->path = strbuf_detach(&worktree_path, NULL);\n+\t\tworktree->git_dir = strbuf_detach(&git_dir, NULL);\n+\t\tworktree->is_bare = is_bare;\n+\t\tworktree->head_ref = NULL;\n+\t\tworktree->is_detached = is_detached;\n+\t\tadd_head_info(&head_ref, worktree);\n+\t}\n+\treturn worktree;\n }\n \n-static char *find_linked_symref(const char *symref, const char *branch,\n-\t\t\t\tconst char *id)\n+static struct worktree *get_linked_worktree(const char *id)\n {\n-\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct worktree *worktree = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf gitdir = STRBUF_INIT;\n-\tchar *existing = NULL;\n+\tstruct strbuf worktree_path = STRBUF_INIT;\n+\tstruct strbuf git_dir = STRBUF_INIT;\n+\tstruct strbuf head_ref = STRBUF_INIT;\n+\tint is_detached = 0;\n \n \tif (!id)\n \t\tgoto done;\n-\t/*\n-\t * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside\n-\t * $GIT_DIR so resolve_ref_unsafe() won't work (it uses\n-\t * git_path). Parse the ref ourselves.\n-\t */\n-\tstrbuf_addf(&path, \"%s/worktrees/%s/%s\", get_git_common_dir(), id, symref);\n-\n-\tif (parse_ref(path.buf, &sb, NULL) == -1)\n-\t\tgoto done;\n-\tif (strcmp(sb.buf, branch))\n+\n+\tstrbuf_addf(&git_dir, \"%s/worktrees/%s\",\n+\t\t\tabsolute_path(get_git_common_dir()), id);\n+\tstrbuf_addf(&path, \"%s/gitdir\", git_dir.buf);\n+\tif (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)\n+\t\t/* invalid gitdir file */\n \t\tgoto done;\n+\n+\tstrbuf_rtrim(&worktree_path);\n+\tif (!strbuf_strip_suffix(&worktree_path, \"/.git\")) {\n+\t\tstrbuf_reset(&worktree_path);\n+\t\tstrbuf_addstr(&worktree_path, absolute_path(\".\"));\n+\t\tstrbuf_strip_suffix(&worktree_path, \"/.\");\n+\t}\n+\n \tstrbuf_reset(&path);\n-\tstrbuf_addf(&path, \"%s/worktrees/%s/gitdir\", get_git_common_dir(), id);\n-\tif (strbuf_read_file(&gitdir, path.buf, 0) <= 0)\n+\tstrbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n+\n+\tif (parse_ref(path.buf, &head_ref, &is_detached))\n \t\tgoto done;\n-\tstrbuf_rtrim(&gitdir);\n-\tstrbuf_strip_suffix(&gitdir, \".git\");\n \n-\texisting = strbuf_detach(&gitdir, NULL);\n+\tworktree = xmalloc(sizeof(struct worktree));\n+\tworktree->path = strbuf_detach(&worktree_path, NULL);\n+\tworktree->git_dir = strbuf_detach(&git_dir, NULL);\n+\tworktree->is_bare = 0;\n+\tworktree->head_ref = NULL;\n+\tworktree->is_detached = is_detached;\n+\tadd_head_info(&head_ref, worktree);\n+\n done:\n \tstrbuf_release(&path);\n-\tstrbuf_release(&sb);\n-\tstrbuf_release(&gitdir);\n+\tstrbuf_release(&git_dir);\n+\tstrbuf_release(&head_ref);\n+\tstrbuf_release(&worktree_path);\n+\treturn worktree;\n+}\n \n-\treturn existing;\n+/**\n+ * get the estimated worktree count for use in sizing the worktree array\n+ * Note that the minimum count should be 2 (main worktree + NULL).  Using the\n+ * file count in $GIT_COMMON_DIR/worktrees includes '.' and '..' so the\n+ * minimum is satisfied by counting those entries.\n+ */\n+static int get_estimated_worktree_count()\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tDIR *dir;\n+\tint count = 0;\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\twhile (readdir(dir))\n+\t\t\tcount++;\n+\t\tclosedir(dir);\n+\t}\n+\n+\tif (!count)\n+\t\tcount = 2;\n+\n+\treturn count;\n }\n \n-char *find_shared_symref(const char *symref, const char *target)\n+struct worktree **get_worktrees(void)\n {\n+\tstruct worktree **list = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n \tDIR *dir;\n \tstruct dirent *d;\n-\tchar *existing;\n+\tint counter = 0;\n+\n+\tlist = xmalloc(get_estimated_worktree_count() * sizeof(struct worktree *));\n \n-\tif ((existing = find_main_symref(symref, target)))\n-\t\treturn existing;\n+\tif ((list[counter] = get_main_worktree()))\n+\t\tcounter++;\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 NULL;\n-\n-\twhile ((d = readdir(dir)) != NULL) {\n-\t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n-\t\t\tcontinue;\n-\t\texisting = find_linked_symref(symref, target, d->d_name);\n-\t\tif (existing)\n-\t\t\tgoto done;\n+\tif (dir) {\n+\t\twhile ((d = readdir(dir)) != NULL) {\n+\t\t\tstruct worktree *linked = NULL;\n+\t\t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n+\t\t\t\tcontinue;\n+\n+\t\t\tif ((linked = get_linked_worktree(d->d_name))) {\n+\t\t\t\tlist[counter++] = linked;\n+\t\t\t}\n+\t\t}\n+\t\tclosedir(dir);\n \t}\n-done:\n-\tclosedir(dir);\n+\tlist[counter] = NULL;\n+\treturn list;\n+}\n+\n+char *find_shared_symref(const char *symref, const char *target)\n+{\n+\tchar *existing = NULL;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct worktree **worktrees = get_worktrees();\n+\tint symref_is_head = !strcmp(\"HEAD\", symref);\n+\n+\tfor (int i = 0; worktrees[i]; i++) {\n+\t\tif (!symref_is_head) {\n+\t\t\tstrbuf_reset(&path);\n+\t\t\tstrbuf_reset(&sb);\n+\t\t\tstrbuf_addf(&path, \"%s/%s\", worktrees[i]->git_dir, symref);\n+\n+\t\t\tif (parse_ref(path.buf, &sb, NULL)) {\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\tif (!strcmp(sb.buf, target)) {\n+\t\t\t\texisting = xstrdup(worktrees[i]->path);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tif (worktrees[i]->head_ref && !strcmp(worktrees[i]->head_ref, target)) {\n+\t\t\t\texisting = xstrdup(worktrees[i]->path);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tstrbuf_release(&path);\n+\tstrbuf_release(&sb);\n+\tfree_worktrees(worktrees);\n \n \treturn existing;\n }\ndiff --git a/worktree.h b/worktree.h\nindex 71b1409..b4b3dda 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -1,6 +1,32 @@\n #ifndef WORKTREE_H\n #define WORKTREE_H\n \n+struct worktree {\n+\tchar *path;\n+\tchar *git_dir;\n+\tchar *head_ref;\n+\tunsigned char head_sha1[20];\n+\tint is_detached;\n+\tint is_bare;\n+};\n+\n+/* Functions for acting on the information about worktrees. */\n+\n+/*\n+ * Get the worktrees.  The primary worktree will always be the first returned,\n+ * and linked worktrees will be pointed to by 'next' in each subsequent\n+ * worktree.  No specific ordering is done on the linked worktrees.\n+ *\n+ * The caller is responsible for freeing the memory from the returned\n+ * worktree(s).\n+ */\n+extern struct worktree **get_worktrees(void);\n+\n+/*\n+ * Free up the memory for worktree(s)\n+ */\n+extern void free_worktrees(struct worktree **);\n+\n /*\n  * Check if a per-worktree symref points to a ref in the main worktree\n  * or any linked worktree, and return the path to the exising worktree\n-- \n2.5.0\n"},{"id":"270288","messageId":"1442583027-47653-5-git-send-email-rappazzo@gmail.com","threadId":"40380","inReplyTo":"1442583027-47653-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v8 4/4] worktree: add 'list' command","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-09-18T13:30:27Z","receivedAt":"2015-09-18T13:30:27Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"'git worktree list' iterates through the worktree list, and outputs\ndetails of the worktree including the path to the worktree, the currently\nchecked out revision and branch, and if the work tree is bare.  There is\nalso porcelain format option available.\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n Documentation/git-worktree.txt | 15 +++++++-\n builtin/worktree.c             | 78 ++++++++++++++++++++++++++++++++++++++\n t/t2027-worktree-list.sh       | 86 ++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 178 insertions(+), 1 deletion(-)\n create mode 100755 t/t2027-worktree-list.sh\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex fb68156..8ee65c6 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -11,6 +11,7 @@ SYNOPSIS\n [verse]\n 'git worktree add' [-f] [--detach] [-b <new-branch>] <path> [<branch>]\n 'git worktree prune' [-n] [-v] [--expire <expire>]\n+'git worktree list' [--porcelain]\n \n DESCRIPTION\n -----------\n@@ -59,6 +60,13 @@ prune::\n \n Prune working tree information in $GIT_DIR/worktrees.\n \n+list::\n+\n+List details of each worktree.  The main worktree is listed first, followed by\n+each of the linked worktrees.  The output details include if the worktree is\n+bare, the revision currently checked out, and the branch currently checked out\n+(or 'detached HEAD' if none).\n+\n OPTIONS\n -------\n \n@@ -86,6 +94,11 @@ OPTIONS\n \tWith `prune`, do not remove anything; just report what it would\n \tremove.\n \n+--porcelain::\n+\tWith `list`, output in an easy-to-parse format for scripts.\n+\tThis format will remain stable across Git versions and regardless of user\n+\tconfiguration.\n+\n -v::\n --verbose::\n \tWith `prune`, report all removals.\n@@ -93,6 +106,7 @@ OPTIONS\n --expire <time>::\n \tWith `prune`, only expire unused working trees older than <time>.\n \n+\n DETAILS\n -------\n Each linked working tree has a private sub-directory in the repository's\n@@ -167,7 +181,6 @@ performed manually, such as:\n - `remove` to remove a linked working tree and its administrative files (and\n   warn if the working tree is dirty)\n - `mv` to move or rename a working tree and update its administrative files\n-- `list` to list linked working trees\n - `lock` to prevent automatic pruning of administrative files (for instance,\n   for a working tree on a portable device)\n \ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 71bb770..e6e36ac 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -8,10 +8,13 @@\n #include \"run-command.h\"\n #include \"sigchain.h\"\n #include \"refs.h\"\n+#include \"utf8.h\"\n+#include \"worktree.h\"\n \n static const char * const worktree_usage[] = {\n \tN_(\"git worktree add [<options>] <path> <branch>\"),\n \tN_(\"git worktree prune [<options>]\"),\n+\tN_(\"git worktree list [<options>]\"),\n \tNULL\n };\n \n@@ -359,6 +362,79 @@ static int add(int ac, const char **av, const char *prefix)\n \treturn add_worktree(path, branch, &opts);\n }\n \n+static void show_worktree_porcelain(struct worktree *worktree)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tstrbuf_addf(&sb, \"worktree %s\\n\", worktree->path);\n+\tif (worktree->is_bare)\n+\t\tstrbuf_addstr(&sb, \"bare\");\n+\telse {\n+\t\tif (worktree->is_detached)\n+\t\t\tstrbuf_addf(&sb, \"detached at %s\", find_unique_abbrev(worktree->head_sha1, DEFAULT_ABBREV));\n+\t\telse\n+\t\t\tstrbuf_addf(&sb, \"branch %s\", shorten_unambiguous_ref(worktree->head_ref, 0));\n+\t}\n+\n+\tprintf(\"%s\\n\\n\", sb.buf);\n+\n+\tstrbuf_release(&sb);\n+}\n+static void show_worktree(struct worktree *worktree, int path_maxlen)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tint cur_len = strlen(worktree->path);\n+\tint utf8_adj = cur_len - utf8_strwidth(worktree->path);\n+\tstrbuf_addf(&sb, \"%-*s \", 1 + path_maxlen + utf8_adj, worktree->path);\n+\tif (worktree->is_bare)\n+\t\tstrbuf_addstr(&sb, \"(bare)\");\n+\telse {\n+\t\tstrbuf_addf(&sb, \"%s \", find_unique_abbrev(worktree->head_sha1, DEFAULT_ABBREV));\n+\t\tif (!worktree->is_detached)\n+\t\t\tstrbuf_addf(&sb, \"[%s]\", shorten_unambiguous_ref(worktree->head_ref, 0));\n+\t\telse\n+\t\t\tstrbuf_addstr(&sb, \"(detached HEAD)\");\n+\t}\n+\tprintf(\"%s\\n\", sb.buf);\n+\n+\tstrbuf_release(&sb);\n+}\n+\n+static int list(int ac, const char **av, const char *prefix)\n+{\n+\tint porcelain = 0;\n+\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"porcelain\", &porcelain, N_(\"machine-readable output\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tac = parse_options(ac, av, prefix, options, worktree_usage, 0);\n+\tif (ac)\n+\t\tusage_with_options(worktree_usage, options);\n+\telse {\n+\t\tstruct worktree **worktrees = get_worktrees();\n+\t\tint path_maxlen = 0;\n+\n+\t\tif (!porcelain) {\n+\t\t\tfor (int i = 0; worktrees[i]; i++) {\n+\t\t\t\tint len = strlen(worktrees[i]->path);\n+\t\t\t\tif (len > path_maxlen)\n+\t\t\t\t\tpath_maxlen = len;\n+\t\t\t}\n+\t\t}\n+\t\tfor (int i = 0; worktrees[i]; i++) {\n+\t\t\tif (porcelain)\n+\t\t\t\tshow_worktree_porcelain(worktrees[i]);\n+\t\t\telse\n+\t\t\t\tshow_worktree(worktrees[i], path_maxlen);\n+\t\t}\n+\t\tfree_worktrees(worktrees);\n+\t}\n+\treturn 0;\n+}\n+\n int cmd_worktree(int ac, const char **av, const char *prefix)\n {\n \tstruct option options[] = {\n@@ -371,5 +447,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)\n \t\treturn add(ac - 1, av + 1, prefix);\n \tif (!strcmp(av[1], \"prune\"))\n \t\treturn prune(ac - 1, av + 1, prefix);\n+\tif (!strcmp(av[1], \"list\"))\n+\t\treturn list(ac - 1, av + 1, prefix);\n \tusage_with_options(worktree_usage, options);\n }\ndiff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\nnew file mode 100755\nindex 0000000..b68dfb4\n--- /dev/null\n+++ b/t/t2027-worktree-list.sh\n@@ -0,0 +1,86 @@\n+#!/bin/sh\n+\n+test_description='test git worktree list'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_commit init\n+'\n+\n+test_expect_success '\"list\" all worktrees from main' '\n+\techo \"$(git rev-parse --show-toplevel)       $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n+\tgit worktree add --detach here master &&\n+\ttest_when_finished \"rm -rf here && git worktree prune\" &&\n+\techo \"$(git -C here rev-parse --show-toplevel)  $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n+\tgit worktree list >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '\"list\" all worktrees from linked' '\n+\techo \"$(git rev-parse --show-toplevel)       $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n+\tgit worktree add --detach here master &&\n+\ttest_when_finished \"rm -rf here && git worktree prune\" &&\n+\techo \"$(git -C here rev-parse --show-toplevel)  $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n+\tgit -C here worktree list >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '\"list\" all worktrees --porcelain' '\n+\techo \"worktree $(git rev-parse --show-toplevel)\" >expect &&\n+\techo \"branch $(git symbolic-ref --short HEAD)\" >>expect &&\n+\techo >>expect &&\n+\tgit worktree add --detach here master &&\n+\ttest_when_finished \"rm -rf here && git worktree prune\" &&\n+\techo \"worktree $(git -C here rev-parse --show-toplevel)\" >>expect &&\n+\techo \"detached at $(git rev-parse --short HEAD)\" >>expect &&\n+\techo >>expect &&\n+\tgit worktree list --porcelain >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bare repo setup' '\n+\tgit init --bare bare1 &&\n+\techo \"data\" >file1 &&\n+\tgit add file1 &&\n+\tgit commit -m\"File1: add data\" &&\n+\tgit push bare1 master &&\n+\tgit reset --hard HEAD^\n+'\n+\n+test_expect_success '\"list\" all worktrees from bare main' '\n+\tgit -C bare1 worktree add --detach ../there master &&\n+\ttest_when_finished \"rm -rf there && git -C bare1 worktree prune\" &&\n+\techo \"$(pwd)/bare1  (bare)\" >expect &&\n+\techo \"$(git -C there rev-parse --show-toplevel)  $(git -C there rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n+\tgit -C bare1 worktree list >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '\"list\" all worktrees --porcelain from bare main' '\n+\tgit -C bare1 worktree add --detach ../there master &&\n+\ttest_when_finished \"rm -rf there && git -C bare1 worktree prune\" &&\n+\techo \"worktree $(pwd)/bare1\" >expect &&\n+\techo \"bare\" >>expect &&\n+\techo >>expect &&\n+\techo \"worktree $(git -C there rev-parse --show-toplevel)\" >>expect &&\n+\techo \"detached at $(git -C there rev-parse --short HEAD)\" >>expect &&\n+\techo >>expect &&\n+\tgit -C bare1 worktree list --porcelain >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '\"list\" all worktrees from linked with a bare main' '\n+\tgit -C bare1 worktree add --detach ../there master &&\n+\ttest_when_finished \"rm -rf there && git -C bare1 worktree prune\" &&\n+\techo \"$(pwd)/bare1  (bare)\" >expect &&\n+\techo \"$(git -C there rev-parse --show-toplevel)  $(git -C there rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n+\tgit -C there worktree list >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bare repo cleanup' '\n+\trm -rf bare1\n+'\n+\n+test_done\n-- \n2.5.0\n"},{"id":"270496","messageId":"xmqqr3lq4bo0.fsf@gitster.mtv.corp.google.com","threadId":"40380","inReplyTo":"1442583027-47653-2-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v8 1/4] worktree: add top-level worktree.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-22T17:16:31Z","receivedAt":"2015-09-22T17:16:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Rappazzo <rappazzo@gmail.com> writes:\n\n> worktree.c contains functions to work with and get information from\n> worktrees.  This introduction moves functions related to worktrees\n> from branch.c into worktree.c\n>\n> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n> ---\n>  Makefile        |  1 +\n>  branch.c        | 79 +-----------------------------------------------------\n>  branch.h        |  8 ------\n>  builtin/notes.c |  1 +\n>  worktree.c      | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>  worktree.h      | 12 +++++++++\n>  6 files changed, 97 insertions(+), 86 deletions(-)\n>  create mode 100644 worktree.c\n>  create mode 100644 worktree.h\n> ...\n> diff --git a/branch.h b/branch.h\n> index d3446ed..58aa45f 100644\n> --- a/branch.h\n> +++ b/branch.h\n> @@ -59,12 +59,4 @@ extern int read_branch_desc(struct strbuf *, const char *branch_name);\n>   */\n>  extern void die_if_checked_out(const char *branch);\n>  \n> -/*\n> - * Check if a per-worktree symref points to a ref in the main worktree\n> - * or any linked worktree, and return the path to the exising worktree\n> - * if it is.  Returns NULL if there is no existing ref.  The caller is\n> - * responsible for freeing the returned path.\n> - */\n> -extern char *find_shared_symref(const char *symref, const char *target);\n> -\n>  #endif\n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index 3608c64..8b30334 100644\n> --- a/builtin/notes.c\n> +++ b/builtin/notes.c\n> @@ -20,6 +20,7 @@\n>  #include \"notes-merge.h\"\n>  #include \"notes-utils.h\"\n>  #include \"branch.h\"\n> +#include \"worktree.h\"\n\nI think you no longer need to include branch.h after this change.\n\nOther than that, this step looks uncontroversial.\n\nThanks.\n"},{"id":"270498","messageId":"xmqqmvwe4adl.fsf@gitster.mtv.corp.google.com","threadId":"40380","inReplyTo":"1442583027-47653-3-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v8 2/4] worktree: refactor find_linked_symref function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-22T17:44:22Z","receivedAt":"2015-09-22T17:44:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Rappazzo <rappazzo@gmail.com> writes:\n\n> Refactoring will help transition this code to provide additional useful\n> worktree functions.\n>\n> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n> ---\n>  worktree.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++----------------\n>  1 file changed, 64 insertions(+), 22 deletions(-)\n>\n> diff --git a/worktree.c b/worktree.c\n> index 10e1496..5c75875 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -3,6 +3,60 @@\n>  #include \"strbuf.h\"\n>  #include \"worktree.h\"\n>  \n> +/*\n> + * read 'path_to_ref' into 'ref'.  Also if is_detached is not NULL,\n> + * set is_detached to 1 if the ref is detatched.\n\nset is_detached to 1 (0) if the ref is detatched (is not detached).\n\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\tif (!starts_with(ref->buf, \"refs/\")\n> +\t\t\t\t|| check_refname_format(ref->buf, 0))\n\nDon't try to be creative with the format and stick to the original.\n\n\tif (!starts_with(ref->buf, \"refs/\") ||\n\t    check_refname_format(ref->buf, 0))\n\n> +\t\t\treturn -1;\n> +\n\nThis blank makes a strange code by making the \"return -1\" have no\nblank above and one blank below.\n\n> +\t} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {\n> +\t\tif (starts_with(ref->buf, \"ref:\")) {\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} else if (is_detached)\n> +\t\t\t*is_detached = 1;\n\nMinor: I have a suspicion that this would be easier to follow:\n\n\t\tif (!starts_with(...)) {\n\t\t\tif (is_detached)\n\t\t\t\t*is_detached = 1;\n\t\t} else {\n                \tstrbuf_remove(...);\n                        strbuf_trim(...);\n                        if (check_refname_format(...))\n\t\t\t\treturn -1;\n\t\t}\n\n> +\t}\n\nWhat should happen when strbuf_read_file() above fails?  Would it be\na bug (i.e. the caller shouldn't have called us in the first place\nwith such a broken ref), would it be a repository inconsistency\n(i.e. it is worth warning and stop the caller from doing further\ndamage), or is it OK to silently succeed?\n\n> +\treturn 0;\n> +}\n> +\n> +static char *find_main_symref(const char *symref, const char *branch)\n> +{\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tstruct strbuf path = STRBUF_INIT;\n> +\tstruct strbuf gitdir = STRBUF_INIT;\n> +\tchar *existing = NULL;\n> +\n> +\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), symref);\n> +\tif (parse_ref(path.buf, &sb, NULL) == -1)\n> +\t\tgoto done;\n\nI know you described it to \"return -1 on an error\", but as a general\nstyle, for a function that signals a success by returning 0 and\nnegative on error (or on various kinds of errors), it is easier to\nfollow if you followed a more common pattern:\n\n\tif (parse_ref(...) < 0)\n        \tgoto done;\n\n> +\tif (strcmp(sb.buf, branch))\n> +\t\tgoto done;\n> +\tstrbuf_addstr(&gitdir, get_git_common_dir());\n> +\tstrbuf_strip_suffix(&gitdir, \".git\");\n> +\texisting = strbuf_detach(&gitdir, NULL);\n> +done:\n> +\tstrbuf_release(&path);\n> +\tstrbuf_release(&sb);\n> +\tstrbuf_release(&gitdir);\n> +\n> +\treturn existing;\n> +}\n> +\n>  static char *find_linked_symref(const char *symref, const char *branch,\n>  \t\t\t\tconst char *id)\n>  {\n> @@ -11,36 +65,24 @@ static char *find_linked_symref(const char *symref, const char *branch,\n>  \tstruct strbuf gitdir = STRBUF_INIT;\n>  \tchar *existing = NULL;\n>  \n> +\tif (!id)\n> +\t\tgoto done;\n\nA caller that calls this function with id==NULL would always get a\nno-op.  Is that what the caller intended, or should it have called\nthe new find_main_symref() instead?  I'd imagine it is the latter,\nin which case\n\n\tif (!id)\n        \tdie(\"BUG\");\n\nis more appropriate.  On the other hand, if you are trying to keep\nthe old interface to allow id==NULL to mean \"get the information for\nthe primary one\", I'd expect this to call find_main_symref() for the\n(old-style) callers.\n\nIn either case, this \"no id? no-op\" looks funny.\n\n>  \t/*\n>  \t * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside\n>  \t * $GIT_DIR so resolve_ref_unsafe() won't work (it uses\n>  \t * git_path). Parse the ref ourselves.\n>  \t */\n\nYou moved this comment to parse_ref(), which is a more appropriate\nhome for it.  Perhaps you want to remove this copy from here?\n\n> -\tif (id)\n> -\t\tstrbuf_addf(&path, \"%s/worktrees/%s/%s\", get_git_common_dir(), id, symref);\n> -\telse\n> -\t\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), symref);\n> +\tstrbuf_addf(&path, \"%s/worktrees/%s/%s\", get_git_common_dir(), id, symref);\n>  \n> -\tif (!strbuf_readlink(&sb, path.buf, 0)) {\n> -\t\tif (!starts_with(sb.buf, \"refs/\") ||\n> -\t\t    check_refname_format(sb.buf, 0))\n> -\t\t\tgoto done;\n> -\t} else if (strbuf_read_file(&sb, path.buf, 0) >= 0 &&\n> -\t    starts_with(sb.buf, \"ref:\")) {\n> -\t\tstrbuf_remove(&sb, 0, strlen(\"ref:\"));\n> -\t\tstrbuf_trim(&sb);\n> -\t} else\n> +\tif (parse_ref(path.buf, &sb, NULL) == -1)\n>  \t\tgoto done;\n\nSame comment as in find_main_symref() applies here.\n\nI can see the value of splitting out parse_ref() into a separate\nfunction, but it is not immediately obvious how it would be an\noverall win to duplicate major parts of the logic that used to be\nshared for \"primary\" and \"linked\" cases in a single function into\ntwo separate, simpler and similar functions at this point in the\nseries (note that I did not say \"it clearly made it worse\").\nPerhaps reviews on the callers will help us answer that.\n\nThanks.\n"},{"id":"270500","messageId":"xmqqio72495x.fsf@gitster.mtv.corp.google.com","threadId":"40380","inReplyTo":"1442583027-47653-4-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v8 3/4] worktree: add functions to get worktree details","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-22T18:10:34Z","receivedAt":"2015-09-22T18:10:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Rappazzo <rappazzo@gmail.com> writes:\n\n> +/**\n> + * get the main worktree\n> + */\n> +static struct worktree *get_main_worktree()\n\nstatic struct worktree *get_main_worktree(void)\n\n> +{\n> +\tstruct worktree *worktree = NULL;\n>  \tstruct strbuf path = STRBUF_INIT;\n> +\tstruct strbuf worktree_path = STRBUF_INIT;\n> +\tstruct strbuf git_dir = STRBUF_INIT;\n> +\tstruct strbuf head_ref = STRBUF_INIT;\n> +\tint is_bare = 0;\n> +\tint is_detached = 0;\n>  \n> +\tstrbuf_addf(&git_dir, \"%s\", absolute_path(get_git_common_dir()));\n> +\tstrbuf_addf(&worktree_path, \"%s\", absolute_path(get_git_common_dir()));\n\nWhy not strbuf_add(&git_dir, absolute_path(get_git_common_dir()))\nfollowed by strbuf_addbuf(&git_dir, &worktree_path)?\n\n> +\tis_bare = !strbuf_strip_suffix(&worktree_path, \"/.git\");\n> +\tif (is_bare)\n> +\t\tstrbuf_strip_suffix(&worktree_path, \"/.\");\n\nHmm, it is not immediately obvious which part is meant to be a\nfaithful translations from the old find_main_symref() that grab the\nsame kind of information the old code used to discover (and record\nin a new mechanism that is \"struct worktree\"), and which part is a\nnew code that is needed to grab new pieces of information.  The\neffort in [PATCH 2/4] to make it easier to follow sort of lost by\nthis rewrite.\n\nI presume that discovery of the git_dir is from the original and\neverything else including is_bare bit, head_ref, is_detached are\nnew?\n\nSplitting this patch further into two may help showing the changes\nbetter.  First move to the \"grab information into an element of the\nworktree array\" code structure without adding new information, and\nthen \"now make the worktree structure richer\" as a follow up,\nperhaps?\n\n> +/**\n> + * get the estimated worktree count for use in sizing the worktree array\n> + * Note that the minimum count should be 2 (main worktree + NULL).  Using the\n> + * file count in $GIT_COMMON_DIR/worktrees includes '.' and '..' so the\n> + * minimum is satisfied by counting those entries.\n> + */\n> +static int get_estimated_worktree_count()\n> +{\n> +...\n>  }\n\nThis is new.  My gut-feeling is that we would probably not want to\ndo this (see below).\n\n> +struct worktree **get_worktrees(void)\n>  {\n> +\tstruct worktree **list = NULL;\n>  \tstruct strbuf path = STRBUF_INIT;\n>  \tDIR *dir;\n>  \tstruct dirent *d;\n> -\tchar *existing;\n> +\tint counter = 0;\n> +\n> +\tlist = xmalloc(get_estimated_worktree_count() * sizeof(struct worktree *));\n\nHere you scanned the directory to see how many possible worktrees\nthere are \"at this moment\".\n\nTime passes.\n\nHow do you know that nobody added more worktrees in the meantime?\nYou don't.\n\n>  \n> -\tif ((existing = find_main_symref(symref, target)))\n> -\t\treturn existing;\n> +\tif ((list[counter] = get_main_worktree()))\n> +\t\tcounter++;\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\twhile ((d = readdir(dir)) != NULL) {\n> +\t\t\tstruct worktree *linked = NULL;\n> +\t\t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tif ((linked = get_linked_worktree(d->d_name))) {\n> +\t\t\t\tlist[counter++] = linked;\n\nWhich means that nothing stops you from overrunning the list[] with\nthis iteration.\n\nBy the way, when the body of \"if\" and \"else\" both consists of a\nsingle statement, we tend to drop braces.\n\n> +\t\t\t}\n\nAnd in order to avoid overrunning, you would need to do the usual\nALLOC_GROW() dance on list[] _anyway_.  So there is no much point,\nother than to optimize for a case where you have thousands of\nworktrees and the usual ALLOC_GROW() approach would have to do\nrealloc(3) too many times, to have the initial \"guestimate\" scan.\n"},{"id":"270515","messageId":"xmqq37y644x7.fsf@gitster.mtv.corp.google.com","threadId":"40380","inReplyTo":"1442583027-47653-5-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v8 4/4] worktree: add 'list' command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-22T19:42:12Z","receivedAt":"2015-09-22T19:42:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Rappazzo <rappazzo@gmail.com> writes:\n\n>  \n> +--porcelain::\n> +\tWith `list`, output in an easy-to-parse format for scripts.\n> +\tThis format will remain stable across Git versions and regardless of user\n> +\tconfiguration.\n\n... and exactly what does it output?  That would be the first\nquestion a reader of this documentation would ask.\n\n\n> @@ -93,6 +106,7 @@ OPTIONS\n>  --expire <time>::\n>  \tWith `prune`, only expire unused working trees older than <time>.\n>  \n> +\n\n?\n\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index 71bb770..e6e36ac 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -8,10 +8,13 @@\n>  #include \"run-command.h\"\n>  #include \"sigchain.h\"\n>  #include \"refs.h\"\n> +#include \"utf8.h\"\n> +#include \"worktree.h\"\n>  \n>  static const char * const worktree_usage[] = {\n>  \tN_(\"git worktree add [<options>] <path> <branch>\"),\n>  \tN_(\"git worktree prune [<options>]\"),\n> +\tN_(\"git worktree list [<options>]\"),\n>  \tNULL\n>  };\n>  \n> @@ -359,6 +362,79 @@ static int add(int ac, const char **av, const char *prefix)\n>  \treturn add_worktree(path, branch, &opts);\n>  }\n>  \n> +static void show_worktree_porcelain(struct worktree *worktree)\n> +{\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\n> +\tstrbuf_addf(&sb, \"worktree %s\\n\", worktree->path);\n> +\tif (worktree->is_bare)\n> +\t\tstrbuf_addstr(&sb, \"bare\");\n> +\telse {\n> +\t\tif (worktree->is_detached)\n> +\t\t\tstrbuf_addf(&sb, \"detached at %s\", find_unique_abbrev(worktree->head_sha1, DEFAULT_ABBREV));\n> +\t\telse\n> +\t\t\tstrbuf_addf(&sb, \"branch %s\", shorten_unambiguous_ref(worktree->head_ref, 0));\n> +\t}\n\nWriting the above like this:\n\n\tif (worktree->is_bare)\n        \t...\n\telse if (worktree->is_detached)\n\t\t...\n\telse\n                ...\n\nwould make it a lot more clear that there are three cases.\n\nAlso, I doubt --porcelain output wants shorten or abbrev.\n\nHuman-readability is not a goal.  Reproducibility is.  When you run\n\"worktree list\" today and save the output, you want the output from\n\"worktree list\" taken tomorrow to exactly match it, even after\ncreating many objects and tags with conflicting names with branches,\nas long as you didn't change their HEADs in the meantime.\n\n> +\n> +\tprintf(\"%s\\n\\n\", sb.buf);\n> +\n> +\tstrbuf_release(&sb);\n\nI am not sure what the point of use of a strbuf is in this function,\nthough.  Two printf's for each case (one for the common \"worktree\n%s\", the other inside if/elseif/else cascade) should be sufficient.\n\n> +static void show_worktree(struct worktree *worktree, int path_maxlen)\n> +{\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\n\nRemove this blank line.  You are still declaring variables.\n\n> +\tint cur_len = strlen(worktree->path);\n> +\tint utf8_adj = cur_len - utf8_strwidth(worktree->path);\n\nHave a blank line here, instead, as now you start your statements.\n\n> +\tstrbuf_addf(&sb, \"%-*s \", 1 + path_maxlen + utf8_adj, worktree->path);\n> +\tif (worktree->is_bare)\n> +\t\tstrbuf_addstr(&sb, \"(bare)\");\n> +\telse {\n> +\t\tstrbuf_addf(&sb, \"%s \", find_unique_abbrev(worktree->head_sha1, DEFAULT_ABBREV));\n\nPersonally I am not a big fan of the the alignment and use of\nutf8_strwidth(), but by using find_unique_abbrev() here, you are\nbreaking the alignment, aren't you?  \" [branchname]\" that follows\nthe commit object name would not start at the same column, when\nyou have many objects that default-abbrev is not enough to uniquely\nidentify them.\n\nAnd it can easily be fixed by computing the unique-abbrev length for\nall the non-bare worktree's HEADs in the same loop you computed\npath_maxlen() in the caller, passing that to this function, and use\nthat as mininum abbrev length when computing the unique-abbrev.\n\n> +\t\tif (!worktree->is_detached)\n> +\t\t\tstrbuf_addf(&sb, \"[%s]\", shorten_unambiguous_ref(worktree->head_ref, 0));\n> +\t\telse\n> +\t\t\tstrbuf_addstr(&sb, \"(detached HEAD)\");\n> +\t}\n> +\tprintf(\"%s\\n\", sb.buf);\n\n\n> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\n> new file mode 100755\n> index 0000000..b68dfb4\n> --- /dev/null\n> +++ b/t/t2027-worktree-list.sh\n> @@ -0,0 +1,86 @@\n> +#!/bin/sh\n> +\n> +test_description='test git worktree list'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> +\ttest_commit init\n> +'\n> +\n> +test_expect_success '\"list\" all worktrees from main' '\n> +\techo \"$(git rev-parse --show-toplevel)       $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n\nAre the number of SPs here significant and if so in what way?  Does\nit depend on your environment or will there always be six of them?\nEither way feels like an indication of a problem.\n\n> +\tgit worktree add --detach here master &&\n> +\ttest_when_finished \"rm -rf here && git worktree prune\" &&\n\nAren't these two the other way around?  When \"add\" fails in the\nmiddle, you would want it to be removed to proceed to the next test\nwithout leaving 'here' in the list of worktrees, no?\n"},{"id":"270620","messageId":"CANoM8SX28TPt_U8v9HO4yo=M46-JKn5S11DrhZ0-EdrjkDT3EQ@mail.gmail.com","threadId":"40380","inReplyTo":"xmqq37y644x7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v8 4/4] worktree: add 'list' command","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-09-24T02:58:53Z","receivedAt":"2015-09-24T02:58:53Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Tue, Sep 22, 2015 at 3:42 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Michael Rappazzo <rappazzo@gmail.com> writes:\n>\n>>\n>> +--porcelain::\n>> +     With `list`, output in an easy-to-parse format for scripts.\n>> +     This format will remain stable across Git versions and regardless of user\n>> +     configuration.\n>\n> ... and exactly what does it output?  That would be the first\n> question a reader of this documentation would ask.\n>\n\nI will add that description.  I have mostly followed what Eric\nsuggested in the v7 series for a porcelain format.  Does the porcelain\nformat restrict additive changes?  That is, is it OK for a future\npatch to add another field in the format, as long as it doesn't alter\nthe other values?  Is the format that I have used here acceptable\n(assuming the changes proposed below are made)?\n\n>\n>> @@ -93,6 +106,7 @@ OPTIONS\n>>  --expire <time>::\n>>       With `prune`, only expire unused working trees older than <time>.\n>>\n>> +\n>\n> ?\n>\n>> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>> index 71bb770..e6e36ac 100644\n>> --- a/builtin/worktree.c\n>> +++ b/builtin/worktree.c\n>> @@ -8,10 +8,13 @@\n>>  #include \"run-command.h\"\n>>  #include \"sigchain.h\"\n>>  #include \"refs.h\"\n>> +#include \"utf8.h\"\n>> +#include \"worktree.h\"\n>>\n>>  static const char * const worktree_usage[] = {\n>>       N_(\"git worktree add [<options>] <path> <branch>\"),\n>>       N_(\"git worktree prune [<options>]\"),\n>> +     N_(\"git worktree list [<options>]\"),\n>>       NULL\n>>  };\n>>\n>> @@ -359,6 +362,79 @@ static int add(int ac, const char **av, const char *prefix)\n>>       return add_worktree(path, branch, &opts);\n>>  }\n>>\n>> +static void show_worktree_porcelain(struct worktree *worktree)\n>> +{\n>> +     struct strbuf sb = STRBUF_INIT;\n>> +\n>> +     strbuf_addf(&sb, \"worktree %s\\n\", worktree->path);\n>> +     if (worktree->is_bare)\n>> +             strbuf_addstr(&sb, \"bare\");\n>> +     else {\n>> +             if (worktree->is_detached)\n>> +                     strbuf_addf(&sb, \"detached at %s\", find_unique_abbrev(worktree->head_sha1, DEFAULT_ABBREV));\n>> +             else\n>> +                     strbuf_addf(&sb, \"branch %s\", shorten_unambiguous_ref(worktree->head_ref, 0));\n>> +     }\n>\n> Writing the above like this:\n>\n>         if (worktree->is_bare)\n>                 ...\n>         else if (worktree->is_detached)\n>                 ...\n>         else\n>                 ...\n>\n> would make it a lot more clear that there are three cases.\n>\n> Also, I doubt --porcelain output wants shorten or abbrev.\n>\n> Human-readability is not a goal.  Reproducibility is.  When you run\n> \"worktree list\" today and save the output, you want the output from\n> \"worktree list\" taken tomorrow to exactly match it, even after\n> creating many objects and tags with conflicting names with branches,\n> as long as you didn't change their HEADs in the meantime.\n>\n>> +\n>> +     printf(\"%s\\n\\n\", sb.buf);\n>> +\n>> +     strbuf_release(&sb);\n>\n> I am not sure what the point of use of a strbuf is in this function,\n> though.  Two printf's for each case (one for the common \"worktree\n> %s\", the other inside if/elseif/else cascade) should be sufficient.\n>\n>> +static void show_worktree(struct worktree *worktree, int path_maxlen)\n>> +{\n>> +     struct strbuf sb = STRBUF_INIT;\n>> +\n>\n> Remove this blank line.  You are still declaring variables.\n>\n>> +     int cur_len = strlen(worktree->path);\n>> +     int utf8_adj = cur_len - utf8_strwidth(worktree->path);\n>\n> Have a blank line here, instead, as now you start your statements.\n>\n>> +     strbuf_addf(&sb, \"%-*s \", 1 + path_maxlen + utf8_adj, worktree->path);\n>> +     if (worktree->is_bare)\n>> +             strbuf_addstr(&sb, \"(bare)\");\n>> +     else {\n>> +             strbuf_addf(&sb, \"%s \", find_unique_abbrev(worktree->head_sha1, DEFAULT_ABBREV));\n>\n> Personally I am not a big fan of the the alignment and use of\n> utf8_strwidth(), but by using find_unique_abbrev() here, you are\n> breaking the alignment, aren't you?  \" [branchname]\" that follows\n> the commit object name would not start at the same column, when\n> you have many objects that default-abbrev is not enough to uniquely\n> identify them.\n>\n> And it can easily be fixed by computing the unique-abbrev length for\n> all the non-bare worktree's HEADs in the same loop you computed\n> path_maxlen() in the caller, passing that to this function, and use\n> that as mininum abbrev length when computing the unique-abbrev.\n>\n\nI only intended for the path to be right padded, since paths can vary\nin length so widely.  I found it much easier to read with the path\nright-padded.  I think that doing a full column align isn't the best\nlooking output in this case.  I tried to model this output after `git\nbranch -vv`.  I didn't notice if that does a full align if the\nshortened refs are differently sized.  I will try it out, and see if\nit makes a significant visual impact.\n\n>> +             if (!worktree->is_detached)\n>> +                     strbuf_addf(&sb, \"[%s]\", shorten_unambiguous_ref(worktree->head_ref, 0));\n>> +             else\n>> +                     strbuf_addstr(&sb, \"(detached HEAD)\");\n>> +     }\n>> +     printf(\"%s\\n\", sb.buf);\n>\n>\n>> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\n>> new file mode 100755\n>> index 0000000..b68dfb4\n>> --- /dev/null\n>> +++ b/t/t2027-worktree-list.sh\n>> @@ -0,0 +1,86 @@\n>> +#!/bin/sh\n>> +\n>> +test_description='test git worktree list'\n>> +\n>> +. ./test-lib.sh\n>> +\n>> +test_expect_success 'setup' '\n>> +     test_commit init\n>> +'\n>> +\n>> +test_expect_success '\"list\" all worktrees from main' '\n>> +     echo \"$(git rev-parse --show-toplevel)       $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n>\n> Are the number of SPs here significant and if so in what way?  Does\n> it depend on your environment or will there always be six of them?\n> Either way feels like an indication of a problem.\n\nThe number of spaces is significant in this case, but it should not be\nplatform dependent.  It is just the padding for the different worktree\npaths.\n\n>\n>> +     git worktree add --detach here master &&\n>> +     test_when_finished \"rm -rf here && git worktree prune\" &&\n>\n> Aren't these two the other way around?  When \"add\" fails in the\n> middle, you would want it to be removed to proceed to the next test\n> without leaving 'here' in the list of worktrees, no?\n\nSure, I'll switch it.\n"},{"id":"270621","messageId":"xmqqvbb0v2bi.fsf@gitster.mtv.corp.google.com","threadId":"40380","inReplyTo":"CANoM8SX28TPt_U8v9HO4yo=M46-JKn5S11DrhZ0-EdrjkDT3EQ@mail.gmail.com","subject":"Re: [PATCH v8 4/4] worktree: add 'list' command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-24T05:00:49Z","receivedAt":"2015-09-24T05:00:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Rappazzo <rappazzo@gmail.com> writes:\n\n> ...  Does the porcelain\n> format restrict additive changes?  That is, is it OK for a future\n> patch to add another field in the format, as long as it doesn't alter\n> the other values?  Is the format that I have used here acceptable\n> (assuming the changes proposed below are made)?\n\nIt for you, as the designer of the format, to decide what to put in\nthe proposed specification, but I'm sure that we would want it to be\nextensible.  I guess your design is essentially a series of records,\neach of which begins with a \"worktree <path>\" line, followed by\nvarious attributes, one per line, about that worktree, and each\nattribute line begins with some fixed keyword so that the reader can\ntell what attribute the line is talking about (or if the line\ndescribes an attribute that the reader does not yet know about), and\nI think that is an acceptable format.\n\nYou need to decide and describe if the value for some attribute can\nspan multiple lines (and define the quoting mechanism if that is the\ncase), what the set of keywords currently defined and what each of\nthese keywords means, and document that the readers are expected to\nskip a line that begins with an unknown keyword.\n\n>> Are the number of SPs here significant and if so in what way?  Does\n>> it depend on your environment or will there always be six of them?\n>> Either way feels like an indication of a problem.\n>\n> The number of spaces is significant in this case, but it should not be\n> platform dependent.  It is just the padding for the different worktree\n> paths.\n\nBy environment, I didn't just mean 'platform'.  For example, the\noutput from --show-toplevel will be different for me and you as it\nis likely that the absolute path where I have my repository and you\nhave yours would be different, so the contents of \"expect\" would\nalso be.  I was wondering where that SIX comes from.\n\nThe reason why I said it feels like an indication of a problem is\nbecause it invites this question: If it is uniformly SIX no matter\nwhat the user's environment is, why does it have to be SIX and not\nother value, say, ONE, for example?\n"}]}