On Mon, Feb 26, 2024 at 3:28 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > The reason why we require the <git-compat-util.h> file to be the > first header file to be included is because it insulates other > header files and source files from platform differences, like which > system header files must be included in what order, and what C > preprocessor feature macros must be defined to trigger certain > features we want out of the system. > > We tried to clarify the rule in the coding guidelines document, but > the wording was a bit fuzzy that can lead to misinterpretations like > you can include <xdiff/xinclude.h> only to avoid having to include > <git-compat-util.h> even if you have nothing to do with the xdiff > implementation, for example. "You do not have to include more than > one of these" was also misleading and would have been puzzling if > you _needed_ to depend on more than one of these approved headers > (answer: you are allowed to include them all if you need the > declarations in them for reasons other than that you want to avoid > including compat-util yourself). > > Instead of using the phrase "approved headers", enumerate them as > exceptions, each labeled with its intended audiences, to avoid such > misinterpretations. The structure also makes it easier to add new > exceptions, so add the description of "t/unit-tests/test-lib.h" > being an exception only for the unit tests implementation as an > example. > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > > * Updated the leading phrase introducing the list of exceptions. > I think this is now clear enough to be ready for 'next'? > > Range-diff: > 1: 2e7082d2d2 ! 1: 470f33a078 doc: clarify the wording on <git-compat-util.h> requirement > @@ Documentation/CodingGuidelines: For C programs: > - "t/helper/test-tool.h", "xdiff/xinclude.h", or > - "reftable/system.h".) You do not have to include more than one of > - these. > -+ implementations and sha1dc/, must be "git-compat-util.h". This > ++ implementations and sha1dc/, must be <git-compat-util.h>. This > + header file insulates other header files and source files from > + platform differences, like which system header files must be > + included in what order, and what C preprocessor feature macros must > + be defined to trigger certain features we expect out of the system. > ++ A collorary to this is that C files should not directly include > ++ system header files themselves. > + > -+ In addition: > ++ There are some exceptions, because certain group of files that > ++ implement an API all have to include the same header file that > ++ defines the API and it is convenient to include <git-compat-util.h> > ++ there. Namely: > + > + - the implementation of the built-in commands in the "builtin/" > + directory that include "builtin.h" for the cmd_foo() prototype > -+ definition > ++ definition, > + > + - the test helper programs in the "t/helper/" directory that include > -+ "t/helper/test-tool.h" for the cmd__foo() prototype definition > ++ "t/helper/test-tool.h" for the cmd__foo() prototype definition, > + > + - the xdiff implementation in the "xdiff/" directory that includes > -+ "xdiff/xinclude.h" for the xdiff machinery internals > ++ "xdiff/xinclude.h" for the xdiff machinery internals, > + > + - the unit test programs in "t/unit-tests/" directory that include > + "t/unit-tests/test-lib.h" that gives them the unit-tests > -+ framework > ++ framework, and > + > + - the source files that implement reftable in the "reftable/" > + directory that include "reftable/system.h" for the reftable > -+ internals > ++ internals, > + > + are allowed to assume that they do not have to include > -+ "git-compat-util.h" themselves, as it is included as the first > ++ <git-compat-util.h> themselves, as it is included as the first > + '#include' in these header files. These headers must be the first > + header file to be "#include"d in them, though. > > > Documentation/CodingGuidelines | 41 +++++++++++++++++++++++++++++----- > 1 file changed, 35 insertions(+), 6 deletions(-) > > diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines > index 578587a471..806979f75b 100644 > --- a/Documentation/CodingGuidelines > +++ b/Documentation/CodingGuidelines > @@ -446,12 +446,41 @@ For C programs: > detail. > > - The first #include in C files, except in platform specific compat/ > - implementations and sha1dc/, must be either "git-compat-util.h" or > - one of the approved headers that includes it first for you. (The > - approved headers currently include "builtin.h", > - "t/helper/test-tool.h", "xdiff/xinclude.h", or > - "reftable/system.h".) You do not have to include more than one of > - these. > + implementations and sha1dc/, must be <git-compat-util.h>. This > + header file insulates other header files and source files from > + platform differences, like which system header files must be > + included in what order, and what C preprocessor feature macros must > + be defined to trigger certain features we expect out of the system. > + A collorary to this is that C files should not directly include > + system header files themselves. > + > + There are some exceptions, because certain group of files that > + implement an API all have to include the same header file that > + defines the API and it is convenient to include <git-compat-util.h> > + there. Namely: > + > + - the implementation of the built-in commands in the "builtin/" > + directory that include "builtin.h" for the cmd_foo() prototype > + definition, > + > + - the test helper programs in the "t/helper/" directory that include > + "t/helper/test-tool.h" for the cmd__foo() prototype definition, > + > + - the xdiff implementation in the "xdiff/" directory that includes > + "xdiff/xinclude.h" for the xdiff machinery internals, > + > + - the unit test programs in "t/unit-tests/" directory that include > + "t/unit-tests/test-lib.h" that gives them the unit-tests > + framework, and > + > + - the source files that implement reftable in the "reftable/" > + directory that include "reftable/system.h" for the reftable > + internals, > + > + are allowed to assume that they do not have to include > + <git-compat-util.h> themselves, as it is included as the first > + '#include' in these header files. These headers must be the first > + header file to be "#include"d in them, though. > > - A C file must directly include the header files that declare the > functions and the types it uses, except for the functions and types > -- > 2.44.0 > Looks good
