Automake Support for cyrus-imapd 2.5

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

Hello,

My comments on Greg's 2nd comments on my comments on my commits:

> commit "*/Makefile.in: add top_(builddir,srcdir) to CPPFLAGS"
>
> +CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib -I$(top_builddir)
> -I$(top_builddir)/lib @COM_ERR_CPPFLAGS@ @CPPFLAGS@ @SASLFLAGS@
>
> Surely the correct order is
>
> -I$(top_builddir)
> -I$(top_srcdir)
> -I$(top_builddir)/lib
> -I$(top_srcdir)/lib
>
> ?

Yes.

>
> Also, why do we have "-I. -I$(srcdir)" in some CPPFLAGS but not others?

The final Makefile.am does not have -I$(srcdir), probably as 
intermediate step -I$(srcdir) was necessary.

> commit "perl/Makefile*: enable building perl modules out of srcdir"
>
> +                       if [ -f ../${srcdir}/$$d/Makefile.PL -a ! -f
> Makefile ]; then \
>
> This breaks if $srcdir is an absolute rather than a relative path.  In
> fact so does this
>
> +                          srcdir="../${srcdir}/$$d" \
> +                          top_srcdir="../$(top_srcdir)" \
>
> It might be a whole lot simpler to generate Makefile.PL from
> Makefile.PL.in at configure time.  Then, you could run the Makefile.PL
> from configure using AC_CONFIG_COMMANDS.
>
> +    INSTALLDIRS =>  'vendor',
>
> Please don't do that!  I just had to do a whole bunch of futzing to
> avoid it :(

INSTALLDIRS =>  'vendor' was initially there, then it was removed by you 
on Friday with commit abd6dcd95abd329b3a (Revert "Bug 3652 - install 
Perl modules into 'vendor' dirs"), but somehow was not merged very 
correctly in my branch.

I think the perl builds deserve a separate discussion.

> commit "imap/Makefile.in: order everything alphabetically"
>
> My comments from lib/Makefile.in about splitting these up 1 per line
> also apply here.
>
> In LOBJS, imapparse,o, message.o, saslclient.o, saslserver.o are out of
> order.
>
> In IMAPDOBJS, proxy.o is out of order.
>
> In PROGS, cyr_dbtool and cyrdump are out of order
>
> There's a reason why $SERVICE ought to be linked first, or at the very
> least before any libraries from outside the project: it contains main()
> and must have the first definition of main() seen by the linker even if
> some library also defines main().

Yes, but in the final Makefile.am things are correct.

In master:Makefile is written: SUBDIRS = @SERVER_SUBDIRS", and 
SERVER_SUBDIRS is "master imap" according to master:configure.in.

master:master/Makefile.in contains
LOBJS = service.o service-thread.o
all: $(LOBJS)

so I thought the reason for the ordering of master/ before imap/ was to 
make sure, that service.o is compiled in master/, before changing to imap/.

As with linking, in Automake there are _SOURCES and _LDADD.  $SERVICE 
goes into _SOURCES, any libraries go into _LDADD.  It is not really 
possible with Automake to force linking a library before linking $SERVICE.

Please feel entitled to correct the order of cyr_df and cyrdump.

> cyr_info links masterconf.o but does not depend on it.

cyr_info does depend on masterconf.o .
> commit "rename $service_path to $servicedir"
>
>
> +AC_DEFINE_UNQUOTED(SERVICE_PATH,"$servicedir",[Directory to use for
> service binaries])
>
> Don't you want to rename SERVICE_PATH too?

Yes, this could be also done.

> Also, the difference between $servicedir and $bindir is slight; in the
> default arrangement one will be /usr/cyrus/bin and the other /usr/bin.
> I would hope that eventually we could eliminate $cyrus_prefix and
> replace $servicedir with $bindir.

Yes, but this is not really related to the switch to Automake.
>
>   commit "remove cyrus_prefix from every Makefile.in, as it is unused"
>
> Since three days ago, it is used, in the perl/ directories to expand
> $PERL_PREINSTALL.  Otherwise, looks good.

Yes, please fix it somehow.

> commit "Initial Automake support"
>
> Do we need the 'depend' target?  Surely automake handles that for us.
>
> +       SIEVE_LIBS="../sieve/libsieve.a"
>
> $(top_builddir)

The depend target is removed in subsequent commits together with makedepend.

SIEVE_LIBS can be extended with $(top_builddir).  In fact at the end, 
SIEVE_LIBS and SIEVE_OBJS are used only for Cunit checks.  So if 
Cunit/Makefile.in can be merged to /Makefile.am SIEVE_LIBS and 
SIEVE_OBJS can be completely removed from configure.ac .

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