[PATCH] pathspec: remove check_path_for_gitlink

[PATCH] pathspec: remove check_path_for_gitlink

[Stefan Beller]

`check_path_for_gitlink` was introduced in 9d67b61f739a (2013-01-06,
add.c: extract check_path_for_gitlink() from treat_gitlinks() for reuse)
but the implementation was removed in 5a76aff1a6 (2013-07-14, add:
convert to use parse_pathspec).

Remove the declaration from the header as well.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 pathspec.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pathspec.h b/pathspec.h
index 0c11262..b596aed 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -96,7 +96,6 @@ static inline int ps_strcmp(const struct pathspec_item *item,
 
 extern char *find_pathspecs_matching_against_index(const struct pathspec *pathspec);
 extern void add_pathspec_matches_against_index(const struct pathspec *pathspec, char *seen);
-extern const char *check_path_for_gitlink(const char *path);
 extern void die_if_path_beyond_symlink(const char *path, const char *prefix);
 
 #endif /* PATHSPEC_H */
-- 
2.8.0.1.g3af9c03

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> `check_path_for_gitlink` was introduced in 9d67b61f739a (2013-01-06,
> add.c: extract check_path_for_gitlink() from treat_gitlinks() for reuse)
> but the implementation was removed in 5a76aff1a6 (2013-07-14, add:
> convert to use parse_pathspec).
>
> Remove the declaration from the header as well.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  pathspec.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/pathspec.h b/pathspec.h
> index 0c11262..b596aed 100644
> --- a/pathspec.h
> +++ b/pathspec.h
> @@ -96,7 +96,6 @@ static inline int ps_strcmp(const struct pathspec_item *item,
>  
>  extern char *find_pathspecs_matching_against_index(const struct pathspec *pathspec);
>  extern void add_pathspec_matches_against_index(const struct pathspec *pathspec, char *seen);
> -extern const char *check_path_for_gitlink(const char *path);
>  extern void die_if_path_beyond_symlink(const char *path, const char *prefix);
>  
>  #endif /* PATHSPEC_H */

Interesting.

I wonder if the patches mentioned have something to do with the "git
add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
repository in some way?

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Stefan Beller]

On Thu, May 5, 2016 at 4:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> `check_path_for_gitlink` was introduced in 9d67b61f739a (2013-01-06,
>> add.c: extract check_path_for_gitlink() from treat_gitlinks() for reuse)
>> but the implementation was removed in 5a76aff1a6 (2013-07-14, add:
>> convert to use parse_pathspec).
>>
>> Remove the declaration from the header as well.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>  pathspec.h | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/pathspec.h b/pathspec.h
>> index 0c11262..b596aed 100644
>> --- a/pathspec.h
>> +++ b/pathspec.h
>> @@ -96,7 +96,6 @@ static inline int ps_strcmp(const struct pathspec_item *item,
>>
>>  extern char *find_pathspecs_matching_against_index(const struct pathspec *pathspec);
>>  extern void add_pathspec_matches_against_index(const struct pathspec *pathspec, char *seen);
>> -extern const char *check_path_for_gitlink(const char *path);
>>  extern void die_if_path_beyond_symlink(const char *path, const char *prefix);
>>
>>  #endif /* PATHSPEC_H */
>
> Interesting.
>
> I wonder if the patches mentioned have something to do with the "git
> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
> repository in some way?

Which is considered a feature now. Maybe we should add tests for that?

http://debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

>> I wonder if the patches mentioned have something to do with the "git
>> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
>> repository in some way?
>
> Which is considered a feature now. Maybe we should add tests for that?
>
> http://debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb

That is a bug, plain and simple.  Duy any ideas where we went wrong?

I think we already have code to avoid adding beyond symlinks.
"git add deep/in/the/tree" should refuse if deep/in is a symbolic
link (and happens to point at a directory that has the/tree in it).
We used not to catch that long time ago, but I think we fixed it.

The logic and the places to do the checks for "no, that thing may be
a directory but is an unrelated repository" should be the same.

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Stefan Beller]

On Thu, May 5, 2016 at 4:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>> I wonder if the patches mentioned have something to do with the "git
>>> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
>>> repository in some way?
>>
>> Which is considered a feature now. Maybe we should add tests for that?
>>
>> http://debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb
>
> That is a bug, plain and simple.  Duy any ideas where we went wrong?

That was my first reaction as well. However after a while of thought I actually
like that bug. Consider the possibilities how gitk/git-gui or other subsystems
can be developed. When accepting a patch for that you can either apply the
patch in the outer or inner repository, depending on what the sender used.

I am not so sure if it is a bug plain and simple, but devolved into a
"feature" now.

>
> I think we already have code to avoid adding beyond symlinks.
> "git add deep/in/the/tree" should refuse if deep/in is a symbolic
> link (and happens to point at a directory that has the/tree in it).
> We used not to catch that long time ago, but I think we fixed it.
>
> The logic and the places to do the checks for "no, that thing may be
> a directory but is an unrelated repository" should be the same.

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> That was my first reaction as well. However after a while of thought I actually
> like that bug. Consider the possibilities how gitk/git-gui or other subsystems
> can be developed. When accepting a patch for that you can either apply the
> patch in the outer or inner repository, depending on what the sender used.
>
> I am not so sure if it is a bug plain and simple, but devolved into a
> "feature" now.

I'd freely admit that I have not considered its possible upsides at
all.  When deep/in/ is an unrelated repository, and running either

    git add deep/in/the
    git add deep/in/the/tree

would add deep/in/the/tree/is-a-leaf.txt to my index, but if I did

    git add deep/in

I'd lose that and suddenly everything there turns into a submodule.

And that is enough for me to declare that it is not worth my time to
consider possible upside of that hole.  Can you tell offhand what
would happen if you do "git add deep" (before adding deep/in as a
submodule) without experimenting?

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Stefan Beller]

On Thu, May 5, 2016 at 4:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> That was my first reaction as well. However after a while of thought I actually
>> like that bug. Consider the possibilities how gitk/git-gui or other subsystems
>> can be developed. When accepting a patch for that you can either apply the
>> patch in the outer or inner repository, depending on what the sender used.
>>
>> I am not so sure if it is a bug plain and simple, but devolved into a
>> "feature" now.
>
> I'd freely admit that I have not considered its possible upsides at
> all.  When deep/in/ is an unrelated repository, and running either
>
>     git add deep/in/the
>     git add deep/in/the/tree

I think that doesn't work (I did not test), but the crucial part is to add
a trailing '/'. E.g.  `git add deep/in/the/` adds the 'the/**' tree of
the nested
repository.

>
> would add deep/in/the/tree/is-a-leaf.txt to my index, but if I did
>
>     git add deep/in
>
> I'd lose that and suddenly everything there turns into a submodule.

Yes.

    git add deep/in # adds a submodule

however:

    git add deep/in/ # adds all files of the sub-"repo"  as indpendent files
    git commit -a -m "new files"
    git -C deep/in reset --hard HEAD^
    git diff
    # shows a difference in deep/in/the/tree/is-a-leaf.txt

>
> And that is enough for me to declare that it is not worth my time to
> consider possible upside of that hole.  Can you tell offhand what
> would happen if you do "git add deep" (before adding deep/in as a
> submodule) without experimenting?
>

Not really. My expectation is to add everything *but* the deep/in/ repo
as this is not exercising the bug/feature.

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> When deep/in/ is an unrelated repository, and running either
>
>     git add deep/in/the
>     git add deep/in/the/tree
>
> would add deep/in/the/tree/is-a-leaf.txt to my index, but if I did
>
>     git add deep/in
>
> I'd lose that and suddenly everything there turns into a submodule.

Also, I recall that you floated an idea to declare that

    git add deep/in/the/tree/is-a-leaf.txt

must always behave as if this is done instead:

    git -C deep/in/the/tree/ add is-a-leaf.txt

Even though I am not a huge fan of an operation that crosses module
boundaries, I think that is a sensible semantics of a "cross module
boundary operation" (the actual implementation should not be
iterating over pathspecs and chdir(2)ing around for each and every
one of them, though), if we need "cross module boundary operation"
in order to support end users working on a project with one or more
submodules at the same time.

But treating the bug under discussion as a "feature" will destroy
that future.

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Stefan Beller]

On Thu, May 5, 2016 at 11:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> When deep/in/ is an unrelated repository, and running either
>>
>>     git add deep/in/the
>>     git add deep/in/the/tree
>>
>> would add deep/in/the/tree/is-a-leaf.txt to my index, but if I did
>>
>>     git add deep/in
>>
>> I'd lose that and suddenly everything there turns into a submodule.
>
> Also, I recall that you floated an idea to declare that
>
>     git add deep/in/the/tree/is-a-leaf.txt
>
> must always behave as if this is done instead:
>
>     git -C deep/in/the/tree/ add is-a-leaf.txt

In a different thread, yeah. I am not sure about that any more.

>
> Even though I am not a huge fan of an operation that crosses module
> boundaries, I think that is a sensible semantics of a "cross module
> boundary operation" (the actual implementation should not be
> iterating over pathspecs and chdir(2)ing around for each and every
> one of them, though), if we need "cross module boundary operation"
> in order to support end users working on a project with one or more
> submodules at the same time.

I agree.

>
> But treating the bug under discussion as a "feature" will destroy
> that future.

The bug under discussion was blogged about in 2010 and still persists.
I'll try to find out if people actually use it.

If that bug was fixed, but I still wanted to enjoy the upsides of it,
I could to that with pointing core.worktree into deep/in/tree.
e.g. I have git.git and gitk-git as separate repositories,
then I could still do a

    GIT_DIR=gitk-git/.git git -C git.git/gitk-git git checkout
<sha1-of-gitk-repo>

This looks more complicated than this bug/feature though.

There are 2 fundamental cases though.
 (1) The bug we're talking about (as explained in that blog), refers to 2
    independent repositories, whose work trees are nested
 (2) You seemed to bring in the notion that the nested repo is considered
    a submodule of the outer repo, i.e. they have a relationship.

I don't mind (1). It's a neat hack as these 2 repos are totally unrelated
(except for the working tree in the file system being the same files).
You could also achieve a similar handling by hardlinking gitk-git/gitk
and git.git/gitk-git/gitk.

In (2), we have a gitlink, which by definition takes up the whole directory.
So any file in that directory in the file system which represents the root of
the submodule should belong to the submodule.

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> There are 2 fundamental cases though.
>  (1) The bug we're talking about (as explained in that blog), refers to 2
>     independent repositories, whose work trees are nested
>  (2) You seemed to bring in the notion that the nested repo is considered
>     a submodule of the outer repo, i.e. they have a relationship.
>
> I don't mind (1). It's a neat hack as these 2 repos are totally unrelated
> (except for the working tree in the file system being the same files).
> You could also achieve a similar handling by hardlinking gitk-git/gitk
> and git.git/gitk-git/gitk.
>
> In (2), we have a gitlink, which by definition takes up the whole directory.
> So any file in that directory in the file system which represents the root of
> the submodule should belong to the submodule.

I certainly didn't mean to "bring in the notion" as if it is
something entirely alien to the discussion.  Before you "git add",
it may be a "nested independent" repository, but that is merely a
submodule that is untracked, yet to be added.  Just like tracked
files were once untracked before they got added, these are possible
submodules that happened to be not tracked yet.

I do not see there is any difference between the two at all.

If deep/in is a repository yet to be added as a submodule,

    $ git add deep/in/the/tree/is-a-leaf.txt
    $ git add deep/in

in the hypothetical "git add A is equivalent to git -C $(dirname A)
add $(basename A)" world should behave the same regardless of the
order of these two commands (otherwise the behaviour is way too
confusing).

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Duy Nguyen]

On Fri, May 6, 2016 at 6:27 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>> I wonder if the patches mentioned have something to do with the "git
>>> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
>>> repository in some way?

The same functionality is added in 8745024 (parse_pathspec: support
stripping/checking submodule paths - 2013-07-14) so if it didn't fail
to notice that before 5a76aff1a6 and did after, it's a bug.

>>
>> Which is considered a feature now. Maybe we should add tests for that?
>>
>> http://debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb
>
> That is a bug, plain and simple.  Duy any ideas where we went wrong?

I vaguely recall this symptom. It has something to do with the index,
the check we do requires a gitlink in the index, I think. So if the
gitlink entry is not in the index, our protection line fails. I think
doing all this at pathspec level is wrong. We should wait at least
after read_directory() is done, by then we have a lot more info to
decide.

> I think we already have code to avoid adding beyond symlinks.
> "git add deep/in/the/tree" should refuse if deep/in is a symbolic
> link (and happens to point at a directory that has the/tree in it).
> We used not to catch that long time ago, but I think we fixed it.
>
> The logic and the places to do the checks for "no, that thing may be
> a directory but is an unrelated repository" should be the same.
-- 
Duy

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Stefan Beller]

On Fri, May 6, 2016 at 3:30 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Fri, May 6, 2016 at 6:27 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>>> I wonder if the patches mentioned have something to do with the "git
>>>> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
>>>> repository in some way?
>
> The same functionality is added in 8745024 (parse_pathspec: support
> stripping/checking submodule paths - 2013-07-14) so if it didn't fail
> to notice that before 5a76aff1a6 and did after, it's a bug.

The bug seems to have existed before. However in the bug we are talking
about the nested repo is not a submodule yet.

>
>>>
>>> Which is considered a feature now. Maybe we should add tests for that?
>>>
>>> http://debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb
>>
>> That is a bug, plain and simple.  Duy any ideas where we went wrong?
>
> I vaguely recall this symptom. It has something to do with the index,
> the check we do requires a gitlink in the index, I think. So if the
> gitlink entry is not in the index, our protection line fails. I think
> doing all this at pathspec level is wrong. We should wait at least
> after read_directory() is done, by then we have a lot more info to
> decide.
>
>> I think we already have code to avoid adding beyond symlinks.
>> "git add deep/in/the/tree" should refuse if deep/in is a symbolic
>> link (and happens to point at a directory that has the/tree in it).
>> We used not to catch that long time ago, but I think we fixed it.
>>
>> The logic and the places to do the checks for "no, that thing may be
>> a directory but is an unrelated repository" should be the same.
> --
> Duy

Here are some tests to have a clear picture of what is happening:
(diff against origin/master)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index f99f674..c9dfa11 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -37,6 +37,22 @@ test_expect_success 'setup - repository in init
subdirectory' '
                git add a &&
                git commit -m "submodule commit 1" &&
                git tag -a -m "rev-1" rev-1
+       ) &&
+       mkdir init_slash1 &&
+       (
+               cd init_slash1 &&
+               git init &&
+               echo a >a &&
+               git add a &&
+               git commit -m "first commit"
+       ) &&
+       mkdir init_slash2 &&
+       (
+               cd init_slash2 &&
+               git init &&
+               echo a >a &&
+               git add a &&
+               git commit -m "first commit"
        )
 '

@@ -44,7 +60,30 @@ test_expect_success 'setup - commit with gitlink' '
        echo a >a &&
        echo z >z &&
        git add a init z &&
-       git commit -m "super commit 1"
+       git commit -m "super commit 1" &&
+       git ls-tree HEAD |grep init >actual &&
+       grep "160000 commit" actual
+'
+
+test_expect_success 'setup - commit with gitlink/ in between' '
+       echo a >a &&
+       echo z >z &&
+       git add a init_slash1/ z &&
+       git commit -m "super commit 1" &&
+       git ls-tree HEAD |grep init_slash1 >actual &&
+       grep "160000 commit" actual
+'
+
+test_expect_failure 'setup - commit with gitlink/ only' '
+       git add init_slash2/ &&
+       git commit -m "super commit 1" &&
+       git ls-tree HEAD |grep init_slash2 >actual &&
+       grep "160000 commit" actual
+'
+
+test_expect_success 'tear down slash tests' '
+       rm -rf init_slash* &&
+       git commit -a -m "removing init_slash*"
 '

 test_expect_success 'setup - hide init subdirectory' '

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> On Fri, May 6, 2016 at 3:30 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Fri, May 6, 2016 at 6:27 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Stefan Beller <sbeller@google.com> writes:
>>>
>>>>> I wonder if the patches mentioned have something to do with the "git
>>>>> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
>>>>> repository in some way?
>>
>> The same functionality is added in 8745024 (parse_pathspec: support
>> stripping/checking submodule paths - 2013-07-14) so if it didn't fail
>> to notice that before 5a76aff1a6 and did after, it's a bug.
>
> The bug seems to have existed before. However in the bug we are talking
> about the nested repo is not a submodule yet.

That agrees with Duy's recollection below:

>> I vaguely recall this symptom. It has something to do with the index,
>> the check we do requires a gitlink in the index, I think. So if the
>> gitlink entry is not in the index, our protection line fails.

So are we all on the same page that this is a bug now?

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Stefan Beller]

On Fri, May 6, 2016 at 12:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> On Fri, May 6, 2016 at 3:30 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>>> On Fri, May 6, 2016 at 6:27 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Stefan Beller <sbeller@google.com> writes:
>>>>
>>>>>> I wonder if the patches mentioned have something to do with the "git
>>>>>> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
>>>>>> repository in some way?
>>>
>>> The same functionality is added in 8745024 (parse_pathspec: support
>>> stripping/checking submodule paths - 2013-07-14) so if it didn't fail
>>> to notice that before 5a76aff1a6 and did after, it's a bug.
>>
>> The bug seems to have existed before. However in the bug we are talking
>> about the nested repo is not a submodule yet.
>
> That agrees with Duy's recollection below:
>
>>> I vaguely recall this symptom. It has something to do with the index,
>>> the check we do requires a gitlink in the index, I think. So if the
>>> gitlink entry is not in the index, our protection line fails.
>
> So are we all on the same page that this is a bug now?

It was a bug, but now people in the outside world consider it a feature.
Search for "Git fake submodules" and you'll find a few users who use this
technique successfully.

I do not think fixing this bug would do good. So maybe we just let it slip?

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> It was a bug, but now people in the outside world consider it a feature.
> Search for "Git fake submodules" and you'll find a few users who use this
> technique successfully.
>
> I do not think fixing this bug would do good. So maybe we just let it slip?

I am OK with leaving it unfixed, iow, we just say this:

    If deep/in is a different repository, whether it is a submodule,
    "git add deep/in/the/tree/is-a-leaf.txt" will give an undefined
    result.

But that is totally different from accepting it as a feature.  If we
were to accept it as a feature (and we will not), then

    I did "git add deep/in/the/tree/is-a-leaf.txt" and have kept the
    path tracked.  Today I did "git add deep/*" and then the path
    disappeared from my project--I now only have deep/in as a
    submodule, which is not what I want.

would become a valid bug report.  I do not want to see that happen.

I.e. I am *NOT* OK with polluting the codebase with a hack to
respond to such a bug report, e.g. by adding a rule that says "if a
file deep/in/the/tree/is-a-leaf.txt is tracked and deep/in is a
repository, 'git add deep/in' must fail".

The stance "It is a bug, but we do not fix it right now.  The
behaviour is undefined" also leaves the door open for a future
enhancement that allows 'git add deep/in/the/tree/is-a-leaf.txt' to
be an equivalent to 'git -C deep/in/the/tree add is-a-leaf.txt'.

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Duy Nguyen]

On Sat, May 7, 2016 at 2:18 AM, Stefan Beller <sbeller@google.com> wrote:
> On Fri, May 6, 2016 at 12:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> On Fri, May 6, 2016 at 3:30 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>>>> On Fri, May 6, 2016 at 6:27 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>> Stefan Beller <sbeller@google.com> writes:
>>>>>
>>>>>>> I wonder if the patches mentioned have something to do with the "git
>>>>>>> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
>>>>>>> repository in some way?
>>>>
>>>> The same functionality is added in 8745024 (parse_pathspec: support
>>>> stripping/checking submodule paths - 2013-07-14) so if it didn't fail
>>>> to notice that before 5a76aff1a6 and did after, it's a bug.
>>>
>>> The bug seems to have existed before. However in the bug we are talking
>>> about the nested repo is not a submodule yet.
>>
>> That agrees with Duy's recollection below:
>>
>>>> I vaguely recall this symptom. It has something to do with the index,
>>>> the check we do requires a gitlink in the index, I think. So if the
>>>> gitlink entry is not in the index, our protection line fails.
>>
>> So are we all on the same page that this is a bug now?
>
> It was a bug, but now people in the outside world consider it a feature.
> Search for "Git fake submodules" and you'll find a few users who use this
> technique successfully.
>
> I do not think fixing this bug would do good. So maybe we just let it slip?

I think it's because people do have some use cases for multiple
worktrees (typically from different repos) to share the same
filesystem portion. We never actually support that. There are a bunch
of questions on how these worktrees/repos should interact. Maybe
someone will finally add support for that. But this is still a bug,
even though nobody has time to fix it yet. So I agree with Junio
calling this "undefined behavior", not "feature".
-- 
Duy