On 19/09/18 18:49, Martin Ågren wrote:
> Hi Ramsay,
> 
> On Wed, 19 Sep 2018 at 02:07, Ramsay Jones <ram...@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

> 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
> 

Reply via email to