Re: [PATCH 2/2] tools: Drop libsystemd as a dependency

[Anthony PERARD <anthony.perard@xxxxxxxxx>]

On Thu, Apr 25, 2024 at 06:32:16PM +0100, Andrew Cooper wrote:
> diff --git a/m4/systemd.m4 b/m4/systemd.m4
> index 112dc11b5e05..aa1ebe94f56c 100644
> --- a/m4/systemd.m4
> +++ b/m4/systemd.m4
> @@ -41,15 +41,6 @@ AC_DEFUN([AX_ALLOW_SYSTEMD_OPTS], [
>  ])
>  
>  AC_DEFUN([AX_CHECK_SYSTEMD_LIBS], [
> -     PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon],,
> -             [PKG_CHECK_MODULES([SYSTEMD], [libsystemd >= 209])]
> -        )
> -     dnl pkg-config older than 0.24 does not set these for
> -     dnl PKG_CHECK_MODULES() worth also noting is that as of version 208
> -     dnl of systemd pkg-config --cflags currently yields no extra flags yet.
> -     AC_SUBST([SYSTEMD_CFLAGS])
> -     AC_SUBST([SYSTEMD_LIBS])
> -
>       AS_IF([test "x$SYSTEMD_DIR" = x], [
>           dnl In order to use the line below we need to fix upstream systemd
>           dnl to properly ${prefix} for child variables in
> @@ -83,23 +74,11 @@ AC_DEFUN([AX_CHECK_SYSTEMD_LIBS], [
>  AC_DEFUN([AX_CHECK_SYSTEMD], [
>       dnl Respect user override to disable
>       AS_IF([test "x$enable_systemd" != "xno"], [
> -          AS_IF([test "x$systemd" = "xy" ], [
> -             AC_DEFINE([HAVE_SYSTEMD], [1], [Systemd available and enabled])
> -                     systemd=y
> -                     AX_CHECK_SYSTEMD_LIBS()

I think you need to keep calling AX_CHECK_SYSTEMD_LIBS() here,
otherwise, nothing sets $SYSTEMD_DIR or $SYSTEMD_MODULES_LOAD.

> -         ],[
> -             AS_IF([test "x$enable_systemd" = "xyes"],
> -                     [AC_MSG_ERROR([Unable to find systemd development 
> library])],
> -                     [systemd=n])
> -         ])
> +     systemd=y
>       ],[systemd=n])
>  ])
>  
>  AC_DEFUN([AX_CHECK_SYSTEMD_ENABLE_AVAILABLE], [
> -     PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon], [systemd="y"],[
> -             PKG_CHECK_MODULES([SYSTEMD], [libsystemd >= 209],
> -                               [systemd="y"],[systemd="n"])
> -     ])

Instead, or in addition, you could AX_AVAILABLE_SYSTEMD() in
configure.ac by AX_ENABLE_SYSTEMD(). (Or AX_ALLOW_SYSTEMD()).

With the current patch, AX_CHECK_SYSTEMD() will enable systemd
"support", even if it supposed to be disabled by default. So it's better
to use AX_ENABLE_SYSTEMD() as this will set the correct help message.

And can you add an entry in CHANGELOG? As systemd support isn't
automatically enabled with the presence of the libs anymore.

Thanks,

-- 
Anthony PERARD