Automake Support for cyrus-imapd 2.5

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

Hello,

> Third and final tranche of review comments.

> commit "Makefile.am: merge com_err/et/Makefile.in"
>
> I think this has already been covered, but using top_builddir in the
> Makefile.in rather than in configure.ac 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/imap_err.et
         cd imap && $(COMPILE_ET) ../$(top_srcdir)/imap/imap_err.et

and the same for sieve/sieve_err.et, imap/mupdate_err.et and 
imap/nntp_err.et

> commit "Makefile.am: merge lib/Makefile.in"
>
> +AM_CFLAGS = @PERL_CCCDLFLAGS@ $(EXTRACFLAGS)
>
> 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 
https://bugzilla.cyrusimap.org/show_bug.cgi?id=3678 issue.

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

> +#this is from lib/test/Makefile.in, 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 "Makefile.am: merge sieve/Makefile.in"
>
> +       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 "Makefile.am: merge notifyd/Makefile.in"
>
> 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 "Makefile.am: merge syslog/Makefile.in"
>
> 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 configure.ac can be deleted, too.

> commit "Makefile.am: merge ptclient/Makefile.in"
>
> +#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/
Fixed.

> commit "Makefile.am: merge installsieve/Makefile.in"
>
>>     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/sieveshell.pl 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 "Makefile.am: merge doc/(Makefile.dist, text/Makefile)"
>
> +man/cyradm.1.html:
> +       @$(MKDIR_P) man
> +       pod2html $(top_srcdir)/perl/imap/cyradm.sh>  man/cyradm.1.html
> +       rm man/pod2htm*
>
>
> man/cyradm.1.html should depend on $(top_srcdir)/perl/imap/cyradm.sh

The dependency on cyradm.sh is fixed now.

> Also, why the rm??

master:doc/Makefile.in contains:

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

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

         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.

Fixed.

> commit "Makefile.am: merge imap/Makefile.in"
>
> +         $(LN_S) -f lmtpd lmtpprodyx
>
> s/lmtpprodyx/lmtpproxy/
Fixed.

> +       cd imap&&  $(top_builddir)/../$(COMPILE_ET)
> ../$(top_srcdir)/imap/imap_err.et
>
> 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
Fixed.

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