Re: [PATCH v8 1/5] Introduce bulk-move detection in diffcore.

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 25, 2010, 17:19 UTC
Message-ID: <AANLkTimGOkv7db2YW_+-V_oJEheuwBiUW-3fFpHBOt4v@mail.gmail.com>
In-Reply-To: <20101125150218.GA16049@burratino>

On Thu, Nov 25, 2010 at 16:02, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 18 quoted lines

> Ævar Arnfjörð Bjarmason wrote:
>> On Fri, Oct 29, 2010 at 00:08, Yann Dirson <ydirson@altern.org> wrote:
>
>>> +       slash = memrchr(src, '/', len);
>>
>> I can't compile pu now on Solaris due to this bit. Are you planning on
>> picking up the patch / configure / Makefile detection discussed in the
>> "[PATCH] compat: add memrchr()" thread? Just wondering what the
>> progress was on that.
>
> BTW, remember that the configure / Makefile detection is only an extra
> nicety.  [1] teaches git to rely on the system memrchr on glibc
> systems only; on other systems (unless they define memrchr as a macro,
> which would be a strange thing to do), gitmemrchr is used.
>
> Upshot: it should be safe to use [1] without the configure / Makefile
> support.  Later, an interested person can write a patch for improved
> memrchr[2] on *BSD and Plan 9.

Sure. It's just a bit of a hack since we already have a standard-ish way of doing this. Which is to optionally have configure.ac support, then add platform-specific stuff to the Makefile & drop something in compat/memrchr.c.

Doing it that way and actually detecting it means we use e.g. the native memrchr on FreeBSD. I just don't see a reason to do it like the [1] patch when it's just as easy to do it the "proper way", where it's more extendable.

Anyway, it's a very minor issue. I'll probably do a patch for this series like I did for that other thing that needed autoconf detection recently. Thanks everyone.