On Do, 2011-06-16 at 08:02 +0100, Salvatore Iovene wrote: > please review the following patch: > > diff --git a/configure-pre.in b/configure-pre.in > index 65a06b5..9d6cc50 100644 > --- a/configure-pre.in > +++ b/configure-pre.in > @@ -501,15 +501,17 @@ if test $enable_dbus_service = "yes"; then > AC_DEFINE(HAS_NOTIFY, 1, [define if libnotify could be used in dbus > service]) > fi > > - # Here we're using QtGui too because mlite fails to depend on it, > - # despite using QAction. > - PKG_CHECK_MODULES(MLITE, [mlite QtGui], HAVE_MLITE=yes, HAVE_MLITE=no) > + AS_IF([test "x$enable_mlite" = "xyes"], > + # Here we're using QtGui too because mlite fails to depend on it, > + # despite using QAction. > + PKG_CHECK_MODULES(MLITE, [mlite QtGui], HAVE_MLITE=yes, > HAVE_MLITE=no), > + []) > AC_ARG_ENABLE(mlite, > AS_HELP_STRING([--enable-mlite], > [send notifications for automatic sync > events, using mlite]), > - [ test "$enableval" = "no" || test $HAVE_MLITE = "yes" || > AC_ERROR([required mlite package not found]) ], > - [ test $HAVE_MLITE = "yes" || AC_ERROR([required mlite > package not found, use --disable-mlite to compile without mlite based > notifications]) ]) > - if test $HAVE_MLITE = "yes"; then > + [test "$enableval" = "yes" && test $HAVE_MLITE = "no" && > AC_ERROR([requested mlite package not found]) ], > + [ test $HAVE_MLITE = "yes" && enable_mlite=yes ]) > + if test $enable_mlite = "yes"; then > AC_DEFINE(HAS_MLITE, 1, [define if mlite could be used in dbus > service]) > fi > AC_DEFINE(DBUS_SERVICE, 1, [define if dbus service is enabled])
I find the ordering of setting HAVE_MLITE and checking the enable flag suspicious. Isn't this a cyclic dependency which simply can't work? Indeed, without --enable-mlite I get: /home/pohly/syncevolution/syncevolution/configure: line 16865: test: =: unary operator expected /home/pohly/syncevolution/syncevolution/configure: line 16868: test: =: unary operator expected That is for: 16865: test $HAVE_MLITE = "yes" && enable_mlite=yes ... 16868: if test $enable_mlite = "yes"; then It fails because HAVE_MLITE is not set yet at that line. I'll solve this as in the check for libnotify: first do the PKG_CONFIG check (unconditionally), then use the result in AC_ENABLE. The default also still was to use mlite when installed, independent of the enable flag. I'd prefer to have it enabled explicitly. I also wonder about the explicit --enable case: enable_mlite was never set.... diff --git a/src/NotificationBackendMLite.cpp b/src/NotificationBackendMLite.cpp index 040fdd0..e3abb1f 100644 --- a/src/NotificationBackendMLite.cpp +++ b/src/NotificationBackendMLite.cpp @@ -17,6 +17,8 @@ * 02110-1301 USA */ +#ifdef HAS_MLITE + You need to include config.h first. Otherwise HAS_MLITE will never be set. I think I have fixed all issues and will commit to "master" for today's SyncEvolution snapshot. I tried the relevant combinations (mlite installed/not installed, no enable flag/--enable-mlite/--disable-mlite) and it worked. I know that the flexibility has a price of increased complexity, but it is worth it. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. _______________________________________________ SyncEvolution mailing list [email protected] http://lists.syncevolution.org/listinfo/syncevolution
