On Tue, Dec 28, 2021 at 10:03 AM Derrick Stolee <stolee@xxxxxxxxx> wrote: > On 12/21/2021 7:45 PM, Eric Sunshine wrote: > > 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. > >> + 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. > >> + /* > >> + * 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> > > 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.