On Tue, Aug 28, 2018 at 05:20:23PM -0400, Eric Sunshine wrote:
> For safety, "git worktree add <path>" will refuse to add a new
> worktree at <path> if <path> is already associated with a worktree
> entry, even if <path> 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 <path> 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 <path>").
>
> 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.
This makes sense to me, and...
> Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
> ---
> Documentation/git-worktree.txt | 8 ++++++--
> builtin/worktree.c | 10 ++++++++--
> t/t2025-worktree-add.sh | 13 ++++++++++++-
> 3 files changed, 26 insertions(+), 5 deletions(-)
The patch looks quite good. One minor comment:
> 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;
> + }
This "unable to re-add" seemed funny to me at first, since the failure
is in deletion. I guess we're relying on delete_git_dir() to already
have said "I had trouble deleting $GIT_DIR/worktrees/foo", and this is
just the follow-up to tell that the whole operation is cancelled. So
that makes sense.
I wonder if we should volunteer the information that we're overwriting
an existing worktree. I guess the user would generally know that
already, though, since they just specified "-f", so it's probably just
being overly chatty to do so.
-Peff
