[PATCH] Documentation/CodingGuidelines: improve header includes rules

From: Christian Couder <chriscool@tuxfamily.org>
Date: Apr 13, 2009, 22:34 UTC
Message-ID: <20090414003433.39cbdea2.chriscool@tuxfamily.org>

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/CodingGuidelines |   26 ++++++++++++++++++++++----
 1 files changed, 22 insertions(+), 4 deletions(-)

	Sorry for the delay but I have been busy with other things.

	The goal of these rule is to codify the current practice and
	to keep things simple to use for the developer while keeping
	some sanity.

	So there are 4 rules and I hope it's enough:

	- The first one was already in the document but is strenghtened
	a bit.

	- The second one existed informaly.

	- The third one means that for example if we have "revision.h"
	that includes "diff.h" and "commit.h", then it's ok to include
	"revision.h" in a C file, only if at least one feature from
	"revision.h" is actually used in the C file.

	It is not ok to include "revision.h" if features from "diff.h"
	and "commit.h" are used but no feature from "revision.h" is
	used.

	But on the other hand if features from bith "revision.h" and
	"diff.h" are used in a C file, then the rule does not state that
	"diff.h" has to be included if "revision.h" is included. So in
	practice this means that it would be ok to remove an include of
	"diff.h" if "revision.h" is included.

	- The fourth and last rule tries to keep things simple to use
	for the developper and quite sane. Here are some part of an email
	I already sent. This will hopefully explain why I think this rule
	is needed:

	> Well, I wanted to say that for a header file frotz.h in the
	> project:
	>
        > $ cat >1.c <<\EOF
        > #include "cache.h"
        > #include "frotz.h"
        > EOF
        > $ cc -Wall -c 1.c
        >
	> should not fail.
	>
	> (Ok, in practice, let's say that something like:
	>
	> $ cc -Wall -DSHA1_HEADER='<openssl/sha.h>' -c 1.c
	>
	> should not fail.)
	>
	> I think [the fact that the above fails] may become a problem if
	> a header needs more headers to be included before it, and those
	> latter headers again need more headers and so on.
	>
	> The advantage of having and using "cache.h" is that things are
	> quite simple. 
	> You include cache.h and then a few more headers for special
	> features you need, and, here you go, you can code up something
	> quite interesting without worrying too much about includes.
	> (Though the compilation time is perhaps a little longer than it
	> could be.)
	>
	> If we loose this simplicity because we don't take care or for
	> some theoretical reason, then I think we have lost everything
	> that really matters.

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index b8bf618..181cb2d 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -115,10 +115,6 @@ For C programs:
 
  - When you come up with an API, document it.
 
- - The first #include in C files, except in platform specific
-   compat/ implementations, should be git-compat-util.h or another
-   header file that includes it, such as cache.h or builtin.h.
-
  - If you are planning a new command, consider writing it in shell
    or perl first, so that changes in semantics can be easily
    changed and discussed.  Many git commands started out like
@@ -132,3 +128,25 @@ For C programs:
 
  - When we pass <string, length> pair to functions, we should try to
    pass them in that order.
+
+About header file includes:
+
+ - The first #include in C files, except in platform specific
+   "compat/" implementations, should be "git-compat-util.h" or
+   "cache.h" or "builtin.h".
+
+ - System header files should be included in "git-compat-util.h",
+   except for specific system headers like "syslog.h" in "daemon.c".
+   (This is because for portability reasons we want the compiler to
+   always see the same system headers in the same order.)
+
+ - After the first #include in a C file, only header files containing
+   features that are actually used in the C file should be included.
+   (This means that it is not ok to include an header file only
+   because this header file includes other header files with features
+   that are used in the C file.)
+
+ - Git header files, when they are included in a C file after
+   "cache.h", should not require any other header file to be included
+   before them for the C file to compile cleanly. (This is because we
+   want to keep it simple to use features defined in our headers.)

-- 
1.6.2.2.572.g4420a