git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Kyle Lippincott <spectral@google.com>,
	 Calvin Wan <calvinwan@google.com>,
	 Jonathan Tan <jonathantanmy@google.com>,
	 Elijah Newren <newren@gmail.com>
Subject: [PATCH v2] doc: clarify the wording on <git-compat-util.h> requirement
Date: Sat, 24 Feb 2024 14:36:57 -0800	[thread overview]
Message-ID: <xmqqh6hxlcae.fsf@gitster.g> (raw)
In-Reply-To: <xmqq4jdxmx2e.fsf@gitster.g> (Junio C. Hamano's message of "Sat, 24 Feb 2024 12:22:49 -0800")

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@pobox.com>
---

 * The most notable change since the first one is that the reason
   why the requirement exists is added to the document, not just
   telling them what to do but also telling them why.

   Also the path to "test-lib.h" used in the unit-test framework has
   been spelled out relative to the root of the working tree, like
   all other header files.

Range-diff:
1:  dde3a79470 ! 1:  b9f3d36e9a doc: clarify the wording on <git-compat-util.h> requirement
    @@ Metadata
      ## Commit message ##
         doc: clarify the wording on <git-compat-util.h> requirement
     
    -    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.
    +    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.
     
    -    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).
    +    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 intended audiences, to avoid such
    +    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
    @@ 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".  In
    -+   addition:
    ++   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.
    ++
    ++   In addition:
     +
     +   - the implementation of the built-in commands in the "builtin/"
     +     directory that include "builtin.h" for the cmd_foo() prototype
    @@ Documentation/CodingGuidelines: For C programs:
     +     "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
    ++     "t/unit-tests/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
    ++   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

 Documentation/CodingGuidelines | 36 ++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 578587a471..291b2898a2 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -446,12 +446,36 @@ 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.
+
+   In addition:
+
+   - 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
+
+   - 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


  reply	other threads:[~2024-02-24 22:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-24 20:22 [PATCH] doc: clarify the wording on <git-compat-util.h> requirement Junio C Hamano
2024-02-24 22:36 ` Junio C Hamano [this message]
2024-02-26 23:28   ` [PATCH v3] " Junio C Hamano
2024-02-26 23:57     ` Kyle Lippincott
2024-02-27  4:25     ` Elijah Newren
2024-02-27  5:35       ` Junio C Hamano
2024-02-24 22:38 ` [PATCH] " Kyle Lippincott
2024-02-24 22:54   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh6hxlcae.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=calvinwan@google.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=newren@gmail.com \
    --cc=spectral@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).