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

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

[Eli Schwartz]

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.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 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'
 
-# 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
 
-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'
-- 
2.49.0

[PATCH 2/6] meson: check for getpagesize before using it

[Eli Schwartz]

It is deprecated and removed in SUS v3 / POSIX 2001, so various systems
may not include it. Solaris, in particular, carefully refrains from
defining it except inside of a maze of `#ifdef` to make sure you have
kept your nose clean and only used it in code that *targets* SUS v2 or
earlier.

config.mak.uname defines this automatically, though only for QNX.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson.build b/meson.build
index 6c147c22a4..f5d9ffcd7f 100644
--- a/meson.build
+++ b/meson.build
@@ -1300,6 +1300,8 @@ checkfuncs = [
   'mkdtemp',
   # no compat
   'initgroups',
+  # no compat
+  'getpagesize',
 ]
 
 if host_machine.system() == 'windows'
-- 
2.49.0

[PATCH 3/6] meson: do a full usage-based compile check for sysinfo

[Eli Schwartz]

On Solaris, sys/sysinfo.h is a completely different file and doesn't
resemble the linux file at all. There is also a sysinfo() function, but
it takes a totally different call signature, which asks for:

- the field you wish to receive
- a `char *buf` to copy the data to

and is very useful IFF you want to know, say, the hardware provider. Or,
get *specific* fields from uname(2).

https://docs.oracle.com/cd/E86824_01/html/E54765/sysinfo-2.html

It is surely possible to do this manually via `sysconf(3)` without the
nice API. I can't find anything more direct. Either way, I'm not very
attached to Solaris, so someone who cares can add it. Either way, it's
wrong to assume that sysinfo.h contains what we are looking for.

Check that sysinfo.h defines the struct we actually utilize in
builtins/gc.c, which will correctly fail on systems that don't have it.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index f5d9ffcd7f..8037e536dd 100644
--- a/meson.build
+++ b/meson.build
@@ -1058,10 +1058,6 @@ if compiler.has_header('alloca.h')
   libgit_c_args += '-DHAVE_ALLOCA_H'
 endif
 
-if compiler.has_header('sys/sysinfo.h')
-  libgit_c_args += '-DHAVE_SYSINFO'
-endif
-
 # Windows has libgen.h and a basename implementation, but we still need our own
 # implementation to threat things like drive prefixes specially.
 if host_machine.system() == 'windows' or not compiler.has_header('libgen.h')
@@ -1272,6 +1268,10 @@ if host_machine.system() != 'windows'
   endif
 endif
 
+if compiler.has_member('struct sysinfo', 'totalram', prefix: '#include <sys/sysinfo.h>')
+  libgit_c_args += '-DHAVE_SYSINFO'
+endif
+
 if compiler.has_member('struct stat', 'st_mtimespec.tv_nsec', prefix: '#include <sys/stat.h>')
   libgit_c_args += '-DUSE_ST_TIMESPEC'
 elif not compiler.has_member('struct stat', 'st_mtim.tv_nsec', prefix: '#include <sys/stat.h>')
-- 
2.49.0

[PATCH 4/6] meson: add a couple missing networking dependencies

[Eli Schwartz]

As evidenced in config.mak.uname and configure.ac, there are various
possible scenarios where these libraries are default-enabled in the
build, which mainly boils down to: SunOS. -lresolv is simply not the
only library that, when it exists, probably needs to be linked to for
networking.

Check for and add -lnsl -lsocket as well.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index 8037e536dd..8fad10379a 100644
--- a/meson.build
+++ b/meson.build
@@ -1080,10 +1080,11 @@ if host_machine.system() == 'windows'
     networking_dependencies += winsock
   endif
 else
-  libresolv = compiler.find_library('resolv', required: false)
-  if libresolv.found()
-    networking_dependencies += libresolv
-  endif
+  networking_dependencies += [
+    compiler.find_library('nsl', required: false),
+    compiler.find_library('resolv', required: false),
+    compiler.find_library('socket', required: false),
+  ]
 endif
 libgit_dependencies += networking_dependencies
 
-- 
2.49.0

[PATCH 5/6] meson: fix typo in function check that prevented checking for hstrerror

[Eli Schwartz]

Nowhere in the codebase do we otherwise check for strerror. Nowhere in
the codebase do we make use of -DNO_STRERROR. `strerror` is not a
networking function at all.

We do utilize `hstrerror` though, which is a networking function we
should have been checking here.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 8fad10379a..1b7e55756b 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,7 +1088,7 @@ else
 endif
 libgit_dependencies += networking_dependencies
 
-foreach symbol : ['inet_ntop', 'inet_pton', 'strerror']
+foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
   if not compiler.has_function(symbol, dependencies: networking_dependencies)
     libgit_c_args += '-DNO_' + symbol.to_upper()
   endif
-- 
2.49.0

[PATCH 6/6] meson: only check for missing networking syms on non-Windows; add compat impls

[Eli Schwartz]

These are added in the Makefile, but not in meson. They probably won't
work well on systems without them.

CMake adds them, but only on non-Windows. Actually, it only performs
compiler checks for hstrerror, but excludes that check on Windows with
the note that it is "incompatible with the Windows build". This seems to
be misleading -- it is not incompatible, it simply doesn't exist. Still,
the compat version should not be used.

I interpret this cmake logic to mean we shouldn't even be checking for
symbol availability on Windows. In addition to making it simple to add
compat definitions, this also probably shaves off a second or two of
configure time on Windows as no compiler check needs to be performed.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index 1b7e55756b..24b304fb57 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,11 +1088,14 @@ else
 endif
 libgit_dependencies += networking_dependencies
 
-foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
-  if not compiler.has_function(symbol, dependencies: networking_dependencies)
-    libgit_c_args += '-DNO_' + symbol.to_upper()
-  endif
-endforeach
+if host_machine.system() != 'windows'
+  foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
+    if not compiler.has_function(symbol, dependencies: networking_dependencies)
+      libgit_c_args += '-DNO_' + symbol.to_upper()
+      libgit_sources += 'compat/' + symbol + '.c'
+    endif
+  endforeach
+endif
 
 has_ipv6 = compiler.has_function('getaddrinfo', dependencies: networking_dependencies)
 if not has_ipv6
-- 
2.49.0

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

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 4850 bytes --]

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@gentoo.org>
> ---
>  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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Junio C Hamano]

Eli Schwartz <eschwartz@gentoo.org> writes:

> 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".


As there are non-zero number of meson related topics in flight, I'd
like to know where this new series is meant to apply, if you need
some of them before we can apply it, and what is the overall goal
this series has ("there is no theme, they are just random set of
changes to do such and such things" is perfectly acceptable answer).

And the best place to describe these things is in the cover letter
[PATCH 0/6] of the series.

Thanks.

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

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 2253 bytes --]

On 4/21/25 8:33 PM, Junio C Hamano wrote:
> Eli Schwartz <eschwartz@gentoo.org> writes:
> 
>> 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".
> 
> 
> As there are non-zero number of meson related topics in flight, I'd
> like to know where this new series is meant to apply, if you need
> some of them before we can apply it, and what is the overall goal
> this series has ("there is no theme, they are just random set of
> changes to do such and such things" is perfectly acceptable answer).
> 
> And the best place to describe these things is in the cover letter
> [PATCH 0/6] of the series.


My apologies. There was no big theme other than that they were things I
determined were relevant to more closely match the Makefile
expectations, while investigating a badly worded report (in fact, a
wholly uncommunicated :( local patch ) of git not building on Gentoo's
Solaris environment ( https://wiki.gentoo.org/wiki/Project:Prefix )

e.g. I simplified the repetitive lists because it made it easier to do
the followup patch adding a new check for getpagesize (which I needed,
because it needed to be checked on Solaris).

I think that I sort of subconsciously assumed that "if in doubt, assume
it's independently developed against the current state of the master
branch".

I do not need any other series merged, I think it should apply to
`master` independently of all of them. I can't see anything available in
origin/master..origin/seen that would clash, at least.


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Patrick Steinhardt]

On Mon, Apr 21, 2025 at 01:51:45PM -0400, 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.

Yeah, this is somewhat unfortunate indeed. I think in the long term we
might want to unify our approach so that we consistently use e.g.
`HAVE_SOME_FUNCTION` or `NO_SOME_FUNCTION`.

> Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
> ---
>  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

Our current code style puts a space both before and after the colon.

> +  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

I think this is a bit too magic-y. An alternative might be to have
`checkfuncs` be a dictionary where the value of each function is the
compat sources that we fall back to.

Other than that I really like the simplifications introduced by this
patch, thanks!

Patrick

Re: [PATCH 2/6] meson: check for getpagesize before using it

[Patrick Steinhardt]

On Mon, Apr 21, 2025 at 01:51:46PM -0400, Eli Schwartz wrote:
> It is deprecated and removed in SUS v3 / POSIX 2001, so various systems
> may not include it. Solaris, in particular, carefully refrains from
> defining it except inside of a maze of `#ifdef` to make sure you have
> kept your nose clean and only used it in code that *targets* SUS v2 or
> earlier.
> 
> config.mak.uname defines this automatically, though only for QNX.

Ah, interesting. I mostly went by our autoconf infrastructure when
converting the checks, which didn't have a check for `getpagesize()`
either. We might want to teach autoconf to check for this function while
at it.

In all honesty though, I rather hope that we're soon in a state where we
can just drop autoconf altogether in favor of Meson. The only two
blockers I'm aware of are wiring up git-gui and gitk. The former project
has already been adapted upstream, the latter is still in review. But
once those have landed, we should be ready to mark Meson as stable and
then we can start deprecating autoconf unless there are good reasons not
to do so.

Patrick

Re: [PATCH 3/6] meson: do a full usage-based compile check for sysinfo

[Patrick Steinhardt]

On Mon, Apr 21, 2025 at 01:51:47PM -0400, Eli Schwartz wrote:
> diff --git a/meson.build b/meson.build
> index f5d9ffcd7f..8037e536dd 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1058,10 +1058,6 @@ if compiler.has_header('alloca.h')
>    libgit_c_args += '-DHAVE_ALLOCA_H'
>  endif
>  
> -if compiler.has_header('sys/sysinfo.h')
> -  libgit_c_args += '-DHAVE_SYSINFO'
> -endif
> -
>  # Windows has libgen.h and a basename implementation, but we still need our own
>  # implementation to threat things like drive prefixes specially.
>  if host_machine.system() == 'windows' or not compiler.has_header('libgen.h')
> @@ -1272,6 +1268,10 @@ if host_machine.system() != 'windows'
>    endif
>  endif
>  
> +if compiler.has_member('struct sysinfo', 'totalram', prefix: '#include <sys/sysinfo.h>')
> +  libgit_c_args += '-DHAVE_SYSINFO'
> +endif

Makes sense. We do have c9a51775a36 (builtin/gc.c: correct RAM
calculation when using sysinfo, 2025-04-17) in flight which also causes
us to use `struct sysinfo::mem_unit`. But I think it's fine to check for
only one of the members here.

Patrick

Re: [PATCH 4/6] meson: add a couple missing networking dependencies

[Patrick Steinhardt]

On Mon, Apr 21, 2025 at 01:51:48PM -0400, Eli Schwartz wrote:
> As evidenced in config.mak.uname and configure.ac, there are various
> possible scenarios where these libraries are default-enabled in the
> build, which mainly boils down to: SunOS. -lresolv is simply not the
> only library that, when it exists, probably needs to be linked to for
> networking.
> 
> Check for and add -lnsl -lsocket as well.
> 
> Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
> ---
>  meson.build | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 8037e536dd..8fad10379a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1080,10 +1080,11 @@ if host_machine.system() == 'windows'
>      networking_dependencies += winsock
>    endif
>  else
> -  libresolv = compiler.find_library('resolv', required: false)
> -  if libresolv.found()
> -    networking_dependencies += libresolv
> -  endif
> +  networking_dependencies += [
> +    compiler.find_library('nsl', required: false),
> +    compiler.find_library('resolv', required: false),
> +    compiler.find_library('socket', required: false),
> +  ]
>  endif
>  libgit_dependencies += networking_dependencies

Fair. We could extend this check to verify which combination of
libraries we actually require to make desired functions available. But
I'm not sure whether that would really be worth the effort.

Patrick

Re: [PATCH 5/6] meson: fix typo in function check that prevented checking for hstrerror

[Patrick Steinhardt]

On Mon, Apr 21, 2025 at 01:51:49PM -0400, Eli Schwartz wrote:
> Nowhere in the codebase do we otherwise check for strerror. Nowhere in
> the codebase do we make use of -DNO_STRERROR. `strerror` is not a
> networking function at all.
> 
> We do utilize `hstrerror` though, which is a networking function we
> should have been checking here.
> 
> Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
> ---
>  meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index 8fad10379a..1b7e55756b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1088,7 +1088,7 @@ else
>  endif
>  libgit_dependencies += networking_dependencies
>  
> -foreach symbol : ['inet_ntop', 'inet_pton', 'strerror']
> +foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
>    if not compiler.has_function(symbol, dependencies: networking_dependencies)
>      libgit_c_args += '-DNO_' + symbol.to_upper()
>    endif

Good catch and obviously correct, thanks!

Patrick

Re: [PATCH 6/6] meson: only check for missing networking syms on non-Windows; add compat impls

[Patrick Steinhardt]

On Mon, Apr 21, 2025 at 01:51:50PM -0400, Eli Schwartz wrote:
> These are added in the Makefile, but not in meson. They probably won't
> work well on systems without them.
> 
> CMake adds them, but only on non-Windows. Actually, it only performs
> compiler checks for hstrerror, but excludes that check on Windows with
> the note that it is "incompatible with the Windows build". This seems to
> be misleading -- it is not incompatible, it simply doesn't exist. Still,
> the compat version should not be used.

CMake only checks for `hstrerror()` though -- it doesn't check for the
other functions at all.

> I interpret this cmake logic to mean we shouldn't even be checking for
> symbol availability on Windows. In addition to making it simple to add
> compat definitions, this also probably shaves off a second or two of
> configure time on Windows as no compiler check needs to be performed.

I dunno. In this case I'd lean towards just using the check on Windows,
too. The less platform-specific configuration we do the easier the build
system is to reason about.

> Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
> ---
>  meson.build | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 1b7e55756b..24b304fb57 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1088,11 +1088,14 @@ else
>  endif
>  libgit_dependencies += networking_dependencies
>  
> -foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
> -  if not compiler.has_function(symbol, dependencies: networking_dependencies)
> -    libgit_c_args += '-DNO_' + symbol.to_upper()
> -  endif
> -endforeach
> +if host_machine.system() != 'windows'
> +  foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
> +    if not compiler.has_function(symbol, dependencies: networking_dependencies)
> +      libgit_c_args += '-DNO_' + symbol.to_upper()
> +      libgit_sources += 'compat/' + symbol + '.c'
> +    endif
> +  endforeach
> +endif

We do have compat sources for `inet_ntop()` and `inet_pton()` indeed, so
adding those makes sense. But we don't have a replacement for
`hstrerror()`, so if that function wasn't found we would error out
because "compat/hstrerror.c" wasn't found.

Patrick

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

[Patrick Steinhardt]

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.

> 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? :)

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.

> > -# 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?

Inconsistencies like this are what you get in a codebase that is 20
years old. Many things have grown organically, and one hand doesn't
always know what the other hand is doing.

I agree that in the best case we'd unify these. But unfortunately, this
is not trivial because those are part of the build interface for our
Makefiles. People may have `HAVE_GETDELIM = YesPlease` in their
`config.mak` file, and we don't want to break this usecase. So if we
were to change this in the future, we'd have to also introduce shims for
backwards compatibility.

Patrick

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

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> Yeah, this is somewhat unfortunate indeed. I think in the long term we
> might want to unify our approach so that we consistently use e.g.
> `HAVE_SOME_FUNCTION` or `NO_SOME_FUNCTION`.

Yes and no, because the intention, at least in the original form,
was that HAVE_FOO is not necessarily !NO_FOO.  You cannot claim to
HAVE_FOO on a system that does not have FOO, and expect the result
to build and/or function, but you should be able to build with
NO_FOO on a system that supports FOO and use an alternative
implementation that does not rely on system-supplied FOO.  NO_MMAP
and NO_REGEX comes to mind (I often have to build NO_REGEX locally
when doing "make sparse", for example, to avoid warnings triggering
on system headers, for example).

I suspect that majority of these feature symbols do not fall into
the same category as NO_REGEX, i.e. the user/builder may choose to
decline using the system-supplied one.  So for them this is a total
overkill, but conceptually, building with feature FOO enabled should
be done iff (HAVE_FOO && !NO_FOO).

I do not mind changing the stance Makefile takes on HAVE/!NO
division, and see us declare that from now on, even when
auto-detection flips HAVE_FOO on, the way for the builder to decline
use of FOO is to flip HAVE_FOO off manually.  I and others may need
to tweak some scripts and figure out to pass !HAVE_REGEX instead of
NO_REGEX when that happens, but that is a one-time cost to make
things more consistent.

Re: [PATCH 6/6] meson: only check for missing networking syms on non-Windows; add compat impls

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 3260 bytes --]

On 4/22/25 3:31 AM, Patrick Steinhardt wrote:
> On Mon, Apr 21, 2025 at 01:51:50PM -0400, Eli Schwartz wrote:
>> These are added in the Makefile, but not in meson. They probably won't
>> work well on systems without them.
>>
>> CMake adds them, but only on non-Windows. Actually, it only performs
>> compiler checks for hstrerror, but excludes that check on Windows with
>> the note that it is "incompatible with the Windows build". This seems to
>> be misleading -- it is not incompatible, it simply doesn't exist. Still,
>> the compat version should not be used.
> 
> CMake only checks for `hstrerror()` though -- it doesn't check for the
> other functions at all.


Right, that's what I meant.

"cmake, like this patch, adds the compat/*.c when checking function
availability. Actually, it only performs compiler checks for hstrerror,
but does add the compat impl in that case".


>> I interpret this cmake logic to mean we shouldn't even be checking for
>> symbol availability on Windows. In addition to making it simple to add
>> compat definitions, this also probably shaves off a second or two of
>> configure time on Windows as no compiler check needs to be performed.
> 
> I dunno. In this case I'd lean towards just using the check on Windows,
> too. The less platform-specific configuration we do the easier the build
> system is to reason about.


Maybe, but the issue is that it appears on Windows it is not correct to
add the compat impl for hstrerror, which would still be a
platform-specific configuration. Do I check for hstrerror everywhere but
configure Windows to not add the compat impl, but do add it for
inet_ntop and inet_pton? That is more configuration than skipping the
checks on Windows...



>> Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
>> ---
>>  meson.build | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 1b7e55756b..24b304fb57 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -1088,11 +1088,14 @@ else
>>  endif
>>  libgit_dependencies += networking_dependencies
>>  
>> -foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
>> -  if not compiler.has_function(symbol, dependencies: networking_dependencies)
>> -    libgit_c_args += '-DNO_' + symbol.to_upper()
>> -  endif
>> -endforeach
>> +if host_machine.system() != 'windows'
>> +  foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
>> +    if not compiler.has_function(symbol, dependencies: networking_dependencies)
>> +      libgit_c_args += '-DNO_' + symbol.to_upper()
>> +      libgit_sources += 'compat/' + symbol + '.c'
>> +    endif
>> +  endforeach
>> +endif
> 
> We do have compat sources for `inet_ntop()` and `inet_pton()` indeed, so
> adding those makes sense. But we don't have a replacement for
> `hstrerror()`, so if that function wasn't found we would error out
> because "compat/hstrerror.c" wasn't found.


I don't really understand what you mean by this. Of course the file will
be found.

$ grep hstrerror compat/hstrerror.c
const char *githstrerror(int err)


File is there. (The function name is then #defined by compat/posix.h, no
comment.)



-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 3006 bytes --]

On 4/22/25 3:31 AM, Patrick Steinhardt wrote:
> 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...


>> 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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Patrick Steinhardt]

On Tue, Apr 22, 2025 at 11:36:06AM -0400, Eli Schwartz wrote:
> On 4/22/25 3:31 AM, Patrick Steinhardt wrote:
> > On Mon, Apr 21, 2025 at 04:04:30PM -0400, Eli Schwartz wrote:
> >> On 4/21/25 1:51 PM, Eli Schwartz wrote:
> > 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.

The question to me is whether this is still an issue that we need to
care about nowadays. If we _know_ that it is still an issue that we need
to address then I'm okay with adapting as required. But if we think that
it's probably not an issue anymore then I'd rather wait and see whether
anybody complains. Because if nobody does, then we can eventually just
throw out this logic altogether.

I treat this as a bit of a canary. Git is quite old by now, so we need
to question existing infrastructure every now and then so that we can in
the best case throw out unneeded bits and pieces every once in a while
as the surrounding ecosystem matures.

Patrick

Re: [PATCH 6/6] meson: only check for missing networking syms on non-Windows; add compat impls

[Patrick Steinhardt]

On Tue, Apr 22, 2025 at 11:27:03AM -0400, Eli Schwartz wrote:
> On 4/22/25 3:31 AM, Patrick Steinhardt wrote:
> > On Mon, Apr 21, 2025 at 01:51:50PM -0400, Eli Schwartz wrote:
> > We do have compat sources for `inet_ntop()` and `inet_pton()` indeed, so
> > adding those makes sense. But we don't have a replacement for
> > `hstrerror()`, so if that function wasn't found we would error out
> > because "compat/hstrerror.c" wasn't found.
> 
> 
> I don't really understand what you mean by this. Of course the file will
> be found.
> 
> $ grep hstrerror compat/hstrerror.c
> const char *githstrerror(int err)
> 
> 
> File is there. (The function name is then #defined by compat/posix.h, no
> comment.)

Oops, I somehow missed this file altogether. Please ignore this.

Patrick

Re: [PATCH 2/6] meson: check for getpagesize before using it

[Junio C Hamano]

Eli Schwartz <eschwartz@gentoo.org> writes:

> It is deprecated and removed in SUS v3 / POSIX 2001, so various systems
> may not include it. Solaris, in particular, carefully refrains from
> defining it except inside of a maze of `#ifdef` to make sure you have
> kept your nose clean and only used it in code that *targets* SUS v2 or
> earlier.
>
> config.mak.uname defines this automatically, though only for QNX.
>
> Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
> ---
>  meson.build | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 6c147c22a4..f5d9ffcd7f 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1300,6 +1300,8 @@ checkfuncs = [
>    'mkdtemp',
>    # no compat
>    'initgroups',
> +  # no compat
> +  'getpagesize',
>  ]
>  
>  if host_machine.system() == 'windows'

Is this related to this breakge we started seeing for 'seen'
recently?

https://github.com/git/git/actions/runs/14653573748/job/41124519642

Re: [PATCH 2/6] meson: check for getpagesize before using it

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 1489 bytes --]

On 4/24/25 7:48 PM, Junio C Hamano wrote:
> Eli Schwartz <eschwartz@gentoo.org> writes:
> 
>> It is deprecated and removed in SUS v3 / POSIX 2001, so various systems
>> may not include it. Solaris, in particular, carefully refrains from
>> defining it except inside of a maze of `#ifdef` to make sure you have
>> kept your nose clean and only used it in code that *targets* SUS v2 or
>> earlier.
>>
>> config.mak.uname defines this automatically, though only for QNX.
>>
>> Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
>> ---
>>  meson.build | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 6c147c22a4..f5d9ffcd7f 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -1300,6 +1300,8 @@ checkfuncs = [
>>    'mkdtemp',
>>    # no compat
>>    'initgroups',
>> +  # no compat
>> +  'getpagesize',
>>  ]
>>  
>>  if host_machine.system() == 'windows'
> 
> Is this related to this breakge we started seeing for 'seen'
> recently?
> 
> https://github.com/git/git/actions/runs/14653573748/job/41124519642


Yes.

compat/mingw.c defines mingw_getpagesize, and posix.h defines -- when
the function is detected as missing:

#define getpagesize()  sysconf(_SC_PAGESIZE)


That means we can't check for it on mingw or we end up with two
definitions. I will move it to the else block in:

if host_machine.system() == 'windows'
  libgit_c_args += '-DUSE_WIN32_MMAP'
else



-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[PATCH v2 0/6] meson: miscellaneous system detection fixes

[Eli Schwartz]

Principally motivated to handle an issue where these were failing to
detect the system properly, on Solaris.

Eli Schwartz (6):
  meson: simplify and parameterize various standard function checks
  meson: check for getpagesize before using it
  meson: do a full usage-based compile check for sysinfo
  meson: add a couple missing networking dependencies
  meson: fix typo in function check that prevented checking for
    hstrerror
  meson: only check for missing networking syms on non-Windows; add
    compat impls

 meson.build | 105 ++++++++++++++++++++++++----------------------------
 1 file changed, 49 insertions(+), 56 deletions(-)

Range-diff against v1:
1:  e137afaff2 = 1:  e137afaff2 meson: simplify and parameterize various standard function checks
2:  df82ee7872 ! 2:  3c4918a7b3 meson: check for getpagesize before using it
    @@ Commit message
         config.mak.uname defines this automatically, though only for QNX.
     
      ## meson.build ##
    -@@ meson.build: checkfuncs = [
    -   'mkdtemp',
    -   # no compat
    -   'initgroups',
    -+  # no compat
    -+  'getpagesize',
    - ]
    +@@ meson.build: else
    +     'mmap',
    +     # unsetenv is provided by compat/mingw.c.
    +     'unsetenv',
    ++    # no compat, is provided by compat/mingw.c
    ++    'getpagesize',
    +   ]
    + endif
      
    - if host_machine.system() == 'windows'
3:  2ec759d7be = 3:  2c19f04f3e meson: do a full usage-based compile check for sysinfo
4:  61f2addd47 = 4:  e62057094f meson: add a couple missing networking dependencies
5:  c67d10a337 = 5:  a6a7bdacd1 meson: fix typo in function check that prevented checking for hstrerror
6:  c5b8b89dd4 = 6:  15f48b8991 meson: only check for missing networking syms on non-Windows; add compat impls
-- 
2.49.0

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

[Eli Schwartz]

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.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 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'
 
-# 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
 
-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'
-- 
2.49.0

[PATCH v2 2/6] meson: check for getpagesize before using it

[Eli Schwartz]

It is deprecated and removed in SUS v3 / POSIX 2001, so various systems
may not include it. Solaris, in particular, carefully refrains from
defining it except inside of a maze of `#ifdef` to make sure you have
kept your nose clean and only used it in code that *targets* SUS v2 or
earlier.

config.mak.uname defines this automatically, though only for QNX.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---

v2: add this only for !windows

 meson.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson.build b/meson.build
index 6c147c22a4..29c188af99 100644
--- a/meson.build
+++ b/meson.build
@@ -1309,6 +1309,8 @@ else
     'mmap',
     # unsetenv is provided by compat/mingw.c.
     'unsetenv',
+    # no compat, is provided by compat/mingw.c
+    'getpagesize',
   ]
 endif
 
-- 
2.49.0

[PATCH v2 3/6] meson: do a full usage-based compile check for sysinfo

[Eli Schwartz]

On Solaris, sys/sysinfo.h is a completely different file and doesn't
resemble the linux file at all. There is also a sysinfo() function, but
it takes a totally different call signature, which asks for:

- the field you wish to receive
- a `char *buf` to copy the data to

and is very useful IFF you want to know, say, the hardware provider. Or,
get *specific* fields from uname(2).

https://docs.oracle.com/cd/E86824_01/html/E54765/sysinfo-2.html

It is surely possible to do this manually via `sysconf(3)` without the
nice API. I can't find anything more direct. Either way, I'm not very
attached to Solaris, so someone who cares can add it. Either way, it's
wrong to assume that sysinfo.h contains what we are looking for.

Check that sysinfo.h defines the struct we actually utilize in
builtins/gc.c, which will correctly fail on systems that don't have it.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index 29c188af99..ea0722a216 100644
--- a/meson.build
+++ b/meson.build
@@ -1058,10 +1058,6 @@ if compiler.has_header('alloca.h')
   libgit_c_args += '-DHAVE_ALLOCA_H'
 endif
 
-if compiler.has_header('sys/sysinfo.h')
-  libgit_c_args += '-DHAVE_SYSINFO'
-endif
-
 # Windows has libgen.h and a basename implementation, but we still need our own
 # implementation to threat things like drive prefixes specially.
 if host_machine.system() == 'windows' or not compiler.has_header('libgen.h')
@@ -1272,6 +1268,10 @@ if host_machine.system() != 'windows'
   endif
 endif
 
+if compiler.has_member('struct sysinfo', 'totalram', prefix: '#include <sys/sysinfo.h>')
+  libgit_c_args += '-DHAVE_SYSINFO'
+endif
+
 if compiler.has_member('struct stat', 'st_mtimespec.tv_nsec', prefix: '#include <sys/stat.h>')
   libgit_c_args += '-DUSE_ST_TIMESPEC'
 elif not compiler.has_member('struct stat', 'st_mtim.tv_nsec', prefix: '#include <sys/stat.h>')
-- 
2.49.0

[PATCH v2 4/6] meson: add a couple missing networking dependencies

[Eli Schwartz]

As evidenced in config.mak.uname and configure.ac, there are various
possible scenarios where these libraries are default-enabled in the
build, which mainly boils down to: SunOS. -lresolv is simply not the
only library that, when it exists, probably needs to be linked to for
networking.

Check for and add -lnsl -lsocket as well.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index ea0722a216..ff65d36cac 100644
--- a/meson.build
+++ b/meson.build
@@ -1080,10 +1080,11 @@ if host_machine.system() == 'windows'
     networking_dependencies += winsock
   endif
 else
-  libresolv = compiler.find_library('resolv', required: false)
-  if libresolv.found()
-    networking_dependencies += libresolv
-  endif
+  networking_dependencies += [
+    compiler.find_library('nsl', required: false),
+    compiler.find_library('resolv', required: false),
+    compiler.find_library('socket', required: false),
+  ]
 endif
 libgit_dependencies += networking_dependencies
 
-- 
2.49.0

[PATCH v2 5/6] meson: fix typo in function check that prevented checking for hstrerror

[Eli Schwartz]

Nowhere in the codebase do we otherwise check for strerror. Nowhere in
the codebase do we make use of -DNO_STRERROR. `strerror` is not a
networking function at all.

We do utilize `hstrerror` though, which is a networking function we
should have been checking here.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index ff65d36cac..7927c54dc3 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,7 +1088,7 @@ else
 endif
 libgit_dependencies += networking_dependencies
 
-foreach symbol : ['inet_ntop', 'inet_pton', 'strerror']
+foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
   if not compiler.has_function(symbol, dependencies: networking_dependencies)
     libgit_c_args += '-DNO_' + symbol.to_upper()
   endif
-- 
2.49.0

[PATCH v2 6/6] meson: only check for missing networking syms on non-Windows; add compat impls

[Eli Schwartz]

These are added in the Makefile, but not in meson. They probably won't
work well on systems without them.

CMake adds them, but only on non-Windows. Actually, it only performs
compiler checks for hstrerror, but excludes that check on Windows with
the note that it is "incompatible with the Windows build". This seems to
be misleading -- it is not incompatible, it simply doesn't exist. Still,
the compat version should not be used.

I interpret this cmake logic to mean we shouldn't even be checking for
symbol availability on Windows. In addition to making it simple to add
compat definitions, this also probably shaves off a second or two of
configure time on Windows as no compiler check needs to be performed.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index 7927c54dc3..59a7782da0 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,11 +1088,14 @@ else
 endif
 libgit_dependencies += networking_dependencies
 
-foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
-  if not compiler.has_function(symbol, dependencies: networking_dependencies)
-    libgit_c_args += '-DNO_' + symbol.to_upper()
-  endif
-endforeach
+if host_machine.system() != 'windows'
+  foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
+    if not compiler.has_function(symbol, dependencies: networking_dependencies)
+      libgit_c_args += '-DNO_' + symbol.to_upper()
+      libgit_sources += 'compat/' + symbol + '.c'
+    endif
+  endforeach
+endif
 
 has_ipv6 = compiler.has_function('getaddrinfo', dependencies: networking_dependencies)
 if not has_ipv6
-- 
2.49.0

Re: [PATCH v2 0/6] meson: miscellaneous system detection fixes

[Patrick Steinhardt]

On Thu, Apr 24, 2025 at 08:13:29PM -0400, Eli Schwartz wrote:
> Principally motivated to handle an issue where these were failing to
> detect the system properly, on Solaris.

I was expecting to also see my comments addressed around the one style
issue as well as the comment regarding `fs.exists()` being a bit too
magical (both mentioned in [1]). This is the only remaining item that
I'd like to see addressed, and other than that this series looks good to
me.

Patrick

[1]: <aAdFysi-n_5Aa4Au@pks.im>

[PATCH v3 0/6] meson: miscellaneous system detection fixes

[Eli Schwartz]

Principally motivated to handle an issue where these were failing to
detect the system properly, on Solaris.

Changes:

v2: getpagesize check moved to !windows
v3: style fixes, pass source files in

Eli Schwartz (6):
  meson: simplify and parameterize various standard function checks
  meson: check for getpagesize before using it
  meson: do a full usage-based compile check for sysinfo
  meson: add a couple missing networking dependencies
  meson: fix typo in function check that prevented checking for
    hstrerror
  meson: only check for missing networking syms on non-Windows; add
    compat impls

 meson.build | 117 +++++++++++++++++++++-------------------------------
 1 file changed, 48 insertions(+), 69 deletions(-)

Range-diff against v2:
1:  e137afaff2 ! 1:  037d2f8610 meson: simplify and parameterize various standard function checks
    @@ Commit message
         prefixes are used.
     
      ## meson.build ##
    +@@ meson.build: else
    +   build_options_config.set('NO_UNIX_SOCKETS', '1')
    + endif
    + 
    +-if not compiler.has_function('pread')
    +-  libgit_c_args += '-DNO_PREAD'
    +-  libgit_sources += 'compat/pread.c'
    +-endif
    +-
    + if host_machine.system() == 'darwin'
    +   libgit_sources += 'compat/precompose_utf8.c'
    +   libgit_c_args += '-DPRECOMPOSE_UNICODE'
     @@ meson.build: if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#include <pwd.h
        libgit_c_args += '-DNO_GECOS_IN_PWENT'
      endif
    @@ meson.build: if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#i
     -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',
    -+]
    ++checkfuncs = {
    ++  'strcasestr' : ['strcasestr.c'],
    ++  'memmem' : ['memmem.c'],
    ++  'strlcpy' : ['strlcpy.c'],
    ++  'strtoull' : [],
    ++  'setenv' : ['setenv.c'],
    ++  'mkdtemp' : ['mkdtemp.c'],
    ++  'initgroups' : [],
    ++  'strtoumax' : ['strtoumax.c', 'strtoimax.c'],
    ++  'pread' : ['pread.c'],
    ++}
      
     -if not compiler.has_function('strcasestr')
     -  libgit_c_args += '-DNO_STRCASESTR'
    @@ meson.build: if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#i
     +if host_machine.system() == 'windows'
     +  libgit_c_args += '-DUSE_WIN32_MMAP'
     +else
    -+  checkfuncs += [
    -+    'mmap',
    -+    # unsetenv is provided by compat/mingw.c.
    -+    'unsetenv',
    -+  ]
    ++  checkfuncs += {
    ++    'mmap' : ['mmap.c'],
    ++    # provided by compat/mingw.c.
    ++    'unsetenv' : ['unsetenv.c'],
    ++  }
      endif
      
     -if not compiler.has_function('memmem')
     -  libgit_c_args += '-DNO_MEMMEM'
     -  libgit_sources += 'compat/memmem.c'
     -endif
    -+foreach func: checkfuncs
    ++foreach func, impls : 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
    ++    foreach impl : impls
    ++      libgit_sources += 'compat/' + impl
    ++    endforeach
     +  endif
     +endforeach
      
    @@ meson.build: if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#i
      endif
      
      if not compiler.has_function('strdup')
    -@@ meson.build: if not compiler.has_function('strtoumax')
    -   ]
    +@@ meson.build: if not compiler.has_function('strdup')
    +   libgit_sources += 'compat/strdup.c'
      endif
      
    +-if not compiler.has_function('strtoumax')
    +-  libgit_c_args += '-DNO_STRTOUMAX'
    +-  libgit_sources += [
    +-    'compat/strtoumax.c',
    +-    'compat/strtoimax.c',
    +-  ]
    +-endif
    +-
     -if not compiler.has_function('strtoull')
     -  libgit_c_args += '-DNO_STRTOULL'
     -endif
2:  3c4918a7b3 ! 2:  291ac2579f meson: check for getpagesize before using it
    @@ Commit message
     
      ## meson.build ##
     @@ meson.build: else
    -     'mmap',
    -     # unsetenv is provided by compat/mingw.c.
    -     'unsetenv',
    -+    # no compat, is provided by compat/mingw.c
    -+    'getpagesize',
    -   ]
    +     'mmap' : ['mmap.c'],
    +     # provided by compat/mingw.c.
    +     'unsetenv' : ['unsetenv.c'],
    ++    # provided by compat/mingw.c.
    ++    'getpagesize' : [],
    +   }
      endif
      
3:  2c19f04f3e = 3:  9af41a0c23 meson: do a full usage-based compile check for sysinfo
4:  e62057094f = 4:  041859574f meson: add a couple missing networking dependencies
5:  a6a7bdacd1 = 5:  6e20afb77f meson: fix typo in function check that prevented checking for hstrerror
6:  15f48b8991 = 6:  a60c55bb02 meson: only check for missing networking syms on non-Windows; add compat impls
-- 
2.49.0

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

[Eli Schwartz]

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.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---

v3:
- use a dictionary for checkfuncs, and list the filenames
- now we can handle strtoumax too, so, do that
- pread was overlooked previously; move it down and handle it with the
  other function checks

 meson.build | 85 ++++++++++++++++++-----------------------------------
 1 file changed, 29 insertions(+), 56 deletions(-)

diff --git a/meson.build b/meson.build
index c47cb79af0..ed0359b9c9 100644
--- a/meson.build
+++ b/meson.build
@@ -1133,11 +1133,6 @@ else
   build_options_config.set('NO_UNIX_SOCKETS', '1')
 endif
 
-if not compiler.has_function('pread')
-  libgit_c_args += '-DNO_PREAD'
-  libgit_sources += 'compat/pread.c'
-endif
-
 if host_machine.system() == 'darwin'
   libgit_sources += 'compat/precompose_utf8.c'
   libgit_c_args += '-DPRECOMPOSE_UNICODE'
@@ -1290,23 +1285,39 @@ 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' : ['strcasestr.c'],
+  'memmem' : ['memmem.c'],
+  'strlcpy' : ['strlcpy.c'],
+  'strtoull' : [],
+  'setenv' : ['setenv.c'],
+  'mkdtemp' : ['mkdtemp.c'],
+  'initgroups' : [],
+  'strtoumax' : ['strtoumax.c', 'strtoimax.c'],
+  'pread' : ['pread.c'],
+}
 
-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' : ['mmap.c'],
+    # provided by compat/mingw.c.
+    'unsetenv' : ['unsetenv.c'],
+  }
 endif
 
-if not compiler.has_function('memmem')
-  libgit_c_args += '-DNO_MEMMEM'
-  libgit_sources += 'compat/memmem.c'
-endif
+foreach func, impls : checkfuncs
+  if not compiler.has_function(func)
+    libgit_c_args += '-DNO_' + func.to_upper()
+    foreach impl : impls
+      libgit_sources += 'compat/' + impl
+    endforeach
+  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')
@@ -1314,53 +1325,15 @@ if not compiler.has_function('strdup')
   libgit_sources += 'compat/strdup.c'
 endif
 
-if not compiler.has_function('strtoumax')
-  libgit_c_args += '-DNO_STRTOUMAX'
-  libgit_sources += [
-    'compat/strtoumax.c',
-    'compat/strtoimax.c',
-  ]
-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'
 
-# 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
 
-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'
-- 
2.49.0

[PATCH v3 2/6] meson: check for getpagesize before using it

[Eli Schwartz]

It is deprecated and removed in SUS v3 / POSIX 2001, so various systems
may not include it. Solaris, in particular, carefully refrains from
defining it except inside of a maze of `#ifdef` to make sure you have
kept your nose clean and only used it in code that *targets* SUS v2 or
earlier.

config.mak.uname defines this automatically, though only for QNX.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---

v2: add this only for !windows

v3: rebase now that it is a dict

 meson.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson.build b/meson.build
index ed0359b9c9..e575231129 100644
--- a/meson.build
+++ b/meson.build
@@ -1304,6 +1304,8 @@ else
     'mmap' : ['mmap.c'],
     # provided by compat/mingw.c.
     'unsetenv' : ['unsetenv.c'],
+    # provided by compat/mingw.c.
+    'getpagesize' : [],
   }
 endif
 
-- 
2.49.0

[PATCH v3 3/6] meson: do a full usage-based compile check for sysinfo

[Eli Schwartz]

On Solaris, sys/sysinfo.h is a completely different file and doesn't
resemble the linux file at all. There is also a sysinfo() function, but
it takes a totally different call signature, which asks for:

- the field you wish to receive
- a `char *buf` to copy the data to

and is very useful IFF you want to know, say, the hardware provider. Or,
get *specific* fields from uname(2).

https://docs.oracle.com/cd/E86824_01/html/E54765/sysinfo-2.html

It is surely possible to do this manually via `sysconf(3)` without the
nice API. I can't find anything more direct. Either way, I'm not very
attached to Solaris, so someone who cares can add it. Either way, it's
wrong to assume that sysinfo.h contains what we are looking for.

Check that sysinfo.h defines the struct we actually utilize in
builtins/gc.c, which will correctly fail on systems that don't have it.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index e575231129..b21b191d25 100644
--- a/meson.build
+++ b/meson.build
@@ -1058,10 +1058,6 @@ if compiler.has_header('alloca.h')
   libgit_c_args += '-DHAVE_ALLOCA_H'
 endif
 
-if compiler.has_header('sys/sysinfo.h')
-  libgit_c_args += '-DHAVE_SYSINFO'
-endif
-
 # Windows has libgen.h and a basename implementation, but we still need our own
 # implementation to threat things like drive prefixes specially.
 if host_machine.system() == 'windows' or not compiler.has_header('libgen.h')
@@ -1267,6 +1263,10 @@ if host_machine.system() != 'windows'
   endif
 endif
 
+if compiler.has_member('struct sysinfo', 'totalram', prefix: '#include <sys/sysinfo.h>')
+  libgit_c_args += '-DHAVE_SYSINFO'
+endif
+
 if compiler.has_member('struct stat', 'st_mtimespec.tv_nsec', prefix: '#include <sys/stat.h>')
   libgit_c_args += '-DUSE_ST_TIMESPEC'
 elif not compiler.has_member('struct stat', 'st_mtim.tv_nsec', prefix: '#include <sys/stat.h>')
-- 
2.49.0

[PATCH v3 4/6] meson: add a couple missing networking dependencies

[Eli Schwartz]

As evidenced in config.mak.uname and configure.ac, there are various
possible scenarios where these libraries are default-enabled in the
build, which mainly boils down to: SunOS. -lresolv is simply not the
only library that, when it exists, probably needs to be linked to for
networking.

Check for and add -lnsl -lsocket as well.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index b21b191d25..66b69f2471 100644
--- a/meson.build
+++ b/meson.build
@@ -1080,10 +1080,11 @@ if host_machine.system() == 'windows'
     networking_dependencies += winsock
   endif
 else
-  libresolv = compiler.find_library('resolv', required: false)
-  if libresolv.found()
-    networking_dependencies += libresolv
-  endif
+  networking_dependencies += [
+    compiler.find_library('nsl', required: false),
+    compiler.find_library('resolv', required: false),
+    compiler.find_library('socket', required: false),
+  ]
 endif
 libgit_dependencies += networking_dependencies
 
-- 
2.49.0

[PATCH v3 5/6] meson: fix typo in function check that prevented checking for hstrerror

[Eli Schwartz]

Nowhere in the codebase do we otherwise check for strerror. Nowhere in
the codebase do we make use of -DNO_STRERROR. `strerror` is not a
networking function at all.

We do utilize `hstrerror` though, which is a networking function we
should have been checking here.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 66b69f2471..25bac8d89f 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,7 +1088,7 @@ else
 endif
 libgit_dependencies += networking_dependencies
 
-foreach symbol : ['inet_ntop', 'inet_pton', 'strerror']
+foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
   if not compiler.has_function(symbol, dependencies: networking_dependencies)
     libgit_c_args += '-DNO_' + symbol.to_upper()
   endif
-- 
2.49.0

[PATCH v3 6/6] meson: only check for missing networking syms on non-Windows; add compat impls

[Eli Schwartz]

These are added in the Makefile, but not in meson. They probably won't
work well on systems without them.

CMake adds them, but only on non-Windows. Actually, it only performs
compiler checks for hstrerror, but excludes that check on Windows with
the note that it is "incompatible with the Windows build". This seems to
be misleading -- it is not incompatible, it simply doesn't exist. Still,
the compat version should not be used.

I interpret this cmake logic to mean we shouldn't even be checking for
symbol availability on Windows. In addition to making it simple to add
compat definitions, this also probably shaves off a second or two of
configure time on Windows as no compiler check needs to be performed.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 meson.build | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index 25bac8d89f..fbe43be949 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,11 +1088,14 @@ else
 endif
 libgit_dependencies += networking_dependencies
 
-foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
-  if not compiler.has_function(symbol, dependencies: networking_dependencies)
-    libgit_c_args += '-DNO_' + symbol.to_upper()
-  endif
-endforeach
+if host_machine.system() != 'windows'
+  foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']
+    if not compiler.has_function(symbol, dependencies: networking_dependencies)
+      libgit_c_args += '-DNO_' + symbol.to_upper()
+      libgit_sources += 'compat/' + symbol + '.c'
+    endif
+  endforeach
+endif
 
 has_ipv6 = compiler.has_function('getaddrinfo', dependencies: networking_dependencies)
 if not has_ipv6
-- 
2.49.0

Re: [PATCH v2 0/6] meson: miscellaneous system detection fixes

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 766 bytes --]

On 4/25/25 12:39 AM, Patrick Steinhardt wrote:
> On Thu, Apr 24, 2025 at 08:13:29PM -0400, Eli Schwartz wrote:
>> Principally motivated to handle an issue where these were failing to
>> detect the system properly, on Solaris.
> 
> I was expecting to also see my comments addressed around the one style
> issue as well as the comment regarding `fs.exists()` being a bit too
> magical (both mentioned in [1]). This is the only remaining item that
> I'd like to see addressed, and other than that this series looks good to
> me.


I overlooked the style comment, will fix.

Regarding the fs.exists I was worried that it would be repetitive and
unwieldy but I think I found a good solution, as a bonus I can now
handle strtoumax.


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH v3 0/6] meson: miscellaneous system detection fixes

[Patrick Steinhardt]

On Fri, Apr 25, 2025 at 01:25:39AM -0400, Eli Schwartz wrote:
> Principally motivated to handle an issue where these were failing to
> detect the system properly, on Solaris.
> 
> Changes:
> 
> v2: getpagesize check moved to !windows
> v3: style fixes, pass source files in

This looks as expected to me now. Thanks for working on it!

Patrick