Automake Support for cyrus-imapd 2.5

[Dilyan Palauzov]

Hello,

on Greg's first comments:

> commit "imap/Makefile.in: add .c.snmp: recipe"
>
> The old makefile will rebuild pushstats.c if the snmpgen script changes.
>  The new one won't.
I fixed this.

>
> commit "lib/mkchartable.pl move output from stderr to stdout"
>
> In mkchartable.pl, you need to handle the open() failing.
>
> Also, mkchartable.pl can fail partway through, e.g. if one of the
> charset/*.t files is not readable.  In the old makefile, such a failure
> would trigger this code
>
> -        || (rm -f chartable.c && exit 1)
>
> which would break the build and avoid half-creating a chartable.c so
> that the next attempt to build was also (correctly) fail.  In the new
> makefile there is no such safeguard: a partly finished chartable.c would
> be created and the next attempt to build would (incorrectly) not fail at
> this point.

I fixed this now.

>> > commit "Prepend the path in #include to .et ->  .h files"
>> > [...] I don't see why you would change
>> >
>> > -#include "imap_err.h"
>> > +#include "imap/imap_err.h"
>> >
>> > without changing any of the other #include lines around it.
>> >
>
>> >
>> > Generally, I think it would be a good thing to have a clear consistent
>> > and documented policy for how we #include headers.  We don't really have
>> > that now, and I don't think this commit gets us any closer.
>>
>> These files
>> are prefixed with the path, as in #include "imap/imap_err.h", so that
>> the build works (this was my conclusion, when I was trying to build with
>> VPATH).
>
> Why make the way in which files are #include'd different depending on
> whether they are built sources or not?  That seems very confusing.  I
> would very much prefer to have every #include line use the same format,
> either
>
> #include "imap/imap_err.h"  /* relative path down from
> top_{build|src}dir */
>
> OR
>
> #include "imap_err.h"  /* filename only, rely on lots of -I options */
>
> but not confusing admixtures of both.  I don't care which one, just that
> it's consistent.  It would also be nice if the convention was documented
> in doc/internal/hacking.
> If you were experiencing problems with build order due to built headers
> not being available when a .c file was being compiled, then that's what
> BUILT_SOURCES is for.

I prefer to keep (AM_)CPPFLAGS minimalistic and have everthing  
relative to the top_dirs.  Automake includes automatically -I.  
-I$(srcdir) and -I(the directory with config.h = top_builddir), so it  
is not necessary to add those explicitly (even if it is done for  
top?builddir).  Specifying the path to generated and included *.h  
files is the minimal effort required to keep AM_CPPFLAGS as small as  
possible (top_dir and top_dir/lib).


>> > commit "lib/Makefile.in: Reorder LIBCYR(M)_(OBJS, HDRS) alphabetically"
>> >
>> > Looks good.  While you're doing that, how about splitting them up one
>> > per line?  We have all sorts of headaches doing git rebases of commits
>> > which add or remove .c files because of this.
>>
>> Okay, we can split them one per line, if you want.  But you can also put
>> them all on the same line.  Having them all on the same line, it is
>> quite easy to find where the change is.
>
> Ah, but *finding* is not the problem; my text editor has a working
> search function.
>
> The problem is when you're rebasing a commit that adds a single source
> file, and the upstream changes you're rebasing around add or remove
> another source file.  With multiple source files per line, you are far
> more likely to get spurious context damage which prevents the git merge
> from succeeding, which then requires a manual merge conflict fix, which
> is annoying slow and error-prone.  When Bron and I were actively working
> on the conversations branch we would suffer through dozens of these
> spurious merge conflicts per week.  See commit
> 3aa2792f402a27687cf0ecdcd38654716436ec0c

The problem would probably not have appeared, if all the files were on  
a single, very long line. I prefer to have them on one line (even not  
split with \)

> commit "lib/Makefile.in: expand ACL and AUTH"
> commit "lib/Makefile.am: move strarray from libcyrus.a to
> libcyrus_min.a"
>
> Looks good, but here
>
> +master: master.o masterconf.o cyrusMasterMIB.o
>
> the master program needs to depend on libcyrus.a in some way.  For
> example, in imapd/Makefile.in we do

In Makefile.am the dependencies are right.

Greetings
   Dilian