On Wed, 19 Sep 2018 at 22:15, Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> wrote: > On 19/09/18 18:49, Martin Ågren wrote: > > On Wed, 19 Sep 2018 at 02:07, Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> wrote: > >> +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. ;-) > 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. Ah, I wasn't aware of how unicode-width.h works, thanks. > 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. With the above background, I'd tend to agree. > I guess I could eliminate > that macro, absorbing it into EXCEPT_HDRS, if that would > lead to less confusion ... I'm just a single data point, so don't trust my experience too much. > > 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. > > 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. Just thinking out loud, I suppose you could use $(GENERATED_H) instead of hard-coding command-list.h in your patch. But with the background you explained above, I think there's a good counter-argument to be made: When we gain new auto-generated headers, we wouldn't actually mind `make hdr-check` failing. We'd get the opportunity to decide whether making the new header pass the check is worth it, or whether the correct solution is to blacklist the auto-generated header. That's probably better than just blacklisting the new header by default so that we don't even notice that it has a potential problem. So I think your approach makes sense. I stumbled on the similarity of the name to something we already have. Maybe something like # Ignore various generated files, rather than updating the # generating scripts for little or no benefit. GEN_HDRS := ... or a remark in the commit message, or rolling this into EXCEPT_HDRS, but I'd be perfectly fine just leaving this as it is. Please trust your own judgment more than mine. Thanks Martin
