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

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Sep 19, 2018, 19:37 UTC
Message-ID: <89478060-85c1-416a-0ad9-54d3f8f1111b@ramsayjones.plus.com>
In-Reply-To: <CAN0heSpNT25PrNwdbQKZo=Q8kpSfwnzuOz34xKHtfF+Fq9ZXmg@mail.gmail.com>

On 19/09/18 18:49, Martin Ågren wrote:

Show 13 quoted lines

> Hi Ramsay,
> 
> On Wed, 19 Sep 2018 at 02:07, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>> @@ -2675,6 +2676,17 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
>>  .PHONY: sparse $(SP_OBJ)
>>  sparse: $(SP_OBJ)
>>
>> +GEN_HDRS := command-list.h unicode-width.h
> 
> Most of the things happening around here are obvious steps towards the
> end-goal and seem to logically belong here, together. This one though,
> "generated headers"(?) seems like it is more general in nature, so could
> perhaps live somewhere else.

Yes, generated headers, but no, not more general. ;-)

I originally included those headers directly in the EXCEPT_HDRS macro, along with the list of excluded directories (which was much longer at one point).

The 'command-list.h' is generated as part of the build (and so may or may not be part of the LIB_H macro), whereas the unicode-width.h header is only re-generated when someone runs the 'contrib/update-unicode/update_unicode.sh' script (presumably when a new standard version is announced) and commits the result.

Both headers fail the 'hdr-check', although both generator scripts could be 'fixed' so that they passed. I just didn't think it was worth the effort - neither header was likely to be #included anywhere else. I guess I could eliminate that macro, absorbing it into EXCEPT_HDRS, if that would lead to less confusion ...

[I suspect the fact that LIB_H (almost always) contains 'command-list.h' has not been noticed ... :-P ]

> Actually, we have a variable `GENERATED_H` which seems to try to do more
> or less the same thing. It lists just one file, though, command-list.h.
> And unicode-width.h seems to be tracked in git.git.

Hmm, GENERATED_H seems only to be used by the i18n part of the makefile and, as a result of its appearance in LIB_H, sometimes results in command-list.h appearing twice in LOCALIZED_C. (which is probably not a problem).

ATB, Ramsay Jones

Show 17 quoted lines

> Maybe use `GENERATED_H` instead, and list unicode-width.h on the next
> line instead? Or am I completely misreading "GEN_HDRS"?
> 
>> +EXCEPT_HDRS := $(GEN_HDRS) compat% xdiff%
>> +CHK_HDRS = $(filter-out $(EXCEPT_HDRS),$(patsubst ./%,%,$(LIB_H)))
>> +HCO = $(patsubst %.h,%.hco,$(CHK_HDRS))
>> +
>> +$(HCO): %.hco: %.h FORCE
>> +       $(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $<
>> +
>> +.PHONY: hdr-check $(HCO)
>> +hdr-check: $(HCO)
>> +
>>  .PHONY: style
>>  style:
>>         git clang-format --style file --diff --extensions c,h
>