Re: [PATCH 0/5] Header includes cleanup

[Junio C Hamano <gitster@pobox.com>]

Christian Couder <chriscool@tuxfamily.org> writes:

> Ok, so I suggest the following simple guiding principles:
>
> - git-compat-util.h or cache.h or builtin.h should always be the first 
> #include in C files,
>
> - header files should include other incude files if and only if the other 
> includes are needed to compile them,

This is unclear.

Long before I started touching git, I used to be religious about making
header files self contained.  For a header file frotz.h in the project, I
used to insist that

	$ cat >1.c <<\EOF
	#include "frotz.h"
        EOF
        $ cc -Wall -c 1.c

did not fail.  

Are you talking about that by "to compile them"?

I stopped doing that long time ago, partly because the rule was cumbersome
to enforce, but primarily because it was not helping much in the larger
picture.

Such a rule, together with strict rules such as the order of including
other header files in the header files themselves, may make life easier
for programmers who touch .c files but never .h files, because they can
include only the necessary .h files and in any order.  But in practice,
people need to touch both .h and .c files and when they need to include
new system header files, they need to follow the inclusion order rule
somewhere anyway---at that point, it does not matter much if the rule
applies to only .h files or both .h and .c files.

So for example, you cannot compile

	$ cat >1.c <<\EOF
        #include "revision.h"
        EOF
        $ cc -Wll -c 1.c

in git.git project, but I do not think it is a problem.

> - a header file should be included in a C file only if it is needed to 
> compile the C file (it is not ok to include it only because it includes 
> many other headers that are needed)

If that is the rule, perhaps the problem lies not in a .c program that
includes such a .h header, but in the .h itself that includes many other
header files.

> - other than the above rules, it is ok to reduce the number of includes as 
> much as possible
>
> What do you think?

What you did not write and I forgot to mention, which is a logical
conclusion of the first rule, is that C files should not directly include
common system header files such as unistd, sys/stat, etc.

There are exceptions to any rule.  For example, inclusion of syslog.h in
daemon.c is OK because most of the rest of the system does not even use
syslog.  If we later find a platform whose syslog.h has some funny
inter-header dependencies, however, we will need to include it in the
git-compat-util.h and resolve the dependencies there, like we do for other
system header files.

> Or perhaps Junio would prefer that you work on a C file by C file basis? 
> Like for example:
>
> "delete useless includes in 'builtin-diff-files.c'"
> "delete useless includes in 'builtin-diff-index.c'"

If the series does not involve .h file clean-up, then a series that
consists of one patch per .c file would be easier to handle, I think.