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

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 22, 2025, 07:31 UTC
Message-ID: <aAdF3eu1heCycaLJ@pks.im>
In-Reply-To: <20250421175247.240971-6-eschwartz@gentoo.org>

On Mon, Apr 21, 2025 at 01:51:50PM -0400, Eli Schwartz wrote:

Show 8 quoted lines

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

Show 26 quoted lines

> 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