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

From: Duy Nguyen <pclouds@gmail.com>
Date: Jun 14, 2015, 13:16 UTC
Message-ID: <CACsJy8A5ikjGmj+ysTUt9FFi9X0WzVXvnFSVbmFoe8rzHdNNoA@mail.gmail.com>
In-Reply-To: <CAPc5daUA30ZbbJVK9S_FM9Z8qyyEXUez9hGD2JNhTm78QYb5YQ@mail.gmail.com>

On Thu, Apr 23, 2015 at 10:47 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

> On Wed, Apr 22, 2015 at 3:32 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> ...
>>> 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.
>>
>> The user having removed subrepo/.git is just one reason for that.
>> Another is a user adding a file in an unpopulated work tree of a
>> not initialized submodule. I doubt that simply nuking the 160000
>> entry would be the right thing to do in this case, I expect this
>> to be a pilot error we should barf about ;-)
>
> OK, that sounds sensible.

There are more to this "submodule vs the world". When .git is gone, if you expect to warn instead of deleting 160000. You may want the same for the opposite direction: when b/c is in the index and you add b/.git, you may want "git add b" to complain too (right not it ignores b/.git and updates b/c).

We may need more surgery around this area. It seems to be the "Path %s is in submodule" check (in pathspec.c) is at the wrong place. We have d/f check in add_index_entry_with_check. That should catch it, at the core, not from call sites like add or update-index. But that check is not active...

The first version of "path is in submodule" is in 2ce53f9 (git add: do not add files from a submodule - 2009-01-02). Back then dir.c does not intervene. If you have "b" as 160000 in the index and tries to add b/c, the ok_to_replace feature kicks in and kills 'b'. This is why d/f check is turned off. This ok_to_replace is to deal with symlinks, see 192268c (Add git-update-cache --replace option. - 2005-05-07).

I think we should stop the ok-to-replace feature when submodules are involved, we consider submdules much more valuable than symlinks. If we do this, I think we can delete those "Path is in submodule" trick becaue the index core can handle it well.

-- 
Duy