{"thread":{"id":"49246","subject":"[PATCH 0/9] worktree: fix bugs and broaden --force applicability","startedAt":"2018-08-28T21:20:50Z","lastAt":"2018-08-31T21:57:51Z","messageCount":28,"participants":["Eric Sunshine","Jeff King","Junio C Hamano","Ramsay Jones"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"356740","messageId":"20180828212026.21989-1-sunshine@sunshineco.com","threadId":"49246","inReplyTo":null,"subject":"[PATCH 0/9] worktree: fix bugs and broaden --force applicability","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:20:17Z","receivedAt":"2018-08-28T21:20:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This series started as a fix for a bug reported by Peff[1] in which the\n\"database\" of worktrees could be corrupted (or at least become\ninternally inconsistent) by having multiple worktree entries associated\nwith the same path.\n\nPeff's particular use-case for git-worktree is Documentation/doc-diff\nwhich wants to re-use a worktree if it exists or create it anew if it\ndoesn't. Unfortunately, this is a bit more difficult than it should be\nif the worktree directory is deleted manually (without pruning the\nworktree entry) between script invocations. To simplify this use-case\nfor tools, it was suggested[2] that \"git worktree add --force\" could\ndeal with the problem of a registered-but-missing worktree (much as a\ntool might rely upon \"mkdir -p\" to create or re-use an existing\ndirectory). This series implements that proposal, as well.\n\nFixing the original bug revealed another existing bug, and after several\nadditional \"while we're here\" changes, the series ended up a bit longer\nthan expected.\n\n[1]: https://public-inbox.org/git/20180821193556.GA859@sigill.intra.peff.net/\n[2]: https://public-inbox.org/git/CAPig+cTghgbBo5VfZN+VP2VM00nPkhUqm0dOUqO37arxraxBKw@mail.gmail.com/\n\nEric Sunshine (9):\n  worktree: don't die() in library function find_worktree()\n  worktree: move delete_git_dir() earlier in file for upcoming new\n    callers\n  worktree: generalize delete_git_dir() to reduce code duplication\n  worktree: prepare for more checks of whether path can become worktree\n  worktree: disallow adding same path multiple times\n  worktree: teach 'add' to respect --force for registered but missing\n    path\n  worktree: teach 'move' to override lock when --force given twice\n  worktree: teach 'remove' to override lock when --force given twice\n  worktree: delete .git/worktrees if empty after 'remove'\n\n Documentation/git-worktree.txt |  12 +++-\n builtin/worktree.c             | 113 ++++++++++++++++++++++-----------\n t/t2025-worktree-add.sh        |  18 ++++++\n t/t2028-worktree-move.sh       |  44 +++++++++++++\n worktree.c                     |   6 +-\n 5 files changed, 154 insertions(+), 39 deletions(-)\n\n-- \n2.19.0.rc1.350.ge57e33dbd1\n\n"},{"id":"356741","messageId":"20180828212026.21989-2-sunshine@sunshineco.com","threadId":"49246","inReplyTo":"20180828212026.21989-1-sunshine@sunshineco.com","subject":"[PATCH 1/9] worktree: don't die() in library function find_worktree()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:20:18Z","receivedAt":"2018-08-28T21:20:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Callers don't expect library function find_worktree() to die(); they\nexpect it to return the named worktree if found, or NULL if not.\nAlthough find_worktree() itself never invokes die(), it calls\nreal_pathdup() with 'die_on_error' incorrectly set to 'true', thus will\ndie() indirectly if the user-provided path is not to real_pathdup()'s\nliking. This can be observed, for instance, with any git-worktree\ncommand which searches for an existing worktree:\n\n    $ git worktree unlock foo\n    fatal: 'foo' is not a working tree\n    $ git worktree unlock foo/bar\n    fatal: Invalid path '.../foo': No such file or directory\n\nThe first error message is the expected one from \"git worktree unlock\"\nnot finding the specified worktree; the second is from find_worktree()\ninvoking real_pathdup() incorrectly and die()ing prematurely.\n\nAside from the inconsistent error message between the two cases, this\nbug hasn't otherwise been a serious problem since existing callers all\ndie() anyhow when the worktree can't be found. However, that may not be\ntrue of callers added in the future, so fix find_worktree() to avoid\ndie()ing.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t2028-worktree-move.sh | 8 ++++++++\n worktree.c               | 6 +++++-\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh\nindex 5f7d45b7b7..60aba7c41a 100755\n--- a/t/t2028-worktree-move.sh\n+++ b/t/t2028-worktree-move.sh\n@@ -141,4 +141,12 @@ test_expect_success 'NOT remove missing-but-locked worktree' '\n \ttest_path_is_dir .git/worktrees/gone-but-locked\n '\n \n+test_expect_success 'proper error when worktree not found' '\n+\tfor i in noodle noodle/bork\n+\tdo\n+\t\ttest_must_fail git worktree lock $i 2>err &&\n+\t\ttest_i18ngrep \"not a working tree\" err || return 1\n+\tdone\n+'\n+\n test_done\ndiff --git a/worktree.c b/worktree.c\nindex 97cda5f97b..b0d0b5426d 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -217,7 +217,11 @@ struct worktree *find_worktree(struct worktree **list,\n \n \tif (prefix)\n \t\targ = to_free = prefix_filename(prefix, arg);\n-\tpath = real_pathdup(arg, 1);\n+\tpath = real_pathdup(arg, 0);\n+\tif (!path) {\n+\t\tfree(to_free);\n+\t\treturn NULL;\n+\t}\n \tfor (; *list; list++)\n \t\tif (!fspathcmp(path, real_path((*list)->path)))\n \t\t\tbreak;\n-- \n2.19.0.rc1.350.ge57e33dbd1\n\n"},{"id":"356742","messageId":"20180828212026.21989-3-sunshine@sunshineco.com","threadId":"49246","inReplyTo":"20180828212026.21989-1-sunshine@sunshineco.com","subject":"[PATCH 2/9] worktree: move delete_git_dir() earlier in file for upcoming new callers","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:20:19Z","receivedAt":"2018-08-28T21:20:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This is a pure code movement to avoid having to forward-declare the\nfunction when new callers are subsequently added.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n builtin/worktree.c | 28 ++++++++++++++--------------\n 1 file changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 41e7714396..a8128289cc 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -47,6 +47,20 @@ static int git_worktree_config(const char *var, const char *value, void *cb)\n \treturn git_default_config(var, value, cb);\n }\n \n+static int delete_git_dir(struct worktree *wt)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tint ret = 0;\n+\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+\treturn ret;\n+}\n+\n static int prune_worktree(const char *id, struct strbuf *reason)\n {\n \tstruct stat st;\n@@ -822,20 +836,6 @@ static int delete_git_work_tree(struct worktree *wt)\n \treturn ret;\n }\n \n-static int delete_git_dir(struct worktree *wt)\n-{\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tint ret = 0;\n-\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-\treturn ret;\n-}\n-\n static int remove_worktree(int ac, const char **av, const char *prefix)\n {\n \tint force = 0;\n-- \n2.19.0.rc1.350.ge57e33dbd1\n\n"},{"id":"356744","messageId":"20180828212026.21989-5-sunshine@sunshineco.com","threadId":"49246","inReplyTo":"20180828212026.21989-1-sunshine@sunshineco.com","subject":"[PATCH 4/9] worktree: prepare for more checks of whether path can become worktree","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:20:21Z","receivedAt":"2018-08-28T21:20:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Certain conditions must be met for a path to be a valid candidate as the\nlocation of a new worktree; for instance, the path must not exist or\nmust be an empty directory. Although the number of conditions is small,\nnew conditions will soon be added so factor out the existing checks into\na separate function to avoid further bloating add_worktree().\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n builtin/worktree.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 0affcb476c..46c93771e8 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -219,6 +219,12 @@ static const char *worktree_basename(const char *path, int *olen)\n \treturn name;\n }\n \n+static void validate_worktree_add(const char *path, const struct add_opts *opts)\n+{\n+\tif (file_exists(path) && !is_empty_dir(path))\n+\t\tdie(_(\"'%s' already exists\"), path);\n+}\n+\n static int add_worktree(const char *path, const char *refname,\n \t\t\tconst struct add_opts *opts)\n {\n@@ -233,8 +239,7 @@ static int add_worktree(const char *path, const char *refname,\n \tstruct commit *commit = NULL;\n \tint is_branch = 0;\n \n-\tif (file_exists(path) && !is_empty_dir(path))\n-\t\tdie(_(\"'%s' already exists\"), path);\n+\tvalidate_worktree_add(path, opts);\n \n \t/* is 'refname' a branch or commit? */\n \tif (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n-- \n2.19.0.rc1.350.ge57e33dbd1\n\n"},{"id":"356743","messageId":"20180828212026.21989-4-sunshine@sunshineco.com","threadId":"49246","inReplyTo":"20180828212026.21989-1-sunshine@sunshineco.com","subject":"[PATCH 3/9] worktree: generalize delete_git_dir() to reduce code duplication","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:20:20Z","receivedAt":"2018-08-28T21:20:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"prune_worktrees() and delete_git_dir() both remove worktree\nadministrative entries from .git/worktrees, and their implementations\nare nearly identical. The only difference is that prune_worktrees() is\nalso capable of removing a bogus non-worktree-related file from\n.git/worktrees.\n\nSimplify by extending delete_git_dir() to handle the little bit of\nextra functionality needed by prune_worktrees(), and drop the\neffectively duplicate code from the latter.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n builtin/worktree.c | 25 +++++++++----------------\n 1 file changed, 9 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex a8128289cc..0affcb476c 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -47,16 +47,17 @@ static int git_worktree_config(const char *var, const char *value, void *cb)\n \treturn git_default_config(var, value, cb);\n }\n \n-static int delete_git_dir(struct worktree *wt)\n+static int delete_git_dir(const char *id)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n-\tint ret = 0;\n+\tint ret;\n \n-\tstrbuf_addstr(&sb, git_common_path(\"worktrees/%s\", wt->id));\n-\tif (remove_dir_recursively(&sb, 0)) {\n+\tstrbuf_addstr(&sb, git_common_path(\"worktrees/%s\", id));\n+\tret = remove_dir_recursively(&sb, 0);\n+\tif (ret < 0 && errno == ENOTDIR)\n+\t\tret = unlink(sb.buf);\n+\tif (ret)\n \t\terror_errno(_(\"failed to delete '%s'\"), sb.buf);\n-\t\tret = -1;\n-\t}\n \tstrbuf_release(&sb);\n \treturn ret;\n }\n@@ -130,10 +131,8 @@ static int prune_worktree(const char *id, struct strbuf *reason)\n static void prune_worktrees(void)\n {\n \tstruct strbuf reason = STRBUF_INIT;\n-\tstruct strbuf path = STRBUF_INIT;\n \tDIR *dir = opendir(git_path(\"worktrees\"));\n \tstruct dirent *d;\n-\tint ret;\n \tif (!dir)\n \t\treturn;\n \twhile ((d = readdir(dir)) != NULL) {\n@@ -146,18 +145,12 @@ static void prune_worktrees(void)\n \t\t\tprintf(\"%s\\n\", reason.buf);\n \t\tif (show_only)\n \t\t\tcontinue;\n-\t\tgit_path_buf(&path, \"worktrees/%s\", d->d_name);\n-\t\tret = remove_dir_recursively(&path, 0);\n-\t\tif (ret < 0 && errno == ENOTDIR)\n-\t\t\tret = unlink(path.buf);\n-\t\tif (ret)\n-\t\t\terror_errno(_(\"failed to remove '%s'\"), path.buf);\n+\t\tdelete_git_dir(d->d_name);\n \t}\n \tclosedir(dir);\n \tif (!show_only)\n \t\trmdir(git_path(\"worktrees\"));\n \tstrbuf_release(&reason);\n-\tstrbuf_release(&path);\n }\n \n static int prune(int ac, const char **av, const char *prefix)\n@@ -882,7 +875,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix)\n \t * continue on even if ret is non-zero, there's no going back\n \t * from here.\n \t */\n-\tret |= delete_git_dir(wt);\n+\tret |= delete_git_dir(wt->id);\n \n \tfree_worktrees(worktrees);\n \treturn ret;\n-- \n2.19.0.rc1.350.ge57e33dbd1\n\n"},{"id":"356745","messageId":"20180828212026.21989-7-sunshine@sunshineco.com","threadId":"49246","inReplyTo":"20180828212026.21989-1-sunshine@sunshineco.com","subject":"[PATCH 6/9] worktree: teach 'add' to respect --force for registered but missing path","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:20:23Z","receivedAt":"2018-08-28T21:20:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"For safety, \"git worktree add <path>\" will refuse to add a new\nworktree at <path> if <path> is already associated with a worktree\nentry, even if <path> is missing (for instance, has been deleted or\nresides on non-mounted removable media or network share). The typical\nway to re-create a worktree at <path> in such a situation is either to\nprune all \"broken\" entries (\"git worktree prune\") or to selectively\nremove the worktree entry manually (\"git worktree remove <path>\").\n\nHowever, neither of these approaches (\"prune\" nor \"remove\") is\nespecially convenient, and they may be unsuitable for scripting when a\ntool merely wants to re-use a worktree if it exists or create it from\nscratch if it doesn't (much as a tool might use \"mkdir -p\" to re-use\nor create a directory).\n\nTherefore, teach 'add' to respect --force as a convenient way to\nre-use a path already associated with a worktree entry if the path is\nnon-existent. For a locked worktree, require --force to be specified\ntwice.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n Documentation/git-worktree.txt |  8 ++++++--\n builtin/worktree.c             | 10 ++++++++--\n t/t2025-worktree-add.sh        | 13 ++++++++++++-\n 3 files changed, 26 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex 29a5b7e252..8537692f05 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -120,8 +120,12 @@ OPTIONS\n --force::\n \tBy default, `add` refuses to create a new working tree when\n \t`<commit-ish>` is a branch name and is already checked out by\n-\tanother working tree and `remove` refuses to remove an unclean\n-\tworking tree. This option overrides these safeguards.\n+\tanother working tree, or if `<path>` is already assigned to some\n+\tworking tree but is missing (for instance, if `<path>` was deleted\n+\tmanually). This option overrides these safeguards. To add a missing but\n+\tlocked working tree path, specify `--force` twice.\n++\n+`remove` refuses to remove an unclean working tree unless `--force` is used.\n \n -b <new-branch>::\n -B <new-branch>::\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 1122f27b5f..3eb2f89b0f 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -241,10 +241,16 @@ static void validate_worktree_add(const char *path, const struct add_opts *opts)\n \t\tgoto done;\n \n \tlocked = !!is_worktree_locked(wt);\n+\tif ((!locked && opts->force) || (locked && opts->force > 1)) {\n+\t\tif (delete_git_dir(wt->id))\n+\t\t    die(_(\"unable to re-add worktree '%s'\"), path);\n+\t\tgoto done;\n+\t}\n+\n \tif (locked)\n-\t\tdie(_(\"'%s' is a missing but locked worktree;\\nuse 'unlock' and 'prune' or 'remove' to clear\"), path);\n+\t\tdie(_(\"'%s' is a missing but locked worktree;\\nuse 'add -f -f' to override, or 'unlock' and 'prune' or 'remove' to clear\"), path);\n \telse\n-\t\tdie(_(\"'%s' is a missing but already registered worktree;\\nuse 'prune' or 'remove' to clear\"), path);\n+\t\tdie(_(\"'%s' is a missing but already registered worktree;\\nuse 'add -f' to override, or 'prune' or 'remove' to clear\"), path);\n \n done:\n \tfree_worktrees(worktrees);\ndiff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\nindex 67fccc6591..286bba35d8 100755\n--- a/t/t2025-worktree-add.sh\n+++ b/t/t2025-worktree-add.sh\n@@ -556,7 +556,18 @@ test_expect_success '\"add\" an existing but missing worktree' '\n \tgit worktree add --detach pneu &&\n \ttest_must_fail git worktree add --detach pneu &&\n \trm -fr pneu &&\n-\ttest_must_fail git worktree add --detach pneu\n+\ttest_must_fail git worktree add --detach pneu &&\n+\tgit worktree add --force --detach pneu\n+'\n+\n+test_expect_success '\"add\" an existing locked but missing worktree' '\n+\tgit worktree add --detach gnoo &&\n+\tgit worktree lock gnoo &&\n+\ttest_when_finished \"git worktree unlock gnoo || :\" &&\n+\trm -fr gnoo &&\n+\ttest_must_fail git worktree add --detach gnoo &&\n+\ttest_must_fail git worktree add --force --detach gnoo &&\n+\tgit worktree add --force --force --detach gnoo\n '\n \n test_done\n-- \n2.19.0.rc1.350.ge57e33dbd1\n\n"},{"id":"356747","messageId":"20180828212026.21989-9-sunshine@sunshineco.com","threadId":"49246","inReplyTo":"20180828212026.21989-1-sunshine@sunshineco.com","subject":"[PATCH 8/9] worktree: teach 'remove' to override lock when --force given twice","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:20:25Z","receivedAt":"2018-08-28T21:20:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"For consistency with \"add -f -f\" and \"move -f -f\" which override\nthe lock on a worktree, allow \"remove -f -f\" to do so, as well, as a\nconvenience.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n Documentation/git-worktree.txt |  1 +\n builtin/worktree.c             | 11 ++++++-----\n t/t2028-worktree-move.sh       | 10 ++++++++++\n 3 files changed, 17 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex d08b8d8e4f..e2ee9fc21b 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -129,6 +129,7 @@ OPTIONS\n twice.\n +\n `remove` refuses to remove an unclean working tree unless `--force` is used.\n+To remove a locked working tree, specify `--force` twice.\n \n -b <new-branch>::\n -B <new-branch>::\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 354a6c0eb5..a95fe67da6 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -875,13 +875,13 @@ static int remove_worktree(int ac, const char **av, const char *prefix)\n \tint force = 0;\n \tstruct option options[] = {\n \t\tOPT__FORCE(&force,\n-\t\t\t N_(\"force removing even if the worktree is dirty\"),\n+\t\t\t N_(\"force removal even if worktree is dirty or locked\"),\n \t\t\t PARSE_OPT_NOCOMPLETE),\n \t\tOPT_END()\n \t};\n \tstruct worktree **worktrees, *wt;\n \tstruct strbuf errmsg = STRBUF_INIT;\n-\tconst char *reason;\n+\tconst char *reason = NULL;\n \tint ret = 0;\n \n \tac = parse_options(ac, av, prefix, options, worktree_usage, 0);\n@@ -894,12 +894,13 @@ static int remove_worktree(int ac, const char **av, const char *prefix)\n \t\tdie(_(\"'%s' is not a working tree\"), av[0]);\n \tif (is_main_worktree(wt))\n \t\tdie(_(\"'%s' is a main working tree\"), av[0]);\n-\treason = is_worktree_locked(wt);\n+\tif (force < 2)\n+\t\treason = is_worktree_locked(wt);\n \tif (reason) {\n \t\tif (*reason)\n-\t\t\tdie(_(\"cannot remove a locked working tree, lock reason: %s\"),\n+\t\t\tdie(_(\"cannot remove a locked working tree, lock reason: %s\\nuse 'remove -f -f' to override or unlock first\"),\n \t\t\t    reason);\n-\t\tdie(_(\"cannot remove a locked working tree\"));\n+\t\tdie(_(\"cannot remove a locked working tree;\\nuse 'remove -f -f' to override or unlock first\"));\n \t}\n \tif (validate_worktree(wt, &errmsg, WT_VALIDATE_WORKTREE_MISSING_OK))\n \t\tdie(_(\"validation failed, cannot remove working tree: %s\"),\ndiff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh\nindex 9756ede8f1..1b5079e8fa 100755\n--- a/t/t2028-worktree-move.sh\n+++ b/t/t2028-worktree-move.sh\n@@ -163,4 +163,14 @@ test_expect_success 'proper error when worktree not found' '\n \tdone\n '\n \n+test_expect_success 'remove locked worktree (force)' '\n+\tgit worktree add --detach gumby &&\n+\ttest_when_finished \"git worktree remove gumby || :\" &&\n+\tgit worktree lock gumby &&\n+\ttest_when_finished \"git worktree unlock gumby || :\" &&\n+\ttest_must_fail git worktree remove gumby &&\n+\ttest_must_fail git worktree remove --force gumby &&\n+\tgit worktree remove --force --force gumby\n+'\n+\n test_done\n-- \n2.19.0.rc1.350.ge57e33dbd1\n\n"},{"id":"356746","messageId":"20180828212026.21989-10-sunshine@sunshineco.com","threadId":"49246","inReplyTo":"20180828212026.21989-1-sunshine@sunshineco.com","subject":"[PATCH 9/9] worktree: delete .git/worktrees if empty after 'remove'","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:20:26Z","receivedAt":"2018-08-28T21:20:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"For cleanliness, \"git worktree prune\" deletes the .git/worktrees\ndirectory if it is empty after pruning is complete.\n\nFor consistency, make \"git worktree remove <path>\" likewise delete\n.git/worktrees if it is empty after the removal.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n builtin/worktree.c       |  8 +++++++-\n t/t2028-worktree-move.sh | 12 ++++++++++++\n 2 files changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex a95fe67da6..c4abbde2b8 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -62,6 +62,11 @@ static int delete_git_dir(const char *id)\n \treturn ret;\n }\n \n+static void delete_worktrees_dir_if_empty(void)\n+{\n+\trmdir(git_path(\"worktrees\")); /* ignore failed removal */\n+}\n+\n static int prune_worktree(const char *id, struct strbuf *reason)\n {\n \tstruct stat st;\n@@ -149,7 +154,7 @@ static void prune_worktrees(void)\n \t}\n \tclosedir(dir);\n \tif (!show_only)\n-\t\trmdir(git_path(\"worktrees\"));\n+\t\tdelete_worktrees_dir_if_empty();\n \tstrbuf_release(&reason);\n }\n \n@@ -918,6 +923,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix)\n \t * from here.\n \t */\n \tret |= delete_git_dir(wt->id);\n+\tdelete_worktrees_dir_if_empty();\n \n \tfree_worktrees(worktrees);\n \treturn ret;\ndiff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh\nindex 1b5079e8fa..33c0337733 100755\n--- a/t/t2028-worktree-move.sh\n+++ b/t/t2028-worktree-move.sh\n@@ -173,4 +173,16 @@ test_expect_success 'remove locked worktree (force)' '\n \tgit worktree remove --force --force gumby\n '\n \n+test_expect_success 'remove cleans up .git/worktrees when empty' '\n+\tgit init moog &&\n+\t(\n+\t\tcd moog &&\n+\t\ttest_commit bim &&\n+\t\tgit worktree add --detach goom &&\n+\t\ttest_path_exists .git/worktrees &&\n+\t\tgit worktree remove goom &&\n+\t\ttest_path_is_missing .git/worktrees\n+\t)\n+'\n+\n test_done\n-- \n2.19.0.rc1.350.ge57e33dbd1\n\n"},{"id":"356749","messageId":"20180828212026.21989-8-sunshine@sunshineco.com","threadId":"49246","inReplyTo":"20180828212026.21989-1-sunshine@sunshineco.com","subject":"[PATCH 7/9] worktree: teach 'move' to override lock when --force given twice","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:20:24Z","receivedAt":"2018-08-28T21:20:59Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"For consistency with \"add -f -f\", which allows a missing but locked\nworktree path to be re-used, allow \"move -f -f\" to override a lock,\nas well, as a convenience.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n Documentation/git-worktree.txt |  3 +++\n builtin/worktree.c             | 13 +++++++++----\n t/t2028-worktree-move.sh       | 14 ++++++++++++++\n 3 files changed, 26 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex 8537692f05..d08b8d8e4f 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -125,6 +125,9 @@ OPTIONS\n \tmanually). This option overrides these safeguards. To add a missing but\n \tlocked working tree path, specify `--force` twice.\n +\n+`move` refuses to move a locked working tree unless `--force` is specified\n+twice.\n++\n `remove` refuses to remove an unclean working tree unless `--force` is used.\n \n -b <new-branch>::\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 3eb2f89b0f..354a6c0eb5 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -740,13 +740,17 @@ static void validate_no_submodules(const struct worktree *wt)\n \n static int move_worktree(int ac, const char **av, const char *prefix)\n {\n+\tint force = 0;\n \tstruct option options[] = {\n+\t\tOPT__FORCE(&force,\n+\t\t\t N_(\"force move even if worktree is dirty or locked\"),\n+\t\t\t PARSE_OPT_NOCOMPLETE),\n \t\tOPT_END()\n \t};\n \tstruct worktree **worktrees, *wt;\n \tstruct strbuf dst = STRBUF_INIT;\n \tstruct strbuf errmsg = STRBUF_INIT;\n-\tconst char *reason;\n+\tconst char *reason = NULL;\n \tchar *path;\n \n \tac = parse_options(ac, av, prefix, options, worktree_usage, 0);\n@@ -777,12 +781,13 @@ static int move_worktree(int ac, const char **av, const char *prefix)\n \n \tvalidate_no_submodules(wt);\n \n-\treason = is_worktree_locked(wt);\n+\tif (force < 2)\n+\t\treason = is_worktree_locked(wt);\n \tif (reason) {\n \t\tif (*reason)\n-\t\t\tdie(_(\"cannot move a locked working tree, lock reason: %s\"),\n+\t\t\tdie(_(\"cannot move a locked working tree, lock reason: %s\\nuse 'move -f -f' to override or unlock first\"),\n \t\t\t    reason);\n-\t\tdie(_(\"cannot move a locked working tree\"));\n+\t\tdie(_(\"cannot move a locked working tree;\\nuse 'move -f -f' to override or unlock first\"));\n \t}\n \tif (validate_worktree(wt, &errmsg, 0))\n \t\tdie(_(\"validation failed, cannot move working tree: %s\"),\ndiff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh\nindex 60aba7c41a..9756ede8f1 100755\n--- a/t/t2028-worktree-move.sh\n+++ b/t/t2028-worktree-move.sh\n@@ -98,6 +98,20 @@ test_expect_success 'move worktree to another dir' '\n \ttest_cmp expected2 actual2\n '\n \n+test_expect_success 'move locked worktree (force)' '\n+\ttest_when_finished \"\n+\t\tgit worktree unlock flump || :\n+\t\tgit worktree remove flump || :\n+\t\tgit worktree unlock ploof || :\n+\t\tgit worktree remove ploof || :\n+\t\t\" &&\n+\tgit worktree add --detach flump &&\n+\tgit worktree lock flump &&\n+\ttest_must_fail git worktree move flump ploof\" &&\n+\ttest_must_fail git worktree move --force flump ploof\" &&\n+\tgit worktree move --force --force flump ploof\n+'\n+\n test_expect_success 'remove main worktree' '\n \ttest_must_fail git worktree remove .\n '\n-- \n2.19.0.rc1.350.ge57e33dbd1\n\n"},{"id":"356748","messageId":"20180828212026.21989-6-sunshine@sunshineco.com","threadId":"49246","inReplyTo":"20180828212026.21989-1-sunshine@sunshineco.com","subject":"[PATCH 5/9] worktree: disallow adding same path multiple times","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:20:22Z","receivedAt":"2018-08-28T21:21:00Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"A given path should only ever be associated with a single registered\nworktree. This invariant is enforced by refusing to create a new\nworktree at a given path if that path already exists. For example:\n\n    $ git worktree add -q --detach foo\n    $ git worktree add -q --detach foo\n    fatal: 'foo' already exists\n\nHowever, the check can be fooled, and the invariant broken, if the\npath is missing. Continuing the example:\n\n    $ rm -fr foo\n    $ git worktree add -q --detach foo\n    $ git worktree list\n    ...      eadebfe [master]\n    .../foo  eadebfe (detached HEAD)\n    .../foo  eadebfe (detached HEAD)\n\nThis \"corruption\" leads to the unfortunate situation in which the\nworktree can not be removed:\n\n    $ git worktree remove foo\n    fatal: validation failed, cannot remove working tree: '.../foo'\n    does not point back to '.git/worktrees/foo'\n\nNor can the bogus entry be pruned:\n\n    $ git worktree prune -v\n    $ git worktree list\n    ...      eadebfe [master]\n    .../foo  eadebfe (detached HEAD)\n    .../foo  eadebfe (detached HEAD)\n\nwithout first deleting the worktree directory manually:\n\n    $ rm -fr foo\n    $ git worktree prune -v\n    Removing .../foo: gitdir file points to non-existent location\n    Removing .../foo1: gitdir file points to non-existent location\n    $ git worktree list\n    ...  eadebfe [master]\n\nor by manually deleting the worktree entry in .git/worktrees.\n\nTo address this problem, upgrade \"git worktree add\" validation to\nallow worktree creation only if the given path is not already\nassociated with an existing worktree (even if the path itself is\nnon-existent), thus preventing such bogus worktree entries from being\ncreated in the first place.\n\nReported-by: Jeff King <peff@peff.net>\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n builtin/worktree.c      | 25 +++++++++++++++++++++++++\n t/t2025-worktree-add.sh |  7 +++++++\n 2 files changed, 32 insertions(+)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 46c93771e8..1122f27b5f 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -221,8 +221,33 @@ static const char *worktree_basename(const char *path, int *olen)\n \n static void validate_worktree_add(const char *path, const struct add_opts *opts)\n {\n+\tstruct worktree **worktrees;\n+\tstruct worktree *wt;\n+\tint locked;\n+\n \tif (file_exists(path) && !is_empty_dir(path))\n \t\tdie(_(\"'%s' already exists\"), path);\n+\n+\tworktrees = get_worktrees(0);\n+\t/*\n+\t * find_worktree()'s suffix matching may undesirably find the main\n+\t * rather than a linked worktree (for instance, when the basenames\n+\t * of the main worktree and the one being created are the same).\n+\t * We're only interested in linked worktrees, so skip the main\n+\t * worktree with +1.\n+\t */\n+\twt = find_worktree(worktrees + 1, NULL, path);\n+\tif (!wt)\n+\t\tgoto done;\n+\n+\tlocked = !!is_worktree_locked(wt);\n+\tif (locked)\n+\t\tdie(_(\"'%s' is a missing but locked worktree;\\nuse 'unlock' and 'prune' or 'remove' to clear\"), path);\n+\telse\n+\t\tdie(_(\"'%s' is a missing but already registered worktree;\\nuse 'prune' or 'remove' to clear\"), path);\n+\n+done:\n+\tfree_worktrees(worktrees);\n }\n \n static int add_worktree(const char *path, const char *refname,\ndiff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\nindex 07d292317c..67fccc6591 100755\n--- a/t/t2025-worktree-add.sh\n+++ b/t/t2025-worktree-add.sh\n@@ -552,4 +552,11 @@ test_expect_success '\"add\" in bare repo invokes post-checkout hook' '\n \ttest_cmp hook.expect goozy/hook.actual\n '\n \n+test_expect_success '\"add\" an existing but missing worktree' '\n+\tgit worktree add --detach pneu &&\n+\ttest_must_fail git worktree add --detach pneu &&\n+\trm -fr pneu &&\n+\ttest_must_fail git worktree add --detach pneu\n+'\n+\n test_done\n-- \n2.19.0.rc1.350.ge57e33dbd1\n\n"},{"id":"356936","messageId":"20180830065734.GA11944@sigill.intra.peff.net","threadId":"49246","inReplyTo":"20180828212026.21989-4-sunshine@sunshineco.com","subject":"Re: [PATCH 3/9] worktree: generalize delete_git_dir() to reduce code duplication","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-30T06:57:35Z","receivedAt":"2018-08-30T06:57:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 28, 2018 at 05:20:20PM -0400, Eric Sunshine wrote:\n\n> prune_worktrees() and delete_git_dir() both remove worktree\n> administrative entries from .git/worktrees, and their implementations\n> are nearly identical. The only difference is that prune_worktrees() is\n> also capable of removing a bogus non-worktree-related file from\n> .git/worktrees.\n> \n> Simplify by extending delete_git_dir() to handle the little bit of\n> extra functionality needed by prune_worktrees(), and drop the\n> effectively duplicate code from the latter.\n\nMakes sense. The name \"delete_git_dir()\" is a little funny (I assume it\nmeans \"the git dir\", not a worktree's git-dir), but that is not new (and\nit's static in worktree.c, which helps).\n\nYour patch maybe stretches that a little by deleting non-directories.\nMaybe delete_from_worktrees() would be a better name. Probably not worth\na re-roll, though.\n\n-Peff\n"},{"id":"356945","messageId":"20180830072808.GB11944@sigill.intra.peff.net","threadId":"49246","inReplyTo":"20180828212026.21989-6-sunshine@sunshineco.com","subject":"Re: [PATCH 5/9] worktree: disallow adding same path multiple times","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-30T07:28:08Z","receivedAt":"2018-08-30T07:28:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 28, 2018 at 05:20:22PM -0400, Eric Sunshine wrote:\n\n> A given path should only ever be associated with a single registered\n> worktree. This invariant is enforced by refusing to create a new\n> worktree at a given path if that path already exists. For example:\n> \n>     $ git worktree add -q --detach foo\n>     $ git worktree add -q --detach foo\n>     fatal: 'foo' already exists\n> \n> However, the check can be fooled, and the invariant broken, if the\n> path is missing. Continuing the example:\n> [...]\n\nNicely explained.\n\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index 46c93771e8..1122f27b5f 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -221,8 +221,33 @@ static const char *worktree_basename(const char *path, int *olen)\n>  \n>  static void validate_worktree_add(const char *path, const struct add_opts *opts)\n>  {\n> +\tstruct worktree **worktrees;\n> +\tstruct worktree *wt;\n> +\tint locked;\n> +\n>  \tif (file_exists(path) && !is_empty_dir(path))\n>  \t\tdie(_(\"'%s' already exists\"), path);\n> +\n> +\tworktrees = get_worktrees(0);\n> +\t/*\n> +\t * find_worktree()'s suffix matching may undesirably find the main\n> +\t * rather than a linked worktree (for instance, when the basenames\n> +\t * of the main worktree and the one being created are the same).\n> +\t * We're only interested in linked worktrees, so skip the main\n> +\t * worktree with +1.\n> +\t */\n> +\twt = find_worktree(worktrees + 1, NULL, path);\n\nVery appropriate use of a comment. :)\n\nI wondered whether find_worktree() would discover a collision like this:\n\n  git worktree add --detach foo\n  rm -rf foo\n  git worktree add --detach bar/foo\n\nbut it is smart enough to actually compare the gitdirs and so we\n(correctly) create \"foo\" and \"foo1\" in $GIT_DIR/worktrees.\n\n> +\tif (!wt)\n> +\t\tgoto done;\n> +\n> +\tlocked = !!is_worktree_locked(wt);\n> +\tif (locked)\n> +\t\tdie(_(\"'%s' is a missing but locked worktree;\\nuse 'unlock' and 'prune' or 'remove' to clear\"), path);\n> +\telse\n> +\t\tdie(_(\"'%s' is a missing but already registered worktree;\\nuse 'prune' or 'remove' to clear\"), path);\n\nNice, I like giving separate messages for the locked and unlocked cases\n(which I imagine will become even more obvious as you add --force\nsupport).\n\nThe formatting of the message itself is a little funny:\n\n  $ git worktree add --detach foo\n  fatal: 'foo' is a missing but already registered worktree;\n  use 'prune' or 'remove' to clear\n\nI'd say that the second line would ideally be advise(), since we're\ndying. You could do:\n\n  error(%s is missing...)\n  advise(use prune...)\n  exit(128);\n\nbut that loses the \"fatal\" in the first message. I wonder if just\nmanually writing \"hint:\" would be so bad.\n\n> +done:\n> +\tfree_worktrees(worktrees);\n\nYou could easily avoid this goto with:\n\n  if (wt) {\n     /* check wt or die */\n  }\n\n  free_worktrees(worktrees);\n\nbut it may not be worth it if the logic gets more complicated in future\npatches.\n\n-Peff\n"},{"id":"356946","messageId":"20180830073642.GC11944@sigill.intra.peff.net","threadId":"49246","inReplyTo":"20180828212026.21989-7-sunshine@sunshineco.com","subject":"Re: [PATCH 6/9] worktree: teach 'add' to respect --force for registered but missing path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-30T07:36:42Z","receivedAt":"2018-08-30T07:36:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 28, 2018 at 05:20:23PM -0400, Eric Sunshine wrote:\n\n> For safety, \"git worktree add <path>\" will refuse to add a new\n> worktree at <path> if <path> is already associated with a worktree\n> entry, even if <path> is missing (for instance, has been deleted or\n> resides on non-mounted removable media or network share). The typical\n> way to re-create a worktree at <path> in such a situation is either to\n> prune all \"broken\" entries (\"git worktree prune\") or to selectively\n> remove the worktree entry manually (\"git worktree remove <path>\").\n> \n> However, neither of these approaches (\"prune\" nor \"remove\") is\n> especially convenient, and they may be unsuitable for scripting when a\n> tool merely wants to re-use a worktree if it exists or create it from\n> scratch if it doesn't (much as a tool might use \"mkdir -p\" to re-use\n> or create a directory).\n> \n> Therefore, teach 'add' to respect --force as a convenient way to\n> re-use a path already associated with a worktree entry if the path is\n> non-existent. For a locked worktree, require --force to be specified\n> twice.\n\nThis makes sense to me, and...\n\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n> ---\n>  Documentation/git-worktree.txt |  8 ++++++--\n>  builtin/worktree.c             | 10 ++++++++--\n>  t/t2025-worktree-add.sh        | 13 ++++++++++++-\n>  3 files changed, 26 insertions(+), 5 deletions(-)\n\nThe patch looks quite good. One minor comment:\n\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index 1122f27b5f..3eb2f89b0f 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -241,10 +241,16 @@ static void validate_worktree_add(const char *path, const struct add_opts *opts)\n>  \t\tgoto done;\n>  \n>  \tlocked = !!is_worktree_locked(wt);\n> +\tif ((!locked && opts->force) || (locked && opts->force > 1)) {\n> +\t\tif (delete_git_dir(wt->id))\n> +\t\t    die(_(\"unable to re-add worktree '%s'\"), path);\n> +\t\tgoto done;\n> +\t}\n\nThis \"unable to re-add\" seemed funny to me at first, since the failure\nis in deletion. I guess we're relying on delete_git_dir() to already\nhave said \"I had trouble deleting $GIT_DIR/worktrees/foo\", and this is\njust the follow-up to tell that the whole operation is cancelled. So\nthat makes sense.\n\nI wonder if we should volunteer the information that we're overwriting\nan existing worktree. I guess the user would generally know that\nalready, though, since they just specified \"-f\", so it's probably just\nbeing overly chatty to do so.\n\n-Peff\n"},{"id":"356947","messageId":"20180830073820.GD11944@sigill.intra.peff.net","threadId":"49246","inReplyTo":"20180828212026.21989-8-sunshine@sunshineco.com","subject":"Re: [PATCH 7/9] worktree: teach 'move' to override lock when --force given twice","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-30T07:38:20Z","receivedAt":"2018-08-30T07:38:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 28, 2018 at 05:20:24PM -0400, Eric Sunshine wrote:\n\n> For consistency with \"add -f -f\", which allows a missing but locked\n> worktree path to be re-used, allow \"move -f -f\" to override a lock,\n> as well, as a convenience.\n\nI don't have a strong opinion on this one, as I have never used\n\"worktree mv\" myself. :)\n\nBut anytime I see \"-f -f\", I have to wonder what \"-f\" does. In this\ncase, nothing. Is there some future lesser forcing we might use it for?\n\n-Peff\n"},{"id":"356948","messageId":"20180830074033.GE11944@sigill.intra.peff.net","threadId":"49246","inReplyTo":"20180828212026.21989-9-sunshine@sunshineco.com","subject":"Re: [PATCH 8/9] worktree: teach 'remove' to override lock when --force given twice","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-30T07:40:33Z","receivedAt":"2018-08-30T07:40:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 28, 2018 at 05:20:25PM -0400, Eric Sunshine wrote:\n\n> For consistency with \"add -f -f\" and \"move -f -f\" which override\n> the lock on a worktree, allow \"remove -f -f\" to do so, as well, as a\n> convenience.\n\nThis one makes more sense to me than the last, since \"remove -f\" already\ndoes something useful.\n\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index 354a6c0eb5..a95fe67da6 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -875,13 +875,13 @@ static int remove_worktree(int ac, const char **av, const char *prefix)\n>  \tint force = 0;\n>  \tstruct option options[] = {\n>  \t\tOPT__FORCE(&force,\n> -\t\t\t N_(\"force removing even if the worktree is dirty\"),\n> +\t\t\t N_(\"force removal even if worktree is dirty or locked\"),\n\nI wonder if somebody might assume from this that a single \"-f\" would\noverride a lock. Perhaps not the end of the world, and the manpage does\nmake it clear. And also I don't really know how to be more specific here\nwithout an overly long line.\n\nI'm guessing all those thoughts went through your head before ending up\nhere, too. :)\n\n-Peff\n"},{"id":"356949","messageId":"20180830075431.GF11944@sigill.intra.peff.net","threadId":"49246","inReplyTo":"20180828212026.21989-1-sunshine@sunshineco.com","subject":"Re: [PATCH 0/9] worktree: fix bugs and broaden --force applicability","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-30T07:54:31Z","receivedAt":"2018-08-30T07:54:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 28, 2018 at 05:20:17PM -0400, Eric Sunshine wrote:\n\n> This series started as a fix for a bug reported by Peff[1] in which the\n> \"database\" of worktrees could be corrupted (or at least become\n> internally inconsistent) by having multiple worktree entries associated\n> with the same path.\n> \n> Peff's particular use-case for git-worktree is Documentation/doc-diff\n> which wants to re-use a worktree if it exists or create it anew if it\n> doesn't. Unfortunately, this is a bit more difficult than it should be\n> if the worktree directory is deleted manually (without pruning the\n> worktree entry) between script invocations. To simplify this use-case\n> for tools, it was suggested[2] that \"git worktree add --force\" could\n> deal with the problem of a registered-but-missing worktree (much as a\n> tool might rely upon \"mkdir -p\" to create or re-use an existing\n> directory). This series implements that proposal, as well.\n> \n> Fixing the original bug revealed another existing bug, and after several\n> additional \"while we're here\" changes, the series ended up a bit longer\n> than expected.\n\nThanks for working on this. I think this nicely solves the problem I was\nhaving.  I had a few minor comments which I sent in reply to individual\npatches, but overall it looks very good. I'd be happy to see it picked\nup with or without any changes based on my comments.\n\nI'd want to do this on top. :)\n\n-- >8 --\nSubject: [PATCH] doc-diff: force worktree add\n\nWe avoid re-creating our temporary worktree if it's already\nthere. But we may run into a situation where the worktree\nhas been deleted, but an entry still exists in\n$GIT_DIR/worktrees.\n\nOlder versions of git-worktree would annoyingly create a\nseries of duplicate entries. Recent versions now detect and\nprevent this, allowing you to override with \"-f\". Since we\nknow that the worktree in question was just our temporary\nworkspace, it's safe for us to always pass \"-f\".\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/doc-diff | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/doc-diff b/Documentation/doc-diff\nindex f483fe427c..19d841ddeb 100755\n--- a/Documentation/doc-diff\n+++ b/Documentation/doc-diff\n@@ -54,7 +54,7 @@ fi\n # results that don't differ between the two trees.\n if ! test -d \"$tmp/worktree\"\n then\n-\tgit worktree add --detach \"$tmp/worktree\" \"$from\" &&\n+\tgit worktree add -f --detach \"$tmp/worktree\" \"$from\" &&\n \tdots=$(echo \"$tmp/worktree\" | sed 's#[^/]*#..#g') &&\n \tln -s \"$dots/config.mak\" \"$tmp/worktree/config.mak\"\n fi\n-- \n2.19.0.rc1.539.g3876d0831e\n\n"},{"id":"356952","messageId":"CAPig+cSbUVcs+eg1V_7xPY4ZMUpGZ5dQQ8iD8DWomZ-K=0mkug@mail.gmail.com","threadId":"49246","inReplyTo":"20180830072808.GB11944@sigill.intra.peff.net","subject":"Re: [PATCH 5/9] worktree: disallow adding same path multiple times","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-30T08:22:13Z","receivedAt":"2018-08-30T08:22:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 30, 2018 at 3:28 AM Jeff King <peff@peff.net> wrote:\n> On Tue, Aug 28, 2018 at 05:20:22PM -0400, Eric Sunshine wrote:\n> > +     /*\n> > +      * find_worktree()'s suffix matching may undesirably find the main\n> > +      * rather than a linked worktree (for instance, when the basenames\n> > +      * of the main worktree and the one being created are the same).\n> > +      * We're only interested in linked worktrees, so skip the main\n> > +      * worktree with +1.\n> > +      */\n> > +     wt = find_worktree(worktrees + 1, NULL, path);\n>\n> Very appropriate use of a comment. :)\n\nI tried repeatedly to write the comment as a one-liner but simply\ncouldn't do it in a way which conveyed the necessary information.\n\n> > +     if (locked)\n> > +             die(_(\"'%s' is a missing but locked worktree;\\nuse 'unlock' and 'prune' or 'remove' to clear\"), path);\n> > +     else\n> > +             die(_(\"'%s' is a missing but already registered worktree;\\nuse 'prune' or 'remove' to clear\"), path);\n>\n> Nice, I like giving separate messages for the locked and unlocked cases\n> The formatting of the message itself is a little funny:\n>   $ git worktree add --detach foo\n>   fatal: 'foo' is a missing but already registered worktree;\n>   use 'prune' or 'remove' to clear\n\nI couldn't come up with an improvement, so I went with this.\n\n> I'd say that the second line would ideally be advise(), since we're\n> dying. You could do:\n>   error(%s is missing...)\n>   advise(use prune...)\n>   exit(128);\n> but that loses the \"fatal\" in the first message.\n\nI'm somewhat hesitant to sidestep die() here because die() brings its\nown \"special\" behaviors and cleanups. (Even if they aren't needed\ntoday, perhaps they will be in the future.)\n\n> I wonder if just manually writing \"hint:\" would be so bad.\n\nThat would lose coloring of \"hint\", though.\n\nAnother alternative would be to make it just a long one-liner error\nmessage. I don't feel strongly one way or the other.\n\n> > +done:\n> > +     free_worktrees(worktrees);\n>\n> You could easily avoid this goto with:\n>\n>   if (wt) {\n>      /* check wt or die */\n>   }\n>\n>   free_worktrees(worktrees);\n>\n> but it may not be worth it if the logic gets more complicated in future\n> patches.\n\nI did suspect that a reviewer would comment on this, but, yes, the\nlogic does get more complicated in subsequent patches, and the 'goto'\nmakes a cleaner result in the end.\n"},{"id":"356953","messageId":"CAPig+cSSd4kgdfCsgmJ7omTTEn9Qve1DfDUtmJ7qkL5o0JS-hA@mail.gmail.com","threadId":"49246","inReplyTo":"20180830073642.GC11944@sigill.intra.peff.net","subject":"Re: [PATCH 6/9] worktree: teach 'add' to respect --force for registered but missing path","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-30T08:26:31Z","receivedAt":"2018-08-30T08:26:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 30, 2018 at 3:36 AM Jeff King <peff@peff.net> wrote:\n> On Tue, Aug 28, 2018 at 05:20:23PM -0400, Eric Sunshine wrote:\n> > +     if ((!locked && opts->force) || (locked && opts->force > 1)) {\n> > +             if (delete_git_dir(wt->id))\n> > +                 die(_(\"unable to re-add worktree '%s'\"), path);\n> > +             goto done;\n> > +     }\n>\n> This \"unable to re-add\" seemed funny to me at first, since the failure\n> is in deletion. I guess we're relying on delete_git_dir() to already\n> have said \"I had trouble deleting $GIT_DIR/worktrees/foo\", and this is\n> just the follow-up to tell that the whole operation is cancelled. So\n> that makes sense.\n\nCorrect, delete_git_dir() has already used error() to explain that it\ncouldn't delete .git/worktrees/<id>, so this finalizing die() doesn't\nneed to repeat that.\n\n> I wonder if we should volunteer the information that we're overwriting\n> an existing worktree. I guess the user would generally know that\n> already, though, since they just specified \"-f\", so it's probably just\n> being overly chatty to do so.\n\nThat's my conclusion, as well. An explicit use of --force shouldn't\nneed such chattiness.\n"},{"id":"356954","messageId":"CAPig+cSadH2LrhLvY0Dsrv_HhFibQo5wUq5GDPbNJJMq5MS8fA@mail.gmail.com","threadId":"49246","inReplyTo":"20180830073820.GD11944@sigill.intra.peff.net","subject":"Re: [PATCH 7/9] worktree: teach 'move' to override lock when --force given twice","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-30T08:30:09Z","receivedAt":"2018-08-30T08:30:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 30, 2018 at 3:38 AM Jeff King <peff@peff.net> wrote:\n> On Tue, Aug 28, 2018 at 05:20:24PM -0400, Eric Sunshine wrote:\n> > For consistency with \"add -f -f\", which allows a missing but locked\n> > worktree path to be re-used, allow \"move -f -f\" to override a lock,\n> > as well, as a convenience.\n>\n> I don't have a strong opinion on this one, as I have never used\n> \"worktree mv\" myself. :)\n\nI don't have strong feelings about this either (nor about \"remove -f -f\").\n\n> But anytime I see \"-f -f\", I have to wonder what \"-f\" does. In this\n> case, nothing. Is there some future lesser forcing we might use it for?\n\nI had the same concern. A single --force probably ought to be\nsufficient (given that there is no other meaning presently for a\nsingle --force), but it somehow seemed wrong to override a lock with a\nsingle --force when the other commands demand specifying it twice.\n\nThe strictness could always be downgraded later to require only a\nsingle --force if it becomes obvious that that makes more sense.\n"},{"id":"356955","messageId":"CAPig+cSXx39jAJ_hsMtAsxCUVgBO64=RPaa+yF5JjZAK1CXZLQ@mail.gmail.com","threadId":"49246","inReplyTo":"20180830074033.GE11944@sigill.intra.peff.net","subject":"Re: [PATCH 8/9] worktree: teach 'remove' to override lock when --force given twice","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-30T08:33:38Z","receivedAt":"2018-08-30T08:33:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 30, 2018 at 3:40 AM Jeff King <peff@peff.net> wrote:\n> On Tue, Aug 28, 2018 at 05:20:25PM -0400, Eric Sunshine wrote:\n> > -                      N_(\"force removing even if the worktree is dirty\"),\n> > +                      N_(\"force removal even if worktree is dirty or locked\"),\n>\n> I wonder if somebody might assume from this that a single \"-f\" would\n> override a lock. Perhaps not the end of the world, and the manpage does\n> make it clear. And also I don't really know how to be more specific here\n> without an overly long line.\n\nPrecisely, on all counts. Plus, if they try a single --force and it\nfails, the hint printed when it fails says explicitly to use --force\ntwice, so the \"solution\" is easily discoverable.\n\n> I'm guessing all those thoughts went through your head before ending up\n> here, too. :)\n\nYou guess correctly and you came to the same conclusion as I did.\nThese -h summary lines are just to short for any real discussion.\n"},{"id":"356956","messageId":"CAPig+cR90mzorzLfQTuq73R0zx_DrAa0hThHDsghdZo=cxOWSw@mail.gmail.com","threadId":"49246","inReplyTo":"20180830065734.GA11944@sigill.intra.peff.net","subject":"Re: [PATCH 3/9] worktree: generalize delete_git_dir() to reduce code duplication","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-30T08:45:30Z","receivedAt":"2018-08-30T08:45:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 30, 2018 at 2:57 AM Jeff King <peff@peff.net> wrote:\n> On Tue, Aug 28, 2018 at 05:20:20PM -0400, Eric Sunshine wrote:\n> > Simplify by extending delete_git_dir() to handle the little bit of\n> > extra functionality needed by prune_worktrees(), and drop the\n> > effectively duplicate code from the latter.\n>\n> Makes sense. The name \"delete_git_dir()\" is a little funny (I assume it\n> means \"the git dir\", not a worktree's git-dir), but that is not new (and\n> it's static in worktree.c, which helps).\n\nIt's not necessarily the best name, but, as you say, it's not a new\nissue, and it is local to that file.\n\nAlso, it's such a small function, and it's quite clear from the\nimplementation that it's deleting stuff from .git/worktrees that it's\nprobably okay to leave the name as-is for now.\n\n> Your patch maybe stretches that a little by deleting non-directories.\n> Maybe delete_from_worktrees() would be a better name. Probably not worth\n> a re-roll, though.\n\nThat's perhaps a bit better, though still somewhat ambiguous, as I can\nalso read it as deleting something from every worktree directory, as\nopposed to from .git/worktrees.\n"},{"id":"356957","messageId":"CAPig+cQ1a7eND5dOK7AKffBoQyNYHCgeqr9wkhB0dch4iAn4iw@mail.gmail.com","threadId":"49246","inReplyTo":"20180830075431.GF11944@sigill.intra.peff.net","subject":"Re: [PATCH 0/9] worktree: fix bugs and broaden --force applicability","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-30T08:57:23Z","receivedAt":"2018-08-30T08:57:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 30, 2018 at 3:54 AM Jeff King <peff@peff.net> wrote:\n> On Tue, Aug 28, 2018 at 05:20:17PM -0400, Eric Sunshine wrote:\n> > This series started as a fix for a bug reported by Peff[1] in which the\n> > \"database\" of worktrees could be corrupted (or at least become\n> > internally inconsistent) by having multiple worktree entries associated\n> > with the same path.\n>\n> Thanks for working on this. I think this nicely solves the problem I was\n> having.  I had a few minor comments which I sent in reply to individual\n> patches, but overall it looks very good. I'd be happy to see it picked\n> up with or without any changes based on my comments.\n\nThanks for the review comments. Hopefully, the series can land as-is\nwithout a re-roll.\n\nIt would be nice to find a better way to format the hint attached to\nthe fatal message, but that can always be tweaked with a follow-on\npatch if I don't have to re-roll for some other reason.\n"},{"id":"356959","messageId":"CAPig+cS3-oRzQrZGM_-pXUVFE3HvMdtODurPkmYgw1vFV8zb3Q@mail.gmail.com","threadId":"49246","inReplyTo":"20180830075431.GF11944@sigill.intra.peff.net","subject":"Re: [PATCH 0/9] worktree: fix bugs and broaden --force applicability","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-30T09:04:32Z","receivedAt":"2018-08-30T09:04:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 30, 2018 at 3:54 AM Jeff King <peff@peff.net> wrote:\n> Subject: [PATCH] doc-diff: force worktree add\n>\n> We avoid re-creating our temporary worktree if it's already\n> there. But we may run into a situation where the worktree\n> has been deleted, but an entry still exists in\n> $GIT_DIR/worktrees.\n\nCan \"clean\" or \"distclean\" also be augmented to remove this worktree\n(and directory)? I guess that \"distclean\" would make more sense than\n\"clean\"(?).\n\n> Older versions of git-worktree would annoyingly create a\n> series of duplicate entries. Recent versions now detect and\n> prevent this, allowing you to override with \"-f\". Since we\n> know that the worktree in question was just our temporary\n> workspace, it's safe for us to always pass \"-f\".\n\nMakes sense, and the patch looks correct.\n\n> Signed-off-by: Jeff King <peff@peff.net>\n"},{"id":"357009","messageId":"20180830194634.GE19685@sigill.intra.peff.net","threadId":"49246","inReplyTo":"CAPig+cS3-oRzQrZGM_-pXUVFE3HvMdtODurPkmYgw1vFV8zb3Q@mail.gmail.com","subject":"Re: [PATCH 0/9] worktree: fix bugs and broaden --force applicability","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-30T19:46:34Z","receivedAt":"2018-08-30T19:46:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 30, 2018 at 05:04:32AM -0400, Eric Sunshine wrote:\n\n> On Thu, Aug 30, 2018 at 3:54 AM Jeff King <peff@peff.net> wrote:\n> > Subject: [PATCH] doc-diff: force worktree add\n> >\n> > We avoid re-creating our temporary worktree if it's already\n> > there. But we may run into a situation where the worktree\n> > has been deleted, but an entry still exists in\n> > $GIT_DIR/worktrees.\n> \n> Can \"clean\" or \"distclean\" also be augmented to remove this worktree\n> (and directory)? I guess that \"distclean\" would make more sense than\n> \"clean\"(?).\n\nI suppose so. I don't think I've _ever_ used distclean, and I only\nrarely use \"clean\" (a testament to our Makefile's efforts to accurately\ntrack dependencies). I'd usually use \"git clean\" when I want something\npristine (because I don't want to trust the Makefile at all).\n\n-Peff\n"},{"id":"357021","messageId":"xmqqva7r4nof.fsf@gitster-ct.c.googlers.com","threadId":"49246","inReplyTo":"20180830194634.GE19685@sigill.intra.peff.net","subject":"Re: [PATCH 0/9] worktree: fix bugs and broaden --force applicability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-30T20:14:24Z","receivedAt":"2018-08-30T20:14:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I suppose so. I don't think I've _ever_ used distclean, and I only\n> rarely use \"clean\" (a testament to our Makefile's efforts to accurately\n> track dependencies). I'd usually use \"git clean\" when I want something\n> pristine (because I don't want to trust the Makefile at all).\n\nI do not trust \"git clean\" all that much, and pre-cleaning with\n\"make distclean\" and then running \"git clean -x\" has become my bad\nhabit.  I jump around quite a bit during the day, which would end up\nlittering the working tree with *.o files that are only known to one\nbut not both of {maint,pu}/Makefile's distclean rules.  I even do\n\"for i in pu maint master next; do git checkout $i; make distclean; done\"\nsometimes before running \"git clean -x\" ;-)\n"},{"id":"357057","messageId":"c9d51d68-c7ae-64f9-b3e9-a72f33c5afc3@ramsayjones.plus.com","threadId":"49246","inReplyTo":"xmqqva7r4nof.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/9] worktree: fix bugs and broaden --force applicability","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-08-30T23:49:39Z","receivedAt":"2018-08-30T23:49:43Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 30/08/18 21:14, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n>> I suppose so. I don't think I've _ever_ used distclean, and I only\n>> rarely use \"clean\" (a testament to our Makefile's efforts to accurately\n>> track dependencies). I'd usually use \"git clean\" when I want something\n>> pristine (because I don't want to trust the Makefile at all).\n> \n> I do not trust \"git clean\" all that much, and pre-cleaning with\n> \"make distclean\" and then running \"git clean -x\" has become my bad\n> habit.  I jump around quite a bit during the day, which would end up\n> littering the working tree with *.o files that are only known to one\n> but not both of {maint,pu}/Makefile's distclean rules.  I even do\n> \"for i in pu maint master next; do git checkout $i; make distclean; done\"\n> sometimes before running \"git clean -x\" ;-)\n> \n\n'git clean -x' always removes _way_ more than I want it\nto - in particular, I lost my config.mak more than once.\n\nSo, no I don't trust it. ;-)\n\nATB,\nRamsay Jones\n\n"},{"id":"357058","messageId":"20180831005426.GA2858@sigill.intra.peff.net","threadId":"49246","inReplyTo":"c9d51d68-c7ae-64f9-b3e9-a72f33c5afc3@ramsayjones.plus.com","subject":"Re: [PATCH 0/9] worktree: fix bugs and broaden --force applicability","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-31T00:54:26Z","receivedAt":"2018-08-31T00:54:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 31, 2018 at 12:49:39AM +0100, Ramsay Jones wrote:\n\n> On 30/08/18 21:14, Junio C Hamano wrote:\n> > Jeff King <peff@peff.net> writes:\n> > \n> >> I suppose so. I don't think I've _ever_ used distclean, and I only\n> >> rarely use \"clean\" (a testament to our Makefile's efforts to accurately\n> >> track dependencies). I'd usually use \"git clean\" when I want something\n> >> pristine (because I don't want to trust the Makefile at all).\n> > \n> > I do not trust \"git clean\" all that much, and pre-cleaning with\n> > \"make distclean\" and then running \"git clean -x\" has become my bad\n> > habit.  I jump around quite a bit during the day, which would end up\n> > littering the working tree with *.o files that are only known to one\n> > but not both of {maint,pu}/Makefile's distclean rules.  I even do\n> > \"for i in pu maint master next; do git checkout $i; make distclean; done\"\n> > sometimes before running \"git clean -x\" ;-)\n> > \n> \n> 'git clean -x' always removes _way_ more than I want it\n> to - in particular, I lost my config.mak more than once.\n\nHeh. I have done that, too, but fortunately mine is a symlink to a copy\nthat is held in a git repository. ;)\n\n-Peff\n"},{"id":"357140","messageId":"d2f70c00-5247-7af1-b059-a0f423030977@ramsayjones.plus.com","threadId":"49246","inReplyTo":"20180831005426.GA2858@sigill.intra.peff.net","subject":"Re: [PATCH 0/9] worktree: fix bugs and broaden --force applicability","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-08-31T21:57:46Z","receivedAt":"2018-08-31T21:57:51Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 31/08/18 01:54, Jeff King wrote:\n> On Fri, Aug 31, 2018 at 12:49:39AM +0100, Ramsay Jones wrote:\n> \n>> On 30/08/18 21:14, Junio C Hamano wrote:\n>>> Jeff King <peff@peff.net> writes:\n>>>\n>>>> I suppose so. I don't think I've _ever_ used distclean, and I only\n>>>> rarely use \"clean\" (a testament to our Makefile's efforts to accurately\n>>>> track dependencies). I'd usually use \"git clean\" when I want something\n>>>> pristine (because I don't want to trust the Makefile at all).\n>>>\n>>> I do not trust \"git clean\" all that much, and pre-cleaning with\n>>> \"make distclean\" and then running \"git clean -x\" has become my bad\n>>> habit.  I jump around quite a bit during the day, which would end up\n>>> littering the working tree with *.o files that are only known to one\n>>> but not both of {maint,pu}/Makefile's distclean rules.  I even do\n>>> \"for i in pu maint master next; do git checkout $i; make distclean; done\"\n>>> sometimes before running \"git clean -x\" ;-)\n>>>\n>>\n>> 'git clean -x' always removes _way_ more than I want it\n>> to - in particular, I lost my config.mak more than once.\n> \n> Heh. I have done that, too, but fortunately mine is a symlink to a copy\n> that is held in a git repository. ;)\n\n:-D\n\nNow, why didn't I think of that! ;-)\n\nATB,\nRamsay Jones\n\n"}]}