On Sat, Feb 24, 2024 at 12:22 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > The reason why we insist including the compat-util header as the > very first thing is because it is our mechanism to absorb the > differences across platforms, like the order in which system header > files must be included, and C preprocessor feature macros that are > needed to trigger certain features we want out of the systems, and > insulate other headers and sources from such minutiae. > > Earlier 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 file even if you have > nothing to do with xdiff implementation, for example. "You do not > have to include more than one of these" were 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 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> > --- > > * git-std-lib folks CC'ed to show them where to put their exception > when things start to stabilize; Elijah CC'ed for his 8bff5ca0 > (treewide: ensure one of the appropriate headers is sourced > first, 2023-02-24) and bc5c5ec0 (cache.h: remove this > no-longer-used header, 2023-05-16). > > Documentation/CodingGuidelines | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git c/Documentation/CodingGuidelines w/Documentation/CodingGuidelines > index 578587a471..b3443dd773 100644 > --- c/Documentation/CodingGuidelines > +++ w/Documentation/CodingGuidelines > @@ -446,12 +446,30 @@ 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". In > + addition: This "In addition" ties to the "are allowed to" 19 lines below, which was confusing for me until I intentionally ignored the "In addition", continued reading, and finally caught the other piece of it. Perhaps either `Exceptions:`, or something like `The following cases are allowed to assume that their header file includes "git-compat-util.h", and they do not have to include "git-compat-util.h" themselves:` -- I have a slight preference for the latter form, but I worry that the "These headers must be the first header file to be "#include"d in them" bit will be missed. So maybe if we went with the latter version, we change each bullet point to include that qualification. Example: - the implementation of the built-in commands in the "builtin/" directory that include "builtin.h" as the first header". I don't know if we need the reasoning why you'd #include these files in the bullets below, which is why I didn't include it here. I'm assuming there's a concern about implying that builtin/foo.c should include builtin.h instead of git-compat-util.h (even if foo.c doesn't use cmd_foo()?). > + > + - 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 > + "test-lib.h" that gives them the unit-tests framework > + > + - the source files that implement reftable in the "reftable/" > + directory that include "reftable/system.h" for the reftable > + internals > + > + are allowed to assume that their header file includes > + "git-compat-util.h", and they do not have to include > + "git-compat-util.h" themselves. 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
