Re: [PATCH] status: report ignored yet tracked directories
- From
Jeff King <peff@peff.net>
- Date
- Jan 7, 2013, 08:33 UTC
- Message-ID
- <20130107083318.GA18052@sigill.intra.peff.net>
- In-Reply-To
- <CALWbr2yRQai2Z08G2qFbA3AvsgivR-8kQ64SZ4pEktyrf+ZXiQ@mail.gmail.com>
On Sun, Jan 06, 2013 at 05:40:46PM +0100, Antoine Pelisse wrote:
Show 22 quoted lines
> > Looking at your fix and remembering how the index hashing works, I think > > the answer is that: > > > > 1. This bug only affects directories, because they are the only thing > > that can be simultaneously "ignored and untracked" and "tracked" > > (i.e., they have entries of both, and we are using > > DIR_SHOW_OTHER_DIRECTORIES). > > > > 2. When core.ignorecase is false, the index name hash contains only > > the file entries, and cache_name_exists returns an exact match. So > > it doesn't matter if we make an extra check when adding the > > directory via dir_add_name; we know that it will not be there, and > > the final check is a no-op. > > > > 3. When core.ignorecase is true, we also store directory entries in > > the index name hash, and this extra check is harmful; the entry > > does not really exist in the index, and we still need to add it. > > Yes, because of this couple of lines I guess (name-hash.c, hash_index_entry()): > > if (ignore_case) > hash_index_entry_directories(istate, ce);
Exactly. I couldn't remember at first why this was the case, but after reading 5102c61 (Add case insensitivity support for directories when using git status, 2010-10-03) again, I think it is because we cannot do a partial-name lookup via the hash (i.e., the hash for "foo/" and "foo/bar" have no relation to each other). Not related to your patch, obviously, but it was the missing piece for me to understand why the code was doing what it does.
Show 7 quoted lines
> > I think in the normal file case, we'd expect treat_path to just tell us > > that it is handled, and we would not ever call dir_add_name in the first > > place. But what if we have an index entry for a file, but the working > > tree now contains a directory? > > The directory is treated as any other untracked directory (it never > matches indexed file because of the trailing /).
Ah, right. That makes sense.
Show 8 quoted lines
> > I _think_ we still do not hit this code path in that instance, because > > we will end up in treat_directory, and we will end up checking > > directory_exists_in_index. And I cannot get it to misbehave in practice. > > So I think your fix is correct, but the exact how and why is a bit > > subtle. > > Thanks a lot for the help, I will try to come up with a better commit > message now.
Thanks. I think the patch is right, but the reasoning is just a bit subtle.
-Peff