commit "mboxevent: Rewrite JSON formatting"

[Дилян Палаузов]

Hello Michel,

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

>> As libjson supports the .pc format, you can detect libjson in configure.ac with
>>    PKG_CHECK_MODULES ([libjson], [json >= 0.10], [check_libjson=yes],
>> [check_libjson=no])
>> and remove the configure.ac:AC_CHECK_LIB 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 : http://www.flameeyes.eu/autotools-mythbuster/pkgconfig/pkg_check_modules.html#idp1027760
>
> - some people discourage the use of such macro : http://stackoverflow.com/questions/10220946/pkg-check-modules-considered-harmful

I am in favour of PKG_(PROG_)PKG_CONFIG, but if you prefer not to use 
it, then you can leave configure.ac as it is.

>> 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.

>> (in the beginning of Makefile.am, 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.

Със здраве
   Дилян

>
> Cheers,
>
> Sébastien
>
> -----Message d'origine-----
> De : Дилян Палаузов [mailto:dilyan.palauzov at aegee.org]
> Envoyé : dimanche 1 juillet 2012 16:37
> À : Michel Sébastien
> Objet : commit "mboxevent: Rewrite JSON formatting"
>
> Hello Sebastien,
>
> you have put the support for -ljson in libcyrus_min .
>
> According to my understanding, in libcyrus_min come only the files needed by the master process (as it does not link with libcyrus).  If you use libjson in the imap/ folder, and not all command line utilities need the functionality from json, then I would at your place shift the handling of json from libcyrus_min to libcyrus_imap., so that the library is not loaded unnecessary.
>
> As libjson supports the .pc format, you can detect libjson in configure.ac with
>     PKG_CHECK_MODULES ([libjson], [json >= 0.10], [check_libjson=yes],
> [check_libjson=no])
> and remove the configure.ac:AC_CHECK_LIB 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.
>
> Then shift the lib/xjson.c and lib/xjson.h to imap/,
>
> intergrate xjson.c conditionally with libcyrus_imap with Makefile.am (or unconditionally, if the files are compiled even if libjson is not found, in this case the above AM_CONDITIONAL is not necessary):
> if LIBJSON
> imap_libcyrus_imap_la_SOURCES += imap/xjson.c imap/xjson.h imap_libcyrus_imap_la_LIBADD += $(libjson_LIBS) endif
>
> (in the beginning of Makefile.am, for correct CPPFLAGS) AM_CFLAGS = ....
> $(libjson_CFLAGS)
>
> Please delete in the branch ticket/3605 the file imap/Makefile.in added by commit "added support for event notifications on message store based of RFC 5423" from August 2011 (I know there was no automake support then, but now there is).
>
> Please add imap/mboxevent.h to imap_libcyrus_imap_SOURCES, otherwise the file will not get into the tarball.
>
> Със здраве
>     Дилян
>
>
> Ce message et les pièces jointes sont confidentiels et réservés à l'usage exclusif de ses destinataires. Il peut également être protégé par le secret professionnel. Si vous recevez ce message par erreur, merci d'en avertir immédiatement l'expéditeur et de le détruire. L'intégrité du message ne pouvant être assurée sur Internet, la responsabilité d'Atos ne pourra être recherchée quant au contenu de ce message. Bien que les meilleurs efforts soient faits pour maintenir cette transmission exempte de tout virus, l'expéditeur ne donne aucune garantie à cet égard et sa responsabilité ne saurait être recherchée pour tout dommage résultant d'un virus transmis.
>
> This e-mail and the documents attached are confidential and intended solely for the addressee; it may also be privileged. If you receive this e-mail in error, please notify the sender immediately and destroy it. As its integrity cannot be secured on the Internet, the Atos liability cannot be triggered for the message content. Although the sender endeavours to maintain a computer virus-free network, the sender does not warrant that this transmission is virus-free and will not be liable for any damages resulting from any virus transmitted.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dilyan_palauzov.vcf
Type: text/x-vcard
Size: 380 bytes
Desc: not available
Url : http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20120813/c432afa3/attachment.vcf