Re: [PATCH v3 5/5] attr.c: respect core.ignorecase when matching attribute patterns

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Oct 9, 2011 at 10:16 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> On 10/06/2011 08:22 PM, Brandon Casey wrote:
>> The last set of tests is performed only on a case-insensitive filesystem.
>> Those tests make 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 match the portion of the path below the directory holding the
>> .gitignore file according to the setting of core.ignorecase.
>
> Isn't this a rather perverse scenario?

No argument here. :)

> It is hard to imagine that
> anybody *wants* part of a single filename to be matched
> case-insensitively and another part to be matched case-sensitively.
> ISTM that a person who is using a case-insensitive filesystem and has
> core-ignorecase=false is (1) a glutton for punishment and (2) probably
> very careful to make sure that the case of all pathnames is correct.  So
> such a person is not likely to benefit from this schizophrenic behavior.
>
>> [...]  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.
>
> Your decision to treat path names as partly case-insensitive...

You are giving more credit to this patch than it deserves.  It really
doesn't do much.  It's not a design decision that I made to treat path
names as case-insensitive.  That is a consequence of using a
case-insensitive file system.  When you give git the path A/b/c and
there exists a/.gitattributes, then on a case insensitive file system
git will try to read A/.gitattributes and the filesystem will return a
handle for a/.gitattributes and git won't be able to tell the
difference.  git will then apply the rules in the file to the paths
below a/ even when the path is supplied like A/.  So, at the moment,
regardless of the setting of core.ignorecase, the path above a
.gitignore file is treated as case-insensitive on a case-insensitive
filesystem.

The purpose of the tests are primarily to ensure that nothing special
needs to be done concerning paths leading up to a directory containing
a .gitignore file in the core.ignorecase=1 case.  For example, it is
_not_ necessary to use strncmp_icase instead of strncmp on the leading
portion of the path.

What I meant when I said "If someone makes a change like that in the
future, these tests will notice", is that they will notice that they
must now use strncmp_icase when comparing the portion of the path
above the .gitattributes file.

> will make
> this kind of improvement considerably more complicated.
> Therefore, weighing the negligible benefit of declaring this
> schizophrenic behavior against the potential big pain of remaining
> backwards-compatible with this behavior, I suggest that we either (1)
> declare that when core.ignorecase=false then the *whole* filenames
> (including the path leading up to the .gitignore file) must match
> case-sensitively, or (2) declare the behavior in this situation to be
> undefined so that nobody thinks they should depend on it.

I do not plan to make git treat leading paths case-sensitively on a
case-insensitive file system when core.ignorecase=0 any more than I
plan to make git treat leading paths case-insensitively on a
case-sensitive file system when core.ignorecase=1.  For justification,
I refer back to your original comment about perversion. :)

So, #2 gets my vote.


Maybe my commit message is not clear that it is describing the current
behavior and not defining it.  Instead of

   git should only match the portion of the path below the directory
   holding the .gitignore file according to the setting of
   core.ignorecase.

maybe I should say

    git will currently only match the portion of the path...

I could also remove the following test from the CASE_INSENSITIVE_FS
tests since it is really a dontcare:

   attr_check A/b/h a/b/h "-c core.ignorecase=0"

We don't care what happens when the user supplies A/b/h and a/b/h
exists on disk when core.ignorecase=0, we only care that A/b/h is
interpreted correctly when core.ignorecase=1.

-Brandon
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]