Re: Automake Support for cyrus-imapd 2.5

2012-05-08 Thread Bron Gondwana
Sorry, I should have reviewed all this a while ago too! On Tue, Apr 17, 2012 at 10:50:08PM +0200, Дилян Палаузов wrote: 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/

Re: Automake Support for cyrus-imapd 2.5

2012-04-20 Thread Greg Banks
G'day, On Wed, Apr 18, 2012, at 09:13 AM, Дилян Палаузов wrote: Hello Greg, I've done a bunch more testing on this branch and fixed a few issues which I found. Remaining issues are: * The cunit/ directory is proving resistant to automakification, but successfully builds and runs unit

Re: Automake Support for cyrus-imapd 2.5

2012-04-20 Thread Jeroen van Meeuwen
On Friday, April 20, 2012 06:58:05 PM Greg Banks wrote: G'day, On Wed, Apr 18, 2012, at 09:13 AM, Дилян Палаузов wrote: Hello Greg, I've done a bunch more testing on this branch and fixed a few issues which I found. Remaining issues are: * The cunit/ directory is proving resistant to

Re: Automake Support for cyrus-imapd 2.5

2012-04-18 Thread Дилян Палаузов
Hello Greg, On 18.04.2012 06:31, Greg Banks wrote: On Tue, Apr 17, 2012, at 03:09 PM, Dilyan Palauzov wrote: 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

Re: Automake Support for cyrus-imapd 2.5

2012-04-17 Thread 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

Re: Automake Support for cyrus-imapd 2.5

2012-04-17 Thread Дилян Палаузов
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 =

Re: Automake Support for cyrus-imapd 2.5

2012-04-17 Thread Greg Banks
On Tue, Apr 17, 2012, at 03:09 PM, Dilyan Palauzov wrote: 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.

Re: Automake Support for cyrus-imapd 2.5

2012-04-16 Thread Greg Banks
G'day, On Sun, Apr 15, 2012, at 01:31 AM, Дилян Палаузов wrote: Hello, at git://demo.aegee.org:143/cyrus-imapd.git , branch dpa/automake I have patched cyrus-imapd/master to support Automake . Some reviews follow. commit rename configure.in to configure.ac Looks good, but there's

Re: Automake Support for cyrus-imapd 2.5

2012-04-16 Thread Дилян Палаузов
Hello, 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. Please feel entitled to fix it. commit lib/mkchartable.pl move output from stderr to stdout In mkchartable.pl, you need to handle the open()

Re: Automake Support for cyrus-imapd 2.5

2012-04-16 Thread Greg Banks
On Mon, Apr 16, 2012, at 04:44 PM, Greg Banks wrote: Have to go now, more later. commit */Makefile.in: add top_builddir Looks good. commit */Makefile.in: insert $(top_builddir) in all DEPLIBS Looks good commit */Makefile.in: add top_(builddir,srcdir) to CPPFLAGS +CPPFLAGS =

Re: Automake Support for cyrus-imapd 2.5

2012-04-16 Thread Дилян Палаузов
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

Re: Automake Support for cyrus-imapd 2.5

2012-04-15 Thread Jeroen van Meeuwen
On Sunday, April 15, 2012 01:31:54 AM Дилян Палаузов wrote: Hello, at git://demo.aegee.org:143/cyrus-imapd.git , branch dpa/automake I have patched cyrus-imapd/master to support Automake . Hi Dilyan, Thanks for your efforts, I'm giving it a go right now. I've logged bug #3669 as a tracker

Re: Automake Support for cyrus-imapd 2.5

2012-04-15 Thread Jeroen van Meeuwen (Kolab Systems)
On 2012-04-15 11:27, Bron Gondwana wrote: On Sun, Apr 15, 2012, at 01:31 AM, Дилян Палаузов wrote: Hello, at git://demo.aegee.org:143/cyrus-imapd.git , branch dpa/automake I have patched cyrus-imapd/master to support Automake . With the exception of perl and CUnit, I it works fine and

Re: Automake Support for cyrus-imapd 2.5

2012-04-15 Thread Jeroen van Meeuwen
On Sunday, April 15, 2012 01:31:54 AM Дилян Палаузов wrote: Hello, at git://demo.aegee.org:143/cyrus-imapd.git , branch dpa/automake I have patched cyrus-imapd/master to support Automake . Hi Dilyan, I've gotten the contents of your branch to almost the same stage as origin/master. See the

Re: Automake Support for cyrus-imapd 2.5

2012-04-15 Thread Greg Banks
On Sun, Apr 15, 2012, at 08:27 PM, Bron Gondwana wrote: On Sun, Apr 15, 2012, at 01:31 AM, Дилян Палаузов wrote: Hello, at git://demo.aegee.org:143/cyrus-imapd.git , branch dpa/automake I have patched cyrus-imapd/master to support Automake . With the exception of perl and