From: Jonathan Tan <jonathantanmy@google.com>
To: pclouds@gmail.com
Cc: jonathantanmy@google.com, git@vger.kernel.org,
Johannes.Schindelin@gmx.de, rappazzo@gmail.com
Subject: Re: [PATCH] worktree: update is_bare heuristics
Date: Thu, 18 Apr 2019 11:30:00 -0700 [thread overview]
Message-ID: <20190418183000.78138-1-jonathantanmy@google.com> (raw)
In-Reply-To: <CACsJy8BygpTQg5=8+2KkFCpaJBEkKx+ocVZoa0yRBAQvnkXVSw@mail.gmail.com>
> 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.
next prev parent reply other threads:[~2019-04-18 18:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-17 21:21 [PATCH] worktree: update is_bare heuristics Jonathan Tan
2019-04-18 2:16 ` Junio C Hamano
2019-04-18 9:59 ` Johannes Schindelin
2019-04-18 18:59 ` Jonathan Tan
2019-04-18 10:11 ` Duy Nguyen
2019-04-18 18:30 ` Jonathan Tan [this message]
2019-04-18 18:42 ` Jeff King
2019-04-19 1:33 ` Duy Nguyen
2019-04-19 10:50 ` Duy Nguyen
2019-04-19 17:21 ` [PATCH v2] " Jonathan Tan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190418183000.78138-1-jonathantanmy@google.com \
--to=jonathantanmy@google.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
--cc=rappazzo@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).