> You actually didn't spell out the problem with "git branch -D", or at
> least the consequence (i.e. the submodule branch is deleted even if
> it's checked out).
Thanks - I'll do that in the commit message.
> > strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
> > - is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
> > - if (is_bare)
> > + if (!strbuf_strip_suffix(&worktree_path, "/.git"))
> > strbuf_strip_suffix(&worktree_path, "/.");
>
> We can just call these two calls unconditionally, right? No harm done
> if we don't strip.
We can, and no harm done. But this if/then pattern is also repeated in
other parts of the file (e.g. get_linked_worktree()) so I'll leave it in
for consistency. (Also, for what it's worth, it's slightly faster if
only one strip is done.)
> > strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
> >
> > worktree = xcalloc(1, sizeof(*worktree));
> > worktree->path = strbuf_detach(&worktree_path, NULL);
> > - worktree->is_bare = is_bare;
> > + worktree->is_bare = (is_bare_repository_cfg == 1) ||
>
> core.bare and core.worktree are special. When you access them standing
> from the main worktree, you'll see them. But when you stand from a
> secondary worktree, they are ignored.
Just checking: I think that is_bare_repository_cfg ignores core.bare
only if the config.worktree file is present? In the t2402 test '"list"
all worktrees from linked with a bare main', "git worktree list" still
observes the main worktree as bare. But in any case, you are right that
core.bare is sometimes ignored.
> It's more obvious with
> core.worktree because if that affects all worktrees, what's the point
> of having multiple worktrees. Git will always go to the place
> core.worktree points out.
That's true.
> So if this function is called from a secondary worktree, I'm not sure
> if it still works as expected because is_bare_repo may be false then.
I think you're right that is_bare_repository() will always return false
here. So let's look at the cases where, running from a secondary
worktree, we think that the main worktree should be observed as bare:
- main worktree did not define core.bare
- I don't know if this is possible (remember that we're running from
a secondary worktree). But if it is, it seems that
is_bare_repository_cfg will be -1, and worktree->is_bare will be
set to 0 regardless of whether or not it is bare.
- main worktree defines core.bare as 1; no config.worktree
- is_bare_repository_cfg is 1, so we see the main worktree as bare.
(This case is tested in t2402 '"list" all worktrees from linked
with a bare main'.)
- main worktree defines core.bare as 1, and secondary worktree defines
core.bare as 0
- I think that we'll see is_bare_repository_cfg as 0, so we won't see
the main worktree as bare.
The only potentially problematic case seems to be the 3rd one.
> For the submodule case, you always stand at the submodule's main
> worktree, so it still works.
Yes.
> I don't think multiple-worktrees-on-submodules will be coming soon, so
> it's probably ok. But maybe leave a note here.
Observing that the problematic case is the 3rd one above, would this
note work:
NEEDSWORK: If this function is called from a secondary worktree and
config.worktree is present, is_bare_repository_cfg will reflect the
contents of config.worktree, not the contents of the main worktree.
This means that worktree->is_bare may be set to 0 even if the main
worktree is configured to be bare.
