Automake Support for cyrus-imapd 2.5

Дилян Палаузов dilyan.palauzov at
Tue Apr 17 16:50:08 EDT 2012


> Third and final tranche of review comments.

> commit " merge com_err/et/"
> I think this has already been covered, but using top_builddir in the
> rather than in will break when the library is
> actually found in /usr/lib
> +LIBS = ${top_builddir}/@COM_ERR_LIBS@

It is now fixed.

imap/imap_err.h imap/imap_err.c: imap/
         cd imap && $(COMPILE_ET) ../$(top_srcdir)/imap/

and the same for sieve/, imap/ and 

> commit " merge lib/"
> This is wrong.  PERL_CCCDLFLAGS should only be used for the code which
> is built into the perl extension shared library.  In particular, it
> contains -fPIC which we do not want anywhere else if we can avoid it.

Greg, I admit that you have more understanding about Perl than me.  I 
kindly ask you to fix the Perl stuff, including the issue.

Why are those EXTRACFLAGS necessary at all?  Can't AM_CFLAGS be used 

> +#this is from lib/test/, however testglob.c does not exist
> in
> +#/lib/test, instead testglob2.c is there.
> +#lib_test_testglob_LIRBARIES = lib/libcyrus.a lib/libcyrus_min.a
> -ldb-4.0
> Everything in lib/test/ will be either moved to cunit/ or removed,
> eventually, and is currently not built. You probably shouldn't bother
> with it.

Yes, but in order for "make dist" to put the all files in the tarball, 
these files need to mentioned somehow.  Anyway, once the 
automake-building works, I suggest to have a separate discussion on how 
to implement "make snapshot"/"make dist"/generation of xversion.h, etc. 
  Let's stick now to the dependancy and compilation issues.

> commit " merge sieve/"
> +       sieve/test/testExtension ....
> Not only is this line far too long, but the directory is sieve/tests/
> not sieve/test/

I renamed all sieve/test/ to sieve/tests/ .

> commit " merge notifyd/"
> Looks good, although I note that notifytest is not built at all
> currently but is built in the new Makefile.

notifytest is build by "make check".

> commit " merge syslog/"
> I think it's time we deleted the entire syslog/ directory.  I don't
> believe there's any platform on which it will be built, nor be useful.

I deleted the syslog/ files and the SYSLOG Automake conditional.

Please check if the syslog checks in can be deleted, too.

> commit " merge ptclient/"
> +#ptclient_ptextract_SOURCES = imap/cli_fatal.o imap/mutex_fake.o
> ptclient/ptextrac.c
> s/ptextrac/ptextract/
> +ptclient_ptloader_SOURCES = ptclient/ptloader.c ptclient/ptloader.h
> ptclient/afskrb.oc ptclient/ldap.c imap/mutex_fake.o
> master/service-thread.c
> s/afskrb.oc/afskrb.c/

> commit " merge installsieve/"
>>     It is unclear why this directory exist, since there is no place, that
>>     installs installsieve.
> Another relic, let's remove it too.

I deleted installsieve/* .

> commit "Include SIEVE/scripts/ in "make dist""
>>      It is unclear why this script is neede (and installsieve/installsieve)
>>     in parallel to perl/sieve/scripts/(sieveshell|installsieve).pl
> I don't see any good reason for this to continue existing.

I deleted it.

> commit " merge doc/(Makefile.dist, text/Makefile)"
> +man/cyradm.1.html:
> +       @$(MKDIR_P) man
> +       pod2html $(top_srcdir)/perl/imap/>  man/cyradm.1.html
> +       rm man/pod2htm*
> man/cyradm.1.html should depend on $(top_srcdir)/perl/imap/

The dependency on is fixed now.

> Also, why the rm??

master:doc/ contains:

         mkdir -p man
         for man in ../man/*.[1-9]; \
         do \
                 echo "=== $$man ==="; \
                 groff -man -Thtml $$man > man/`basename $$man`.html; \

         pod2html ../perl/imap/ > man/cyradm.1.html
         pod2man ../perl/sieve/scripts/ > ../man/sieveshell.1
         pod2html ../perl/sieve/scripts/ > 

         rm -f groff-html-*.png pod2htm*

         fig2dev -L png murder.fig murder.png
         fig2dev -L png netnews.fig netnews.png

         (cd text; make)

And the rm comes from there.

> +       @$(MKDIR_P) doc
> +       @$(MKDIR_P) doc/text
> The first of these lines is redundant.


> commit " merge imap/"
> +         $(LN_S) -f lmtpd lmtpprodyx
> s/lmtpprodyx/lmtpproxy/

> +       cd imap&&  $(top_builddir)/../$(COMPILE_ET)
> ../$(top_srcdir)/imap/
> This is wrong in so many ways.
See above.

> +AM_CONDITIONAL([WITH_OPENSSL], [test "$with_openssl" != "no"])
> For consistency, the name of the conditional should be OPENSSL

Със здраве

More information about the Cyrus-devel mailing list