{"thread":{"id":"45021","subject":"[PATCH 00/11] nd/worktree-move update","startedAt":"2017-02-02T08:50:20Z","lastAt":"2017-02-04T11:57:42Z","messageCount":25,"participants":["Nguyễn Thái Ngọc Duy","Johannes Schindelin","Duy Nguyen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"310702","messageId":"20170202085007.21418-1-pclouds@gmail.com","threadId":"45021","inReplyTo":null,"subject":"[PATCH 00/11] nd/worktree-move update","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:49:56Z","receivedAt":"2017-02-02T08:50:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This squashes two changes from Johannes and Ramsay:\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 339c622e20..a1c91f1542 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -528,7 +528,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)\n \n static void validate_no_submodules(const struct worktree *wt)\n {\n-\tstruct index_state istate = {0};\n+\tstruct index_state istate = { NULL };\n \tint i, found_submodules = 0;\n \n \tif (read_index_from(&istate, worktree_git_path(wt, \"index\")) > 0) {\ndiff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh\nindex 084acc6c6d..b3105eaaed 100755\n--- a/t/t2028-worktree-move.sh\n+++ b/t/t2028-worktree-move.sh\n@@ -71,13 +71,14 @@ test_expect_success 'move locked worktree' '\n '\n \n test_expect_success 'move worktree' '\n+\ttoplevel=\"$(pwd)\" &&\n \tgit worktree move source destination &&\n \ttest_path_is_missing source &&\n \tgit worktree list --porcelain | grep \"^worktree\" >actual &&\n \tcat <<-EOF >expected &&\n-\tworktree $TRASH_DIRECTORY\n-\tworktree $TRASH_DIRECTORY/destination\n-\tworktree $TRASH_DIRECTORY/elsewhere\n+\tworktree $toplevel\n+\tworktree $toplevel/destination\n+\tworktree $toplevel/elsewhere\n \tEOF\n \ttest_cmp expected actual &&\n \tgit -C destination log --format=%s >actual2 &&\n\n\nNguyễn Thái Ngọc Duy (11):\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 worktree.c: add validate_worktree()\n worktree.c: add update_worktree_location()\n worktree move: new command\n worktree move: accept destination as directory\n worktree move: refuse to move worktrees with submodules\n worktree remove: new command\n\n Documentation/git-worktree.txt | 28 ++++--\n branch.c | 2 +-\n builtin/branch.c | 2 +-\n builtin/worktree.c | 176 +++++++++++++++++++++++++++++++--\n contrib/completion/git-completion.bash | 5 +-\n t/t2027-worktree-list.sh | 40 ++++++++\n t/t2028-worktree-move.sh | 57 +++++++++++\n worktree.c | 126 +++++++++++++++++++----\n worktree.h | 15 ++-\n 9 files changed, 410 insertions(+), 41 deletions(-)\n\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"310703","messageId":"20170202085007.21418-2-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 01/11] worktree.c: zero new 'struct worktree' on allocation","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:49:57Z","receivedAt":"2017-02-02T08:50:30Z","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 f7869f8d60..f7c1b5e24d 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.11.0.157.gd943d85\n\n"},{"id":"310704","messageId":"20170202085007.21418-3-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 02/11] worktree: reorder an if statement","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:49:58Z","receivedAt":"2017-02-02T08:50:34Z","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 \nSigned-off-by: Junio C Hamano \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 5c4854d3e4..8a654e4ad3 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.11.0.157.gd943d85\n\n"},{"id":"310705","messageId":"20170202085007.21418-4-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 03/11] 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":"2017-02-02T08:49:59Z","receivedAt":"2017-02-02T08:50:40Z","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 \nSigned-off-by: Junio C Hamano \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 8a654e4ad3..b835b91f63 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 1b1b65a6b0..98b5f340e5 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 f7c1b5e24d..3145522536 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.11.0.157.gd943d85\n\n"},{"id":"310706","messageId":"20170202085007.21418-6-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 05/11] worktree list: keep the list sorted","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:50:01Z","receivedAt":"2017-02-02T08:50:45Z","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 \nSigned-off-by: Junio C Hamano \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 d7d195cd95..9a97e37a3f 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 98b5f340e5..465eeeacd3 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 ead088e43c..eb6121263b 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 2e68d4ad86..d59ce1fee8 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.11.0.157.gd943d85\n\n"},{"id":"310707","messageId":"20170202085007.21418-5-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 04/11] worktree.c: get_worktrees() takes a new flag argument","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:50:00Z","receivedAt":"2017-02-02T08:50:52Z","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 \nSigned-off-by: Junio C Hamano \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 0d459b3cfe..c431cbf6a9 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 60cc5c8e8d..475707528a 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 b835b91f63..d7d195cd95 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 3145522536..ead088e43c 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 90e1311fa7..2e68d4ad86 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.11.0.157.gd943d85\n\n"},{"id":"310708","messageId":"20170202085007.21418-7-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 06/11] worktree.c: add validate_worktree()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:50:02Z","receivedAt":"2017-02-02T08:50:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This function is later used by \"worktree move\" and \"worktree remove\"\nto ensure that we have a good connection between the repository and\nthe worktree. For example, if a worktree is moved manually, the\nworktree location recorded in $GIT_DIR/worktrees/.../gitdir is\nincorrect and we should not move that one.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Junio C Hamano \n---\n worktree.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n worktree.h | 5 +++++\n 2 files changed, 68 insertions(+)\n\ndiff --git a/worktree.c b/worktree.c\nindex eb6121263b..929072ad89 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -291,6 +291,69 @@ const char *is_worktree_locked(struct worktree *wt)\n \treturn wt->lock_reason;\n }\n \n+static int report(int quiet, const char *fmt, ...)\n+{\n+\tva_list params;\n+\n+\tif (quiet)\n+\t\treturn -1;\n+\n+\tva_start(params, fmt);\n+\tvfprintf(stderr, fmt, params);\n+\tfputc('\\n', stderr);\n+\tva_end(params);\n+\treturn -1;\n+}\n+\n+int validate_worktree(const struct worktree *wt, int quiet)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *path;\n+\tint err;\n+\n+\tif (is_main_worktree(wt)) {\n+\t\t/*\n+\t\t * Main worktree using .git file to point to the\n+\t\t * repository would make it impossible to know where\n+\t\t * the actual worktree is if this function is executed\n+\t\t * from another worktree. No .git file support for now.\n+\t\t */\n+\t\tstrbuf_addf(&sb, \"%s/.git\", wt->path);\n+\t\tif (!is_directory(sb.buf)) {\n+\t\t\tstrbuf_release(&sb);\n+\t\t\treturn report(quiet, _(\"'%s/.git' at main worktree is not the repository directory\"),\n+\t\t\t\t wt->path);\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n+\t/*\n+\t * Make sure \"gitdir\" file points to a real .git file and that\n+\t * file points back here.\n+\t */\n+\tif (!is_absolute_path(wt->path))\n+\t\treturn report(quiet, _(\"'%s' file does not contain absolute path to the worktree location\"),\n+\t\t\t git_common_path(\"worktrees/%s/gitdir\", wt->id));\n+\n+\tstrbuf_addf(&sb, \"%s/.git\", wt->path);\n+\tif (!file_exists(sb.buf)) {\n+\t\tstrbuf_release(&sb);\n+\t\treturn report(quiet, _(\"'%s/.git' does not exist\"), wt->path);\n+\t}\n+\n+\tpath = read_gitfile_gently(sb.buf, &err);\n+\tstrbuf_release(&sb);\n+\tif (!path)\n+\t\treturn report(quiet, _(\"'%s/.git' is not a .git file, error code %d\"),\n+\t\t\t wt->path, err);\n+\n+\tif (fspathcmp(path, real_path(git_common_path(\"worktrees/%s\", wt->id))))\n+\t\treturn report(quiet, _(\"'%s' does not point back to\"),\n+\t\t\t wt->path, git_common_path(\"worktrees/%s\", wt->id));\n+\n+\treturn 0;\n+}\n+\n int is_worktree_being_rebased(const struct worktree *wt,\n \t\t\t const char *target)\n {\ndiff --git a/worktree.h b/worktree.h\nindex d59ce1fee8..4433db2eb3 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -53,6 +53,11 @@ extern int is_main_worktree(const struct worktree *wt);\n extern const char *is_worktree_locked(struct worktree *wt);\n \n /*\n+ * Return zero if the worktree is in good condition.\n+ */\n+extern int validate_worktree(const struct worktree *wt, int quiet);\n+\n+/*\n * Free up the memory for worktree(s)\n */\n extern void free_worktrees(struct worktree **);\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"310709","messageId":"20170202085007.21418-9-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 08/11] worktree move: new command","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:50:04Z","receivedAt":"2017-02-02T08:51:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"There are two options to move the main worktree, but both have\ncomplications, so it's not implemented yet. Anyway the options are:\n\n - convert the main worktree to a linked one and move it away, leave the\n git repository where it is. The repo essentially becomes bare after\n this move.\n\n - move the repository with the main worktree. The tricky part is make\n sure all file descriptors to the repository are closed, or it may\n fail on Windows.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/git-worktree.txt | 7 +++++-\n builtin/worktree.c | 43 ++++++++++++++++++++++++++++++++++\n contrib/completion/git-completion.bash | 2 +-\n t/t2028-worktree-move.sh | 31 ++++++++++++++++++++++++\n 4 files changed, 81 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex e257c19ebe..13105138a7 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -12,6 +12,7 @@ SYNOPSIS\n 'git worktree add' [-f] [--detach] [--checkout] [-b ] []\n 'git worktree list' [--porcelain]\n 'git worktree lock' [--reason ] \n+'git worktree move' \n 'git worktree prune' [-n] [-v] [--expire ]\n 'git worktree unlock' \n \n@@ -71,6 +72,11 @@ files from being pruned automatically. This also prevents it from\n being moved or deleted. Optionally, specify a reason for the lock\n with `--reason`.\n \n+move::\n+\n+Move a working tree to a new location. Note that the main working tree\n+cannot be moved yet.\n+\n prune::\n \n Prune working tree information in $GIT_DIR/worktrees.\n@@ -252,7 +258,6 @@ performed manually, such as:\n \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 \n GIT\n ---\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 9a97e37a3f..0d8b57ceb3 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -15,6 +15,7 @@ static const char * const worktree_usage[] = {\n \tN_(\"git worktree add [] []\"),\n \tN_(\"git worktree list []\"),\n \tN_(\"git worktree lock [] \"),\n+\tN_(\"git worktree move \"),\n \tN_(\"git worktree prune []\"),\n \tN_(\"git worktree unlock \"),\n \tNULL\n@@ -524,6 +525,46 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)\n \treturn ret;\n }\n \n+static int move_worktree(int ac, const char **av, const char *prefix)\n+{\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\tstruct worktree **worktrees, *wt;\n+\tstruct strbuf dst = STRBUF_INIT;\n+\tconst char *reason;\n+\n+\tac = parse_options(ac, av, prefix, options, worktree_usage, 0);\n+\tif (ac != 2)\n+\t\tusage_with_options(worktree_usage, options);\n+\n+\tstrbuf_addstr(&dst, prefix_filename(prefix,\n+\t\t\t\t\t strlen(prefix),\n+\t\t\t\t\t av[1]));\n+\tif (file_exists(dst.buf))\n+\t\tdie(_(\"target '%s' already exists\"), av[1]);\n+\n+\tworktrees = get_worktrees(0);\n+\twt = find_worktree(worktrees, prefix, av[0]);\n+\tif (!wt)\n+\t\tdie(_(\"'%s' is not a working directory\"), av[0]);\n+\tif (is_main_worktree(wt))\n+\t\tdie(_(\"'%s' is a main working directory\"), av[0]);\n+\treason = is_worktree_locked(wt);\n+\tif (reason) {\n+\t\tif (*reason)\n+\t\t\tdie(_(\"already locked, reason: %s\"), reason);\n+\t\tdie(_(\"already locked, no reason\"));\n+\t}\n+\tif (validate_worktree(wt, 0))\n+\t\treturn -1;\n+\n+\tif (rename(wt->path, dst.buf) == -1)\n+\t\tdie_errno(_(\"failed to move '%s' to '%s'\"), wt->path, dst.buf);\n+\n+\treturn update_worktree_location(wt, dst.buf);\n+}\n+\n int cmd_worktree(int ac, const char **av, const char *prefix)\n {\n \tstruct option options[] = {\n@@ -546,5 +587,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)\n \t\treturn lock_worktree(ac - 1, av + 1, prefix);\n \tif (!strcmp(av[1], \"unlock\"))\n \t\treturn unlock_worktree(ac - 1, av + 1, prefix);\n+\tif (!strcmp(av[1], \"move\"))\n+\t\treturn move_worktree(ac - 1, av + 1, prefix);\n \tusage_with_options(worktree_usage, options);\n }\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 21016bf8df..613e03b879 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2715,7 +2715,7 @@ _git_whatchanged ()\n \n _git_worktree ()\n {\n-\tlocal subcommands=\"add list lock prune unlock\"\n+\tlocal subcommands=\"add list lock move prune unlock\"\n \tlocal subcommand=\"$(__git_find_on_cmdline \"$subcommands\")\"\n \tif [ -z \"$subcommand\" ]; then\n \t\t__gitcomp \"$subcommands\"\ndiff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh\nindex 8298aaf97f..bef420a086 100755\n--- a/t/t2028-worktree-move.sh\n+++ b/t/t2028-worktree-move.sh\n@@ -59,4 +59,35 @@ test_expect_success 'unlock worktree twice' '\n \ttest_path_is_missing .git/worktrees/source/locked\n '\n \n+test_expect_success 'move non-worktree' '\n+\tmkdir abc &&\n+\ttest_must_fail git worktree move abc def\n+'\n+\n+test_expect_success 'move locked worktree' '\n+\tgit worktree lock source &&\n+\ttest_must_fail git worktree move source destination &&\n+\tgit worktree unlock source\n+'\n+\n+test_expect_success 'move worktree' '\n+\ttoplevel=\"$(pwd)\" &&\n+\tgit worktree move source destination &&\n+\ttest_path_is_missing source &&\n+\tgit worktree list --porcelain | grep \"^worktree\" >actual &&\n+\tcat <<-EOF >expected &&\n+\tworktree $toplevel\n+\tworktree $toplevel/destination\n+\tworktree $toplevel/elsewhere\n+\tEOF\n+\ttest_cmp expected actual &&\n+\tgit -C destination log --format=%s >actual2 &&\n+\techo init >expected2 &&\n+\ttest_cmp expected2 actual2\n+'\n+\n+test_expect_success 'move main worktree' '\n+\ttest_must_fail git worktree move . def\n+'\n+\n test_done\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"310710","messageId":"20170202085007.21418-8-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 07/11] worktree.c: add update_worktree_location()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:50:03Z","receivedAt":"2017-02-02T08:51:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Junio C Hamano \n---\n worktree.c | 21 +++++++++++++++++++++\n worktree.h | 6 ++++++\n 2 files changed, 27 insertions(+)\n\ndiff --git a/worktree.c b/worktree.c\nindex 929072ad89..7684951da5 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -354,6 +354,27 @@ int validate_worktree(const struct worktree *wt, int quiet)\n \treturn 0;\n }\n \n+int update_worktree_location(struct worktree *wt, const char *path_)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tint ret = 0;\n+\n+\tif (is_main_worktree(wt))\n+\t\treturn 0;\n+\n+\tstrbuf_add_absolute_path(&path, path_);\n+\tif (fspathcmp(wt->path, path.buf)) {\n+\t\twrite_file(git_common_path(\"worktrees/%s/gitdir\",\n+\t\t\t\t\t wt->id),\n+\t\t\t \"%s/.git\", real_path(path.buf));\n+\t\tfree(wt->path);\n+\t\twt->path = strbuf_detach(&path, NULL);\n+\t\tret = 0;\n+\t}\n+\tstrbuf_release(&path);\n+\treturn ret;\n+}\n+\n int is_worktree_being_rebased(const struct worktree *wt,\n \t\t\t const char *target)\n {\ndiff --git a/worktree.h b/worktree.h\nindex 4433db2eb3..1ee03f4d06 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -58,6 +58,12 @@ extern const char *is_worktree_locked(struct worktree *wt);\n extern int validate_worktree(const struct worktree *wt, int quiet);\n \n /*\n+ * Update worktrees/xxx/gitdir with the new path.\n+ */\n+extern int update_worktree_location(struct worktree *wt,\n+\t\t\t\t const char *path_);\n+\n+/*\n * Free up the memory for worktree(s)\n */\n extern void free_worktrees(struct worktree **);\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"310711","messageId":"20170202085007.21418-10-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 09/11] worktree move: accept destination as directory","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:50:05Z","receivedAt":"2017-02-02T08:51:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Similar to \"mv a b/\", which is actually \"mv a b/a\", we extract basename\nof source worktree and create a directory of the same name at\ndestination if dst path is a directory.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Junio C Hamano \n---\n builtin/worktree.c | 19 ++++++++++++++++++-\n 1 file changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 0d8b57ceb3..900b68bb5d 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -541,7 +541,13 @@ static int move_worktree(int ac, const char **av, const char *prefix)\n \tstrbuf_addstr(&dst, prefix_filename(prefix,\n \t\t\t\t\t strlen(prefix),\n \t\t\t\t\t av[1]));\n-\tif (file_exists(dst.buf))\n+\tif (is_directory(dst.buf))\n+\t\t/*\n+\t\t * keep going, dst will be appended after we get the\n+\t\t * source's absolute path\n+\t\t */\n+\t\t;\n+\telse if (file_exists(dst.buf))\n \t\tdie(_(\"target '%s' already exists\"), av[1]);\n \n \tworktrees = get_worktrees(0);\n@@ -559,6 +565,17 @@ static int move_worktree(int ac, const char **av, const char *prefix)\n \tif (validate_worktree(wt, 0))\n \t\treturn -1;\n \n+\tif (is_directory(dst.buf)) {\n+\t\tconst char *sep = find_last_dir_sep(wt->path);\n+\n+\t\tif (!sep)\n+\t\t\tdie(_(\"could not figure out destination name from '%s'\"),\n+\t\t\t wt->path);\n+\t\tstrbuf_addstr(&dst, sep);\n+\t\tif (file_exists(dst.buf))\n+\t\t\tdie(_(\"target '%s' already exists\"), dst.buf);\n+\t}\n+\n \tif (rename(wt->path, dst.buf) == -1)\n \t\tdie_errno(_(\"failed to move '%s' to '%s'\"), wt->path, dst.buf);\n \n-- \n2.11.0.157.gd943d85\n\n"},{"id":"310712","messageId":"20170202085007.21418-12-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 11/11] worktree remove: new command","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:50:07Z","receivedAt":"2017-02-02T08:51:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-worktree.txt | 21 +++++----\n builtin/worktree.c | 79 ++++++++++++++++++++++++++++++++++\n contrib/completion/git-completion.bash | 5 ++-\n t/t2028-worktree-move.sh | 26 +++++++++++\n 4 files changed, 121 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex 13105138a7..bbde6b8110 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -14,6 +14,7 @@ SYNOPSIS\n 'git worktree lock' [--reason ] \n 'git worktree move' \n 'git worktree prune' [-n] [-v] [--expire ]\n+'git worktree remove' [--force] \n 'git worktree unlock' \n \n DESCRIPTION\n@@ -81,6 +82,13 @@ prune::\n \n Prune working tree information in $GIT_DIR/worktrees.\n \n+remove::\n+\n+Remove a working tree. Only clean working trees (no untracked files\n+and no modification in tracked files) can be removed. Unclean working\n+trees can be removed with `--force`. The main working tree cannot be\n+removed.\n+\n unlock::\n \n Unlock a working tree, allowing it to be pruned, moved or deleted.\n@@ -90,9 +98,10 @@ OPTIONS\n \n -f::\n --force::\n-\tBy default, `add` refuses to create a new working tree when ``\n-\tis already checked out by another working tree. This option overrides\n-\tthat safeguard.\n+\tBy default, `add` refuses to create a new working tree when\n+\t`` is already checked out by another working tree and\n+\t`remove` refuses to remove an unclean working tree. This option\n+\toverrides that safeguard.\n \n -b ::\n -B ::\n@@ -253,12 +262,6 @@ Multiple checkout in general is still experimental, and the support\n for submodules is incomplete. It is NOT recommended to make multiple\n checkouts of a superproject.\n \n-git-worktree could provide more automation for tasks currently\n-performed manually, such as:\n-\n-- `remove` to remove a linked working tree and its administrative files (and\n- warn if the working tree is dirty)\n-\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 6c58d620ce..a1c91f1542 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -17,6 +17,7 @@ static const char * const worktree_usage[] = {\n \tN_(\"git worktree lock [] \"),\n \tN_(\"git worktree move \"),\n \tN_(\"git worktree prune []\"),\n+\tN_(\"git worktree remove [] \"),\n \tN_(\"git worktree unlock \"),\n \tNULL\n };\n@@ -605,6 +606,82 @@ static int move_worktree(int ac, const char **av, const char *prefix)\n \treturn update_worktree_location(wt, dst.buf);\n }\n \n+static int remove_worktree(int ac, const char **av, const char *prefix)\n+{\n+\tint force = 0;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"force\", &force,\n+\t\t\t N_(\"force removing even if the worktree is dirty\")),\n+\t\tOPT_END()\n+\t};\n+\tstruct worktree **worktrees, *wt;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *reason;\n+\tint ret = 0;\n+\n+\tac = parse_options(ac, av, prefix, options, worktree_usage, 0);\n+\tif (ac != 1)\n+\t\tusage_with_options(worktree_usage, options);\n+\n+\tworktrees = get_worktrees(0);\n+\twt = find_worktree(worktrees, prefix, av[0]);\n+\tif (!wt)\n+\t\tdie(_(\"'%s' is not a working directory\"), av[0]);\n+\tif (is_main_worktree(wt))\n+\t\tdie(_(\"'%s' is a main working directory\"), av[0]);\n+\treason = is_worktree_locked(wt);\n+\tif (reason) {\n+\t\tif (*reason)\n+\t\t\tdie(_(\"already locked, reason: %s\"), reason);\n+\t\tdie(_(\"already locked, no reason\"));\n+\t}\n+\tif (validate_worktree(wt, 0))\n+\t\treturn -1;\n+\n+\tif (!force) {\n+\t\tstruct argv_array child_env = ARGV_ARRAY_INIT;\n+\t\tstruct child_process cp;\n+\t\tchar buf[1];\n+\n+\t\targv_array_pushf(&child_env, \"%s=%s/.git\",\n+\t\t\t\t GIT_DIR_ENVIRONMENT, wt->path);\n+\t\targv_array_pushf(&child_env, \"%s=%s\",\n+\t\t\t\t GIT_WORK_TREE_ENVIRONMENT, wt->path);\n+\t\tmemset(&cp, 0, sizeof(cp));\n+\t\targv_array_pushl(&cp.args, \"status\", \"--porcelain\", NULL);\n+\t\tcp.env = child_env.argv;\n+\t\tcp.git_cmd = 1;\n+\t\tcp.dir = wt->path;\n+\t\tcp.out = -1;\n+\t\tret = start_command(&cp);\n+\t\tif (ret)\n+\t\t\tdie_errno(_(\"failed to run git-status on '%s', code %d\"),\n+\t\t\t\t av[0], ret);\n+\t\tret = xread(cp.out, buf, sizeof(buf));\n+\t\tif (ret)\n+\t\t\tdie(_(\"'%s' is dirty, use --force to delete it\"), av[0]);\n+\t\tclose(cp.out);\n+\t\tret = finish_command(&cp);\n+\t\tif (ret)\n+\t\t\tdie_errno(_(\"failed to run git-status on '%s', code %d\"),\n+\t\t\t\t av[0], ret);\n+\t}\n+\tstrbuf_addstr(&sb, wt->path);\n+\tif (remove_dir_recursively(&sb, 0)) {\n+\t\terror_errno(_(\"failed to delete '%s'\"), sb.buf);\n+\t\tret = -1;\n+\t}\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addstr(&sb, git_common_path(\"worktrees/%s\", wt->id));\n+\tif (remove_dir_recursively(&sb, 0)) {\n+\t\terror_errno(_(\"failed to delete '%s'\"), sb.buf);\n+\t\tret = -1;\n+\t}\n+\tstrbuf_release(&sb);\n+\tfree_worktrees(worktrees);\n+\treturn ret;\n+}\n+\n int cmd_worktree(int ac, const char **av, const char *prefix)\n {\n \tstruct option options[] = {\n@@ -629,5 +706,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)\n \t\treturn unlock_worktree(ac - 1, av + 1, prefix);\n \tif (!strcmp(av[1], \"move\"))\n \t\treturn move_worktree(ac - 1, av + 1, prefix);\n+\tif (!strcmp(av[1], \"remove\"))\n+\t\treturn remove_worktree(ac - 1, av + 1, prefix);\n \tusage_with_options(worktree_usage, options);\n }\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 613e03b879..f6855af1f6 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2715,7 +2715,7 @@ _git_whatchanged ()\n \n _git_worktree ()\n {\n-\tlocal subcommands=\"add list lock move prune unlock\"\n+\tlocal subcommands=\"add list lock move prune remove unlock\"\n \tlocal subcommand=\"$(__git_find_on_cmdline \"$subcommands\")\"\n \tif [ -z \"$subcommand\" ]; then\n \t\t__gitcomp \"$subcommands\"\n@@ -2733,6 +2733,9 @@ _git_worktree ()\n \t\tprune,--*)\n \t\t\t__gitcomp \"--dry-run --expire --verbose\"\n \t\t\t;;\n+\t\tremove,--*)\n+\t\t\t__gitcomp \"--force\"\n+\t\t\t;;\n \t\t*)\n \t\t\t;;\n \t\tesac\ndiff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh\nindex bef420a086..b3105eaaed 100755\n--- a/t/t2028-worktree-move.sh\n+++ b/t/t2028-worktree-move.sh\n@@ -90,4 +90,30 @@ test_expect_success 'move main worktree' '\n \ttest_must_fail git worktree move . def\n '\n \n+test_expect_success 'remove main worktree' '\n+\ttest_must_fail git worktree remove .\n+'\n+\n+test_expect_success 'remove locked worktree' '\n+\tgit worktree lock destination &&\n+\ttest_must_fail git worktree remove destination &&\n+\tgit worktree unlock destination\n+'\n+\n+test_expect_success 'remove worktree with dirty tracked file' '\n+\techo dirty >>destination/init.t &&\n+\ttest_must_fail git worktree remove destination\n+'\n+\n+test_expect_success 'remove worktree with untracked file' '\n+\tgit -C destination checkout init.t &&\n+\t: >destination/untracked &&\n+\ttest_must_fail git worktree remove destination\n+'\n+\n+test_expect_success 'force remove worktree with untracked file' '\n+\tgit worktree remove --force destination &&\n+\ttest_path_is_missing destination\n+'\n+\n test_done\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"310713","messageId":"20170202085007.21418-11-pclouds@gmail.com","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"[PATCH 10/11] worktree move: refuse to move worktrees with submodules","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T08:50:06Z","receivedAt":"2017-02-02T08:51:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Submodules contains .git files with relative paths. After a worktree\nmove, these files need to be updated or they may point to nowhere.\n\nThis is a bandage patch to make sure \"worktree move\" don't break\npeople's worktrees by accident. When .git file update code is in\nplace, this validate_no_submodules() could be removed.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/worktree.c | 23 +++++++++++++++++++++++\n 1 file changed, 23 insertions(+)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 900b68bb5d..6c58d620ce 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -525,6 +525,27 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)\n \treturn ret;\n }\n \n+static void validate_no_submodules(const struct worktree *wt)\n+{\n+\tstruct index_state istate = { NULL };\n+\tint i, found_submodules = 0;\n+\n+\tif (read_index_from(&istate, worktree_git_path(wt, \"index\")) > 0) {\n+\t\tfor (i = 0; i < istate.cache_nr; i++) {\n+\t\t\tstruct cache_entry *ce = istate.cache[i];\n+\n+\t\t\tif (S_ISGITLINK(ce->ce_mode)) {\n+\t\t\t\tfound_submodules = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\n+\tdiscard_index(&istate);\n+\n+\tif (found_submodules)\n+\t\tdie(_(\"This working tree contains submodules and cannot be moved yet\"));\n+}\n+\n static int move_worktree(int ac, const char **av, const char *prefix)\n {\n \tstruct option options[] = {\n@@ -565,6 +586,8 @@ static int move_worktree(int ac, const char **av, const char *prefix)\n \tif (validate_worktree(wt, 0))\n \t\treturn -1;\n \n+\tvalidate_no_submodules(wt);\n+\n \tif (is_directory(dst.buf)) {\n \t\tconst char *sep = find_last_dir_sep(wt->path);\n \n-- \n2.11.0.157.gd943d85\n\n"},{"id":"310717","messageId":"alpine.DEB.2.20.1702021015160.3496@virtualbox","threadId":"45021","inReplyTo":"20170202085007.21418-1-pclouds@gmail.com","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-02-02T09:16:20Z","receivedAt":"2017-02-02T09:16:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:\n\n> This squashes two changes from Johannes and Ramsay: [...]\n\nSorry, I lost track of the worktree discussions... Could you remind me\nwhich patch is supposed to fix my continuous reflog corruption?\n\nThanks,\nDscho"},{"id":"310719","messageId":"CACsJy8B3bdokeYVt6aEyZVSzO50PiQRn+0sid9mSDTZ9q-mnww@mail.gmail.com","threadId":"45021","inReplyTo":"alpine.DEB.2.20.1702021015160.3496@virtualbox","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T09:27:30Z","receivedAt":"2017-02-02T09:28:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin\n wrote:\n> Hi Duy,\n>\n> On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:\n>\n>> This squashes two changes from Johannes and Ramsay: [...]\n>\n> Sorry, I lost track of the worktree discussions... Could you remind me\n> which patch is supposed to fix my continuous reflog corruption?\n\nThe corruption caused by git-gc? It's not fixed. All the changes in\nthis series is shown here.\n-- \nDuy\n"},{"id":"310724","messageId":"alpine.DEB.2.20.1702021043110.3496@virtualbox","threadId":"45021","inReplyTo":"CACsJy8B3bdokeYVt6aEyZVSzO50PiQRn+0sid9mSDTZ9q-mnww@mail.gmail.com","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-02-02T09:43:45Z","receivedAt":"2017-02-02T09:44:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 2 Feb 2017, Duy Nguyen wrote:\n\n> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin\n> wrote:\n> > Hi Duy,\n> >\n> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:\n> >\n> >> This squashes two changes from Johannes and Ramsay: [...]\n> >\n> > Sorry, I lost track of the worktree discussions... Could you remind me\n> > which patch is supposed to fix my continuous reflog corruption?\n> \n> The corruption caused by git-gc? It's not fixed. All the changes in\n> this series is shown here.\n\nOh sorry, I meant to ask \"and if it is not in this patch series, would you\nmind pointing me at the patch series that has that fix?\"\n\nThanks,\nJohannes"},{"id":"310725","messageId":"CACsJy8A-tuea7W+tj6rNddtM0j_374FODjQqKsT8eHfeZ0qDZg@mail.gmail.com","threadId":"45021","inReplyTo":"alpine.DEB.2.20.1702021043110.3496@virtualbox","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T09:50:05Z","receivedAt":"2017-02-02T09:50:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin\n wrote:\n> Hi Duy,\n>\n> On Thu, 2 Feb 2017, Duy Nguyen wrote:\n>\n>> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin\n>> wrote:\n>> > Hi Duy,\n>> >\n>> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:\n>> >\n>> >> This squashes two changes from Johannes and Ramsay: [...]\n>> >\n>> > Sorry, I lost track of the worktree discussions... Could you remind me\n>> > which patch is supposed to fix my continuous reflog corruption?\n>>\n>> The corruption caused by git-gc? It's not fixed. All the changes in\n>> this series is shown here.\n>\n> Oh sorry, I meant to ask \"and if it is not in this patch series, would you\n> mind pointing me at the patch series that has that fix?\"\n\nYou meant this one [1]? There is nothing substantial since then.\n\n[1] https://public-inbox.org/git/%3C20160601104519.16563-1-pclouds@gmail.com%3E/\n\n-- \nDuy\n"},{"id":"310726","messageId":"alpine.DEB.2.20.1702021136210.3496@virtualbox","threadId":"45021","inReplyTo":"CACsJy8A-tuea7W+tj6rNddtM0j_374FODjQqKsT8eHfeZ0qDZg@mail.gmail.com","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-02-02T10:37:17Z","receivedAt":"2017-02-02T10:38:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 2 Feb 2017, Duy Nguyen wrote:\n\n> On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin\n> wrote:\n> >\n> > On Thu, 2 Feb 2017, Duy Nguyen wrote:\n> >\n> >> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin\n> >> wrote:\n> >> >\n> >> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:\n> >> >\n> >> >> This squashes two changes from Johannes and Ramsay: [...]\n> >> >\n> >> > Sorry, I lost track of the worktree discussions... Could you remind\n> >> > me which patch is supposed to fix my continuous reflog corruption?\n> >>\n> >> The corruption caused by git-gc? It's not fixed. All the changes in\n> >> this series is shown here.\n> >\n> > Oh sorry, I meant to ask \"and if it is not in this patch series, would you\n> > mind pointing me at the patch series that has that fix?\"\n> \n> You meant this one [1]? There is nothing substantial since then.\n> \n> [1] https://public-inbox.org/git/%3C20160601104519.16563-1-pclouds@gmail.com%3E/\n\nI guess I mean that.\n\nGiven that this results in real data loss, it is surprising that this has\nnot made it even into `pu` yet!\n\nWould you mind rebasing and re-submitting?\n\nThanks,\nJohannes"},{"id":"310728","messageId":"CACsJy8CBG_a_nX_syXKrdG2-ren=NO9CNxe6tm94FGnEo1HZLQ@mail.gmail.com","threadId":"45021","inReplyTo":"alpine.DEB.2.20.1702021136210.3496@virtualbox","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-02T11:22:50Z","receivedAt":"2017-02-02T11:23:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Feb 2, 2017 at 5:37 PM, Johannes Schindelin\n wrote:\n> Hi Duy,\n>\n> On Thu, 2 Feb 2017, Duy Nguyen wrote:\n>\n>> On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin\n>> wrote:\n>> >\n>> > On Thu, 2 Feb 2017, Duy Nguyen wrote:\n>> >\n>> >> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin\n>> >> wrote:\n>> >> >\n>> >> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:\n>> >> >\n>> >> >> This squashes two changes from Johannes and Ramsay: [...]\n>> >> >\n>> >> > Sorry, I lost track of the worktree discussions... Could you remind\n>> >> > me which patch is supposed to fix my continuous reflog corruption?\n>> >>\n>> >> The corruption caused by git-gc? It's not fixed. All the changes in\n>> >> this series is shown here.\n>> >\n>> > Oh sorry, I meant to ask \"and if it is not in this patch series, would you\n>> > mind pointing me at the patch series that has that fix?\"\n>>\n>> You meant this one [1]? There is nothing substantial since then.\n>>\n>> [1] https://public-inbox.org/git/%3C20160601104519.16563-1-pclouds@gmail.com%3E/\n>\n> I guess I mean that.\n>\n> Given that this results in real data loss, it is surprising that this has\n> not made it even into `pu` yet!\n\nI could rebase and clean it up a bit if you need it, but I don't\nthink it'll end up in 'pu' or anywhere near since Junio wanted a\ncleaner approach [1]. That means (as far as I can see) a lot more work\naround refs store and backend area before it's ready to handle \"get\nrefs from this worktree store\" (or \"get refs from every reachable\nstores\").\n\n[1] https://public-inbox.org/git/xmqqshwwzyee.fsf@gitster.mtv.corp.google.com/\n\n-- \nDuy\n"},{"id":"310729","messageId":"alpine.DEB.2.20.1702021223320.3496@virtualbox","threadId":"45021","inReplyTo":"CACsJy8CBG_a_nX_syXKrdG2-ren=NO9CNxe6tm94FGnEo1HZLQ@mail.gmail.com","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-02-02T11:31:09Z","receivedAt":"2017-02-02T12:03:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 2 Feb 2017, Duy Nguyen wrote:\n\n> On Thu, Feb 2, 2017 at 5:37 PM, Johannes Schindelin\n> wrote:\n> >\n> > On Thu, 2 Feb 2017, Duy Nguyen wrote:\n> >\n> >> On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin\n> >> wrote:\n> >> >\n> >> > On Thu, 2 Feb 2017, Duy Nguyen wrote:\n> >> >\n> >> >> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin\n> >> >> wrote:\n> >> >> >\n> >> >> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:\n> >> >> >\n> >> >> >> This squashes two changes from Johannes and Ramsay: [...]\n> >> >> >\n> >> >> > Sorry, I lost track of the worktree discussions... Could you\n> >> >> > remind me which patch is supposed to fix my continuous reflog\n> >> >> > corruption?\n> >> >>\n> >> >> The corruption caused by git-gc? It's not fixed. All the changes\n> >> >> in this series is shown here.\n> >> >\n> >> > Oh sorry, I meant to ask \"and if it is not in this patch series,\n> >> > would you mind pointing me at the patch series that has that fix?\"\n> >>\n> >> You meant this one [1]? There is nothing substantial since then.\n> >>\n> >> [1] https://public-inbox.org/git/%3C20160601104519.16563-1-pclouds@gmail.com%3E/\n> >\n> > I guess I mean that.\n> >\n> > Given that this results in real data loss, it is surprising that this\n> > has not made it even into `pu` yet!\n> \n> I could rebase and clean it up a bit if you need it, but I don't think\n> it'll end up in 'pu' or anywhere near since Junio wanted a cleaner\n> approach [1]. That means (as far as I can see) a lot more work around\n> refs store and backend area before it's ready to handle \"get refs from\n> this worktree store\" (or \"get refs from every reachable stores\").\n> \n> [1] https://public-inbox.org/git/xmqqshwwzyee.fsf@gitster.mtv.corp.google.com/\n\nThat is a big, big bummer.\n\nWe are talking about a data corrupting bug here, yes? It should be\npossible to do that redesign work while having a small workaround in place\nthat unbreaks, say, me?\n\nCiao,\nJohannes"},{"id":"310730","messageId":"alpine.DEB.2.20.1702021330040.3496@virtualbox","threadId":"45021","inReplyTo":"alpine.DEB.2.20.1702021223320.3496@virtualbox","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-02-02T12:33:38Z","receivedAt":"2017-02-02T12:33:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 2 Feb 2017, Johannes Schindelin wrote:\n\n> Hi Duy,\n> \n> On Thu, 2 Feb 2017, Duy Nguyen wrote:\n> \n> > On Thu, Feb 2, 2017 at 5:37 PM, Johannes Schindelin\n> > wrote:\n> > >\n> > > On Thu, 2 Feb 2017, Duy Nguyen wrote:\n> > >\n> > >> You meant this one [1]? There is nothing substantial since then.\n> > >>\n> > >> [1] https://public-inbox.org/git/%3C20160601104519.16563-1-pclouds@gmail.com%3E/\n> > \n> > I could rebase and clean it up a bit if you need it, but I don't\n> > think it'll end up in 'pu' or anywhere near since Junio wanted a\n> > cleaner approach [1]. That means (as far as I can see) a lot more work\n> > around refs store and backend area before it's ready to handle \"get\n> > refs from this worktree store\" (or \"get refs from every reachable\n> > stores\").\n> > \n> > [1] https://public-inbox.org/git/xmqqshwwzyee.fsf@gitster.mtv.corp.google.com/\n\nGiven that\nhttps://public-inbox.org/git/xmqqy46ntrhk.fsf@gitster.mtv.corp.google.com/\nseems to have expected something to happen within a reasonable time frame,\nand that 8 months is substantially longer than a reasonable time frame, I\nam not sure that that position can still be defended.\n\nAlso, the more important reply was Peff's reply that suggested that the\nproposed fix was incomplete, as it misses --unpack-unreachable:\nhttps://public-inbox.org/git/20160601160143.GA9219@sigill.intra.peff.net/\n\nCiao,\nJohannes\n"},{"id":"310738","messageId":"xmqqmve4s5r2.fsf@gitster.mtv.corp.google.com","threadId":"45021","inReplyTo":"alpine.DEB.2.20.1702021330040.3496@virtualbox","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-02T20:21:53Z","receivedAt":"2017-02-02T20:22:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Also, the more important reply was Peff's reply that suggested that the\n> proposed fix was incomplete, as it misses --unpack-unreachable:\n> https://public-inbox.org/git/20160601160143.GA9219@sigill.intra.peff.net/\n\nWhile I think that --unpack-unreachable thing is a symptom of the\nbasic approach of patching things up at the wrong level, if you are\nhinting that fix to the issue that gc does not pay attention to\nvarious anchoring point in other worktrees is more important than\nadding new commands to \"worktree\", I fully agree with that. \n\nWe have a basic structure of \"worktree\" working well enough to allow\nadventurous folks to experiment (there is a reason why we keep\ncalling it experimental). mv and other additions are primarily to\nmake things _easier_ to use, but we shouldn't be encouraging its use\nto general public by making it easier for them to hurt themselves,\nif we know there still are sharp edges.\n\nThanks for bringing it up.\n"},{"id":"310790","messageId":"CACsJy8Cq8sY1hL75Xs_MMr9r_+jjr7p+58D+0GhT3mgSgiUEtg@mail.gmail.com","threadId":"45021","inReplyTo":"xmqqmve4s5r2.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-03T08:59:25Z","receivedAt":"2017-02-03T09:00:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Feb 3, 2017 at 3:21 AM, Junio C Hamano wrote:\n> Johannes Schindelin writes:\n>\n>> Also, the more important reply was Peff's reply that suggested that the\n>> proposed fix was incomplete, as it misses --unpack-unreachable:\n>> https://public-inbox.org/git/20160601160143.GA9219@sigill.intra.peff.net/\n>\n> While I think that --unpack-unreachable thing is a symptom of the\n> basic approach of patching things up at the wrong level, if you are\n> hinting that fix to the issue that gc does not pay attention to\n> various anchoring point in other worktrees is more important than\n> adding new commands to \"worktree\", I fully agree with that.\n\nJust to make it clear. It's not like I put new worktree commands on\nhigher priority. \"worktree move/remove\" was more or less ready for a\nlong time and the gc problem was blocked by ref-iterator series (which\nentered master a few moths ago, but then I was busy with other things\nand couldn't get right back to the gc issue).\n\nYou didn't answer Johannes's rhetoric question though: \"It should be\npossible to do that redesign work while having a small workaround in\nplace that unbreaks, say, me?\"\n\nI assume \"the right way\" is still updating refs subsystem so that we\ncan have a ref iterator to traverse all refs, or just one worktree,\netc. Should I keep looking for a middle ground (maybe better than the\nlinked series) to \"unbreak Johannes\"? I ignored all those comments\n(about --unpack-reachable and bisect refs) because I saw no chance of\nan updated series getting in git.git anyway.\n-- \nDuy\n"},{"id":"310811","messageId":"xmqqtw8bf7xn.fsf@gitster.mtv.corp.google.com","threadId":"45021","inReplyTo":"CACsJy8Cq8sY1hL75Xs_MMr9r_+jjr7p+58D+0GhT3mgSgiUEtg@mail.gmail.com","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-03T18:25:24Z","receivedAt":"2017-02-03T18:25:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Fri, Feb 3, 2017 at 3:21 AM, Junio C Hamano wrote:\n>> Johannes Schindelin writes:\n>>\n>>> Also, the more important reply was Peff's reply that suggested that the\n>>> proposed fix was incomplete, as it misses --unpack-unreachable:\n>>> https://public-inbox.org/git/20160601160143.GA9219@sigill.intra.peff.net/\n>>\n>> While I think that --unpack-unreachable thing is a symptom of the\n>> basic approach of patching things up at the wrong level, if you are\n>> hinting that fix to the issue that gc does not pay attention to\n>> various anchoring point in other worktrees is more important than\n>> adding new commands to \"worktree\", I fully agree with that.\n>\n> Just to make it clear. It's not like I put new worktree commands on\n> higher priority.\n\nGood; we are on the same page.\n\n> \"worktree move/remove\" was more or less ready for a\n> long time and the gc problem was blocked by ref-iterator series (which\n> entered master a few moths ago, but then I was busy with other things\n> and couldn't get right back to the gc issue).\n\nOK.\n\n> You didn't answer Johannes's rhetoric question though: \"It should be\n> possible to do that redesign work while having a small workaround in\n> place that unbreaks, say, me?\"\n\nI do not recall seeing that. I however deliberately ignored another\nstatement because I thought it enough to answer, which was:\n\n Given that\n https://public-inbox.org/git/xmqqy46ntrhk.fsf@gitster.mtv.corp.google.com/\n seems to have expected something to happen within a reasonable time frame,\n and that 8 months is substantially longer than a reasonable time frame, I\n am not sure that that position can still be defended.\n\nIn the message xmqqy46ntrhk is a response to, Michael enumerated the\nissues need to be solved with priorities, and listed reachability\nfrom the index and per-worktree detached HEAD are more important\nthan others. I was hoping that even these \"relatively more\nimportant\" ones would turn out be of less importance compared to\nfixing all on the right foundation as long as it won't take forever,\nbut I do agree with Dscho that 8 months is way too long.\n\n> I assume \"the right way\" is still updating refs subsystem so that we\n> can have a ref iterator to traverse all refs, or just one worktree,\n> etc. Should I keep looking for a middle ground (maybe better than the\n> linked series) to \"unbreak Johannes\"? I ignored all those comments\n> (about --unpack-reachable and bisect refs) because I saw no chance of\n> an updated series getting in git.git anyway.\n\nSo, you may have seen no chance 8 months ago; after we have been\nwaiting for a better fix, which hadn't materialized for a while, I\nam very much open to change the priority.\n\nEven if you think \"the right way\" is to add to the iterators, I\nsuspect that we can still do incremental fixes? I agree with the\norder of importance Michael listed in his message (i.e. the index\nand the HEAD first, and then other per-worktree hierarchies at lower\npriority), and I suspect you do, too. I am not sure that is what\nyou called \"middle ground\", but I think such an incremental approach\nis totally fine.\n\n"},{"id":"310814","messageId":"xmqqpoizf6ij.fsf@gitster.mtv.corp.google.com","threadId":"45021","inReplyTo":"xmqqtw8bf7xn.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-03T18:56:04Z","receivedAt":"2017-02-03T18:56:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I do not recall seeing that. I however deliberately ignored another\n> statement because I thought it enough to answer, which was:\n\nOops. \"because I didn't think I thought it enough to answer\" was\nwhat I meant.\n"},{"id":"310848","messageId":"CACsJy8BeEUzEt25nN7_eddcUN8=SrQrMSHY5duYLxKxTztfq4A@mail.gmail.com","threadId":"45021","inReplyTo":"xmqqtw8bf7xn.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 00/11] nd/worktree-move update","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-04T11:57:07Z","receivedAt":"2017-02-04T11:57:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Feb 4, 2017 at 1:25 AM, Junio C Hamano wrote:\n> Even if you think \"the right way\" is to add to the iterators, I\n> suspect that we can still do incremental fixes? I agree with the\n> order of importance Michael listed in his message (i.e. the index\n> and the HEAD first, and then other per-worktree hierarchies at lower\n> priority), and I suspect you do, too. I am not sure that is what\n> you called \"middle ground\", but I think such an incremental approach\n> is totally fine.\n\nYeah index should be fixed independently. Sorry that thought did not\noccur to me. I counted HEAD as a ref too (i.e. to be managed/iterated\nby refs subsystem) but I guess it's special enough to be dealt with\nwithout ref iterator. I will try to fix these two first.\n\nBy middle ground, I was thinking that, now that we have ref iterator\ninterface, we more or less know what the api should look like for a\nref user like rev-list, so perhaps we could add a custom (ugly)\niterator for per-worktree refs (including reflog). The ugliness is\nprobably buried in refs/files-backend.c again until we clean it up.\n-- \nDuy\n"}]}