{"thread":{"id":"40051","subject":"[PATCH v3] worktree: add 'list' command","startedAt":"2015-08-10T20:53:01Z","lastAt":"2015-08-11T15:46:56Z","messageCount":8,"participants":["Michael Rappazzo","Junio C Hamano","David Turner","Mike Rappazzo"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"267793","messageId":"1439239982-42826-1-git-send-email-rappazzo@gmail.com","threadId":"40051","inReplyTo":null,"subject":"[PATCH v3] worktree: add 'list' command","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-08-10T20:53:01Z","receivedAt":"2015-08-10T20:53:01Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Differences from [v2](http://www.mail-archive.com/git@vger.kernel.org/msg75467.html)\n - removed unintended whitespace changes\n - cleanup based on comments from v2\n\nMichael Rappazzo (1):\n worktree: add 'list' command\n\n Documentation/git-worktree.txt | 6 +++-\n builtin/worktree.c | 67 ++++++++++++++++++++++++++++++++++++++++++\n t/t2027-worktree-list.sh | 30 +++++++++++++++++++\n 3 files changed, 102 insertions(+), 1 deletion(-)\n create mode 100755 t/t2027-worktree-list.sh\n\n-- \n2.5.0\n"},{"id":"267794","messageId":"1439239982-42826-2-git-send-email-rappazzo@gmail.com","threadId":"40051","inReplyTo":"1439239982-42826-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v3] worktree: add 'list' command","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-08-10T20:53:02Z","receivedAt":"2015-08-10T20:53:02Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"'git worktree list' will list the main worktree followed by any linked\nworktrees which were created using 'git worktree add'.\n\nSigned-off-by: Michael Rappazzo \n---\n Documentation/git-worktree.txt | 6 +++-\n builtin/worktree.c | 67 ++++++++++++++++++++++++++++++++++++++++++\n t/t2027-worktree-list.sh | 30 +++++++++++++++++++\n 3 files changed, 102 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 3387e2f..efb8e9d 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 ] []\n 'git worktree prune' [-n] [-v] [--expire ]\n+'git worktree list'\n \n DESCRIPTION\n -----------\n@@ -59,6 +60,10 @@ prune::\n \n Prune working tree information in $GIT_DIR/worktrees.\n \n+list::\n+\n+List the main worktree followed by all of the linked worktrees.\n+\n OPTIONS\n -------\n \n@@ -167,7 +172,6 @@ performed manually, such as:\n - `remove` to remove a linked worktree and its administrative files (and\n warn if the worktree is dirty)\n - `mv` to move or rename a worktree and update its administrative files\n-- `list` to list linked worktrees\n - `lock` to prevent automatic pruning of administrative files (for instance,\n for a worktree on a portable device)\n \ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 6a264ee..d90bdee 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -10,6 +10,7 @@\n static const char * const worktree_usage[] = {\n \tN_(\"git worktree add [] \"),\n \tN_(\"git worktree prune []\"),\n+\tN_(\"git worktree list []\"),\n \tNULL\n };\n \n@@ -316,6 +317,70 @@ static int add(int ac, const char **av, const char *prefix)\n \treturn add_worktree(path, cmd.argv);\n }\n \n+static int list(int ac, const char **av, const char *prefix)\n+{\n+\tint main_only = 0;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"main-only\", &main_only, N_(\"only list the main worktree\")),\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+\n+\tstruct strbuf main_path = STRBUF_INIT;\n+\tconst char* common_dir = get_git_common_dir();\n+\tint is_bare = is_bare_repository();\n+\tif (is_bare) {\n+\t\tstrbuf_addstr(&main_path, absolute_path(common_dir));\n+\t\tstrbuf_strip_suffix(&main_path, \"/.\");\n+\t} else if (!strcmp(common_dir, \".git\")) {\n+\t\t/* within a worktree, the common dir only returns \".git\" */\n+\t\tstrbuf_addstr(&main_path, get_git_work_tree());\n+\t} else {\n+\t\tstrbuf_addstr(&main_path, common_dir);\n+\t\tstrbuf_strip_suffix(&main_path, \"/.git\");\n+\t}\n+\n+\tprintf(\"%s\\n\", main_path.buf);\n+\n+\tif (!is_bare) {\n+\t\tstrbuf_addstr(&main_path, \"/.git\");\n+\t}\n+\tstrbuf_addstr(&main_path, \"/worktrees\");\n+\n+\tif (is_directory(main_path.buf)) {\n+\t\tDIR *dir = opendir(main_path.buf);\n+\t\tif (dir) {\n+\t\t\tstruct dirent *d;\n+\t\t\tstruct stat st;\n+\t\t\tstruct strbuf path = STRBUF_INIT;\n+\t\t\tstruct strbuf other_worktree_path = STRBUF_INIT;\n+\t\t\twhile ((d = readdir(dir)) != NULL) {\n+\t\t\t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n+\t\t\t\t\tcontinue;\n+\t\t\t\tstrbuf_reset(&other_worktree_path);\n+\t\t\t\tstrbuf_addf(&other_worktree_path, \"%s/%s/gitdir\", main_path.buf, d->d_name);\n+\n+\t\t\t\tif (stat(other_worktree_path.buf, &st))\n+\t\t\t\t\tcontinue;\n+\n+\t\t\t\tstrbuf_reset(&path);\n+\t\t\t\tstrbuf_read_file(&path, other_worktree_path.buf, st.st_size);\n+\t\t\t\tstrbuf_strip_suffix(&path, \"/.git\\n\");\n+\n+\t\t\t\tprintf(\"%s\\n\", path.buf);\n+\t\t\t}\n+\t\t\tstrbuf_release(&other_worktree_path);\n+\t\t\tstrbuf_release(&path);\n+\t\t}\n+\t\tclosedir(dir);\n+\t}\n+\tstrbuf_release(&main_path);\n+\treturn 0;\n+}\n+\n int cmd_worktree(int ac, const char **av, const char *prefix)\n {\n \tstruct option options[] = {\n@@ -328,5 +393,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..8e3dbbc\n--- /dev/null\n+++ b/t/t2027-worktree-list.sh\n@@ -0,0 +1,30 @@\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+\tgit rev-parse --show-toplevel >expect &&\n+\tgit worktree add --detach here master &&\n+\tgit -C here rev-parse --show-toplevel >>expect &&\n+\tgit worktree list >actual &&\n+\ttest_cmp expect actual &&\n+\trm -rf here &&\n+\tgit worktree prune\n+'\n+test_expect_success '\"list\" all worktrees from linked' '\n+\tgit rev-parse --show-toplevel >expect &&\n+\tgit worktree add --detach here master &&\n+\tgit -C here rev-parse --show-toplevel >>expect &&\n+\tgit -C here worktree list >actual &&\n+\ttest_cmp expect actual &&\n+\trm -rf here &&\n+\tgit worktree prune\n+'\n+\n+test_done\n-- \n2.5.0\n"},{"id":"267795","messageId":"xmqqr3nau74h.fsf@gitster.dls.corp.google.com","threadId":"40051","inReplyTo":"1439239982-42826-2-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v3] worktree: add 'list' command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-10T22:10:54Z","receivedAt":"2015-08-10T22:10:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Rappazzo writes:\n\n> +static int list(int ac, const char **av, const char *prefix)\n> +{\n> +\tint main_only = 0;\n> +\tstruct option options[] = {\n> +\t\tOPT_BOOL(0, \"main-only\", &main_only, N_(\"only list the main worktree\")),\n> +\t\tOPT_END()\n> +\t};\n\nHmm, main-only is still there?\n\n> +\n> +\tac = parse_options(ac, av, prefix, options, worktree_usage, 0);\n> +\tif (ac)\n> +\t\tusage_with_options(worktree_usage, options);\n> +\n> +\tstruct strbuf main_path = STRBUF_INIT;\n> +\tconst char* common_dir = get_git_common_dir();\n\nAsterisks stick to the variable names, not types.\n\n> +\tint is_bare = is_bare_repository();\n\nPlease do not introduce decl-after-stmt.\n\n> +\tif (is_bare) {\n> +\t\tstrbuf_addstr(&main_path, absolute_path(common_dir));\n\nHmm, interesting.\n\nBecause .git/config is shared, core.bare read from that tells us if\nthe \"main\" one is bare, even if you start this command from one of\nits linked worktrees. So in that sense, this test of is_bare\ncorrectly tells if \"main\" one is a bare repository.\n\nBut that by itself feels wrong. Doesn't the presense of a working\ntree mean that you should not get \"is_bare==true\" in such a case\n(i.e. your \"main\" one is bare, you are in a linked worktree of it\nthat has the index and the working tree)?\n\nDuy? Eric? What do you guys think?\n\nThere are many codepaths that change their behaviour (e.g. if we\ncreate reflogs by default) based on the return value of\nis_bare_repository(). If I am reading this correctly, I _think_ a\nnew working area that was prepared with \"git worktree add\" out of a\nbare repository would not work well, as these operations behave as\nif we do not have a working tree. Perhaps is_bare_repository() in\nsuch a working area _should_ say \"No\", even if core.bare in the\nshared bare one is set to true.\n\n> +\t\tstrbuf_strip_suffix(&main_path, \"/.\");\n\nIn any case, what is that stripping of \"/.\" about? Who is adding\nthat extra trailing string?\n\nWhat I am getting at is (1) perhaps it shouldn't be adding that in\nthe first place, and (2) if some other code is randomly adding \"/.\"\nat the end, what guarantees you that you would need to strip it only\nonce here---if the other code added that twice, don't you have to\nrepeatedly remove \"/.\" from the end?\n"},{"id":"267810","messageId":"1439261725.5283.9.camel@twopensource.com","threadId":"40051","inReplyTo":"1439239982-42826-2-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v3] worktree: add 'list' command","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-11T02:55:25Z","receivedAt":"2015-08-11T02:55:25Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2015-08-10 at 16:53 -0400, Michael Rappazzo wrote:\n> +\t\t\twhile ((d = readdir(dir)) != NULL) {\n\nI think it would be useful to break this loop out into a\nfor_each_worktree function.\n\nWhile looking into per-worktree ref stuff, I have just noticed that git\nprune will delete objects that are only referenced in a different\nworktree's detached HEAD. To fix this, git prune will need to walk over\neach worktree, looking at that worktree's HEAD (and other per-worktree\nrefs). It would be useful to be able to reuse some of this code for\nthat task.\n"},{"id":"267822","messageId":"CANoM8SWeqxD2vWLQmEfxxxn8Dz4yPfjGOoOH=Azn1A3So+wz2Q@mail.gmail.com","threadId":"40051","inReplyTo":"xmqqr3nau74h.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3] worktree: add 'list' command","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-08-11T11:41:21Z","receivedAt":"2015-08-11T11:41:21Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Mon, Aug 10, 2015 at 6:10 PM, Junio C Hamano wrote:\n> Michael Rappazzo writes:\n>\n>> +static int list(int ac, const char **av, const char *prefix)\n>> +{\n>> + int main_only = 0;\n>> + struct option options[] = {\n>> + OPT_BOOL(0, \"main-only\", &main_only, N_(\"only list the main worktree\")),\n>> + OPT_END()\n>> + };\n>\n> Hmm, main-only is still there?\n\nSorry, I missed that.\n\n>\n>> + int is_bare = is_bare_repository();\n>\n> Please do not introduce decl-after-stmt.\n\nSince I reused this value below, I thought it would be acceptable.\n\n>> + if (is_bare) {\n>> + strbuf_addstr(&main_path, absolute_path(common_dir));\n>\n> Hmm, interesting.\n>\n> Because .git/config is shared, core.bare read from that tells us if\n> the \"main\" one is bare, even if you start this command from one of\n> its linked worktrees. So in that sense, this test of is_bare\n> correctly tells if \"main\" one is a bare repository.\n>\n> But that by itself feels wrong. Doesn't the presense of a working\n> tree mean that you should not get \"is_bare==true\" in such a case\n> (i.e. your \"main\" one is bare, you are in a linked worktree of it\n> that has the index and the working tree)?\n\nIs is even correct for a bare repo to be included in the list? I\nthink that is part of what you are asking here.\n\n>\n>> + strbuf_strip_suffix(&main_path, \"/.\");\n>\n> In any case, what is that stripping of \"/.\" about? Who is adding\n> that extra trailing string?\n>\n> What I am getting at is (1) perhaps it shouldn't be adding that in\n> the first place, and (2) if some other code is randomly adding \"/.\"\n> at the end, what guarantees you that you would need to strip it only\n> once here---if the other code added that twice, don't you have to\n> repeatedly remove \"/.\" from the end?\n\nIn the case of a common dir, the value returned is just '.'. I wanted\nto resolve that to the full path, so I called\n`absolute_path(common_dir)`. Hence the trailing \"/.\". Similarly, in\nthe main repo, the common dir is just \".git\", and I handled that by\nusing `get_git_work_tree()`.\n"},{"id":"267823","messageId":"CANoM8SUznDcZyH6-O9W2ZS7mwwevUz5ZwX4L_fVFV1cgRrF5xA@mail.gmail.com","threadId":"40051","inReplyTo":"1439261725.5283.9.camel@twopensource.com","subject":"Re: [PATCH v3] worktree: add 'list' command","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-08-11T11:42:51Z","receivedAt":"2015-08-11T11:42:51Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Mon, Aug 10, 2015 at 10:55 PM, David Turner wrote:\n> On Mon, 2015-08-10 at 16:53 -0400, Michael Rappazzo wrote:\n>> + while ((d = readdir(dir)) != NULL) {\n>\n> I think it would be useful to break this loop out into a\n> for_each_worktree function.\n>\n> While looking into per-worktree ref stuff, I have just noticed that git\n> prune will delete objects that are only referenced in a different\n> worktree's detached HEAD. To fix this, git prune will need to walk over\n> each worktree, looking at that worktree's HEAD (and other per-worktree\n> refs). It would be useful to be able to reuse some of this code for\n> that task.\n>\n\nI agree, but I will save that for another round.\n"},{"id":"267832","messageId":"xmqq37zpu8tv.fsf@gitster.dls.corp.google.com","threadId":"40051","inReplyTo":"CANoM8SWeqxD2vWLQmEfxxxn8Dz4yPfjGOoOH=Azn1A3So+wz2Q@mail.gmail.com","subject":"Re: [PATCH v3] worktree: add 'list' command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-11T15:46:20Z","receivedAt":"2015-08-11T15:46:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Rappazzo writes:\n\n>>> + int is_bare = is_bare_repository();\n>>\n>> Please do not introduce decl-after-stmt.\n>\n> Since I reused this value below, I thought it would be acceptable.\n\nUse of a new variable is fine. \"Do not declare one in a block after\nyou already wrote statement\" is what \"decl-after-stmt not allowed\"\nmeans. In your patch:\n\n+static int list(int ac, const char **av, const char *prefix)\n+{\n+\tint main_only = 0;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"main-only\", &main_only, N_(\"only list the main worktree\")),\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+\n+\tstruct strbuf main_path = STRBUF_INIT;\n+\tconst char* common_dir = get_git_common_dir();\n+\tint is_bare = is_bare_repository();\n\nThree variables, main_path, common_dir and is_bare are declared here\nafter statements such as a call to parse_options(). Don't.\n\n+static int list(int ac, const char **av, const char *prefix)\n+{\n+\tint main_only = 0;\n+\tstruct strbuf main_path = STRBUF_INIT;\n+\tconst char *common_dir;\n+\tint is_bare;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"main-only\", &main_only, N_(\"only list the main worktree\")),\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+\n+\tcommon_dir = get_git_common_dir();\n+\tint is_bare = is_bare_repository();\n"},{"id":"267833","messageId":"xmqqy4hhsu8f.fsf@gitster.dls.corp.google.com","threadId":"40051","inReplyTo":"1439261725.5283.9.camel@twopensource.com","subject":"Re: [PATCH v3] worktree: add 'list' command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-11T15:46:56Z","receivedAt":"2015-08-11T15:46:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> On Mon, 2015-08-10 at 16:53 -0400, Michael Rappazzo wrote:\n>> +\t\t\twhile ((d = readdir(dir)) != NULL) {\n>\n> I think it would be useful to break this loop out into a\n> for_each_worktree function.\n\nVery good point.\n"}]}