Re: [PATCH 0/4] Honor core.ignorecase for attribute patterns

From: Brandon Casey <drafnel@gmail.com>
Date: Sep 15, 2011, 20:28 UTC
Message-ID: <CA+sFfMeLgdmUHq6YTymkC2bpMEGScQuwJ1SgyR0GGFjo-Gt=4g@mail.gmail.com>
In-Reply-To: <20110915181258.GA1227@sigill.intra.peff.net>

On Thu, Sep 15, 2011 at 1:12 PM, Jeff King <peff@peff.net> wrote:

Show 18 quoted lines

> On Wed, Sep 14, 2011 at 08:59:35PM -0500, Brandon Casey wrote:
>
>> > I haven't even tested that it runs. :)  No, I was hoping someone
>> > who was more interested would finish it, and maybe even test on
>> > an affected system.
>>
>> Ok, I lied.  Here's a series that needs testing by people on a
>> case-insensitive filesystem and some comments.
>
> Thanks. I was trying to decide if I was interested enough to work on it,
> but procrastination wins again.
>
> I'm not sure I understand why you need a case-insensitive file system
> for the final set of tests. If we have a case-sensitive system, we can
> force the filesystem to show us whatever cases we want, and check
> against them with both core.ignorecase off and on[1]. What are these
> tests checking that requires the actual behavior of a case-insensitive
> filesystem?

This is probably way more detail than this feature deserves, but...

Those tests are making sure that git handles the case where the .gitignore file resides in a subdirectory and the user supplies a path that does not match the case in the filesystem. In that case^H^H^H^Hsituation, part of the path supplied by the user is effectively interpreted case-insensitively, and part of it is dependent on the setting of core.ignorecase. git should only be matching the portion of the path below the directory holding the .gitignore file according to the setting of core.ignorecase.

Imagine a hierarchy that looks like this:

   .gitattributes
   a/.gitattributes

On a case-insensitive filesystem, if you supply the path A/B, regardless of whether ignorecase is true or false, git will read the a/.gitattributes file and use it.

Then if you have:

   $ cat a/.gitattributes
   b/c test=a/b/c

then you should get the following results:

   # the case of a/ does not affect the attr check
   $ git -c core.ignorecase=0 check-attr a/b/c
   a/b/c: test: a/b/c
   $ git -c core.ignorecase=0 check-attr A/b/c
   A/b/c: test: a/b/c
   $ git -c core.ignorecase=0 check-attr a/B/c
   a/B/c: test: unspecified
   $ git -c core.ignorecase=1 check-attr a/B/c
   a/B/c: test: a/b/c
   $ git -c core.ignorecase=0 check-attr A/B/c
   A/B/c: test: unspecified
   $ git -c core.ignorecase=1 check-attr A/B/c
   A/B/c: test: a/b/c
   etc.

On a case-sensitive filesystem, a/.gitattributes would never be read if A/b/c was supplied, regardless of core.ignorecase.

This is also partly future-proofing. Currently, git builds the attr stack based on the path supplied by the user, so we don't have to do anything special (like use strcmp_icase) to handle the parts of that path that don't match the filesystem with respect to case. If git instead built the attr stack by scanning the repository, then the paths in the origin field would not necessarily match the paths supplied by the user. If someone makes a change like that in the future, these tests will notice.

> I'm sure there is something subtle that I'm missing. Can you explain it
> either here or in the commit message?

Yeah, that commit message was really just a place-holder. I meant to add WIP in the subject field of the last patch too. I'll try to explain some of the above when I reroll.

-Brandon