| Message ID | 20191018194542.1316981-3-pjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/4] libgit: Add a read-only helper to test the worktree lock | expand |
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. > + > + 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);
diff --git a/branch.c b/branch.c index 579494738a7..760ef387144 100644 --- a/branch.c +++ b/branch.c @@ -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; + + 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);
Currently if you do, for example: $ git worktree add path foo And "foo" has already been checked out at some other path, but the user has removed it without pruning, and the worktree is not locked, you'll get an error that the branch is already checked out. It isn't meaningfully checked out, the repo's data is just stale and no longer reflects reality. This makes it so that if nothing is present where a worktree is supposedly checked out, and it is not locked, we ignore that the worktree exists, then it should be cleaned up. Signed-off-by: Peter Jones <pjones@redhat.com> --- branch.c | 6 ++++++ 1 file changed, 6 insertions(+)