From 4ddddc1f1d763d3c4e0e57af1153c6d48ca4db9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 24 Jan 2018 16:53:51 +0700 Subject: worktree.c: add validate_worktree() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function is later used by "worktree move" and "worktree remove" to ensure that we have a good connection between the repository and the worktree. For example, if a worktree is moved manually, the worktree location recorded in $GIT_DIR/worktrees/.../gitdir is incorrect and we should not move that one. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/worktree.c b/worktree.c index f5da7d2..b238d87 100644 --- a/worktree.c +++ b/worktree.c @@ -254,6 +254,78 @@ const char *is_worktree_locked(struct worktree *wt) return wt->lock_reason; } +/* convenient wrapper to deal with NULL strbuf */ +static void strbuf_addf_gently(struct strbuf *buf, const char *fmt, ...) +{ + va_list params; + + if (!buf) + return; + + va_start(params, fmt); + strbuf_vaddf(buf, fmt, params); + va_end(params); +} + +int validate_worktree(const struct worktree *wt, struct strbuf *errmsg) +{ + struct strbuf wt_path = STRBUF_INIT; + char *path = NULL; + int err, ret = -1; + + strbuf_addf(&wt_path, "%s/.git", wt->path); + + if (is_main_worktree(wt)) { + if (is_directory(wt_path.buf)) { + ret = 0; + goto done; + } + /* + * Main worktree using .git file to point to the + * repository would make it impossible to know where + * the actual worktree is if this function is executed + * from another worktree. No .git file support for now. + */ + strbuf_addf_gently(errmsg, + _("'%s' at main working tree is not the repository directory"), + wt_path.buf); + goto done; + } + + /* + * Make sure "gitdir" file points to a real .git file and that + * file points back here. + */ + if (!is_absolute_path(wt->path)) { + strbuf_addf_gently(errmsg, + _("'%s' file does not contain absolute path to the working tree location"), + git_common_path("worktrees/%s/gitdir", wt->id)); + goto done; + } + + if (!file_exists(wt_path.buf)) { + strbuf_addf_gently(errmsg, _("'%s' does not exist"), wt_path.buf); + goto done; + } + + path = xstrdup_or_null(read_gitfile_gently(wt_path.buf, &err)); + if (!path) { + strbuf_addf_gently(errmsg, _("'%s' is not a .git file, error code %d"), + wt_path.buf, err); + goto done; + } + + ret = fspathcmp(path, real_path(git_common_path("worktrees/%s", wt->id))); + + if (ret) + strbuf_addf_gently(errmsg, _("'%s' does not point back to '%s'"), + wt->path, git_common_path("worktrees/%s", wt->id)); +done: + free(path); + strbuf_release(&wt_path); + return ret; +} + int is_worktree_being_rebased(const struct worktree *wt, const char *target) { diff --git a/worktree.h b/worktree.h index c28a880..cb577de 100644 --- a/worktree.h +++ b/worktree.h @@ -3,6 +3,8 @@ #include "refs.h" +struct strbuf; + struct worktree { char *path; char *id; @@ -60,6 +62,13 @@ extern int is_main_worktree(const struct worktree *wt); extern const char *is_worktree_locked(struct worktree *wt); /* + * Return zero if the worktree is in good condition. Error message is + * returned if "errmsg" is not NULL. + */ +extern int validate_worktree(const struct worktree *wt, + struct strbuf *errmsg); + +/* * Free up the memory for worktree(s) */ extern void free_worktrees(struct worktree **); -- cgit v0.10.2-6-g49f6 From 9c620fc7a60c64e183a661f4df4f7b9e25501099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 12 Feb 2018 16:49:35 +0700 Subject: worktree.c: add update_worktree_location() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/worktree.c b/worktree.c index b238d87..0373faf 100644 --- a/worktree.c +++ b/worktree.c @@ -326,6 +326,23 @@ done: return ret; } +void update_worktree_location(struct worktree *wt, const char *path_) +{ + struct strbuf path = STRBUF_INIT; + + if (is_main_worktree(wt)) + die("BUG: can't relocate main worktree"); + + strbuf_realpath(&path, path_, 1); + if (fspathcmp(wt->path, path.buf)) { + write_file(git_common_path("worktrees/%s/gitdir", wt->id), + "%s/.git", path.buf); + free(wt->path); + wt->path = strbuf_detach(&path, NULL); + } + strbuf_release(&path); +} + int is_worktree_being_rebased(const struct worktree *wt, const char *target) { diff --git a/worktree.h b/worktree.h index cb577de..a913428 100644 --- a/worktree.h +++ b/worktree.h @@ -69,6 +69,12 @@ extern int validate_worktree(const struct worktree *wt, struct strbuf *errmsg); /* + * Update worktrees/xxx/gitdir with the new path. + */ +extern void update_worktree_location(struct worktree *wt, + const char *path_); + +/* * Free up the memory for worktree(s) */ extern void free_worktrees(struct worktree **); -- cgit v0.10.2-6-g49f6 From 9f792bb4721066ce27ee98c4ad38d6d146b64fcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 12 Feb 2018 16:49:36 +0700 Subject: worktree move: new command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This command allows to relocate linked worktrees. Main worktree cannot (yet) be moved. There are two options to move the main worktree, but both have complications, so it's not implemented yet. Anyway the options are: - convert the main worktree to a linked one and move it away, leave the git repository where it is. The repo essentially becomes bare after this move. - move the repository with the main worktree. The tricky part is make sure all file descriptors to the repository are closed, or it may fail on Windows. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 41585f5..4fa1dd3 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -12,6 +12,7 @@ SYNOPSIS 'git worktree add' [-f] [--detach] [--checkout] [--lock] [-b ] [] 'git worktree list' [--porcelain] 'git worktree lock' [--reason ] +'git worktree move' 'git worktree prune' [-n] [-v] [--expire ] 'git worktree unlock' @@ -34,10 +35,6 @@ The working tree's administrative files in the repository (see `git worktree prune` in the main or any linked working tree to clean up any stale administrative files. -If you move a linked working tree, you need to manually update the -administrative files so that they do not get pruned automatically. See -section "DETAILS" for more information. - If a linked working tree is stored on a portable device or network share which is not always mounted, you can prevent its administrative files from being pruned by issuing the `git worktree lock` command, optionally @@ -79,6 +76,11 @@ files from being pruned automatically. This also prevents it from being moved or deleted. Optionally, specify a reason for the lock with `--reason`. +move:: + +Move a working tree to a new location. Note that the main working tree +cannot be moved. + prune:: Prune working tree information in $GIT_DIR/worktrees. @@ -196,7 +198,7 @@ thumb is do not make any assumption about whether a path belongs to $GIT_DIR or $GIT_COMMON_DIR when you need to directly access something inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path. -If you move a linked working tree, you need to update the 'gitdir' file +If you manually move a linked working tree, you need to update the 'gitdir' file in the entry's directory. For example, if a linked working tree is moved to `/newpath/test-next` and its `.git` file points to `/path/main/.git/worktrees/test-next`, then update @@ -281,7 +283,6 @@ performed manually, such as: - `remove` to remove a linked working tree and its administrative files (and warn if the working tree is dirty) -- `mv` to move or rename a working tree and update its administrative files GIT --- diff --git a/builtin/worktree.c b/builtin/worktree.c index 7cef5b1..8b9482d 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -17,6 +17,7 @@ static const char * const worktree_usage[] = { N_("git worktree add [] []"), N_("git worktree list []"), N_("git worktree lock [] "), + N_("git worktree move "), N_("git worktree prune []"), N_("git worktree unlock "), NULL @@ -605,6 +606,56 @@ static int unlock_worktree(int ac, const char **av, const char *prefix) return ret; } +static int move_worktree(int ac, const char **av, const char *prefix) +{ + struct option options[] = { + OPT_END() + }; + struct worktree **worktrees, *wt; + struct strbuf dst = STRBUF_INIT; + struct strbuf errmsg = STRBUF_INIT; + const char *reason; + char *path; + + ac = parse_options(ac, av, prefix, options, worktree_usage, 0); + if (ac != 2) + usage_with_options(worktree_usage, options); + + path = prefix_filename(prefix, av[1]); + strbuf_addstr(&dst, path); + free(path); + + worktrees = get_worktrees(0); + wt = find_worktree(worktrees, prefix, av[0]); + if (!wt) + die(_("'%s' is not a working tree"), av[0]); + if (is_main_worktree(wt)) + die(_("'%s' is a main working tree"), av[0]); + if (file_exists(dst.buf)) + die(_("target '%s' already exists"), av[1]); + + reason = is_worktree_locked(wt); + if (reason) { + if (*reason) + die(_("cannot move a locked working tree, lock reason: %s"), + reason); + die(_("cannot move a locked working tree")); + } + if (validate_worktree(wt, &errmsg)) + die(_("validation failed, cannot move working tree: %s"), + errmsg.buf); + strbuf_release(&errmsg); + + if (rename(wt->path, dst.buf) == -1) + die_errno(_("failed to move '%s' to '%s'"), wt->path, dst.buf); + + update_worktree_location(wt, dst.buf); + + strbuf_release(&dst); + free_worktrees(worktrees); + return 0; +} + int cmd_worktree(int ac, const char **av, const char *prefix) { struct option options[] = { @@ -627,5 +678,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix) return lock_worktree(ac - 1, av + 1, prefix); if (!strcmp(av[1], "unlock")) return unlock_worktree(ac - 1, av + 1, prefix); + if (!strcmp(av[1], "move")) + return move_worktree(ac - 1, av + 1, prefix); usage_with_options(worktree_usage, options); } diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 3683c77..b87d387 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -3087,7 +3087,7 @@ _git_whatchanged () _git_worktree () { - local subcommands="add list lock prune unlock" + local subcommands="add list lock move prune unlock" local subcommand="$(__git_find_on_cmdline "$subcommands")" if [ -z "$subcommand" ]; then __gitcomp "$subcommands" diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh index 8298aaf..0f8abc0 100755 --- a/t/t2028-worktree-move.sh +++ b/t/t2028-worktree-move.sh @@ -59,4 +59,30 @@ test_expect_success 'unlock worktree twice' ' test_path_is_missing .git/worktrees/source/locked ' +test_expect_success 'move non-worktree' ' + mkdir abc && + test_must_fail git worktree move abc def +' + +test_expect_success 'move locked worktree' ' + git worktree lock source && + test_when_finished "git worktree unlock source" && + test_must_fail git worktree move source destination +' + +test_expect_success 'move worktree' ' + toplevel="$(pwd)" && + git worktree move source destination && + test_path_is_missing source && + git worktree list --porcelain | grep "^worktree.*/destination" && + ! git worktree list --porcelain | grep "^worktree.*/source" >empty && + git -C destination log --format=%s >actual2 && + echo init >expected2 && + test_cmp expected2 actual2 +' + +test_expect_success 'move main worktree' ' + test_must_fail git worktree move . def +' + test_done -- cgit v0.10.2-6-g49f6 From c64a8d200f4109df86c6d4716ea4da58df450e34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 12 Feb 2018 16:49:37 +0700 Subject: worktree move: accept destination as directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to "mv a b/", which is actually "mv a b/a", we extract basename of source worktree and create a directory of the same name at destination if dst path is a directory. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/builtin/worktree.c b/builtin/worktree.c index 8b9482d..6fe4131 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -631,8 +631,17 @@ static int move_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]); + if (is_directory(dst.buf)) { + const char *sep = find_last_dir_sep(wt->path); + + if (!sep) + die(_("could not figure out destination name from '%s'"), + wt->path); + strbuf_trim_trailing_dir_sep(&dst); + strbuf_addstr(&dst, sep); + } if (file_exists(dst.buf)) - die(_("target '%s' already exists"), av[1]); + die(_("target '%s' already exists"), dst.buf); reason = is_worktree_locked(wt); if (reason) { diff --git a/strbuf.c b/strbuf.c index 1df674e..46930ad 100644 --- a/strbuf.c +++ b/strbuf.c @@ -95,6 +95,7 @@ void strbuf_trim(struct strbuf *sb) strbuf_rtrim(sb); strbuf_ltrim(sb); } + void strbuf_rtrim(struct strbuf *sb) { while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1])) @@ -102,6 +103,13 @@ void strbuf_rtrim(struct strbuf *sb) sb->buf[sb->len] = '\0'; } +void strbuf_trim_trailing_dir_sep(struct strbuf *sb) +{ + while (sb->len > 0 && is_dir_sep((unsigned char)sb->buf[sb->len - 1])) + sb->len--; + sb->buf[sb->len] = '\0'; +} + void strbuf_ltrim(struct strbuf *sb) { char *b = sb->buf; diff --git a/strbuf.h b/strbuf.h index 14c8c10..e6cae5f 100644 --- a/strbuf.h +++ b/strbuf.h @@ -179,6 +179,9 @@ extern void strbuf_trim(struct strbuf *); extern void strbuf_rtrim(struct strbuf *); extern void strbuf_ltrim(struct strbuf *); +/* Strip trailing directory separators */ +extern void strbuf_trim_trailing_dir_sep(struct strbuf *); + /** * Replace the contents of the strbuf with a reencoded form. Returns -1 * on error, 0 on success. diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh index 0f8abc0..deb486c 100755 --- a/t/t2028-worktree-move.sh +++ b/t/t2028-worktree-move.sh @@ -85,4 +85,15 @@ test_expect_success 'move main worktree' ' test_must_fail git worktree move . def ' +test_expect_success 'move worktree to another dir' ' + toplevel="$(pwd)" && + mkdir some-dir && + git worktree move destination some-dir && + test_path_is_missing source && + git worktree list --porcelain | grep "^worktree.*/some-dir/destination" && + git -C some-dir/destination log --format=%s >actual2 && + echo init >expected2 && + test_cmp expected2 actual2 +' + test_done -- cgit v0.10.2-6-g49f6 From 78d986b252359351a579dc2629c8384d5c8eb8ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 12 Feb 2018 16:49:38 +0700 Subject: worktree move: refuse to move worktrees with submodules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Submodules contains .git files with relative paths. After a worktree move, these files need to be updated or they may point to nowhere. This is a bandage patch to make sure "worktree move" don't break people's worktrees by accident. When .git file update code is in place, this validate_no_submodules() could be removed. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 4fa1dd3..e6764ee 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -79,7 +79,7 @@ with `--reason`. move:: Move a working tree to a new location. Note that the main working tree -cannot be moved. +or linked working trees containing submodules cannot be moved. prune:: diff --git a/builtin/worktree.c b/builtin/worktree.c index 6fe4131..4789ceb 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -606,6 +606,27 @@ static int unlock_worktree(int ac, const char **av, const char *prefix) return ret; } +static void validate_no_submodules(const struct worktree *wt) +{ + struct index_state istate = { NULL }; + int i, found_submodules = 0; + + if (read_index_from(&istate, worktree_git_path(wt, "index")) > 0) { + for (i = 0; i < istate.cache_nr; i++) { + struct cache_entry *ce = istate.cache[i]; + + if (S_ISGITLINK(ce->ce_mode)) { + found_submodules = 1; + break; + } + } + } + discard_index(&istate); + + if (found_submodules) + die(_("working trees containing submodules cannot be moved")); +} + static int move_worktree(int ac, const char **av, const char *prefix) { struct option options[] = { @@ -643,6 +664,8 @@ static int move_worktree(int ac, const char **av, const char *prefix) if (file_exists(dst.buf)) die(_("target '%s' already exists"), dst.buf); + validate_no_submodules(wt); + reason = is_worktree_locked(wt); if (reason) { if (*reason) -- cgit v0.10.2-6-g49f6 From cc73385cf6c5c229458775bc92e7dbbe24d11611 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 12 Feb 2018 16:49:39 +0700 Subject: worktree remove: new command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This command allows to delete a worktree. Like 'move' you cannot remove the main worktree, or one with submodules inside [1]. For deleting $GIT_WORK_TREE, Untracked files or any staged entries are considered precious and therefore prevent removal by default. Ignored files are not precious. When it comes to deleting $GIT_DIR, there's no "clean" check because there should not be any valuable data in there, except: - HEAD reflog. There is nothing we can do about this until somebody steps up and implements the ref graveyard. - Detached HEAD. Technically it can still be recovered. Although it may be nice to warn about orphan commits like 'git checkout' does. [1] We do 'git status' with --ignore-submodules=all for safety anyway. But this needs a closer look by submodule people before we can allow deletion. For example, if a submodule is totally clean, but its repo not absorbed to the main .git dir, then deleting worktree also deletes the valuable .submodule repo too. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index e6764ee..d322acb 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -14,6 +14,7 @@ SYNOPSIS 'git worktree lock' [--reason ] 'git worktree move' 'git worktree prune' [-n] [-v] [--expire ] +'git worktree remove' [--force] 'git worktree unlock' DESCRIPTION @@ -85,6 +86,13 @@ prune:: Prune working tree information in $GIT_DIR/worktrees. +remove:: + +Remove a working tree. Only clean working trees (no untracked files +and no modification in tracked files) can be removed. Unclean working +trees or ones with submodules can be removed with `--force`. The main +working tree cannot be removed. + unlock:: Unlock a working tree, allowing it to be pruned, moved or deleted. @@ -94,9 +102,10 @@ OPTIONS -f:: --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. This option overrides - that safeguard. + 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 that safeguard. -b :: -B :: @@ -278,12 +287,6 @@ Multiple checkout in general is still experimental, and the support for submodules is incomplete. It is NOT recommended to make multiple checkouts of a superproject. -git-worktree could provide more automation for tasks currently -performed manually, such as: - -- `remove` to remove a linked working tree and its administrative files (and - warn if the working tree is dirty) - GIT --- Part of the linkgit:git[1] suite diff --git a/builtin/worktree.c b/builtin/worktree.c index 4789ceb..990e47b 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -19,6 +19,7 @@ static const char * const worktree_usage[] = { N_("git worktree lock [] "), N_("git worktree move "), N_("git worktree prune []"), + N_("git worktree remove [] "), N_("git worktree unlock "), NULL }; @@ -624,7 +625,7 @@ static void validate_no_submodules(const struct worktree *wt) discard_index(&istate); if (found_submodules) - die(_("working trees containing submodules cannot be moved")); + die(_("working trees containing submodules cannot be moved or removed")); } static int move_worktree(int ac, const char **av, const char *prefix) @@ -688,6 +689,135 @@ static int move_worktree(int ac, const char **av, const char *prefix) return 0; } +/* + * Note, "git status --porcelain" is used to determine if it's safe to + * delete a whole worktree. "git status" does not ignore user + * configuration, so if a normal "git status" shows "clean" for the + * user, then it's ok to remove it. + * + * This assumption may be a bad one. We may want to ignore + * (potentially bad) user settings and only delete a worktree when + * it's absolutely safe to do so from _our_ point of view because we + * know better. + */ +static void check_clean_worktree(struct worktree *wt, + const char *original_path) +{ + struct argv_array child_env = ARGV_ARRAY_INIT; + struct child_process cp; + char buf[1]; + int ret; + + /* + * Until we sort this out, all submodules are "dirty" and + * will abort this function. + */ + validate_no_submodules(wt); + + argv_array_pushf(&child_env, "%s=%s/.git", + GIT_DIR_ENVIRONMENT, wt->path); + argv_array_pushf(&child_env, "%s=%s", + GIT_WORK_TREE_ENVIRONMENT, wt->path); + memset(&cp, 0, sizeof(cp)); + argv_array_pushl(&cp.args, "status", + "--porcelain", "--ignore-submodules=none", + NULL); + cp.env = child_env.argv; + cp.git_cmd = 1; + cp.dir = wt->path; + cp.out = -1; + ret = start_command(&cp); + if (ret) + die_errno(_("failed to run 'git status' on '%s'"), + original_path); + ret = xread(cp.out, buf, sizeof(buf)); + if (ret) + die(_("'%s' is dirty, use --force to delete it"), + original_path); + close(cp.out); + ret = finish_command(&cp); + if (ret) + die_errno(_("failed to run 'git status' on '%s', code %d"), + original_path, ret); +} + +static int delete_git_work_tree(struct worktree *wt) +{ + struct strbuf sb = STRBUF_INIT; + int ret = 0; + + strbuf_addstr(&sb, wt->path); + if (remove_dir_recursively(&sb, 0)) { + error_errno(_("failed to delete '%s'"), sb.buf); + ret = -1; + } + strbuf_release(&sb); + 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; + struct option options[] = { + OPT_BOOL(0, "force", &force, + N_("force removing even if the worktree is dirty")), + OPT_END() + }; + struct worktree **worktrees, *wt; + struct strbuf errmsg = STRBUF_INIT; + const char *reason; + int ret = 0; + + ac = parse_options(ac, av, prefix, options, worktree_usage, 0); + if (ac != 1) + usage_with_options(worktree_usage, options); + + worktrees = get_worktrees(0); + wt = find_worktree(worktrees, prefix, av[0]); + if (!wt) + 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 (reason) { + if (*reason) + die(_("cannot remove a locked working tree, lock reason: %s"), + reason); + die(_("cannot remove a locked working tree")); + } + if (validate_worktree(wt, &errmsg)) + die(_("validation failed, cannot remove working tree: %s"), + errmsg.buf); + strbuf_release(&errmsg); + + if (!force) + check_clean_worktree(wt, av[0]); + + ret |= delete_git_work_tree(wt); + /* + * continue on even if ret is non-zero, there's no going back + * from here. + */ + ret |= delete_git_dir(wt); + + free_worktrees(worktrees); + return ret; +} + int cmd_worktree(int ac, const char **av, const char *prefix) { struct option options[] = { @@ -712,5 +842,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix) return unlock_worktree(ac - 1, av + 1, prefix); if (!strcmp(av[1], "move")) return move_worktree(ac - 1, av + 1, prefix); + if (!strcmp(av[1], "remove")) + return remove_worktree(ac - 1, av + 1, prefix); usage_with_options(worktree_usage, options); } diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index b87d387..ff4a396 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -3087,7 +3087,7 @@ _git_whatchanged () _git_worktree () { - local subcommands="add list lock move prune unlock" + local subcommands="add list lock move prune remove unlock" local subcommand="$(__git_find_on_cmdline "$subcommands")" if [ -z "$subcommand" ]; then __gitcomp "$subcommands" @@ -3105,6 +3105,9 @@ _git_worktree () prune,--*) __gitcomp "--dry-run --expire --verbose" ;; + remove,--*) + __gitcomp "--force" + ;; *) ;; esac diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh index deb486c..4718c45 100755 --- a/t/t2028-worktree-move.sh +++ b/t/t2028-worktree-move.sh @@ -96,4 +96,34 @@ test_expect_success 'move worktree to another dir' ' test_cmp expected2 actual2 ' +test_expect_success 'remove main worktree' ' + test_must_fail git worktree remove . +' + +test_expect_success 'move some-dir/destination back' ' + git worktree move some-dir/destination destination +' + +test_expect_success 'remove locked worktree' ' + git worktree lock destination && + test_when_finished "git worktree unlock destination" && + test_must_fail git worktree remove destination +' + +test_expect_success 'remove worktree with dirty tracked file' ' + echo dirty >>destination/init.t && + test_when_finished "git -C destination checkout init.t" && + test_must_fail git worktree remove destination +' + +test_expect_success 'remove worktree with untracked file' ' + : >destination/untracked && + test_must_fail git worktree remove destination +' + +test_expect_success 'force remove worktree with untracked file' ' + git worktree remove --force destination && + test_path_is_missing destination +' + test_done -- cgit v0.10.2-6-g49f6 From ee6763af0a3b97225803c6c908a29de40336cf38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 12 Feb 2018 16:49:40 +0700 Subject: worktree remove: allow it when $GIT_WORK_TREE is already gone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "git worktree remove" basically consists of two things - delete $GIT_WORK_TREE - delete $GIT_DIR (which is $SUPER_GIT_DIR/worktrees/something) If $GIT_WORK_TREE is already gone for some reason, we should be able to finish the job by deleting $GIT_DIR. Two notes: - $GIT_WORK_TREE _can_ be missing if the worktree is locked. In that case we must not delete $GIT_DIR because the real $GIT_WORK_TREE may be in a usb stick somewhere. This is already handled because we check for lock first. - validate_worktree() is still called because it may do more checks in future (and it already does something else, like checking main worktree, but that's irrelevant in this case) Noticed-by: Kaartic Sivaraam Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/builtin/worktree.c b/builtin/worktree.c index 990e47b..f77ef99 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -674,7 +674,7 @@ static int move_worktree(int ac, const char **av, const char *prefix) reason); die(_("cannot move a locked working tree")); } - if (validate_worktree(wt, &errmsg)) + if (validate_worktree(wt, &errmsg, 0)) die(_("validation failed, cannot move working tree: %s"), errmsg.buf); strbuf_release(&errmsg); @@ -799,15 +799,17 @@ static int remove_worktree(int ac, const char **av, const char *prefix) reason); die(_("cannot remove a locked working tree")); } - if (validate_worktree(wt, &errmsg)) + if (validate_worktree(wt, &errmsg, WT_VALIDATE_WORKTREE_MISSING_OK)) die(_("validation failed, cannot remove working tree: %s"), errmsg.buf); strbuf_release(&errmsg); - if (!force) - check_clean_worktree(wt, av[0]); + if (file_exists(wt->path)) { + if (!force) + check_clean_worktree(wt, av[0]); - ret |= delete_git_work_tree(wt); + ret |= delete_git_work_tree(wt); + } /* * continue on even if ret is non-zero, there's no going back * from here. diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh index 4718c45..082368d 100755 --- a/t/t2028-worktree-move.sh +++ b/t/t2028-worktree-move.sh @@ -126,4 +126,21 @@ test_expect_success 'force remove worktree with untracked file' ' test_path_is_missing destination ' +test_expect_success 'remove missing worktree' ' + git worktree add to-be-gone && + test -d .git/worktrees/to-be-gone && + mv to-be-gone gone && + git worktree remove to-be-gone && + test_path_is_missing .git/worktrees/to-be-gone +' + +test_expect_success 'NOT remove missing-but-locked worktree' ' + git worktree add gone-but-locked && + git worktree lock gone-but-locked && + test -d .git/worktrees/gone-but-locked && + mv gone-but-locked really-gone-now && + test_must_fail git worktree remove gone-but-locked && + test_path_is_dir .git/worktrees/gone-but-locked +' + test_done diff --git a/worktree.c b/worktree.c index 0373faf..28989cf 100644 --- a/worktree.c +++ b/worktree.c @@ -267,7 +267,8 @@ static void strbuf_addf_gently(struct strbuf *buf, const char *fmt, ...) va_end(params); } -int validate_worktree(const struct worktree *wt, struct strbuf *errmsg) +int validate_worktree(const struct worktree *wt, struct strbuf *errmsg, + unsigned flags) { struct strbuf wt_path = STRBUF_INIT; char *path = NULL; @@ -303,6 +304,12 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg) goto done; } + if (flags & WT_VALIDATE_WORKTREE_MISSING_OK && + !file_exists(wt->path)) { + ret = 0; + goto done; + } + if (!file_exists(wt_path.buf)) { strbuf_addf_gently(errmsg, _("'%s' does not exist"), wt_path.buf); goto done; diff --git a/worktree.h b/worktree.h index a913428..fe38ce1 100644 --- a/worktree.h +++ b/worktree.h @@ -61,12 +61,15 @@ extern int is_main_worktree(const struct worktree *wt); */ extern const char *is_worktree_locked(struct worktree *wt); +#define WT_VALIDATE_WORKTREE_MISSING_OK (1 << 0) + /* * Return zero if the worktree is in good condition. Error message is * returned if "errmsg" is not NULL. */ extern int validate_worktree(const struct worktree *wt, - struct strbuf *errmsg); + struct strbuf *errmsg, + unsigned flags); /* * Update worktrees/xxx/gitdir with the new path. -- cgit v0.10.2-6-g49f6 From 7f19def0fc254829839495f7fb44a7d99b161d3d Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Sun, 4 Mar 2018 00:26:47 -0500 Subject: t2028: fix minor error and issues in newly-added "worktree move" tests Recently-added "git worktree move" tests include a minor error and a few small issues. Specifically: * checking non-existence of wrong file ("source" instead of "destination") * unneeded redirect (">empty") * unused variable ("toplevel") * restoring a worktree location by means of a separate test somewhat distant from the test which moved it rather than using test_when_finished() to restore it in a self-contained fashion * having git command on the left-hand-side of a pipe ("git foo | grep") Signed-off-by: Eric Sunshine Acked-by: Duy Nguyen Signed-off-by: Junio C Hamano diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh index 082368d..5d5b363 100755 --- a/t/t2028-worktree-move.sh +++ b/t/t2028-worktree-move.sh @@ -7,7 +7,8 @@ test_description='test git worktree move, remove, lock and unlock' test_expect_success 'setup' ' test_commit init && git worktree add source && - git worktree list --porcelain | grep "^worktree" >actual && + git worktree list --porcelain >out && + grep "^worktree" out >actual && cat <<-EOF >expected && worktree $(pwd) worktree $(pwd)/source @@ -74,8 +75,9 @@ test_expect_success 'move worktree' ' toplevel="$(pwd)" && git worktree move source destination && test_path_is_missing source && - git worktree list --porcelain | grep "^worktree.*/destination" && - ! git worktree list --porcelain | grep "^worktree.*/source" >empty && + git worktree list --porcelain >out && + grep "^worktree.*/destination" out && + ! grep "^worktree.*/source" out && git -C destination log --format=%s >actual2 && echo init >expected2 && test_cmp expected2 actual2 @@ -86,11 +88,12 @@ test_expect_success 'move main worktree' ' ' test_expect_success 'move worktree to another dir' ' - toplevel="$(pwd)" && mkdir some-dir && git worktree move destination some-dir && - test_path_is_missing source && - git worktree list --porcelain | grep "^worktree.*/some-dir/destination" && + test_when_finished "git worktree move some-dir/destination destination" && + test_path_is_missing destination && + git worktree list --porcelain >out && + grep "^worktree.*/some-dir/destination" out && git -C some-dir/destination log --format=%s >actual2 && echo init >expected2 && test_cmp expected2 actual2 @@ -100,10 +103,6 @@ test_expect_success 'remove main worktree' ' test_must_fail git worktree remove . ' -test_expect_success 'move some-dir/destination back' ' - git worktree move some-dir/destination destination -' - test_expect_success 'remove locked worktree' ' git worktree lock destination && test_when_finished "git worktree unlock destination" && -- cgit v0.10.2-6-g49f6