From 4c5fa9e6c4091eea9d8df3466b18ddb2812cd934 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 28 Aug 2018 17:20:18 -0400 Subject: [PATCH 01/10] worktree: don't die() in library function find_worktree() Callers don't expect library function find_worktree() to die(); they expect it to return the named worktree if found, or NULL if not. Although find_worktree() itself never invokes die(), it calls real_pathdup() with 'die_on_error' incorrectly set to 'true', thus will die() indirectly if the user-provided path is not to real_pathdup()'s liking. This can be observed, for instance, with any git-worktree command which searches for an existing worktree: $ git worktree unlock foo fatal: 'foo' is not a working tree $ git worktree unlock foo/bar fatal: Invalid path '.../foo': No such file or directory The first error message is the expected one from "git worktree unlock" not finding the specified worktree; the second is from find_worktree() invoking real_pathdup() incorrectly and die()ing prematurely. Aside from the inconsistent error message between the two cases, this bug hasn't otherwise been a serious problem since existing callers all die() anyhow when the worktree can't be found. However, that may not be true of callers added in the future, so fix find_worktree() to avoid die()ing. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t2028-worktree-move.sh | 8 ++++++++ worktree.c | 6 +++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh index 5f7d45b7b7..60aba7c41a 100755 --- a/t/t2028-worktree-move.sh +++ b/t/t2028-worktree-move.sh @@ -141,4 +141,12 @@ test_expect_success 'NOT remove missing-but-locked worktree' ' test_path_is_dir .git/worktrees/gone-but-locked ' +test_expect_success 'proper error when worktree not found' ' + for i in noodle noodle/bork + do + test_must_fail git worktree lock $i 2>err && + test_i18ngrep "not a working tree" err || return 1 + done +' + test_done diff --git a/worktree.c b/worktree.c index 97cda5f97b..b0d0b5426d 100644 --- a/worktree.c +++ b/worktree.c @@ -217,7 +217,11 @@ struct worktree *find_worktree(struct worktree **list, if (prefix) arg = to_free = prefix_filename(prefix, arg); - path = real_pathdup(arg, 1); + path = real_pathdup(arg, 0); + if (!path) { + free(to_free); + return NULL; + } for (; *list; list++) if (!fspathcmp(path, real_path((*list)->path))) break; From e5353bef550d91655b3f8fb443a26c0ba209153f Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 28 Aug 2018 17:20:19 -0400 Subject: [PATCH 02/10] worktree: move delete_git_dir() earlier in file for upcoming new callers This is a pure code movement to avoid having to forward-declare the function when new callers are subsequently added. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/worktree.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 41e7714396..a8128289cc 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -47,6 +47,20 @@ static int git_worktree_config(const char *var, const char *value, void *cb) return git_default_config(var, value, cb); } +static int delete_git_dir(struct worktree *wt) +{ + struct strbuf sb = STRBUF_INIT; + int ret = 0; + + strbuf_addstr(&sb, git_common_path("worktrees/%s", wt->id)); + if (remove_dir_recursively(&sb, 0)) { + error_errno(_("failed to delete '%s'"), sb.buf); + ret = -1; + } + strbuf_release(&sb); + return ret; +} + static int prune_worktree(const char *id, struct strbuf *reason) { struct stat st; @@ -822,20 +836,6 @@ static int delete_git_work_tree(struct worktree *wt) return ret; } -static int delete_git_dir(struct worktree *wt) -{ - struct strbuf sb = STRBUF_INIT; - int ret = 0; - - strbuf_addstr(&sb, git_common_path("worktrees/%s", wt->id)); - if (remove_dir_recursively(&sb, 0)) { - error_errno(_("failed to delete '%s'"), sb.buf); - ret = -1; - } - strbuf_release(&sb); - return ret; -} - static int remove_worktree(int ac, const char **av, const char *prefix) { int force = 0; From 602aaed03f7f82323d88703d6fa2263a13c37907 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 28 Aug 2018 17:20:20 -0400 Subject: [PATCH 03/10] worktree: generalize delete_git_dir() to reduce code duplication prune_worktrees() and delete_git_dir() both remove worktree administrative entries from .git/worktrees, and their implementations are nearly identical. The only difference is that prune_worktrees() is also capable of removing a bogus non-worktree-related file from .git/worktrees. Simplify by extending delete_git_dir() to handle the little bit of extra functionality needed by prune_worktrees(), and drop the effectively duplicate code from the latter. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/worktree.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index a8128289cc..0affcb476c 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -47,16 +47,17 @@ static int git_worktree_config(const char *var, const char *value, void *cb) return git_default_config(var, value, cb); } -static int delete_git_dir(struct worktree *wt) +static int delete_git_dir(const char *id) { struct strbuf sb = STRBUF_INIT; - int ret = 0; + int ret; - strbuf_addstr(&sb, git_common_path("worktrees/%s", wt->id)); - if (remove_dir_recursively(&sb, 0)) { + strbuf_addstr(&sb, git_common_path("worktrees/%s", id)); + ret = remove_dir_recursively(&sb, 0); + if (ret < 0 && errno == ENOTDIR) + ret = unlink(sb.buf); + if (ret) error_errno(_("failed to delete '%s'"), sb.buf); - ret = -1; - } strbuf_release(&sb); return ret; } @@ -130,10 +131,8 @@ static int prune_worktree(const char *id, struct strbuf *reason) static void prune_worktrees(void) { struct strbuf reason = STRBUF_INIT; - struct strbuf path = STRBUF_INIT; DIR *dir = opendir(git_path("worktrees")); struct dirent *d; - int ret; if (!dir) return; while ((d = readdir(dir)) != NULL) { @@ -146,18 +145,12 @@ static void prune_worktrees(void) printf("%s\n", reason.buf); if (show_only) continue; - git_path_buf(&path, "worktrees/%s", d->d_name); - ret = remove_dir_recursively(&path, 0); - if (ret < 0 && errno == ENOTDIR) - ret = unlink(path.buf); - if (ret) - error_errno(_("failed to remove '%s'"), path.buf); + delete_git_dir(d->d_name); } closedir(dir); if (!show_only) rmdir(git_path("worktrees")); strbuf_release(&reason); - strbuf_release(&path); } static int prune(int ac, const char **av, const char *prefix) @@ -882,7 +875,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix) * continue on even if ret is non-zero, there's no going back * from here. */ - ret |= delete_git_dir(wt); + ret |= delete_git_dir(wt->id); free_worktrees(worktrees); return ret; From 45059e6468ab5710748c8827d5bf1f4c7c69d6d1 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 28 Aug 2018 17:20:21 -0400 Subject: [PATCH 04/10] worktree: prepare for more checks of whether path can become worktree Certain conditions must be met for a path to be a valid candidate as the location of a new worktree; for instance, the path must not exist or must be an empty directory. Although the number of conditions is small, new conditions will soon be added so factor out the existing checks into a separate function to avoid further bloating add_worktree(). Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/worktree.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 0affcb476c..46c93771e8 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -219,6 +219,12 @@ static const char *worktree_basename(const char *path, int *olen) return name; } +static void validate_worktree_add(const char *path, const struct add_opts *opts) +{ + if (file_exists(path) && !is_empty_dir(path)) + die(_("'%s' already exists"), path); +} + static int add_worktree(const char *path, const char *refname, const struct add_opts *opts) { @@ -233,8 +239,7 @@ static int add_worktree(const char *path, const char *refname, struct commit *commit = NULL; int is_branch = 0; - if (file_exists(path) && !is_empty_dir(path)) - die(_("'%s' already exists"), path); + validate_worktree_add(path, opts); /* is 'refname' a branch or commit? */ if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) && From cb56f55c16c128e18449c9417dc045f787c1b663 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 28 Aug 2018 17:20:22 -0400 Subject: [PATCH 05/10] worktree: disallow adding same path multiple times A given path should only ever be associated with a single registered worktree. This invariant is enforced by refusing to create a new worktree at a given path if that path already exists. For example: $ git worktree add -q --detach foo $ git worktree add -q --detach foo fatal: 'foo' already exists However, the check can be fooled, and the invariant broken, if the path is missing. Continuing the example: $ rm -fr foo $ git worktree add -q --detach foo $ git worktree list ... eadebfe [master] .../foo eadebfe (detached HEAD) .../foo eadebfe (detached HEAD) This "corruption" leads to the unfortunate situation in which the worktree can not be removed: $ git worktree remove foo fatal: validation failed, cannot remove working tree: '.../foo' does not point back to '.git/worktrees/foo' Nor can the bogus entry be pruned: $ git worktree prune -v $ git worktree list ... eadebfe [master] .../foo eadebfe (detached HEAD) .../foo eadebfe (detached HEAD) without first deleting the worktree directory manually: $ rm -fr foo $ git worktree prune -v Removing .../foo: gitdir file points to non-existent location Removing .../foo1: gitdir file points to non-existent location $ git worktree list ... eadebfe [master] or by manually deleting the worktree entry in .git/worktrees. To address this problem, upgrade "git worktree add" validation to allow worktree creation only if the given path is not already associated with an existing worktree (even if the path itself is non-existent), thus preventing such bogus worktree entries from being created in the first place. Reported-by: Jeff King Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/worktree.c | 25 +++++++++++++++++++++++++ t/t2025-worktree-add.sh | 7 +++++++ 2 files changed, 32 insertions(+) diff --git a/builtin/worktree.c b/builtin/worktree.c index 46c93771e8..1122f27b5f 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -221,8 +221,33 @@ static const char *worktree_basename(const char *path, int *olen) static void validate_worktree_add(const char *path, const struct add_opts *opts) { + struct worktree **worktrees; + struct worktree *wt; + int locked; + if (file_exists(path) && !is_empty_dir(path)) die(_("'%s' already exists"), path); + + worktrees = get_worktrees(0); + /* + * find_worktree()'s suffix matching may undesirably find the main + * rather than a linked worktree (for instance, when the basenames + * of the main worktree and the one being created are the same). + * We're only interested in linked worktrees, so skip the main + * worktree with +1. + */ + wt = find_worktree(worktrees + 1, NULL, path); + if (!wt) + goto done; + + locked = !!is_worktree_locked(wt); + if (locked) + die(_("'%s' is a missing but locked worktree;\nuse 'unlock' and 'prune' or 'remove' to clear"), path); + else + die(_("'%s' is a missing but already registered worktree;\nuse 'prune' or 'remove' to clear"), path); + +done: + free_worktrees(worktrees); } static int add_worktree(const char *path, const char *refname, diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index 07d292317c..67fccc6591 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -552,4 +552,11 @@ test_expect_success '"add" in bare repo invokes post-checkout hook' ' test_cmp hook.expect goozy/hook.actual ' +test_expect_success '"add" an existing but missing worktree' ' + git worktree add --detach pneu && + test_must_fail git worktree add --detach pneu && + rm -fr pneu && + test_must_fail git worktree add --detach pneu +' + test_done From e19831c94f91fd410fe001c0372b9c88b40d335b Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 28 Aug 2018 17:20:23 -0400 Subject: [PATCH 06/10] worktree: teach 'add' to respect --force for registered but missing path For safety, "git worktree add " will refuse to add a new worktree at if is already associated with a worktree entry, even if is missing (for instance, has been deleted or resides on non-mounted removable media or network share). The typical way to re-create a worktree at in such a situation is either to prune all "broken" entries ("git worktree prune") or to selectively remove the worktree entry manually ("git worktree remove "). However, neither of these approaches ("prune" nor "remove") is especially convenient, and they may be unsuitable for scripting when a tool merely wants to re-use a worktree if it exists or create it from scratch if it doesn't (much as a tool might use "mkdir -p" to re-use or create a directory). Therefore, teach 'add' to respect --force as a convenient way to re-use a path already associated with a worktree entry if the path is non-existent. For a locked worktree, require --force to be specified twice. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/git-worktree.txt | 8 ++++++-- builtin/worktree.c | 10 ++++++++-- t/t2025-worktree-add.sh | 13 ++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 29a5b7e252..8537692f05 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -120,8 +120,12 @@ OPTIONS --force:: By default, `add` refuses to create a new working tree when `` is a branch name and is already checked out by - another working tree and `remove` refuses to remove an unclean - working tree. This option overrides these safeguards. + another working tree, or if `` is already assigned to some + working tree but is missing (for instance, if `` was deleted + manually). This option overrides these safeguards. To add a missing but + locked working tree path, specify `--force` twice. ++ +`remove` refuses to remove an unclean working tree unless `--force` is used. -b :: -B :: diff --git a/builtin/worktree.c b/builtin/worktree.c index 1122f27b5f..3eb2f89b0f 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -241,10 +241,16 @@ static void validate_worktree_add(const char *path, const struct add_opts *opts) goto done; locked = !!is_worktree_locked(wt); + if ((!locked && opts->force) || (locked && opts->force > 1)) { + if (delete_git_dir(wt->id)) + die(_("unable to re-add worktree '%s'"), path); + goto done; + } + if (locked) - die(_("'%s' is a missing but locked worktree;\nuse 'unlock' and 'prune' or 'remove' to clear"), path); + die(_("'%s' is a missing but locked worktree;\nuse 'add -f -f' to override, or 'unlock' and 'prune' or 'remove' to clear"), path); else - die(_("'%s' is a missing but already registered worktree;\nuse 'prune' or 'remove' to clear"), path); + die(_("'%s' is a missing but already registered worktree;\nuse 'add -f' to override, or 'prune' or 'remove' to clear"), path); done: free_worktrees(worktrees); diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index 67fccc6591..286bba35d8 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -556,7 +556,18 @@ test_expect_success '"add" an existing but missing worktree' ' git worktree add --detach pneu && test_must_fail git worktree add --detach pneu && rm -fr pneu && - test_must_fail git worktree add --detach pneu + test_must_fail git worktree add --detach pneu && + git worktree add --force --detach pneu +' + +test_expect_success '"add" an existing locked but missing worktree' ' + git worktree add --detach gnoo && + git worktree lock gnoo && + test_when_finished "git worktree unlock gnoo || :" && + rm -fr gnoo && + test_must_fail git worktree add --detach gnoo && + test_must_fail git worktree add --force --detach gnoo && + git worktree add --force --force --detach gnoo ' test_done From 68a6b3a1bd45eb1814a0c1cedfe347692c3f2ff7 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 28 Aug 2018 17:20:24 -0400 Subject: [PATCH 07/10] worktree: teach 'move' to override lock when --force given twice For consistency with "add -f -f", which allows a missing but locked worktree path to be re-used, allow "move -f -f" to override a lock, as well, as a convenience. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/git-worktree.txt | 3 +++ builtin/worktree.c | 13 +++++++++---- t/t2028-worktree-move.sh | 14 ++++++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 8537692f05..d08b8d8e4f 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -125,6 +125,9 @@ OPTIONS manually). This option overrides these safeguards. To add a missing but locked working tree path, specify `--force` twice. + +`move` refuses to move a locked working tree unless `--force` is specified +twice. ++ `remove` refuses to remove an unclean working tree unless `--force` is used. -b :: diff --git a/builtin/worktree.c b/builtin/worktree.c index 3eb2f89b0f..354a6c0eb5 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -740,13 +740,17 @@ static void validate_no_submodules(const struct worktree *wt) static int move_worktree(int ac, const char **av, const char *prefix) { + int force = 0; struct option options[] = { + OPT__FORCE(&force, + N_("force move even if worktree is dirty or locked"), + PARSE_OPT_NOCOMPLETE), OPT_END() }; struct worktree **worktrees, *wt; struct strbuf dst = STRBUF_INIT; struct strbuf errmsg = STRBUF_INIT; - const char *reason; + const char *reason = NULL; char *path; ac = parse_options(ac, av, prefix, options, worktree_usage, 0); @@ -777,12 +781,13 @@ static int move_worktree(int ac, const char **av, const char *prefix) validate_no_submodules(wt); - reason = is_worktree_locked(wt); + if (force < 2) + reason = is_worktree_locked(wt); if (reason) { if (*reason) - die(_("cannot move a locked working tree, lock reason: %s"), + die(_("cannot move a locked working tree, lock reason: %s\nuse 'move -f -f' to override or unlock first"), reason); - die(_("cannot move a locked working tree")); + die(_("cannot move a locked working tree;\nuse 'move -f -f' to override or unlock first")); } if (validate_worktree(wt, &errmsg, 0)) die(_("validation failed, cannot move working tree: %s"), diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh index 60aba7c41a..9756ede8f1 100755 --- a/t/t2028-worktree-move.sh +++ b/t/t2028-worktree-move.sh @@ -98,6 +98,20 @@ test_expect_success 'move worktree to another dir' ' test_cmp expected2 actual2 ' +test_expect_success 'move locked worktree (force)' ' + test_when_finished " + git worktree unlock flump || : + git worktree remove flump || : + git worktree unlock ploof || : + git worktree remove ploof || : + " && + git worktree add --detach flump && + git worktree lock flump && + test_must_fail git worktree move flump ploof" && + test_must_fail git worktree move --force flump ploof" && + git worktree move --force --force flump ploof +' + test_expect_success 'remove main worktree' ' test_must_fail git worktree remove . ' From f4143101cbb26d189f63f2d29875f4acc07b2730 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 28 Aug 2018 17:20:25 -0400 Subject: [PATCH 08/10] worktree: teach 'remove' to override lock when --force given twice For consistency with "add -f -f" and "move -f -f" which override the lock on a worktree, allow "remove -f -f" to do so, as well, as a convenience. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/git-worktree.txt | 1 + builtin/worktree.c | 11 ++++++----- t/t2028-worktree-move.sh | 10 ++++++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index d08b8d8e4f..e2ee9fc21b 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -129,6 +129,7 @@ OPTIONS twice. + `remove` refuses to remove an unclean working tree unless `--force` is used. +To remove a locked working tree, specify `--force` twice. -b :: -B :: diff --git a/builtin/worktree.c b/builtin/worktree.c index 354a6c0eb5..a95fe67da6 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -875,13 +875,13 @@ static int remove_worktree(int ac, const char **av, const char *prefix) int force = 0; struct option options[] = { OPT__FORCE(&force, - N_("force removing even if the worktree is dirty"), + N_("force removal even if worktree is dirty or locked"), PARSE_OPT_NOCOMPLETE), OPT_END() }; struct worktree **worktrees, *wt; struct strbuf errmsg = STRBUF_INIT; - const char *reason; + const char *reason = NULL; int ret = 0; ac = parse_options(ac, av, prefix, options, worktree_usage, 0); @@ -894,12 +894,13 @@ static int remove_worktree(int ac, const char **av, const char *prefix) die(_("'%s' is not a working tree"), av[0]); if (is_main_worktree(wt)) die(_("'%s' is a main working tree"), av[0]); - reason = is_worktree_locked(wt); + if (force < 2) + reason = is_worktree_locked(wt); if (reason) { if (*reason) - die(_("cannot remove a locked working tree, lock reason: %s"), + die(_("cannot remove a locked working tree, lock reason: %s\nuse 'remove -f -f' to override or unlock first"), reason); - die(_("cannot remove a locked working tree")); + die(_("cannot remove a locked working tree;\nuse 'remove -f -f' to override or unlock first")); } if (validate_worktree(wt, &errmsg, WT_VALIDATE_WORKTREE_MISSING_OK)) die(_("validation failed, cannot remove working tree: %s"), diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh index 9756ede8f1..1b5079e8fa 100755 --- a/t/t2028-worktree-move.sh +++ b/t/t2028-worktree-move.sh @@ -163,4 +163,14 @@ test_expect_success 'proper error when worktree not found' ' done ' +test_expect_success 'remove locked worktree (force)' ' + git worktree add --detach gumby && + test_when_finished "git worktree remove gumby || :" && + git worktree lock gumby && + test_when_finished "git worktree unlock gumby || :" && + test_must_fail git worktree remove gumby && + test_must_fail git worktree remove --force gumby && + git worktree remove --force --force gumby +' + test_done From 3a5404333c6c8b5897e78c9dcb711d623035791b Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 28 Aug 2018 17:20:26 -0400 Subject: [PATCH 09/10] worktree: delete .git/worktrees if empty after 'remove' For cleanliness, "git worktree prune" deletes the .git/worktrees directory if it is empty after pruning is complete. For consistency, make "git worktree remove " likewise delete .git/worktrees if it is empty after the removal. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/worktree.c | 8 +++++++- t/t2028-worktree-move.sh | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index a95fe67da6..c4abbde2b8 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -62,6 +62,11 @@ static int delete_git_dir(const char *id) return ret; } +static void delete_worktrees_dir_if_empty(void) +{ + rmdir(git_path("worktrees")); /* ignore failed removal */ +} + static int prune_worktree(const char *id, struct strbuf *reason) { struct stat st; @@ -149,7 +154,7 @@ static void prune_worktrees(void) } closedir(dir); if (!show_only) - rmdir(git_path("worktrees")); + delete_worktrees_dir_if_empty(); strbuf_release(&reason); } @@ -918,6 +923,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix) * from here. */ ret |= delete_git_dir(wt->id); + delete_worktrees_dir_if_empty(); free_worktrees(worktrees); return ret; diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh index 1b5079e8fa..33c0337733 100755 --- a/t/t2028-worktree-move.sh +++ b/t/t2028-worktree-move.sh @@ -173,4 +173,16 @@ test_expect_success 'remove locked worktree (force)' ' git worktree remove --force --force gumby ' +test_expect_success 'remove cleans up .git/worktrees when empty' ' + git init moog && + ( + cd moog && + test_commit bim && + git worktree add --detach goom && + test_path_exists .git/worktrees && + git worktree remove goom && + test_path_is_missing .git/worktrees + ) +' + test_done From 684e742249b48ec0c5491a98fa3b1427793738ce Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 30 Aug 2018 03:54:31 -0400 Subject: [PATCH 10/10] doc-diff: force worktree add We avoid re-creating our temporary worktree if it's already there. But we may run into a situation where the worktree has been deleted, but an entry still exists in $GIT_DIR/worktrees. Older versions of git-worktree would annoyingly create a series of duplicate entries. Recent versions now detect and prevent this, allowing you to override with "-f". Since we know that the worktree in question was just our temporary workspace, it's safe for us to always pass "-f". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/doc-diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/doc-diff b/Documentation/doc-diff index f483fe427c..19d841ddeb 100755 --- a/Documentation/doc-diff +++ b/Documentation/doc-diff @@ -54,7 +54,7 @@ fi # results that don't differ between the two trees. if ! test -d "$tmp/worktree" then - git worktree add --detach "$tmp/worktree" "$from" && + git worktree add -f --detach "$tmp/worktree" "$from" && dots=$(echo "$tmp/worktree" | sed 's#[^/]*#..#g') && ln -s "$dots/config.mak" "$tmp/worktree/config.mak" fi