Re: [PATCH weston v2] configure.ac: fixup systemd/systemd-login detection

2018-03-16 Thread Emil Velikov
On 15 March 2018 at 17:23, Michael Tretter  wrote:
> On Thu, 15 Mar 2018 14:20:21 +, Emil Velikov wrote:
>> From: Emil Velikov 
>>
>> Current systemd/systemd-login integration requires dbus. Although that
>> is far from clear the way current checks are handled.
>>
>> Be explicit and clear, effectively fixing cases where the systemd
>> auto detection will trip when dbus is explicitly disabled.
>>
>> Using git show -w will make things easier to read ;-)
>>
>> v2: auto disable systemd as dbus is missing
>>
>> Cc: Michael Tretter 
>> Reported-by: Michael Tretter 
>> Signed-off-by: Emil Velikov 
>> ---
>> Thanks for the review Michael!
>>
>> You're right - the else hunk is only for consistency with surrounding
>> code.
>> ---
>>  configure.ac | 28 +---
>>  1 file changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 0b326ccc..a3453d15 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -524,7 +524,12 @@ AC_ARG_ENABLE(systemd-login,
>>AS_HELP_STRING([--enable-systemd-login],
>>   [Enable logind support]),,
>>enable_systemd_login=auto)
>> -if test x$enable_systemd_login != xno -a x$have_dbus != xno; then
>> +
>> +if test x$enable_systemd_login = xyes -a x$enable_dbus = xno; then
>> +  AC_MSG_ERROR([systemd-login support explicitly requested, but dbus not 
>> available])
>> +fi
>> +
>> +if test x$enable_systemd_login = xauto -a x$enable_dbus = xno; then
>
> If systemd-login is explicitly enabled, it will not enable
> systemd-login. The second condition is also wrong, because dbus must be
> enabled (or auto). It should read
>
> if test x$enable_systemd_login != xno -a x$enable_dbus != xno; then
>
Indeed, things are fiddly and subtle.

Pekka pulled your simpler fix so this lovely patch can go in the bin.

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v2] configure.ac: fixup systemd/systemd-login detection

2018-03-15 Thread Michael Tretter
On Thu, 15 Mar 2018 14:20:21 +, Emil Velikov wrote:
> From: Emil Velikov 
> 
> Current systemd/systemd-login integration requires dbus. Although that
> is far from clear the way current checks are handled.
> 
> Be explicit and clear, effectively fixing cases where the systemd
> auto detection will trip when dbus is explicitly disabled.
> 
> Using git show -w will make things easier to read ;-)
> 
> v2: auto disable systemd as dbus is missing
> 
> Cc: Michael Tretter 
> Reported-by: Michael Tretter 
> Signed-off-by: Emil Velikov 
> ---
> Thanks for the review Michael!
> 
> You're right - the else hunk is only for consistency with surrounding
> code.
> ---
>  configure.ac | 28 +---
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 0b326ccc..a3453d15 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -524,7 +524,12 @@ AC_ARG_ENABLE(systemd-login,
>AS_HELP_STRING([--enable-systemd-login],
>   [Enable logind support]),,
>enable_systemd_login=auto)
> -if test x$enable_systemd_login != xno -a x$have_dbus != xno; then
> +
> +if test x$enable_systemd_login = xyes -a x$enable_dbus = xno; then
> +  AC_MSG_ERROR([systemd-login support explicitly requested, but dbus not 
> available])
> +fi
> +
> +if test x$enable_systemd_login = xauto -a x$enable_dbus = xno; then

If systemd-login is explicitly enabled, it will not enable
systemd-login. The second condition is also wrong, because dbus must be
enabled (or auto). It should read

if test x$enable_systemd_login != xno -a x$enable_dbus != xno; then

Michael

>PKG_CHECK_MODULES(SYSTEMD_LOGIN,
>  [libsystemd >= 209],
>  [have_systemd_login_209=yes;have_systemd_login=yes],
> @@ -537,21 +542,22 @@ if test x$enable_systemd_login != xno -a x$have_dbus != 
> xno; then
>[have_systemd_login=yes],
>[have_systemd_login=no])
>  ])
> +
> +  if test "x$have_systemd_login" = "xno" -a "x$enable_systemd_login" = 
> "xyes"; then
> +AC_MSG_ERROR([systemd-login support explicitly requested, but can't find 
> libsystemd>=209 or libsystemd-login])
> +  fi
> +
> +  if test "x$have_systemd_login" = "xyes"; then
> +AC_DEFINE([HAVE_SYSTEMD_LOGIN], [1], [Have systemd-login])]
> +  fi
> +
> +  AS_IF([test "x$have_systemd_login_209" = "xyes"],
> +[AC_DEFINE([HAVE_SYSTEMD_LOGIN_209], [1], [Have systemd-login >= 
> 209])])
>  else
>have_systemd_login=no
>  fi
> -
> -if test "x$have_systemd_login" = "xno" -a "x$enable_systemd_login" = "xyes"; 
> then
> -  AC_MSG_ERROR([systemd-login support explicitly enabled, but can't find 
> libsystemd>=209, libsystemd-login or dbus])
> -fi
> -
> -AS_IF([test "x$have_systemd_login" = "xyes"],
> -  [AC_DEFINE([HAVE_SYSTEMD_LOGIN], [1], [Have systemd-login])])
>  AM_CONDITIONAL(HAVE_SYSTEMD_LOGIN, test "x$have_systemd_login" = "xyes")
>  
> -AS_IF([test "x$have_systemd_login_209" = "xyes"],
> -  [AC_DEFINE([HAVE_SYSTEMD_LOGIN_209], [1], [Have systemd-login >= 
> 209])])
> -
>  
>  # Note that other features might want libxml2, or this feature might use
>  # alternative xml libraries at some point. Therefore the feature and
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v2] configure.ac: fixup systemd/systemd-login detection

2018-03-15 Thread Emil Velikov
From: Emil Velikov 

Current systemd/systemd-login integration requires dbus. Although that
is far from clear the way current checks are handled.

Be explicit and clear, effectively fixing cases where the systemd
auto detection will trip when dbus is explicitly disabled.

Using git show -w will make things easier to read ;-)

v2: auto disable systemd as dbus is missing

Cc: Michael Tretter 
Reported-by: Michael Tretter 
Signed-off-by: Emil Velikov 
---
Thanks for the review Michael!

You're right - the else hunk is only for consistency with surrounding
code.
---
 configure.ac | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/configure.ac b/configure.ac
index 0b326ccc..a3453d15 100644
--- a/configure.ac
+++ b/configure.ac
@@ -524,7 +524,12 @@ AC_ARG_ENABLE(systemd-login,
   AS_HELP_STRING([--enable-systemd-login],
  [Enable logind support]),,
   enable_systemd_login=auto)
-if test x$enable_systemd_login != xno -a x$have_dbus != xno; then
+
+if test x$enable_systemd_login = xyes -a x$enable_dbus = xno; then
+  AC_MSG_ERROR([systemd-login support explicitly requested, but dbus not 
available])
+fi
+
+if test x$enable_systemd_login = xauto -a x$enable_dbus = xno; then
   PKG_CHECK_MODULES(SYSTEMD_LOGIN,
 [libsystemd >= 209],
 [have_systemd_login_209=yes;have_systemd_login=yes],
@@ -537,21 +542,22 @@ if test x$enable_systemd_login != xno -a x$have_dbus != 
xno; then
   [have_systemd_login=yes],
   [have_systemd_login=no])
 ])
+
+  if test "x$have_systemd_login" = "xno" -a "x$enable_systemd_login" = "xyes"; 
then
+AC_MSG_ERROR([systemd-login support explicitly requested, but can't find 
libsystemd>=209 or libsystemd-login])
+  fi
+
+  if test "x$have_systemd_login" = "xyes"; then
+AC_DEFINE([HAVE_SYSTEMD_LOGIN], [1], [Have systemd-login])]
+  fi
+
+  AS_IF([test "x$have_systemd_login_209" = "xyes"],
+[AC_DEFINE([HAVE_SYSTEMD_LOGIN_209], [1], [Have systemd-login >= 
209])])
 else
   have_systemd_login=no
 fi
-
-if test "x$have_systemd_login" = "xno" -a "x$enable_systemd_login" = "xyes"; 
then
-  AC_MSG_ERROR([systemd-login support explicitly enabled, but can't find 
libsystemd>=209, libsystemd-login or dbus])
-fi
-
-AS_IF([test "x$have_systemd_login" = "xyes"],
-  [AC_DEFINE([HAVE_SYSTEMD_LOGIN], [1], [Have systemd-login])])
 AM_CONDITIONAL(HAVE_SYSTEMD_LOGIN, test "x$have_systemd_login" = "xyes")
 
-AS_IF([test "x$have_systemd_login_209" = "xyes"],
-  [AC_DEFINE([HAVE_SYSTEMD_LOGIN_209], [1], [Have systemd-login >= 209])])
-
 
 # Note that other features might want libxml2, or this feature might use
 # alternative xml libraries at some point. Therefore the feature and
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel