Automake Support for cyrus-imapd 2.5

Дилян Палаузов dilyan.palauzov at aegee.org
Wed Apr 18 03:13:51 EDT 2012


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 #include lines around it.
>>>>>
>> 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).
>
> Yes, I agree so far.
>
>> 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).
>
> I have two problems with this statement.
>
> Firstly, I don't see why generated .h files need to be treated any
> differently.  If they're mentioned in BUILT_SOURCES then they get built
> before everything else in the 'all' target, so by the time any other .c
> file is compiled or the dependencies are extracted the generated .h
> already exists and is found correctly.

Because generated files go in $(builddir), and the others are in $(srcdir).

> Secondly, lib/ is not the only directory from which .h files are
> included.  I did a quick count based on gcc-generated .dep files and
> found
>
> 3578 headers included into .c files, comprising
> 1456 included from "."
> 174 include from top_dir into files in other directories (e.g. config.h)
> 1872 included from top_dir/lib into files in other directories
> 61 included from top_dir/imap into files in other directories
> 14 included from top_dir/sieve into files in other directories
> 1 included from top_dir/master into files in other directories

Yes.  I have prefixed those files also with the path.

If you want to put path-prefixes to all included *h files, and nobody 
has objections, you can add the path-prefixes and insert in 
AM_INIT_AUTOMAKE([...]) "nostdinc".

>>>>> 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.
>
> On the contrary, such an arrangement is the worst possible case because
> every change to the list of files is a merge conflict waiting to happen.
>   It would have resulted in conflicts every single time we had to rebase
> around a commit which added a source file, rather than about half of
> them.

If you want to put each SOURCE file on a separate line in Makefile.am , 
you are free to do so.

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


More information about the Cyrus-devel mailing list