On 11/5/21 01:38, Jeff King wrote:
If we get the non-default worktree here, it might be nice to tell the
user which worktree has it checked out. Otherwise it may lead to
head-scratching as they peek at their current HEAD.
I’d agree if this case were reachable, but as per below, it seems not to
be. So I didn’t bother figuring out how to shoehorn such a message into
the arguments expected by format_display().
I also notice that find_shared_symref() will catch cases where we're on
a detached HEAD, but it's because we're rebasing or bisecting on a given
branch. Arguably that's a sensible thing to do for this check, but it is
a change from the current behavior (even if you are not using worktrees
at all).
That’s a good observation. I agree it seems sensible, but should be
noted in the commit message.
So if the earlier hunk covered this case, then what is this hunk for? :)
Yeah, that duplication confused me too. The relevant call paths seem to be:
1. do_fetch calls check_not_current_branch (later hunk);
2. do_fetch calls fetch_and_consume_refs → store_updated_refs →
update_local_ref (earlier hunk);
3. do_fetch calls backfill_tags → fetch_and_consume_refs →
store_updated_refs → update_local_ref (earlier hunk);
in that order. That suggests there should be no way to trip the earlier
hunk’s check without first tripping the latter hunk’s check.
$ git fetch . main:main
fatal: Refusing to fetch into branch 'refs/heads/main' checked out at '/home/peff/tmp'
It's technically correct, but perhaps it would be nicer to keep the old
message if the worktree we found is the current one.
I note that this new message is more consistent with
$ git branch -d main
error: Cannot delete branch 'main' checked out at '/home/anders/tmp'
While looking for other related messages, I noticed another bug:
receive-pack doesn’t stop you from updating or deleting a branch checked
out in a worktree for a bare repository.
$ git init
$ git commit --allow-empty -qm foo
$ git clone --bare . bare.git
$ git -C bare.git worktree add wt
$ git -C bare.git/wt push .. :wt
To ..
- [deleted] wt
This is_bare_repository() check in update() in builtin/receive-pack.c
seems wrong:
const struct worktree *worktree = is_bare_repository() ? NULL :
find_shared_symref("HEAD", name);
Anders
