Re: [PATCH v2 3/5] worktree: add upgrade_to_worktree_config()

From
Eric Sunshine <hidden>
Date
2021-12-28 16:58:37
Reply to
Derrick Stolee, 2021-12-28
On Tue, Dec 28, 2021 at 10:03 AM Derrick Stolee [off-list ref] wrote:
On 12/21/2021 7:45 PM, Eric Sunshine wrote:
quoted
It would be a good idea to drop the final sentence since there is no
such thing as a bare worktree (either conceptually or practically),
and end the first sentence at "case": i.e. "... stops that special
case."
Bare worktrees don't exist, that is correct. But if one existed it
would be a directory where you could operate as if it is a bare repo,
but it has its own HEAD different from the base repo's HEAD. Not sure
why one would want it.
I'm not following. I also still don't know what "base repo" is or
where two HEADs would arise.
quoted
quoted
+       char *base_config_file = xstrfmt("%s/config", r->commondir);
+       char *base_worktree_file = xstrfmt("%s/config.worktree", r->commondir);
Per path.c:strbuf_worktree_gitdir(), this use of `r->commondir` is
correct. Good.

Can we use more meaningful variable names? It's not at all clear what
"base" means in this context (I don't think it has any analog in Git
terminology). Perhaps name these `shared_config` and `repo_config`,
respectively.
'repo_config' is too generic, because I want the worktree config for
the "original" repo. I chose to call that the "base" repo and its
worktree config. Shared_config is a good name, though.
There seems to be some terminology confusion or conflict at play here.
We're dealing with only a single repository and zero or more
worktrees, so I'm still having trouble understanding your references
to "original repo" and "base repo", which seem to indicate multiple
repositories.
quoted
quoted
+       /*
+        * If the base repository is bare, then we need to move core.bare=true
+        * out of the base config file and into the base repository's
+        * config.worktree file.
+        */
Here, too, it's not clear what "base" means. I think you want to say
that it needs to "move `core.bare` from the shared config to the
repo-specific config".
Yes, but specific to the original/root/bare repo. I'm open to preferences
here, but "repo" isn't specific enough.
There's only a single repository, so this should be clear, however,
there appears to be some terminology mismatch. I'll enumerate a few
items in an effort to clarify how I'm using the terminology...

    .git/ -- the repository residing within the main worktree

    bare.git/ -- a bare repository

    .git/config -- configuration shared by the repository and all worktrees

    bare.git/config -- configuration shared by the repository and all worktrees

    .git/config.worktree -- configuration specific to the main worktree

    bare.git/config.worktree -- configuration specific to the bare repository

    .git/worktrees/<id>/config -- configuration specific to worktree <id>

    bare.git/worktrees/<id>/config -- configuration specific to worktree <id>
quoted
However, there is a much more _severe_ problem with this
implementation: it is incomplete. As documented in git-worktree.txt
(and mentioned several times during this discussion), this utility
function _must_ relocate both `core.bare` _and_ `core.worktree` (if
they exist) from .git/config to .git/config.worktree. This
implementation neglects to relocate `core.worktree`, which can leave
things in quite a broken state (just as neglecting to relocate
`core.bare` can).
It took me a long time to actually understand the purpose of
core.worktree, since it seems in conflict with the 'git worktree'
feature. Indeed, it is special-cased the same way core.bare is, so
this relocation is required.
Indeed, the situation is unfortunately confusing in this area.
`core.worktree` predates multiple-worktree support (i.e.
`git-worktree`) by quite a long time and is a mechanism for allowing
the repository (.git/) to exist at a distinct location from the
worktree (by which I mean "main worktree" since there was no such
thing as a "linked worktree" at a time). `git-worktree` generalized
the concept by making multiple worktrees first-class citizens, but
`core.worktree` and GIT_WORKTREE still need to be supported for
backward compatibility even though they conflict (or can conflict)
rather badly with multiple-worktrees.
Thread (121 messages) 121 messages, 8 authors, last reply 2022-02-16
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help