This patch allows core.ignorecase=true to work properly with gitignore
exclusions.
This is especially beneficial when using Windows and Perforce and the
git-p4 bridge. Perforce preserves a given file's full path including
case. When syncing a file down, directories are created, if necessary,
using the case as stored with the filename. Unfortunately, two files in
the same directory can have differing cases for their respective paths,
such as /dirA/file1.c and /DirA/file2.c. Depending on sync order, DirA/
may get created instead of dirA/.
Trying to catch all case combinations for a set of gitignore entries is
very difficult. Having the exclusions honor the core.ignorecase=true
makes the process less error prone.
---
dir.c | 49 ++++++++++++++++++++++++++++++++++++-------------
1 files changed, 36 insertions(+), 13 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 22:47:03
On Wed, Jul 15, 2009 at 11:19:05PM -0600, Joshua Jensen wrote:
This patch allows core.ignorecase=true to work properly with
gitignore exclusions.
Makes sense, though I can't help but wonder what would happen with a
filesystem that did more than just case (like the utf8 normalization
that happens on HFS).
Should we actually be converting the filesystem names into a canonical
format as they are read? IIRC, Linus posted some patches a few weeks ago
about "git path" versus "filesystem path", but I didn't actually look
too closely.
That seems like the right way forward to fixing these problems in the
long term, but it may make sense to do something like your patch in the
meantime.
If your patch is the right route, it might be nice to collapse the
comparison into its own function. You end up cutting and pasting a lot
of the related conditionals and returns (like above, where 2 lines
become 9), so it might make sense to do something like:
int filename_cmp(const char *a, const char *b, int ignore_case)
{
return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
}
and then just s/strcmp/filename_cmp/ at the appropriate callsites.
-Peff
----- Original Message -----
From: Jeff King
Date: 7/16/2009 3:42 AM
Makes sense, though I can't help but wonder what would happen with a
filesystem that did more than just case (like the utf8 normalization
that happens on HFS).
Should we actually be converting the filesystem names into a canonical
format as they are read? IIRC, Linus posted some patches a few weeks ago
about "git path" versus "filesystem path", but I didn't actually look
too closely.
I'm game for whatever. Git actually has a lot of places where it
doesn't pay attention to core.ignorecase, and having a standard and
correct method of comparing filenames would make it easier to handle
core.ignorecase=true in a more global fashion.
If your patch is the right route, it might be nice to collapse the
comparison into its own function. You end up cutting and pasting a lot
of the related conditionals and returns (like above, where 2 lines
become 9), so it might make sense to do something like:
int filename_cmp(const char *a, const char *b, int ignore_case)
{
return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
}
and then just s/strcmp/filename_cmp/ at the appropriate callsites.
I started off with this method, but it required two functions, one with
the strcmp() and one for strncmp(). In fact, in other places in the
code, Git uses memcmp() for comparison. Is that, then, three filename
comparison functions, dependent upon intent? At that point, it felt
like my change wasn't as self contained anymore, so I then wrote what I
posted to the list to get feedback.
I'm hoping someone will offer the most correct method to do this, as I
have a number of patches forthcoming to handle core.ignorecase=true in
other areas. The next one covers 'git status' and its reporting of
'missing' directories due to case differences.
Thanks!
Josh
From: Jeff King <hidden> Date: 2016-06-15 22:47:04
On Thu, Jul 16, 2009 at 07:15:26AM -0600, Joshua Jensen wrote:
quoted
Should we actually be converting the filesystem names into a canonical
format as they are read? IIRC, Linus posted some patches a few weeks ago
about "git path" versus "filesystem path", but I didn't actually look
too closely.
I'm game for whatever. Git actually has a lot of places where it
doesn't pay attention to core.ignorecase, and having a standard and
correct method of comparing filenames would make it easier to handle
core.ignorecase=true in a more global fashion.
Like I said, I'm not sure what the status of that is, so probably
something simple like your patch makes sense in the interim (unless we
hear from somebody more clueful).
quoted
If your patch is the right route, it might be nice to collapse the
comparison into its own function. You end up cutting and pasting a lot
of the related conditionals and returns (like above, where 2 lines
become 9), so it might make sense to do something like:
int filename_cmp(const char *a, const char *b, int ignore_case)
{
return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
}
and then just s/strcmp/filename_cmp/ at the appropriate callsites.
I started off with this method, but it required two functions, one
with the strcmp() and one for strncmp(). In fact, in other places in
the code, Git uses memcmp() for comparison. Is that, then, three
filename comparison functions, dependent upon intent? At that point,
it felt like my change wasn't as self contained anymore, so I then
wrote what I posted to the list to get feedback.
IMHO, you are better off even with three wrapper functions, just because
they are all very straightforward. Whereas with your patch, I felt like
the innards of complex functions got harder to read because of big
duplicate conditionals. But that's just my two cents.
-Peff
----- Original Message -----
From: Jeff King
Date: 7/20/2009 9:37 AM
quoted
quoted
If your patch is the right route, it might be nice to collapse the
comparison into its own function. You end up cutting and pasting a lot
of the related conditionals and returns (like above, where 2 lines
become 9), so it might make sense to do something like:
int filename_cmp(const char *a, const char *b, int ignore_case)
{
return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
}
and then just s/strcmp/filename_cmp/ at the appropriate callsites.
IMHO, you are better off even with three wrapper functions, just because
they are all very straightforward. Whereas with your patch, I felt like
the innards of complex functions got harder to read because of big
duplicate conditionals. But that's just my two cents.