commit "mboxevent: Rewrite JSON formatting"

Sébastien Michel sebastien.michel at
Tue Aug 14 10:21:38 EDT 2012

> please delete imap/ : all the build rules are in / .

It's already done. Sorry, I just forgot to mention it.

>>> As libjson supports the .pc format, you can detect libjson in
>>> with
>>>    PKG_CHECK_MODULES ([libjson], [json >= 0.10], [check_libjson=yes],
>>> [check_libjson=no])
>>> and remove the handling.  Possibly add an
>>> AM_CONDITIONAL([LIBJSON], ["$check_libjson" = "yes"]).  I know currently
>>> cyrus-imapd does not use the
>>> PKG_CHECK_MODULES-routine, but on the other side there are no other
>>> external modules, which can be detected by it.
>>> I do not have recipe to how to trivially #define HAVE_LIBJSON when using
>>> libjson, but this shall not be hard to handle.
>> I've used PKG_CHECK_MODULES and it looks fine. However I have noticed some
>> issues :
>> - By default, the macro will set up two variables, joining the given
>> prefix with the suffixes _CFLAGS and _LIBS.
>> The macro will call AC_SUBST on these variables only with pkg-config 0.24
>> and later
>> What do you prefer between always calling AC_SUBST  or testing the
>> pkg-config version ?
>> - Using PKG_CHECK_MODULES in conditional block would raise a failure.
>> Calling PKG_PROG_PKG_CONFIG seems to be one possible solution :
>> - some people discourage the use of such macro :
> I am in favour of PKG_(PROG_)PKG_CONFIG, but if you prefer not to use it,
> then you can leave as it is.

Can we consider that we support only version 0.24 minimum ?

>>> Then shift the lib/xjson.c and lib/xjson.h to imap/,
>> xjson.[ch] doesn't depends on any structure/function in imap folder. As a
>> basic utility I think it should be located in lib.
>> Why do you prefer move it here ?
> Provided that the json code is integrated and used within libcyrus_imap (and
> not libcyrus(_min), then the files have to go under imap/ -- there are all
> libcyrus_imap sources.  While for the future it would be wise, to have the
> source files for libcyrus_imap in a separate directory, for the next major
> release the directory structure will not change.  This has nothing to do
> with being a basic utility or not.

I don't understand than you this comment from :
# BASIC is the libraries that every Cyrus program (except master) will
# need to link with.
# Note that several places in the code use -lcrypto, e.g. for SHA1 or
# MD5 algorithms, without needing SSL.  Currently we have no way of
# minimally linking such code.
LD_BASIC_ADD = lib/ lib/ ${LIBS}

# UTILITY is the libraries that utility programs which use Cyrus'
# mailbox and message handling code need to link with.

As xjson may be linked with every Cyrus program except master (ie.
imapd, pop3d, lmtpd, cyr_expire, ipurge, ...)
I understand that it should be added in or lib/
I noticed that some files like iostat.c and parseaddr.c are not used
within lib/ but are integrated within Thus, I think I
should also integrate xjson.c within (not libcyrus_min)
and not move it to imap/ ...

>>> (in the beginning of, for correct CPPFLAGS) AM_CFLAGS = ....
>>> $(libjson_CFLAGS)
>> I don't know no what is better between that or this declaration below :
>> imap_libcyrus_imap_la_CFLAGS += $(JSON_CFLAGS)
> Having different CFLAGS per target, implies that a single source file can be
> compiled several times for different targets with different -D directives.
> In order to avoid disasters, Automake renames the resulting .o files to be
> very long, and to contain the target, for which the files are compiled.
> This long file names are avoided, if only one (AM_)CFLAGS is used.
> Currently you will not see a difference, since the -fvisibility= switch is
> only half-done, and there are per library CFLAGS, but once I get it right,
> no files in imap/.libs will start with imap_libcyrus_imap_X.o, but be called
> only X.o.


More information about the Cyrus-devel mailing list