Re: init --separate-git-dir does not set core.worktree

[Duy Nguyen <pclouds@gmail.com>]

On Thu, Feb 2, 2017 at 7:37 PM, Kyle Meyer <kyle@kyleam.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Thu, Feb 2, 2017 at 10:55 AM, Kyle Meyer <kyle@kyleam.com> wrote:
>>>
>>> As of 6311cfaf9 (init: do not set unnecessary core.worktree,
>>> 2016-09-25), "git init --separate-git-dir" no longer sets core.worktree
>>> (test below).  Based on the commit message and the corresponding thread
>>> [1], I don't think this change in behavior was intentional, but I wasn't
>>> able to understand things well enough to attempt a patch.
>>
>> I'm missing some context. Why does --separate-git-dir have to set
>> core.worktree? What fails for you exactly?
>
> Sorry for not providing enough information.  I haven't run into a
> failure.
>
> In Magit, we need to determine the top-level of the working tree from
> COMMIT_EDITMSG.  Right now that logic [*1*] looks something like this:

This is much better :)

>  * COMMIT_EDITMSG in .git/modules/<module>/: set working tree to the
>    output of "git rev-parse --show-toplevel"
>
>  * COMMIT_EDITMSG in .git/worktrees/<wtree>/: set working tree to the
>    path in .git/worktrees/<wtree>/gitdir, minus the trailing "/.git"
>
>  * COMMIT_EDITMSG in .git: set working tree to the parent directory
>
> This fails for a repo set up with --separate-git-dir [*2*], where the
> last step will go out into an unrelated repo.  If core.worktree was set
> and "git rev-parse --show-toplevel" returned the working tree like it
> did for submodules, things would work.

OK. If I read this right, given a path of any text file somewhere
within ".git" directory. you are tasked to find out where the
associated worktree is? I.e. this is not an emacsclient executed as
part of "git commit", correct?

So you need some sort of back-link to ".git" location. And
unfortunately there's no such thing for .git file (unless it points to
.git/worktrees/...). I'm hesitant to set core.worktree unless it's
absolutely needed since it may have unexpected interaction with
$GIT_WORK_TREE and others (not sure if it has any interaction with
submodules too). I think adding a new file "gitdir" would be a safer
option.

I'm not entirely sure if enforcing "one worktree - one repository" is
safe though. The first two bullet points are very specific and we can
assume that, but ".git" files alone can be used for anything. In
theory you can always create a secondary worktree (that's not managed
by "git worktree") by setting GIT_WORK_TREE and "git checkout -f"
somewhere. But I guess those would be temporary and nobody would want
magic to point back to them.

As a fall-back mechanism, I think after magit has found the worktree,
it should verify the found location is the correct worktree, with "git
rev-parse --git-dir" or something, and alert the user otherwise. I
think "git rev-parse --git-path COMMIT_MSG" should give back the same
COMMIT_MSG path (and it applies for any files in .git dir, covering
all three cases). The user could add some magit-specific files to tell
magit where the actual worktree is when they hit corner cases.

If the use case is limited to editing COMMIT_EDITMSG only (after it's
generated by git), it may be best to add `pwd` as a comment to that
file. You won't have to go through all the magic rules to find it out
(*). And it helps non-magic users too.

(*) well, you do, because you probably can't expect everybody to have
latest git version.

> Of course, the issue above isn't a reason that --separate-git-dir should
> set core.worktree, but the submodule behavior is why we were wondering
> if it should.

I'm not a submodule person, so I'll pass that "why" question to Stefan.

> And so I was going to ask here whether core.worktree
> should be set, but then I confused myself into thinking 6311cfaf9
> unintentionally changed this behavior.
>
>
> [*1*] This is done by magit-toplevel:
>       https://github.com/magit/magit/blob/e34f4e8eb00f292e8c475489fa7caa286857a421/lisp/magit-git.el#L400
>
> [*2*] https://github.com/magit/magit/issues/2955
>       https://github.com/magit/magit/issues/2981
-- 
Duy