Re: [PATCH 1/6] meson: simplify and parameterize various standard function checks

From: Eli Schwartz <eschwartz@gentoo.org>
Date: Apr 22, 2025, 15:36 UTC
Message-ID: <590d0daa-59ac-44f3-b277-e3e83ad1add5@gentoo.org>
In-Reply-To: <aAdF4DzFCZ3uOJCx@pks.im>

On 4/22/25 3:31 AM, Patrick Steinhardt wrote:

Show 38 quoted lines

> On Mon, Apr 21, 2025 at 04:04:30PM -0400, Eli Schwartz wrote:
>> On 4/21/25 1:51 PM, Eli Schwartz wrote:
>>> diff --git a/meson.build b/meson.build
>>> index c47cb79af0..6c147c22a4 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -1322,45 +1339,15 @@ if not compiler.has_function('strtoumax')
>>>    ]
>>>  endif
>>>  
>>> -if not compiler.has_function('strtoull')
>>> -  libgit_c_args += '-DNO_STRTOULL'
>>> -endif
>>> -
>>> -if not compiler.has_function('setenv')
>>> -  libgit_c_args += '-DNO_SETENV'
>>> -  libgit_sources += 'compat/setenv.c'
>>> -endif
>>> -
>>>  if not compiler.has_function('qsort')
>>>    libgit_c_args += '-DINTERNAL_QSORT'
>>>  endif
>>>  libgit_sources += 'compat/qsort_s.c'
>>
>>
>> ... for example, the Makefile says here:
>>
>>
>> # Define INTERNAL_QSORT to use Git's implementation of qsort(), which
>> # is a simplified version of the merge sort used in glibc. This is
>> # recommended if Git triggers O(n^2) behavior in your platform's
>> # qsort().
>>
>> cmake unconditionally defines it (???)
> 
> Our CMake build instructions shouldn't be treated as canonical source of
> truth. They're good enough for some usecases, but they are not as
> feature complete as any of Makefile/autoconf/Meson.

... yes, which is why I'm using it as a springboard to ask questions? :)

My working theory is it unconditionally defines it because this is the correct behavior on Windows, and the cmake files were primarily written to be used on Windows, which leads us to...

Show 18 quoted lines

>> config.mak.uname says:
>>
>> - AIX:
>>   INTERNAL_QSORT = UnfortunatelyYes
>>
>>   Seems to date back to commit 377d9c409ffe0f0d994b929aeb94716139207b9d.
>>   "Unfortunate" indeed.
>>
>>
>> - MinGW:
>>   INTERNAL_QSORT = YesPlease
>>
>>   Windows claims to have a qsort but perhaps it is very slow and bes
>>   avoided?
>>
>> We should probably stop *checking* for qsort and simply encode the
>> platforms we know are slow and automatically skip it there. Can I get
>> confirmation regarding Windows? :)

... this. config.mak.uname's mingw case appears to agree with my theory about the motivations for the cmake file.

> I'd rather prefer to try and detect this generically instead of adding
> more platform-specific configuration. It is way simpler to maintain, and
> if we ever see that things don't work well on a specific platform we may
> still reconsider at that point in time.

Okay but, how do we generically detect that a platform triggers the Makefile advice "recommended if Git triggers O(n^2) behavior in your platform's qsort()"? I'm not sure how to write a compile-time check for this.

It's easy to write a compile-time check for whether a function exists, but it seems to have been an error that meson assumes some platforms will not provide the function, as that was never the intent of Git's support for internal qsort.

-- 
Eli Schwartz