Re: [PATCH 2/2] Make "git branch -d" prune missing worktrees automatically.
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Oct 17, 2019, 17:28 UTC
- Message-ID
- <CAPig+cS6SzLdgmzffNkg72YSiDQ9eQRqTK12NsraKpGbkJFY_w@mail.gmail.com>
- In-Reply-To
- <20191017162826.1064257-2-pjones@redhat.com>
On Thu, Oct 17, 2019 at 12:28 PM Peter Jones <pjones@redhat.com> wrote:
Show 14 quoted lines
> 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...
Show 17 quoted lines
> Signed-off-by: Peter Jones <pjones@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.
Show 7 quoted lines
> 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.