On Thu, Oct 17, 2019 at 12:28 PM Peter Jones <pjo...@redhat.com> wrote:
> Currently, if you do:
>
> $ git branch zonk origin/master
> $ git worktree add zonk zonk
> $ rm -rf zonk
> $ git branch -d zonk
>
> You get the following error:
>
> $ git branch -d zonk
> error: Cannot delete branch 'zonk' checked out at
> '/home/pjones/devel/kernel.org/git/zonk'
>
> It isn't meaningfully checked out, the repo's data is just stale and no
> longer reflects reality.
Echoing SEZDER's comment on patch 1/2, this behavior is an intentional
design choice and safety feature of the worktree implementation since
worktrees may exist on removable media or remote filesystems which
might not always be mounted; hence, the presence of commands "git
worktree prune" and "git worktree remove".
A couple comment regarding this patch...
> Signed-off-by: Peter Jones <pjo...@redhat.com>
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -133,6 +133,20 @@ static int prune_worktree(const char *id, struct strbuf
> *reason)
> +int prune_worktree_if_missing(const struct worktree *wt)
> +{
> + struct strbuf reason = STRBUF_INIT;
> +
> + if (access(wt->path, F_OK) >= 0 ||
> + (errno != ENOENT && errno == ENOTDIR)) {
> + errno = EEXIST;
> + return -1;
> + }
> +
> + strbuf_addf(&reason, _("Removing worktrees/%s: worktree directory is
> not present"), wt->id);
> + return prune_worktree(wt->id, &reason);
> +}
"git worktree" tries to clean up after itself as much as possible. For
instance, it is careful to remove the .git/worktrees directory when
the last worktree itself is removed (or pruned). So, the caller of
this function would also want to call delete_worktrees_dir_if_empty()
to follow suit.
> diff --git a/worktree.h b/worktree.h
> @@ -132,4 +132,10 @@ void strbuf_worktree_ref(const struct worktree *wt,
> +/*
> + * Prune a worktree if it is no longer present at the checked out location.
> + * Returns < 0 if the checkout is there or if pruning fails.
> + */
> +int prune_worktree_if_missing(const struct worktree *wt);
It's rather ugly that this function is declared in top-level
worktree.h whereas the actual implementation is in builtin/worktree.c.
I'd expect to see a preparatory patch which moves prune_worktree()
(and probably delete_worktrees_dir_if_empty()) to top-level
worktree.c.
These minor implementation comments aside, before considering this
patch series, it would be nice to see a compelling argument as to why
this change of behavior, which undercuts a deliberate design decision,
is really desirable.
