Hello Michel,

according to my understanding, there are no strict rules what goes in libcyrus, libcyrus_min and libcyrus_imap . I am not aware of a guidelines, describing in which library to include what kind of file. While working on getting Automake/libtool in cyrus-imapd, I moved some files between libcyrus and libcyrus_min according to how I felt the things.

lib/parseaddr.c is used within libcyrus_sieve and imap/, so having it in the common library libcyrus seems logical. I do not know, why iostat.c is part of libcyrus.

If nobody else expresses opinion, whether to put xjson in libcyrus or libcyrus_imap, it is up to you. I just told you my opinion.

I have no opinion about the minimal required version for json.

The comment in the Makefile state, that LD_BASIC_ADD is used to link with programs, linking with libcyrus and libcyrus_min, while LD_UTILITY_ADD is used to link with all command line tools (that are not invoked as services from master). -lcrypto is always provided, even if the linked program does not use libcrypto.

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



On 14.08.2012 16:21, Sébastien Michel wrote:
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

<<attachment: dilyan_palauzov.vcf>>

Reply via email to