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

<<attachment: dilyan_palauzov.vcf>>

Reply via email to