On 20/09/18 15:26, Junio C Hamano wrote: > Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> 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