[PATCH/RFC v3 0/8] ab/icase-directory: jj/icase-directory with Makefile + configure checks

On Sun, Oct 3, 2010 at 09:07, Joshua Jensen [off-list ref] wrote:

 ----- Original Message -----
From: Ævar Arnfjörð Bjarmason
Date: 10/3/2010 2:30 AM

quoted

On Sun, Oct 3, 2010 at 04:32, Joshua Jensen[off-list ref]
 wrote

quoted

+int fnmatch_casefold(const char *pattern, const char *string, int flags)
+{
+       char lowerPatternBuf[MAX_PATH];
+       char lowerStringBuf[MAX_PATH];
+       char* lowerPattern;
+       char* lowerString;
+       size_t patternLen;
+       size_t stringLen;
+       char* out;
+       int ret;
+
+       /*
+        * Use the provided stack buffer, if possible.  If the string is
too
+        * large, allocate buffer space.
+        */
+       patternLen = strlen(pattern);
+       if (patternLen + 1>  sizeof(lowerPatternBuf))
+               lowerPattern = xmalloc(patternLen + 1);
+       else
+               lowerPattern = lowerPatternBuf;
+
+       stringLen = strlen(string);
+       if (stringLen + 1>  sizeof(lowerStringBuf))
+               lowerString = xmalloc(stringLen + 1);
+       else
+               lowerString = lowerStringBuf;
+
+       /* Make the pattern and string lowercase to pass to fnmatch. */
+       for (out = lowerPattern; *pattern; ++out, ++pattern)
+               *out = tolower(*pattern);
+       *out = 0;
+
+       for (out = lowerString; *string; ++out, ++string)
+               *out = tolower(*string);
+       *out = 0;
+
+       ret = fnmatch(lowerPattern, lowerString, flags);
+
+       /* Free the pattern or string if it was allocated. */
+       if (lowerPattern != lowerPatternBuf)
+               free(lowerPattern);
+       if (lowerString != lowerStringBuf)
+               free(lowerString);
+       return ret;
+}
+
+int fnmatch_icase(const char *pattern, const char *string, int flags)
+{
+       return ignore_case ? fnmatch_casefold(pattern, string, flags) :
fnmatch(pattern, string, flags);
+}

I liked v1 of this patch better, although it obviously had portability
issues. But I think it would be better to handle this with:

    #ifndef FNM_CASEFOLD
    int fnmatch_casefold(const char *pattern, const char *string, int
flags)
    {
        ...
    }
    #endf

    int fnmatch_icase(const char *pattern, const char *string, int flags)
    {
    #ifndef FNM_CASEFOLD
           return ignore_case ? fnmatch_casefold(pattern, string,
flags) : fnmatch(pattern, string, flags);
    #else
            return fnmatch(pattern, string, flags | (ignore_case ?
FNM_CASEFOLD : 0));
    #endif
    }

Or simply use fnmatch(..., FNM_CASEFOLD) everywhere and include
compat/fnmatch/* on platforms like Solaris that don't have the GNU
extension.

I offered before to help with making this portable, so I've gone ahead
and done it. This series is like your v1, but it has two of my patches
at the front to add Makefile & configure checks & fallbacks for
fnmatch if the function either doesn't exist, or it doesn't support
the FNM_CASEFOLD flag.

The real problem with compat/fnmatch is determining which random platforms
need that support and updating the makefile accordingly.

We already do this for a bunch of NO_WHATEVER= flags. Adding one more
isn't going to be too hard to maintain.

Further, the compat/fnmatch/* code would need to be rejigged
somewhat, so there is no possible conflict (now or in the future)
with the provided symbols.  We discussed this as a potential problem
developers would need to be aware of if the system fnmatch.h (or
whatever it is called) gets #included.

Since we do -Icompat/fnmatch it's going to be our fnmatch.h that's
picked up, so we aren't going to get a symbol conflict I should think.

Anyway, what you describe above creates two code paths.  I would imagine
that would be harder to debug; that is, on some platforms, it uses
fnmatch_casefold and on others, it hands it off to fnmatch(...,
FNM_CASEFOLD).

My ad-hoc example pseudocode created two codepaths, but this version
doesn't.

In any case, I'd like to find a solution to get this series working for
everyone.  I've been out of commission for a month (deploying Git to 80+
programmers at an organization, by the way), but I'm back now and can work
this until it is complete.

This is all from your v1:

Joshua Jensen (6):
  Add string comparison functions that respect the ignore_case
    variable.
  Case insensitivity support for .gitignore via core.ignorecase
  Add case insensitivity support for directories when using git status
  Add case insensitivity support when using git ls-files
  Support case folding for git add when core.ignorecase=true
  Support case folding in git fast-import when core.ignorecase=true

These two are new:

Ævar Arnfjörð Bjarmason (2):
  Makefile & configure: add a NO_FNMATCH flag

This one is a good idea in general. We shouldn't be duplicating setup
code in both the Windows and MinGW portions of the Makefile, and if we
ever get another odd system that doesn't have fnmatch() this will make
things just work there.

Needs testing from someone with Windows.

  Makefile & configure: add a NO_FNMATCH_CASEFOLD flag

The code needed to make Joshua's code portable. On Solaris this
returns with the configure script:

    $ make configure && ./configure | grep -i -e fnmatch && grep -i -e fnmatch config.mak.autogen
        GEN configure
    checking for fnmatch... yes
    checking for library containing fnmatch... none required
    checking whether the fnmatch function supports the FNMATCH_CASEFOLD GNU extension... no
    NO_FNMATCH=
    NO_FNMATCH_CASEFOLD=YesPlease

And on Linux:

    $ make configure && ./configure | grep -i -e fnmatch && grep -i -e fnmatch config.mak.autogen
        GEN configure
    checking for fnmatch... yes
    checking for library containing fnmatch... none required
    checking whether the fnmatch function supports the FNMATCH_CASEFOLD GNU extension... yes
    NO_FNMATCH=
    NO_FNMATCH_CASEFOLD=

 Makefile      |   27 ++++++++++++---
 config.mak.in |    2 +
 configure.ac  |   28 +++++++++++++++
 dir.c         |  106 ++++++++++++++++++++++++++++++++++++++++++++++----------
 dir.h         |    4 ++
 fast-import.c |    7 ++--
 name-hash.c   |   72 ++++++++++++++++++++++++++++++++++++++-
 read-cache.c  |   23 ++++++++++++
 8 files changed, 241 insertions(+), 28 deletions(-)

-- 
1.7.3.159.g610493