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