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

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

[Kyle Meyer]

Hello,

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.

Thanks.

[1] https://public-inbox.org/git/CALqjkKZO_y0DNcRJjooyZ7Eso7yBMGhvZ6fE92oO4Su7JeCeng@mail.gmail.com/T/#u

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index b8fc588b1..444e75865 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -309,6 +309,7 @@ test_expect_success 'init with separate gitdir' '
 	git init --separate-git-dir realgitdir newdir &&
 	echo "gitdir: $(pwd)/realgitdir" >expected &&
 	test_cmp expected newdir/.git &&
+	check_config realgitdir false "$(pwd)/newdir" &&
 	test_path_is_dir realgitdir/refs
 '
 

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

[Duy Nguyen]

On Thu, Feb 2, 2017 at 10:55 AM, Kyle Meyer <kyle@kyleam.com> wrote:
> Hello,
>
> 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?

>
> Thanks.
>
> [1] https://public-inbox.org/git/CALqjkKZO_y0DNcRJjooyZ7Eso7yBMGhvZ6fE92oO4Su7JeCeng@mail.gmail.com/T/#u
>
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index b8fc588b1..444e75865 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -309,6 +309,7 @@ test_expect_success 'init with separate gitdir' '
>         git init --separate-git-dir realgitdir newdir &&
>         echo "gitdir: $(pwd)/realgitdir" >expected &&
>         test_cmp expected newdir/.git &&
> +       check_config realgitdir false "$(pwd)/newdir" &&
>         test_path_is_dir realgitdir/refs
>  '
>



-- 
Duy

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

[Kyle Meyer]

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:

 * 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.

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.  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

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

[Duy Nguyen]

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

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

[Kyle Meyer]

Duy Nguyen <pclouds@gmail.com> writes:

> On Thu, Feb 2, 2017 at 7:37 PM, Kyle Meyer <kyle@kyleam.com> wrote:

[...]

>>  * 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?

Right

> I.e. this is not an emacsclient executed as part of "git commit",
> correct?

... but it is from a "git commit" call.  I think you're asking because,
if we know where "git commit" was called from, we know what the working
tree is.  This is true, but with the current Magit design, the function
for determining the top-level of the working tree, magit-toplevel,
doesn't have access to this information.  From Emacs, Magit calls "git
commit", setting GIT_EDITOR for that process so that git invokes the
current Emacs instance for editing the commit message.  From
COMMIT_EDITMSG, we want the magit-toplevel to be able to determine the
working tree location so that commands can use magit-toplevel to set
their pwd.

The only Magit command that currently relies on magit-toplevel to
determine the working tree from ".git/COMMIT_EDITMSG" is a command that
shows a diff with the staged changes in a separate buffer.  (Even though
"git diff --cached" would work from within ".git/", some functionality
in this buffer depends on its pwd being set top-level of the working
tree.)

So, whatever solution we come up with on the Magit end will likely
involve recording where "git commit" was called so that the diff command
can figure out the working tree.

> 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.

Right, makes sense.

Unfortunately, magit-toplevel was designed thinking that the
--separate-git-dir / core.worktree behavior in Git 2.8.4 and lower was
intentional [*].  We'll need to rework this on our end.

Thanks for your input and for confirming that "git init
--separate-git-dir" does not set core.worktree by design.

[*] https://github.com/magit/magit/issues/460#issuecomment-36035787.

-- 
Kyle

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

[Stefan Beller]

On Fri, Feb 3, 2017 at 5:38 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> 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"

.git/modules/<module>/ itself is a fully fledged git directory, making use of
the core.worktree variable to have the working tree at an "unusual" place.

"git rev-parse --show-toplevel" is the correct place of the working tree

>>
>>  * COMMIT_EDITMSG in .git/worktrees/<wtree>/: set working tree to the
>>    path in .git/worktrees/<wtree>/gitdir, minus the trailing "/.git"

(in worktree/<wtree>:) "git rev-parse --show-toplevel"s output is empty,
but the gitdir file exists unlike in the other cases.

>>
>>  * COMMIT_EDITMSG in .git: set working tree to the parent directory

(in .git:) "git rev-parse --show-toplevel"s output is empty.

>>
>> 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.

When using --separate-git-dir you cannot tell where the working tree is from
within the git dir, despite knowing it has to be *somewhere*:

    [core]
        bare = false

So I would hope we'll set core.worktree again in this case. Otherwise how
would we discover the working tree?

>
> 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?

I always assumed core.worktree is precisely that analogy, i.e.
core.worktree is the backwards pointer to the .git file (which
is true coming from a submodule background).

>
> 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.

How would "gitdir" (should it be called worktree/workingtree instead?)
work together with core.worktree set?
Would it point at the .git file or the root level of the working tree?

>
> 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.

Good question. As said above I always assumed it is the backlink to know where
the working tree of the submodule is.

Digging through history I found d75219b4a, which explains why that is:

    Since recently a submodule with name <name> has its git directory in the
    .git/modules/<name> directory of the superproject while the work tree
    contains a gitfile pointing there. To make that work the git directory has
    the core.worktree configuration set in its config file to point back to
    the work tree.

I remember reading this some time ago and wondered what the
"to make that work" implies though and IIRC there was nothing I found by
experimentation.

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

[Stefan Beller]

> [*] https://github.com/magit/magit/issues/460#issuecomment-36035787.

I would agree with the thinking in that issue.