git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Joshua Jensen <jjensen@workspacewhiz.com>
Cc: Johannes Sixt <j6t@kdbg.org>, git@vger.kernel.org
Subject: Re: [PATCH 1/6] Add string comparison functions that respect the ignore_case variable.
Date: Mon, 30 Aug 2010 14:51:26 +0000	[thread overview]
Message-ID: <AANLkTikbLtfx6xL6K32o_AXqY+qeqKANN5n2g0oYPoi6@mail.gmail.com> (raw)
In-Reply-To: <4C7BC344.9020500@workspacewhiz.com>

On Mon, Aug 30, 2010 at 14:42, Joshua Jensen <jjensen@workspacewhiz.com> wrote:
>  ----- Original Message -----
> From: Ævar Arnfjörð Bjarmason
> Date: 8/29/2010 1:39 PM
>>
>> On Wed, Aug 18, 2010 at 18:32, Johannes Sixt<j6t@kdbg.org>  wrote:
>>>
>>> On Mittwoch, 18. August 2010, Ævar Arnfjörð Bjarmason wrote:
>>>>
>>>> According to some further research at least FreeBSD and NetBSD have
>>>> copied this GNU extension. You may find their versions easier to
>>>> integrate.
>>>
>>> We already have a GNU fnmatch in compat/fnmatch.
>>
>> Do you have any plan to deal with this? I currently have this
>> monkeypatch to build on Solaris:
>>
>>     diff --git a/Makefile b/Makefile
>>     index 62d526a..079fae5 100644
>>     --- a/Makefile
>>     +++ b/Makefile
>>     @@ -863,2 +863,4 @@ endif
>>      ifeq ($(uname_S),SunOS)
>>     +       COMPAT_OBJS = compat/fnmatch/fnmatch.o
>>     +       COMPAT_CFLAGS = -Icompat -Icompat/fnmatch
>>             NEEDS_SOCKET = YesPlease
>>
>> One way to deal with it would be a new NONGNU_FNMATCH=UnfortunatelyYes
>> flag, or the fnmatch_icase() suggestion above which we could bundle
>> and always use. But having next build on systems without GNU
>> extensions would be preferrable.
>
> I am going to deal with this, but I haven't been around.  I hope for some
> time this week.

Sure, no rush. I was just wondering whether you had some plan for it,
or whether I should submit a patch to use the fallback on
e.g. Solaris.

> Short of duplicating fnmatch's code and renaming the function, I am not sure
> how to make this play nice on all systems.

I don't think duplicating the GNU (or *BSD) version into a
compat/fnmatch.c would be a bad thing. See e.g. compat/snprintf.c.

> You added COMPAT_OBJS above, but I think there is no linker
> guarantee it will pick up compat/fnmatch/fnmatch.o over the C
> runtime version?  Perhaps the makefile is architected to do so.

It's probably just an artifact of how the Solaris linker works, it
doesn't go through the trouble of undefining an existing fnmatch
symbol or anything.

> The safest alternative is to allocate character buffers, lowercase the
> filename and match arguments into those buffers, and pass them off to
> fnmatch without any special flags.  I don't like the idea of a double memory
> allocation/free combo per each call to this function, but it would work.  Is
> anyone opposed to this approach?

Just using the GNU extension and providing a fallback is probably
cleaner and easier to maintain.

  reply	other threads:[~2010-08-30 14:51 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-16 19:38 [PATCH 0/6] Extensions of core.ignorecase=true support Johannes Sixt
2010-08-16 19:38 ` [PATCH 1/6] Add string comparison functions that respect the ignore_case variable Johannes Sixt
2010-08-18 12:52   ` Ævar Arnfjörð Bjarmason
2010-08-18 12:53     ` Ævar Arnfjörð Bjarmason
2010-08-18 15:52       ` Joshua Jensen
2010-08-18 16:07         ` Ævar Arnfjörð Bjarmason
2010-08-18 18:32           ` Johannes Sixt
2010-08-18 18:58             ` Ævar Arnfjörð Bjarmason
2010-08-29 19:39             ` Ævar Arnfjörð Bjarmason
2010-08-30 14:42               ` Joshua Jensen
2010-08-30 14:51                 ` Ævar Arnfjörð Bjarmason [this message]
2010-08-30 15:05                   ` Jonathan Nieder
2010-08-30 14:52                 ` Jonathan Nieder
2010-08-30 18:40                 ` Johannes Sixt
2010-08-30 19:57                   ` Ævar Arnfjörð Bjarmason
2010-08-30 20:13                     ` Johannes Sixt
2010-08-16 19:38 ` [PATCH 2/6] Case insensitivity support for .gitignore via core.ignorecase Johannes Sixt
2010-08-16 19:38 ` [PATCH 3/6] Add case insensitivity support for directories when using git status Johannes Sixt
2010-08-16 19:38 ` [PATCH 4/6] Add case insensitivity support when using git ls-files Johannes Sixt
2010-08-16 19:38 ` [PATCH 5/6] Support case folding for git add when core.ignorecase=true Johannes Sixt
2010-08-16 19:38 ` [PATCH 6/6] Support case folding in git fast-import " Johannes Sixt
2010-08-17 19:36 ` [PATCH 0/6] Extensions of core.ignorecase=true support Robert Buck
2010-08-17 21:20   ` Johannes Sixt
2010-08-18  2:41     ` Robert Buck
2010-08-18 18:31       ` Johannes Sixt
2010-08-22  7:23 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AANLkTikbLtfx6xL6K32o_AXqY+qeqKANN5n2g0oYPoi6@mail.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=jjensen@workspacewhiz.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).