Re: [PATCH 1/9] Makefile: add a hdr-check target

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, GIT Mailing-list <git@vger.kernel.org>,
	Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH 1/9] Makefile: add a hdr-check target
Date: Thu, 20 Sep 2018 17:03:05 +0100	[thread overview]
Message-ID: <b7b9b26d-c175-05b9-7144-101bcaf592d4@ramsayjones.plus.com> (raw)
In-Reply-To: <xmqqbm8s2qjd.fsf@gitster-ct.c.googlers.com>

On 20/09/18 15:26, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>> 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.
> 
> It makes sense to have tool do what we do not have to do manually.
> 
> One thing that makes me wonder with the patch is that the new check
> command does not seem to need to see what is on CFLAGS and friends.
> Having seen that "make DEVELOPER=1" adds more -W... on the command
> line of the compiler and makes a build fail on a source that
> otherwise would build, I am wondering if there are some (subset of)
> options that the header-check command line wants to give to the
> compiler.

Yes, this was one of my first concerns (I even asked Elijah what
compiler options he used), but I was getting useful results without
passing CFLAGS, so I just ignored that issue ... :-D

[The 'on-the-fly' compilation units don't correspond to any _actual_
compilation unit, so it's not easy to use existing rules ... but we
could use 'hco' rule specific definitions to add flags, I suppose ...]

> Of course, there are also conditionally compiled sections of code,
> which are affected by the choice of -DMACRO=VALUE; how does this new
> feature handle that?

Indeed. This bothered me as well. The 'compat' directory does not
follow the 'usual pattern' of the main headers and is particularly
sensitive to the lack of various -DMACROs. I had initially included
_all_ sub-directories in the 'exclude list' (to follow what Elijah
had done), but then removed one at a time ...

I am open to suggestions for improvements. ;-)

ATB,
Ramsay Jones