Subject: Re: [PATCH] pathspec: adjust prefixlen after striping trailing slash

From: Junio C Hamano <[email-masked]>
Date: Wed, 22 Apr 2015 12:58:33 -0700

Jens Lehmann <[email-masked]> writes:

> Am 21.04.2015 um 23:08 schrieb Junio C Hamano:
>
>> I looked at the test script update.  The new test does (I am
>> rephrasing to make it clearer):
>>
>>      mkdir -p dir/ectory
>>      git init dir/ectory ;# a new directory inside top-level project
>>      (
>>          cd dir/ectory &&
>>          >test && git add test && git commit -m test
>>      )
>>      git add dir/ectory
>>
>> to set it up.  At this point, the top-level index knows dir/ectory
>> is a submodule.
>>
>> Then the test goes on to turn it a non submodule by
>>
>>      mv dir/ectory/.git dir/ectory/dotgit
>> ...
>
> We already do (2) in the cases you describe:
>
>    $ git add subrepo/a
>    fatal: Pathspec 'subrepo/a' is in submodule 'subrepo'
>    $ git -C subrepo add a
>    fatal: Pathspec 'a' is in submodule 'subrepo'
> ...
> So I'd vote to have (2) also for "git -C subrepo add .", which
> is what started this thread.

Does having "mv subrepo/.git subrepo/dotgit" before that "git add"
change your conclusion?

It is very clear to me that without that "mv" step, (2) is
absolutely the right thing to do, and I agree with you.

But it is unclear if we should still do (2) when "subrepo/.git" is
no longer there.  That has to be done manually and it may be an
indication that is clear enough that the end user wants the
directory to be a normal directory without any submodule involved,
in which case it may match the expectation of the user better to
just nuke the corresponding 160000 entry in the index and replace it
with files in there.  I dunno.

>> In my quick test, it appeared that the behaviour with this patch
>> applied was neither of the two and instead to silently do nothing,
>> and if that is the case I think it is quite wrong.
>
> Yep.