On 4/21/25 1:51 PM, Eli Schwartz wrote:
> This is repetitive logic. We either want to use some -lc function, or if
> it is not available we define it as -DNO_XXX and usually (but not
> always) provide some custom compatibility impl instead.
>
> Checking the intent of each block when reading through the file is slow
> and not very DRY. Switch to taking an array of checkable functions
> instead.
>
> Not all functions are straightforward to move, since different macro
> prefixes are used.
By the way, when reviewing this I was having a slightly hard time
figuring out which stuff belonged here... specifically, because of the
differences in macro prefixes lead me to believe it's not always so
simple as "does it exist".
> Signed-off-by: Eli Schwartz <eschwartz@xxxxxxxxxx>
> ---
> meson.build | 73 ++++++++++++++++++++++-------------------------------
> 1 file changed, 30 insertions(+), 43 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index c47cb79af0..6c147c22a4 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1290,23 +1290,40 @@ if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#include <pwd.h
> libgit_c_args += '-DNO_GECOS_IN_PWENT'
> endif
>
> -if compiler.has_function('sync_file_range')
> - libgit_c_args += '-DHAVE_SYNC_FILE_RANGE'
> -endif
> +checkfuncs = [
> + 'strcasestr',
> + 'memmem',
> + 'strlcpy',
> + # no compat
> + 'strtoull',
> + 'setenv',
> + 'mkdtemp',
> + # no compat
> + 'initgroups',
> +]
>
> -if not compiler.has_function('strcasestr')
> - libgit_c_args += '-DNO_STRCASESTR'
> - libgit_sources += 'compat/strcasestr.c'
> +if host_machine.system() == 'windows'
> + libgit_c_args += '-DUSE_WIN32_MMAP'
> +else
> + checkfuncs += [
> + 'mmap',
> + # unsetenv is provided by compat/mingw.c.
> + 'unsetenv',
> + ]
> endif
>
> -if not compiler.has_function('memmem')
> - libgit_c_args += '-DNO_MEMMEM'
> - libgit_sources += 'compat/memmem.c'
> -endif
> +foreach func: checkfuncs
> + if not compiler.has_function(func)
> + libgit_c_args += '-DNO_' + func.to_upper()
> + impl = 'compat/' + func + '.c'
> + if fs.exists(impl)
> + libgit_sources += impl
> + endif
> + endif
> +endforeach
>
> -if not compiler.has_function('strlcpy')
> - libgit_c_args += '-DNO_STRLCPY'
> - libgit_sources += 'compat/strlcpy.c'
> +if compiler.has_function('sync_file_range')
> + libgit_c_args += '-DHAVE_SYNC_FILE_RANGE'
> endif
>
> if not compiler.has_function('strdup')
> @@ -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 (???)
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? :)
> -# unsetenv is provided by compat/mingw.c.
> -if host_machine.system() != 'windows' and not compiler.has_function('unsetenv')
> - libgit_c_args += '-DNO_UNSETENV'
> - libgit_sources += 'compat/unsetenv.c'
> -endif
> -
> -if not compiler.has_function('mkdtemp')
> - libgit_c_args += '-DNO_MKDTEMP'
> - libgit_sources += 'compat/mkdtemp.c'
> -endif
> -
> -if not compiler.has_function('initgroups')
> - libgit_c_args += '-DNO_INITGROUPS'
> -endif
> -
> if compiler.has_function('getdelim')
> libgit_c_args += '-DHAVE_GETDELIM'
> endif
But stuff like this, why isn't it consistent with the other functions?
What's the difference between HAVE_XXX and NO_XXX?
> -if host_machine.system() == 'windows'
> - libgit_c_args += '-DUSE_WIN32_MMAP'
> -elif not compiler.has_function('mmap')
> - libgit_c_args += '-DNO_MMAP'
> - libgit_sources += 'compat/mmap.c'
> -endif
>
> if compiler.has_function('clock_gettime')
> libgit_c_args += '-DHAVE_CLOCK_GETTIME'
--
Eli Schwartz
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature
