[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 <jjensen@workspacewhiz.com> wrote:
Show 85 quoted lines
> ----- Original Message -----
> From: Ævar Arnfjörð Bjarmason
> Date: 10/3/2010 2:30 AM
>>
>> On Sun, Oct 3, 2010 at 04:32, Joshua Jensen<jjensen@workspacewhiz.com>
>> wrote
>>>
>>> +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.
Show 5 quoted lines
> 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=trueThese 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=YesPleaseAnd 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
Joshua Jensen
