Re: [PATCH] Makefile: drop GEN_HDRS

From: Jeff King <peff@peff.net>
Date: Dec 14, 2019, 00:38 UTC
Message-ID: <20191214003820.GA927924@coredump.intra.peff.net>
In-Reply-To: <xmqq1rt7hkp6.fsf@gitster-ct.c.googlers.com>

On Fri, Dec 13, 2019 at 03:25:41PM -0800, Junio C Hamano wrote:

> Get rid of GEN_HDRS, which is used only once to list the headers we
> do not run hdr-check test on, and instead explicitly list that the
> ones, either tracked or generated, that we exempt from the test.

Yeah, I think this is an improvement by itself.

After reading this, though:

>  - If we value the header cleanliness check, we eventually want to
>    teach our header generating scripts to produce clean headers.
>    Keeping the blanket "generated headers can be left as dirty as we
>    want" exception does not nudge us in the right direction.

I did expect to see the actual hdr-check behavior move towards checking these generated versions. However, both are kind of interesting.

unicode-width.h isn't a "real" header file; it's meant to be included in the middle of a function. I think it _could_ be changed to define "struct interval" itself, and then be a static file-scope variable. But there's not really a compelling reason to do so.

But "command-list.h" is more of a traditional header file, being included at the top of help.c. In theory the hdr-check target could add a dependency on it, and then we could check it along with everything else. But even without that first step, if I remove it from EXCEPT_HDRS, nothing happens!

That's because LIB_H is created by running find in the local filesystem. So until it's generated, we don't realize it's there to check. I kind of wonder if it should be part of LIB_H. I suspect that on some systems, we'd fail to notice a rebuild when command-list.txt is updated (but nobody noticed, because it is only systems that do not have compiler-supported dependency tracking, and most developers are no modern platforms that do).

-Peff