2:21 a.m.
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.