{"thread":{"id":"44528","subject":"[PATCH 0/3] Minor fixes on 'git worktree'","startedAt":"2016-11-22T10:01:04Z","lastAt":"2016-11-28T21:26:13Z","messageCount":19,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"306289","messageId":"20161122100046.8341-1-pclouds@gmail.com","threadId":"44528","inReplyTo":null,"subject":"[PATCH 0/3] Minor fixes on 'git worktree'","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-22T10:00:43Z","receivedAt":"2016-11-22T10:01:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This fixes two things:\n\n - make sure the first item is always the main worktree even if we\n fail to retrieve some info\n\n - keep 'worktree list' order stable (which in turn fixes the random\n failure on my 'worktree-move' series\n\nNguyễn Thái Ngọc Duy (3):\n worktree.c: zero new 'struct worktree' on allocation\n get_worktrees() must return main worktree as first item even on error\n worktree list: keep the list sorted\n\n builtin/worktree.c | 26 ++++++++++++++++++++++----\n worktree.c | 20 ++++----------------\n 2 files changed, 26 insertions(+), 20 deletions(-)\n\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"306290","messageId":"20161122100046.8341-2-pclouds@gmail.com","threadId":"44528","inReplyTo":"20161122100046.8341-1-pclouds@gmail.com","subject":"[PATCH 1/3] worktree.c: zero new 'struct worktree' on allocation","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-22T10:00:44Z","receivedAt":"2016-11-22T10:01:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This keeps things a bit simpler when we add more fields, knowing that\ndefault values are always zero.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n worktree.c | 14 ++------------\n 1 file changed, 2 insertions(+), 12 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex f7869f8..f7c1b5e 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -91,16 +91,11 @@ static struct worktree *get_main_worktree(void)\n \tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n \t\tgoto done;\n \n-\tworktree = xmalloc(sizeof(struct worktree));\n+\tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n-\tworktree->id = NULL;\n \tworktree->is_bare = is_bare;\n-\tworktree->head_ref = NULL;\n \tworktree->is_detached = is_detached;\n-\tworktree->is_current = 0;\n \tadd_head_info(&head_ref, worktree);\n-\tworktree->lock_reason = NULL;\n-\tworktree->lock_reason_valid = 0;\n \n done:\n \tstrbuf_release(&path);\n@@ -138,16 +133,11 @@ static struct worktree *get_linked_worktree(const char *id)\n \tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n \t\tgoto done;\n \n-\tworktree = xmalloc(sizeof(struct worktree));\n+\tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->id = xstrdup(id);\n-\tworktree->is_bare = 0;\n-\tworktree->head_ref = NULL;\n \tworktree->is_detached = is_detached;\n-\tworktree->is_current = 0;\n \tadd_head_info(&head_ref, worktree);\n-\tworktree->lock_reason = NULL;\n-\tworktree->lock_reason_valid = 0;\n \n done:\n \tstrbuf_release(&path);\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"306291","messageId":"20161122100046.8341-3-pclouds@gmail.com","threadId":"44528","inReplyTo":"20161122100046.8341-1-pclouds@gmail.com","subject":"[PATCH 2/3] get_worktrees() must return main worktree as first item even on error","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-22T10:00:45Z","receivedAt":"2016-11-22T10:01:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is required by git-worktree.txt, stating that the main worktree is\nthe first line (especially in --porcelain mode when we can't just change\nbehavior at will).\n\nThere's only one case when get_worktrees() may skip main worktree, when\nparse_ref() fails. Update the code so that we keep first item as main\nworktree and return something sensible in this case:\n\n - In user-friendly mode, since we're not constraint by anything,\n returning \"(error)\" should do the job (we already show \"(detached\n HEAD)\" which is not machine-friendly). Actually errors should be\n printed on stderr by parse_ref() (*)\n\n - In plumbing mode, we do not show neither 'bare', 'detached' or\n 'branch ...', which is possible by the format description if I read\n it right.\n\nCareful readers may realize that when the local variable \"head_ref\" in\nget_main_worktree() is emptied, add_head_info() will do nothing to\nwt->head_sha1. But that's ok because head_sha1 is zero-ized in the\nprevious patch.\n\n(*) Well, it does not. But it's supposed to be a stop gap\n implementation until refs we can reuse code, to parse \"ref: \" stuff\n in HEAD, from resolve_refs_unsafe(). Now may be the time since\n refs refactoring is mostly done.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/worktree.c | 8 +++++---\n worktree.c | 6 ++----\n 2 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 5c4854d..b835b91 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -388,7 +388,7 @@ static void show_worktree_porcelain(struct worktree *wt)\n \t\tprintf(\"HEAD %s\\n\", sha1_to_hex(wt->head_sha1));\n \t\tif (wt->is_detached)\n \t\t\tprintf(\"detached\\n\");\n-\t\telse\n+\t\telse if (wt->head_ref)\n \t\t\tprintf(\"branch %s\\n\", wt->head_ref);\n \t}\n \tprintf(\"\\n\");\n@@ -406,10 +406,12 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)\n \telse {\n \t\tstrbuf_addf(&sb, \"%-*s \", abbrev_len,\n \t\t\t\tfind_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));\n-\t\tif (!wt->is_detached)\n+\t\tif (wt->is_detached)\n+\t\t\tstrbuf_addstr(&sb, \"(detached HEAD)\");\n+\t\telse if (wt->head_ref)\n \t\t\tstrbuf_addf(&sb, \"[%s]\", shorten_unambiguous_ref(wt->head_ref, 0));\n \t\telse\n-\t\t\tstrbuf_addstr(&sb, \"(detached HEAD)\");\n+\t\t\tstrbuf_addstr(&sb, \"(error)\");\n \t}\n \tprintf(\"%s\\n\", sb.buf);\n \ndiff --git a/worktree.c b/worktree.c\nindex f7c1b5e..a674efa 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -89,7 +89,7 @@ static struct worktree *get_main_worktree(void)\n \tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n \n \tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n-\t\tgoto done;\n+\t\tstrbuf_reset(&head_ref);\n \n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n@@ -97,7 +97,6 @@ static struct worktree *get_main_worktree(void)\n \tworktree->is_detached = is_detached;\n \tadd_head_info(&head_ref, worktree);\n \n-done:\n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n \tstrbuf_release(&head_ref);\n@@ -173,8 +172,7 @@ struct worktree **get_worktrees(void)\n \n \tlist = xmalloc(alloc * sizeof(struct worktree *));\n \n-\tif ((list[counter] = get_main_worktree()))\n-\t\tcounter++;\n+\tlist[counter++] = get_main_worktree();\n \n \tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n \tdir = opendir(path.buf);\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"306292","messageId":"20161122100046.8341-4-pclouds@gmail.com","threadId":"44528","inReplyTo":"20161122100046.8341-1-pclouds@gmail.com","subject":"[PATCH 3/3] worktree list: keep the list sorted","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-22T10:00:46Z","receivedAt":"2016-11-22T10:01:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It makes it easier to write tests for. But it should also be good for\nthe user since locating a worktree by eye would be easier once they\nnotice this.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/worktree.c | 18 +++++++++++++++++-\n 1 file changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex b835b91..80ccc51 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -434,6 +434,13 @@ static void measure_widths(struct worktree **wt, int *abbrev, int *maxlen)\n \t}\n }\n \n+static int compare_worktree(const void *a_, const void *b_)\n+{\n+\tconst struct worktree *const *a = a_;\n+\tconst struct worktree *const *b = b_;\n+\treturn fspathcmp((*a)->path, (*b)->path);\n+}\n+\n static int list(int ac, const char **av, const char *prefix)\n {\n \tint porcelain = 0;\n@@ -448,11 +455,20 @@ static int list(int ac, const char **av, const char *prefix)\n \t\tusage_with_options(worktree_usage, options);\n \telse {\n \t\tstruct worktree **worktrees = get_worktrees();\n-\t\tint path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;\n+\t\tint path_maxlen = 0, abbrev = DEFAULT_ABBREV, i, nr;\n \n \t\tif (!porcelain)\n \t\t\tmeasure_widths(worktrees, &abbrev, &path_maxlen);\n \n+\t\tfor (i = nr = 0; worktrees[i]; i++)\n+\t\t\tnr++;\n+\n+\t\t/*\n+\t\t * don't sort the first item (main worktree), which will\n+\t\t * always be the first\n+\t\t */\n+\t\tQSORT(worktrees + 1, nr - 1, compare_worktree);\n+\n \t\tfor (i = 0; worktrees[i]; i++) {\n \t\t\tif (porcelain)\n \t\t\t\tshow_worktree_porcelain(worktrees[i]);\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"306380","messageId":"xmqqbmx6nnpg.fsf@gitster.mtv.corp.google.com","threadId":"44528","inReplyTo":"20161122100046.8341-1-pclouds@gmail.com","subject":"Re: [PATCH 0/3] Minor fixes on 'git worktree'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-23T16:52:11Z","receivedAt":"2016-11-23T16:52:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> This fixes two things:\n>\n> - make sure the first item is always the main worktree even if we\n> fail to retrieve some info\n>\n> - keep 'worktree list' order stable (which in turn fixes the random\n> failure on my 'worktree-move' series\n> Nguyễn Thái Ngọc Duy (3):\n> worktree.c: zero new 'struct worktree' on allocation\n> get_worktrees() must return main worktree as first item even on error\n> worktree list: keep the list sorted\n>\n> builtin/worktree.c | 26 ++++++++++++++++++++++----\n> worktree.c | 20 ++++----------------\n> 2 files changed, 26 insertions(+), 20 deletions(-)\n\nAny tests?\n\n"},{"id":"306381","messageId":"xmqq7f7unnoo.fsf@gitster.mtv.corp.google.com","threadId":"44528","inReplyTo":"20161122100046.8341-2-pclouds@gmail.com","subject":"Re: [PATCH 1/3] worktree.c: zero new 'struct worktree' on allocation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-23T16:52:39Z","receivedAt":"2016-11-23T16:52:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> This keeps things a bit simpler when we add more fields, knowing that\n> default values are always zero.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n\nLooks sensible. Thanks.\n\n\n> worktree.c | 14 ++------------\n> 1 file changed, 2 insertions(+), 12 deletions(-)\n>\n> diff --git a/worktree.c b/worktree.c\n> index f7869f8..f7c1b5e 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -91,16 +91,11 @@ static struct worktree *get_main_worktree(void)\n> \tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n> \t\tgoto done;\n> \n> -\tworktree = xmalloc(sizeof(struct worktree));\n> +\tworktree = xcalloc(1, sizeof(*worktree));\n> \tworktree->path = strbuf_detach(&worktree_path, NULL);\n> -\tworktree->id = NULL;\n> \tworktree->is_bare = is_bare;\n> -\tworktree->head_ref = NULL;\n> \tworktree->is_detached = is_detached;\n> -\tworktree->is_current = 0;\n> \tadd_head_info(&head_ref, worktree);\n> -\tworktree->lock_reason = NULL;\n> -\tworktree->lock_reason_valid = 0;\n> \n> done:\n> \tstrbuf_release(&path);\n> @@ -138,16 +133,11 @@ static struct worktree *get_linked_worktree(const char *id)\n> \tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n> \t\tgoto done;\n> \n> -\tworktree = xmalloc(sizeof(struct worktree));\n> +\tworktree = xcalloc(1, sizeof(*worktree));\n> \tworktree->path = strbuf_detach(&worktree_path, NULL);\n> \tworktree->id = xstrdup(id);\n> -\tworktree->is_bare = 0;\n> -\tworktree->head_ref = NULL;\n> \tworktree->is_detached = is_detached;\n> -\tworktree->is_current = 0;\n> \tadd_head_info(&head_ref, worktree);\n> -\tworktree->lock_reason = NULL;\n> -\tworktree->lock_reason_valid = 0;\n> \n> done:\n> \tstrbuf_release(&path);\n"},{"id":"306382","messageId":"xmqq1sy2nn1g.fsf@gitster.mtv.corp.google.com","threadId":"44528","inReplyTo":"20161122100046.8341-3-pclouds@gmail.com","subject":"Re: [PATCH 2/3] get_worktrees() must return main worktree as first item even on error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-23T17:06:35Z","receivedAt":"2016-11-23T17:06:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index 5c4854d..b835b91 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -388,7 +388,7 @@ static void show_worktree_porcelain(struct worktree *wt)\n> \t\tprintf(\"HEAD %s\\n\", sha1_to_hex(wt->head_sha1));\n> \t\tif (wt->is_detached)\n> \t\t\tprintf(\"detached\\n\");\n> -\t\telse\n> +\t\telse if (wt->head_ref)\n> \t\t\tprintf(\"branch %s\\n\", wt->head_ref);\n\nThis change looks somewhat unrelated to what the title and the log\nmessage claims to do, but the fix is to indicate an error condition\nby leaving wt->head_ref as NULL, so this is a necessary adjustment.\n\nGood.\n\n> @@ -406,10 +406,12 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)\n> \telse {\n> \t\tstrbuf_addf(&sb, \"%-*s \", abbrev_len,\n> \t\t\t\tfind_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));\n> -\t\tif (!wt->is_detached)\n> +\t\tif (wt->is_detached)\n> +\t\t\tstrbuf_addstr(&sb, \"(detached HEAD)\");\n> +\t\telse if (wt->head_ref)\n> \t\t\tstrbuf_addf(&sb, \"[%s]\", shorten_unambiguous_ref(wt->head_ref, 0));\n> \t\telse\n> -\t\t\tstrbuf_addstr(&sb, \"(detached HEAD)\");\n> +\t\t\tstrbuf_addstr(&sb, \"(error)\");\n> \t}\n\nLikewise.\n\n> diff --git a/worktree.c b/worktree.c\n> index f7c1b5e..a674efa 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -89,7 +89,7 @@ static struct worktree *get_main_worktree(void)\n> \tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n> \n> \tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n> -\t\tgoto done;\n> +\t\tstrbuf_reset(&head_ref);\n> \n> \tworktree = xcalloc(1, sizeof(*worktree));\n> \tworktree->path = strbuf_detach(&worktree_path, NULL);\n> @@ -97,7 +97,6 @@ static struct worktree *get_main_worktree(void)\n> \tworktree->is_detached = is_detached;\n> \tadd_head_info(&head_ref, worktree);\n\nOK. The earlier call to _reset() and add_head_info() function\nitself may want to be clarified that a zero-length strbuf signals an\nerror condition with additional comment. It is all too unclear in\nthe code with this patch as it stands.\n\n> -done:\n> \tstrbuf_release(&path);\n> \tstrbuf_release(&worktree_path);\n> \tstrbuf_release(&head_ref);\n\nAfter this there is \"return worktree\" which used to return NULL\nbecause of the \"goto\", but we never return NULL from the function\nafter this change, which is the whole point of this change. Good.\n\n> @@ -173,8 +172,7 @@ struct worktree **get_worktrees(void)\n> \n> \tlist = xmalloc(alloc * sizeof(struct worktree *));\n> \n> -\tif ((list[counter] = get_main_worktree()))\n> -\t\tcounter++;\n> +\tlist[counter++] = get_main_worktree();\n\nHence the conditional, while it does not hurt, becomes unnecessary\nand we can unconditionally throw the primary one to the list.\n\nGood.\n\nOther than the \"these need in-code commenting\", and also that this\nshould have a new test, the patch makes sense to me.\n\nThanks.\n"},{"id":"306383","messageId":"xmqqwpfum801.fsf@gitster.mtv.corp.google.com","threadId":"44528","inReplyTo":"20161122100046.8341-4-pclouds@gmail.com","subject":"Re: [PATCH 3/3] worktree list: keep the list sorted","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-23T17:16:46Z","receivedAt":"2016-11-23T17:16:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> +\t\tfor (i = nr = 0; worktrees[i]; i++)\n> +\t\t\tnr++;\n> +\n> +\t\t/*\n> +\t\t * don't sort the first item (main worktree), which will\n> +\t\t * always be the first\n> +\t\t */\n> +\t\tQSORT(worktrees + 1, nr - 1, compare_worktree);\n> +\n\nThis is somewhat curious.\n\n\tfor (i = 0; worktrees[i]; i++)\n\t\t; /* just counting */\n\tQSORT(worktrees + 1, i - 1, compare_worktree);\n\nwould have been a lot more idiomatic (you do not use nr after sorting).\n\nMore importantly, perhaps get_worktrees() should learn to take an\noptional pointer to int that returns how many items are in the list?\n\nAlternatively, other existing callers of the function do not care\nabout the order, so it may not be such a good idea to always sort\nthe result, but it may be a good idea to teach it to take a boolean\nthat signals that the list should be sorted in a \"natural order\",\nwhich is how \"worktree list\" would show them to the user?\n\nThis should be easily protectable with a new test. Please do.\n"},{"id":"306424","messageId":"CACsJy8C-5f-PErg6zx5OuURaA717o_g_RddjvwMUnO+vKCHrNA@mail.gmail.com","threadId":"44528","inReplyTo":"xmqqwpfum801.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] worktree list: keep the list sorted","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-25T12:19:31Z","receivedAt":"2016-11-25T12:20:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Nov 24, 2016 at 12:16 AM, Junio C Hamano wrote:\n> More importantly, perhaps get_worktrees() should learn to take an\n> optional pointer to int that returns how many items are in the list?\n\nMy first thought was \"yeah I remember there are many counting loop\nlike this\" then grepped and realized this is the only place. For now I\nthink it's ok to leave it as is. When the second place does the same,\nwe'll change get_worktrees()\n\n> Alternatively, other existing callers of the function do not care\n> about the order, so it may not be such a good idea to always sort\n> the result, but it may be a good idea to teach it to take a boolean\n> that signals that the list should be sorted in a \"natural order\",\n> which is how \"worktree list\" would show them to the user?\n\nOK. I'll make it a bit flag, not a boolean though, to avoid changing\nall call sites next time.\n-- \nDuy\n"},{"id":"306425","messageId":"CACsJy8D+vyO0LF_A2Wym+PUg=HOTPyDe-1fVwv8zN797pL-rmw@mail.gmail.com","threadId":"44528","inReplyTo":"xmqqbmx6nnpg.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/3] Minor fixes on 'git worktree'","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-25T12:24:38Z","receivedAt":"2016-11-25T12:25:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Nov 23, 2016 at 11:52 PM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> This fixes two things:\n>>\n>> - make sure the first item is always the main worktree even if we\n>> fail to retrieve some info\n>>\n>> - keep 'worktree list' order stable (which in turn fixes the random\n>> failure on my 'worktree-move' series\n>> Nguyễn Thái Ngọc Duy (3):\n>> worktree.c: zero new 'struct worktree' on allocation\n>> get_worktrees() must return main worktree as first item even on error\n>> worktree list: keep the list sorted\n>>\n>> builtin/worktree.c | 26 ++++++++++++++++++++++----\n>> worktree.c | 20 ++++----------------\n>> 2 files changed, 26 insertions(+), 20 deletions(-)\n>\n> Any tests?\n\nI discarded the idea of adding test for sorting because it relies on\nfilesystems. A passed test may just mean the filesystem happens to\nreturn files in \"good\" order. But I guess a test wouldn't hurt.\nSomewhere out there some user may still have a \"bad\" filesystem and\nthe test could help catch breakage then.\n\nAdding the test for the failed parse_ref() is possible, I think. But\nsince that function is destined to die, as I promised to use\nrefs-provided api instead of rolling out a custom ref parser, and I'm\ngoing to have another hard look at refs subsystem for the\ngc-not-looking-at-worktree-refs problem, may I make another promise to\nadd tests after this function is gone? It should happen \"soon\".\n-- \nDuy\n"},{"id":"306426","messageId":"CACsJy8CpySQs07cfioOnoxoH1TALRePGbeVCc45ZqDr43519qQ@mail.gmail.com","threadId":"44528","inReplyTo":"CACsJy8D+vyO0LF_A2Wym+PUg=HOTPyDe-1fVwv8zN797pL-rmw@mail.gmail.com","subject":"Re: [PATCH 0/3] Minor fixes on 'git worktree'","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-25T13:44:53Z","receivedAt":"2016-11-25T13:45:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Nov 25, 2016 at 7:24 PM, Duy Nguyen wrote:\n> Adding the test for the failed parse_ref() is possible, I think. But\n> since that function is destined to die, as I promised to use\n> refs-provided api instead of rolling out a custom ref parser, and I'm\n> going to have another hard look at refs subsystem for the\n> gc-not-looking-at-worktree-refs problem, may I make another promise to\n> add tests after this function is gone?\n\nNever mind. I went ahead and made a test case anyway because why not.\n\nBut that's not worth an email. An interesting thing maybe worth\nsharing is, if HEAD is broken (the only reason we would fail to create\n\"struct worktree\" for main worktree), then \"git worktree list\" from\nmain worktree would fail too, because repo setup code fails to parse\nHEAD as well and die(). That makes the \"always show main worktree\"\npatch useless, since we won't get far enough to execute\nget_worktrees() in the first place. However, you can still do \"git\nworktree list\" from a linked worktree, gitdir setup code will not stop\nyou (how can it). And the patch is still needed.\n-- \nDuy\n"},{"id":"306474","messageId":"20161128093656.15744-1-pclouds@gmail.com","threadId":"44528","inReplyTo":"20161122100046.8341-1-pclouds@gmail.com","subject":"[PATCH v2 0/5] nd/worktree-list-fixup","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-28T09:36:51Z","receivedAt":"2016-11-28T09:37:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This version\n\n* changes get_worktrees() to take a flag, and adds one flag for\n sorting.\n\n* adds tests for both the 'main worktree always present' and the\n sorting problems.\n\n* reworks 3/5 a bit, keep changes closer, easier to see the cause and\n consequence.\n\nNguyễn Thái Ngọc Duy (5):\n worktree.c: zero new 'struct worktree' on allocation\n worktree: reorder an if statement\n get_worktrees() must return main worktree as first item even on error\n worktree.c: get_worktrees() takes a new flag argument\n worktree list: keep the list sorted\n\n branch.c | 2 +-\n builtin/branch.c | 2 +-\n builtin/worktree.c | 14 ++++++++------\n t/t2027-worktree-list.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n worktree.c | 42 +++++++++++++++++++++---------------------\n worktree.h | 4 +++-\n 6 files changed, 74 insertions(+), 30 deletions(-)\n\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"306475","messageId":"20161128093656.15744-2-pclouds@gmail.com","threadId":"44528","inReplyTo":"20161128093656.15744-1-pclouds@gmail.com","subject":"[PATCH v2 1/5] worktree.c: zero new 'struct worktree' on allocation","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-28T09:36:52Z","receivedAt":"2016-11-28T09:37:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This keeps things a bit simpler when we add more fields, knowing that\ndefault values are always zero.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Junio C Hamano \n---\n worktree.c | 14 ++------------\n 1 file changed, 2 insertions(+), 12 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex f7869f8..f7c1b5e 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -91,16 +91,11 @@ static struct worktree *get_main_worktree(void)\n \tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n \t\tgoto done;\n \n-\tworktree = xmalloc(sizeof(struct worktree));\n+\tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n-\tworktree->id = NULL;\n \tworktree->is_bare = is_bare;\n-\tworktree->head_ref = NULL;\n \tworktree->is_detached = is_detached;\n-\tworktree->is_current = 0;\n \tadd_head_info(&head_ref, worktree);\n-\tworktree->lock_reason = NULL;\n-\tworktree->lock_reason_valid = 0;\n \n done:\n \tstrbuf_release(&path);\n@@ -138,16 +133,11 @@ static struct worktree *get_linked_worktree(const char *id)\n \tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n \t\tgoto done;\n \n-\tworktree = xmalloc(sizeof(struct worktree));\n+\tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->id = xstrdup(id);\n-\tworktree->is_bare = 0;\n-\tworktree->head_ref = NULL;\n \tworktree->is_detached = is_detached;\n-\tworktree->is_current = 0;\n \tadd_head_info(&head_ref, worktree);\n-\tworktree->lock_reason = NULL;\n-\tworktree->lock_reason_valid = 0;\n \n done:\n \tstrbuf_release(&path);\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"306476","messageId":"20161128093656.15744-3-pclouds@gmail.com","threadId":"44528","inReplyTo":"20161128093656.15744-1-pclouds@gmail.com","subject":"[PATCH v2 2/5] worktree: reorder an if statement","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-28T09:36:53Z","receivedAt":"2016-11-28T09:37:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is no-op. But it helps reduce diff noise in the next patch.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/worktree.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 5c4854d..8a654e4 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -406,10 +406,10 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)\n \telse {\n \t\tstrbuf_addf(&sb, \"%-*s \", abbrev_len,\n \t\t\t\tfind_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));\n-\t\tif (!wt->is_detached)\n-\t\t\tstrbuf_addf(&sb, \"[%s]\", shorten_unambiguous_ref(wt->head_ref, 0));\n-\t\telse\n+\t\tif (wt->is_detached)\n \t\t\tstrbuf_addstr(&sb, \"(detached HEAD)\");\n+\t\telse\n+\t\t\tstrbuf_addf(&sb, \"[%s]\", shorten_unambiguous_ref(wt->head_ref, 0));\n \t}\n \tprintf(\"%s\\n\", sb.buf);\n \n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"306477","messageId":"20161128093656.15744-6-pclouds@gmail.com","threadId":"44528","inReplyTo":"20161128093656.15744-1-pclouds@gmail.com","subject":"[PATCH v2 5/5] worktree list: keep the list sorted","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-28T09:36:56Z","receivedAt":"2016-11-28T09:38:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It makes it easier to write tests for. But it should also be good for\nthe user since locating a worktree by eye would be easier once they\nnotice this.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/worktree.c | 2 +-\n t/t2027-worktree-list.sh | 19 +++++++++++++++++++\n worktree.c | 14 ++++++++++++++\n worktree.h | 2 ++\n 4 files changed, 36 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex d7d195c..9a97e37 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -447,7 +447,7 @@ static int list(int ac, const char **av, const char *prefix)\n \tif (ac)\n \t\tusage_with_options(worktree_usage, options);\n \telse {\n-\t\tstruct worktree **worktrees = get_worktrees(0);\n+\t\tstruct worktree **worktrees = get_worktrees(GWT_SORT_LINKED);\n \t\tint path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;\n \n \t\tif (!porcelain)\ndiff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\nindex 98b5f34..465eeea 100755\n--- a/t/t2027-worktree-list.sh\n+++ b/t/t2027-worktree-list.sh\n@@ -117,4 +117,23 @@ test_expect_success 'broken main worktree still at the top' '\n \t)\n '\n \n+test_expect_success 'linked worktrees are sorted' '\n+\tmkdir sorted &&\n+\tgit init sorted/main &&\n+\t(\n+\t\tcd sorted/main &&\n+\t\ttest_tick &&\n+\t\ttest_commit new &&\n+\t\tgit worktree add ../first &&\n+\t\tgit worktree add ../second &&\n+\t\tgit worktree list --porcelain | grep ^worktree >actual\n+\t) &&\n+\tcat >expected <<-EOF &&\n+\tworktree $(pwd)/sorted/main\n+\tworktree $(pwd)/sorted/first\n+\tworktree $(pwd)/sorted/second\n+\tEOF\n+\ttest_cmp expected sorted/main/actual\n+'\n+\n test_done\ndiff --git a/worktree.c b/worktree.c\nindex ead088e..eb61212 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -160,6 +160,13 @@ static void mark_current_worktree(struct worktree **worktrees)\n \tfree(git_dir);\n }\n \n+static int compare_worktree(const void *a_, const void *b_)\n+{\n+\tconst struct worktree *const *a = a_;\n+\tconst struct worktree *const *b = b_;\n+\treturn fspathcmp((*a)->path, (*b)->path);\n+}\n+\n struct worktree **get_worktrees(unsigned flags)\n {\n \tstruct worktree **list = NULL;\n@@ -191,6 +198,13 @@ struct worktree **get_worktrees(unsigned flags)\n \tALLOC_GROW(list, counter + 1, alloc);\n \tlist[counter] = NULL;\n \n+\tif (flags & GWT_SORT_LINKED)\n+\t\t/*\n+\t\t * don't sort the first item (main worktree), which will\n+\t\t * always be the first\n+\t\t */\n+\t\tQSORT(list + 1, counter - 1, compare_worktree);\n+\n \tmark_current_worktree(list);\n \treturn list;\n }\ndiff --git a/worktree.h b/worktree.h\nindex 2e68d4a..d59ce1f 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -15,6 +15,8 @@ struct worktree {\n \n /* Functions for acting on the information about worktrees. */\n \n+#define GWT_SORT_LINKED (1 << 0) /* keeps linked worktrees sorted */\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-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"306478","messageId":"20161128093656.15744-4-pclouds@gmail.com","threadId":"44528","inReplyTo":"20161128093656.15744-1-pclouds@gmail.com","subject":"[PATCH v2 3/5] get_worktrees() must return main worktree as first item even on error","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-28T09:36:54Z","receivedAt":"2016-11-28T09:38:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is required by git-worktree.txt, stating that the main worktree is\nthe first line (especially in --porcelain mode when we can't just change\nbehavior at will).\n\nThere's only one case when get_worktrees() may skip main worktree, when\nparse_ref() fails. Update the code so that we keep first item as main\nworktree and return something sensible in this case:\n\n - In user-friendly mode, since we're not constraint by anything,\n returning \"(error)\" should do the job (we already show \"(detached\n HEAD)\" which is not machine-friendly). Actually errors should be\n printed on stderr by parse_ref() (*)\n\n - In plumbing mode, we do not show neither 'bare', 'detached' or\n 'branch ...', which is possible by the format description if I read\n it right.\n\nCareful readers may realize that when the local variable \"head_ref\" in\nget_main_worktree() is emptied, add_head_info() will do nothing to\nwt->head_sha1. But that's ok because head_sha1 is zero-ized in the\nprevious patch.\n\n(*) Well, it does not. But it's supposed to be a stop gap implementation\n until we can reuse refs code to parse \"ref: \" stuff in HEAD, from\n resolve_refs_unsafe(). Now may be the time since refs refactoring is\n mostly done.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/worktree.c | 6 ++++--\n t/t2027-worktree-list.sh | 21 +++++++++++++++++++++\n worktree.c | 10 +++-------\n 3 files changed, 28 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 8a654e4..b835b91 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -388,7 +388,7 @@ static void show_worktree_porcelain(struct worktree *wt)\n \t\tprintf(\"HEAD %s\\n\", sha1_to_hex(wt->head_sha1));\n \t\tif (wt->is_detached)\n \t\t\tprintf(\"detached\\n\");\n-\t\telse\n+\t\telse if (wt->head_ref)\n \t\t\tprintf(\"branch %s\\n\", wt->head_ref);\n \t}\n \tprintf(\"\\n\");\n@@ -408,8 +408,10 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)\n \t\t\t\tfind_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));\n \t\tif (wt->is_detached)\n \t\t\tstrbuf_addstr(&sb, \"(detached HEAD)\");\n-\t\telse\n+\t\telse if (wt->head_ref)\n \t\t\tstrbuf_addf(&sb, \"[%s]\", shorten_unambiguous_ref(wt->head_ref, 0));\n+\t\telse\n+\t\t\tstrbuf_addstr(&sb, \"(error)\");\n \t}\n \tprintf(\"%s\\n\", sb.buf);\n \ndiff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\nindex 1b1b65a..98b5f34 100755\n--- a/t/t2027-worktree-list.sh\n+++ b/t/t2027-worktree-list.sh\n@@ -96,4 +96,25 @@ test_expect_success 'bare repo cleanup' '\n \trm -rf bare1\n '\n \n+test_expect_success 'broken main worktree still at the top' '\n+\tgit init broken-main &&\n+\t(\n+\t\tcd broken-main &&\n+\t\ttest_commit new &&\n+\t\tgit worktree add linked &&\n+\t\tcat >expected <<-EOF &&\n+\t\tworktree $(pwd)\n+\t\tHEAD $_z40\n+\n+\t\tEOF\n+\t\tcd linked &&\n+\t\techo \"worktree $(pwd)\" >expected &&\n+\t\techo \"ref: .broken\" >../.git/HEAD &&\n+\t\tgit worktree list --porcelain | head -n 3 >actual &&\n+\t\ttest_cmp ../expected actual &&\n+\t\tgit worktree list | head -n 1 >actual.2 &&\n+\t\tgrep -F \"(error)\" actual.2\n+\t)\n+'\n+\n test_done\ndiff --git a/worktree.c b/worktree.c\nindex f7c1b5e..3145522 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -88,16 +88,13 @@ static struct worktree *get_main_worktree(void)\n \n \tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n \n-\tif (parse_ref(path.buf, &head_ref, &is_detached) < 0)\n-\t\tgoto done;\n-\n \tworktree = xcalloc(1, sizeof(*worktree));\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->is_bare = is_bare;\n \tworktree->is_detached = is_detached;\n-\tadd_head_info(&head_ref, worktree);\n+\tif (!parse_ref(path.buf, &head_ref, &is_detached))\n+\t\tadd_head_info(&head_ref, worktree);\n \n-done:\n \tstrbuf_release(&path);\n \tstrbuf_release(&worktree_path);\n \tstrbuf_release(&head_ref);\n@@ -173,8 +170,7 @@ struct worktree **get_worktrees(void)\n \n \tlist = xmalloc(alloc * sizeof(struct worktree *));\n \n-\tif ((list[counter] = get_main_worktree()))\n-\t\tcounter++;\n+\tlist[counter++] = get_main_worktree();\n \n \tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n \tdir = opendir(path.buf);\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"306479","messageId":"20161128093656.15744-5-pclouds@gmail.com","threadId":"44528","inReplyTo":"20161128093656.15744-1-pclouds@gmail.com","subject":"[PATCH v2 4/5] worktree.c: get_worktrees() takes a new flag argument","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-28T09:36:55Z","receivedAt":"2016-11-28T09:38:17Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is another no-op patch, in preparation for get_worktrees() to do\noptional things, like sorting.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n branch.c | 2 +-\n builtin/branch.c | 2 +-\n builtin/worktree.c | 6 +++---\n worktree.c | 4 ++--\n worktree.h | 2 +-\n 5 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 0d459b3..c431cbf 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -348,7 +348,7 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)\n int replace_each_worktree_head_symref(const char *oldref, const char *newref)\n {\n \tint ret = 0;\n-\tstruct worktree **worktrees = get_worktrees();\n+\tstruct worktree **worktrees = get_worktrees(0);\n \tint i;\n \n \tfor (i = 0; worktrees[i]; i++) {\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 60cc5c8..4757075 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -531,7 +531,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \n static void reject_rebase_or_bisect_branch(const char *target)\n {\n-\tstruct worktree **worktrees = get_worktrees();\n+\tstruct worktree **worktrees = get_worktrees(0);\n \tint i;\n \n \tfor (i = 0; worktrees[i]; i++) {\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex b835b91..d7d195c 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -447,7 +447,7 @@ static int list(int ac, const char **av, const char *prefix)\n \tif (ac)\n \t\tusage_with_options(worktree_usage, options);\n \telse {\n-\t\tstruct worktree **worktrees = get_worktrees();\n+\t\tstruct worktree **worktrees = get_worktrees(0);\n \t\tint path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;\n \n \t\tif (!porcelain)\n@@ -478,7 +478,7 @@ static int lock_worktree(int ac, const char **av, const char *prefix)\n \tif (ac != 1)\n \t\tusage_with_options(worktree_usage, options);\n \n-\tworktrees = get_worktrees();\n+\tworktrees = get_worktrees(0);\n \twt = find_worktree(worktrees, prefix, av[0]);\n \tif (!wt)\n \t\tdie(_(\"'%s' is not a working tree\"), av[0]);\n@@ -511,7 +511,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)\n \tif (ac != 1)\n \t\tusage_with_options(worktree_usage, options);\n \n-\tworktrees = get_worktrees();\n+\tworktrees = get_worktrees(0);\n \twt = find_worktree(worktrees, prefix, av[0]);\n \tif (!wt)\n \t\tdie(_(\"'%s' is not a working tree\"), av[0]);\ndiff --git a/worktree.c b/worktree.c\nindex 3145522..ead088e 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -160,7 +160,7 @@ static void mark_current_worktree(struct worktree **worktrees)\n \tfree(git_dir);\n }\n \n-struct worktree **get_worktrees(void)\n+struct worktree **get_worktrees(unsigned flags)\n {\n \tstruct worktree **list = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n@@ -327,7 +327,7 @@ const struct worktree *find_shared_symref(const char *symref,\n \n \tif (worktrees)\n \t\tfree_worktrees(worktrees);\n-\tworktrees = get_worktrees();\n+\tworktrees = get_worktrees(0);\n \n \tfor (i = 0; worktrees[i]; i++) {\n \t\tstruct worktree *wt = worktrees[i];\ndiff --git a/worktree.h b/worktree.h\nindex 90e1311..2e68d4a 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -23,7 +23,7 @@ struct worktree {\n * The caller is responsible for freeing the memory from the returned\n * worktree(s).\n */\n-extern struct worktree **get_worktrees(void);\n+extern struct worktree **get_worktrees(unsigned flags);\n \n /*\n * Return git dir of the worktree. Note that the path may be relative.\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"306514","messageId":"xmqq1sxvjt16.fsf@gitster.mtv.corp.google.com","threadId":"44528","inReplyTo":"CACsJy8CpySQs07cfioOnoxoH1TALRePGbeVCc45ZqDr43519qQ@mail.gmail.com","subject":"Re: [PATCH 0/3] Minor fixes on 'git worktree'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-28T19:36:37Z","receivedAt":"2016-11-28T19:36:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> An interesting thing maybe worth\n> sharing is, if HEAD is broken (the only reason we would fail to create\n> \"struct worktree\" for main worktree), then \"git worktree list\" from\n> main worktree would fail too, because repo setup code fails to parse\n> HEAD as well and die().\n\nYeah, that makes sense.\n\n> That makes the \"always show main worktree\"\n> patch useless, since we won't get far enough to execute\n> get_worktrees() in the first place. However, you can still do \"git\n> worktree list\" from a linked worktree, gitdir setup code will not stop\n> you (how can it). And the patch is still needed.\n\nYes.\n"},{"id":"306519","messageId":"xmqqoa0zi9ei.fsf@gitster.mtv.corp.google.com","threadId":"44528","inReplyTo":"20161128093656.15744-1-pclouds@gmail.com","subject":"Re: [PATCH v2 0/5] nd/worktree-list-fixup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-28T21:25:57Z","receivedAt":"2016-11-28T21:26:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> This version\n>\n> * changes get_worktrees() to take a flag, and adds one flag for\n> sorting.\n>\n> * adds tests for both the 'main worktree always present' and the\n> sorting problems.\n>\n> * reworks 3/5 a bit, keep changes closer, easier to see the cause and\n> consequence.\n>\n> Nguyễn Thái Ngọc Duy (5):\n> worktree.c: zero new 'struct worktree' on allocation\n> worktree: reorder an if statement\n> get_worktrees() must return main worktree as first item even on error\n> worktree.c: get_worktrees() takes a new flag argument\n> worktree list: keep the list sorted\n>\n> branch.c | 2 +-\n> builtin/branch.c | 2 +-\n> builtin/worktree.c | 14 ++++++++------\n> t/t2027-worktree-list.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n> worktree.c | 42 +++++++++++++++++++++---------------------\n> worktree.h | 4 +++-\n> 6 files changed, 74 insertions(+), 30 deletions(-)\n\nThanks for a pleasant read. Will replace what has been queued.\n"}]}