Re: [PATCH v2 3/4] Make die_if_checked_out() prune missing checkouts of unlocked worktrees.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 21, 2019, 02:09 UTC
- Message-ID
- <xmqqimoin92j.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20191018194542.1316981-3-pjones@redhat.com>
Peter Jones <pjones@redhat.com> writes:
[jc: won't repeat comments on the title]
> @@ -360,6 +360,12 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)
> wt = find_shared_symref("HEAD", branch);
> if (!wt || (ignore_current_worktree && wt->is_current))
> return;die-if-checked-out is called from callers that expect to be stopped before they do any harm, so it feels dirty to make a side effect like this.
If the user tries to check out a branch that used to be checked out in an already removed worktree, doesn't that indicate that an earlier worktree removal was done incorrectly, which is something worth reporting to the user and give the user a chance to think and choose what corrective action(s) need to be taken?
For that, instead of automatically losing information like this patch does, it may make more sense to fail the checkout and stop at giving diagnosis (e.g. "our record shows that the branch is checked out in that worktree, but you seem to have lost it. if you forgot to prune it, then here is the command you can give to do so.") without actually touching the filesystem.
Thanks.
Show 9 quoted lines
> +
> + if (prune_worktree_if_missing(wt) >= 0) {
> + delete_worktrees_dir_if_empty();
> + return;
> + }
> +
> skip_prefix(branch, "refs/heads/", &branch);
> die(_("'%s' is already checked out at '%s'"),
> branch, wt->path);