Re: [PATCH 0/9] Add missing includes and forward declares

[Jeff King <peff@peff.net>]

On Sat, Aug 11, 2018 at 01:59:50AM -0700, Elijah Newren wrote:

> The part of my story you snipped in the ellipsis is kind of important,
> though: "...and decided to determine which header files were missing
> their own necessary #include's and forward declarations."  The way I
> did so was making a simple one-line .c file that included exactly one
> header, and checked to see if I could compile it (without any special
> defines), fixed it up as necessary, then repeated that process for
> every toplevel header.

The rule in Git has always been that your very first include must
always be "git-compat-util.h" or an equivalent header that includes it
first (like "cache.h"). This is mentioned in CodingGuidelines.

So I think the better test is a two-line .c file with:

  #include "git-compat-util.h"
  #include $header_to_check

And I'd be fine with fixing any compilation failures there, either with
forward declarations or recursive includes. I think the in the early
days there was some resistance to having a lot of recursive includes,
because it _can_ lead to confusion, but IMHO it mostly helps people. And
I don't recall much discussion on it in recent years.

Including "git-compat-util.h" in many more headers probably doesn't hurt
(after all, it's a noop for every .c file which is following the
existing rule). But I'd just as soon not sprinkle it everywhere, nor
imply that that people don't need to be including it. It's really
important that it comes first because it wants a clean slate and have
subtle effects on other header files due to macros. Worse still, some of
the effects are platform dependent, so we might not realize an ordering
problem until somebody on AIX complains.

I wonder if there's an easy way to check. I guess adding '-Dint=foo' to
the command line, and then putting '#undef int' at the top of
git-compat-util would catch just about any code the compiler sees that
doesn't have git-compat-util included. :)

-Peff