> please delete imap/Makefile.in : all the build rules are in /Makefile.am .
It's already done. Sorry, I just forgot to mention it. > >>> 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. OK. 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 Makefile.am : # 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/libcyrus.la lib/libcyrus_min.la ${LIBS} # UTILITY is the libraries that utility programs which use Cyrus' # mailbox and message handling code need to link with. LD_UTILITY_ADD = imap/libcyrus_imap.la $(LD_BASIC_ADD) $(COM_ERR_LIBS) 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 libcyrus.la or lib/libcyrus_min.la I noticed that some files like iostat.c and parseaddr.c are not used within lib/ but are integrated within libcyrus.la. Thus, I think I should also integrate xjson.c within libcyrus.la (not libcyrus_min) and not move it to imap/ ... >>> (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. > Ok Thanks, Sébastien
