Automake Support for cyrus-imapd 2.5

Greg Banks gnb at
Tue Apr 17 03:14:34 EDT 2012


Third and final tranche of review comments.

On Sun, Apr 15, 2012, at 01:31 AM, Дилян Палаузов wrote:
> Hello,
> at git:// , branch dpa/automake I have 
> patched cyrus-imapd/master to support Automake .

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@

commit " replace mkdir with"

Looks good

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.

+#this is from lib/test/, however testglob.c does not exist
+#/lib/test, instead testglob2.c is there.
+#lib_test_testglob_LIRBARIES = lib/libcyrus.a lib/libcyrus_min.a

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.

commit " merge sieve/"

+       sieve/test/testExtension ....

Not only is this line far too long, but the directory is sieve/tests/
not sieve/test/

commit " merge master/"

Looks good.

commit " merge perl/sieve/lib/"

Looks good

commit " merge netnews/"

Looks good

commit " merge depot/"

Looks good.

commit " merge notifyd/"

Looks good, although I note that notifytest is not built at all
currently but is built in the new Makefile.

commit " merge timsieved/"

+       timsieved/TODO

Given that a) this file has not been updated since 2003, and b) we have
Bugzilla, I think we can delete this.

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.

commit " merge ptclient/"

+#ptclient_ptextract_SOURCES = imap/cli_fatal.o imap/mutex_fake.o


+ptclient_ptloader_SOURCES = ptclient/ptloader.c ptclient/ptloader.h
ptclient/afskrb.oc ptclient/ldap.c imap/mutex_fake.o


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.

commit "Remove makedepend"

Looks good.

commit "Include tools/* and snmp/* in "make dist""

Looks good

commit "Include the files from / in "make dist""

Looks good

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.

commit "Include the files from contrib/ in "make dist""

Looks good

commit " merge imtest/"

Looks good

commit " merge doc/(Makefile.dist, text/Makefile)"

+       @$(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/

Also, why the rm??

+       @$(MKDIR_P) doc
+       @$(MKDIR_P) doc/text

The first of these lines is redundant.

commit " merge perl/(sieve,.)/"

No comment, this was reworked later.

commit " order alphabetically build of service programs"

Looks good

commit " merge imap/"

+         $(LN_S) -f lmtpd lmtpprodyx


+       cd imap && $(top_builddir)/../$(COMPILE_ET)

This is wrong in so many ways.

+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