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 .
Със здраве
Дилян
