Re: [PATCH] Makefile: drop GEN_HDRS

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 17, 2019, 01:40 UTC
Message-ID: <20191217014057.GE10734@google.com>
In-Reply-To: <20191217012756.GQ135450@google.com>

Emily Shaffer wrote:

> On Mon, Dec 16, 2019 at 02:20:14PM -0500, Jeff King wrote:

Show 16 quoted lines

>> But this would enable us to start checking command-list.h. I'm on the
>> fence on whether that's useful or not; the patch below makes it pass,
>> but I'm not sure if that is really turning up any useful problems. I
>> suppose somebody besides help.c could include command-list.h, in which
>> case some of those MAYBE_UNUSED bits could become useful.
>
> Firstly, I think if someone besides help.c includes command-list.h it
> blows up because there's no include guards :)
>
> My gut wants to say, "I need to be sure my generated header is correct!"
> But it seems that will also be checked when I try to include that header
> from something actually important. So maybe it's not actually useful.
> But then it seems like hdr-check target isn't that useful for anybody,
> since those headers should always be included sometime down the road (or
> why have them). So that makes me think I must still be missing
> something, like maybe I parsed hdr-check wrong.

"git log -Shdr-check Makefile" gives a hint:

  $ git log -Shdr-check Makefile
  commit ebb7baf02f69f2164b1f89148945d18c376fc6a8
  Author: Ramsay Jones <ramsay@ramsayjones.plus.com>
  Date:   Wed Sep 19 01:07:08 2018 +0100

      Makefile: add a hdr-check target

      Commit ef3ca95475 ("Add missing includes and forward declarations",
      2018-08-15) resulted from the author employing a manual method to
      create a C file consisting of a pair of pre-processor #include
      lines (for 'git-compat-util.h' and a given toplevel header), and
      fixing any resulting compiler errors or warnings.

      Add a Makefile target to automate this process.

So it's primarily about making sure that each header is #include-able on its own (i.e., that it #includes the headers for any types it relies on).

That seems like something I'd want to hold for command-list.h, too. :)

Thanks, Jonathan