On Wed, Sep 22 2021, Carlo Marcelo Arenas Belón wrote: > 3821c38068 (Makefile: add support for generating JSON compilation > database, 2020-09-03), adds a feature to be used with clang to generate > a compilation database by copying most of what was done before with the > header dependency, but by doing so includes on its availability check > the CFLAGS which became specially problematic once DEVELOPER=1 implied > -pedantic as pointed out by Ævar[1]. > > Remove the unnecessary flags in the availability test, so it will work > regardless of which other warnings are enabled or if the compilers has > been told to error on them. > > [1] https://lore.kernel.org/git/patch-1.1-6b2e9af5e67-20210922T103749Z-avarab@xxxxxxxxx/ > > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx> > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 9df565f27b..d5c6d0ea3b 100644 > --- a/Makefile > +++ b/Makefile > @@ -1302,7 +1302,7 @@ GENERATE_COMPILATION_DATABASE = no > endif > > ifeq ($(GENERATE_COMPILATION_DATABASE),yes) > -compdb_check = $(shell $(CC) $(ALL_CFLAGS) \ > +compdb_check = $(shell $(CC) \ > -c -MJ /dev/null \ > -x c /dev/null -o /dev/null 2>&1; \ > echo $$?) Sorry about the overlap in https://lore.kernel.org/git/patch-v2-2.2-6b18bd08894-20210922T220532Z-avarab@xxxxxxxxx/; I didn't see this thread before sending my version. I think your patch here is better than mine. FWIW I also had this on top of mine, i.e. emitting output to stderr unconditionally: https://github.com/avar/git/commit/d4bcc0e617e52df803870df29df82aa3b2205d84 But thinking about it again I think with the rationale in that not-on-list commit of mine the below is better than either of our versions v1. I.e. for COMPUTE_HEADER_DEPENDENCIES the point of the test is that we turn it on automatically, so it needs to not suck by default. The reason we're doing this is, per the comment in 3821c38068: If this variable is set, check that $(CC) indeed supports the `-MJ` flag, following what is done for automatic dependencies. Anyone using GENERATE_COMPILATION_DATABASE is turning it on explicitly, and I daresay if they're using it at all they're either not using anything but clang, or is keenly aware of the difference. So do we really need to carry those 17 lines of the Makefile logic simply to avoid showing this error on say "CC=gcc GENERATE_COMPILATION_DATABASE=yes": gcc: error: unrecognized command-line option ‘-MJ’; did you mean ‘-J’? It doesn't seem worth it to me, especially as we document that we'll use the "-MJ" flag in the Makefile comment that the person turning on GENERATE_COMPILATION_DATABASE=yes must have read. Anyway, I'll leave you to do what you think is best here, and I'm also fine with just going for the v1 you've got here, it just seems to me like we're both fixing logic that's been copy/pasted from COMPUTE_HEADER_DEPENDENCIES, and the reasons we need it for that facility don't apply at all to GENERATE_COMPILATION_DATABASE. -- >8 -- diff --git a/Makefile b/Makefile index 9df565f27bb..32538f9e858 100644 --- a/Makefile +++ b/Makefile @@ -1301,23 +1301,6 @@ ifndef GENERATE_COMPILATION_DATABASE GENERATE_COMPILATION_DATABASE = no endif -ifeq ($(GENERATE_COMPILATION_DATABASE),yes) -compdb_check = $(shell $(CC) $(ALL_CFLAGS) \ - -c -MJ /dev/null \ - -x c /dev/null -o /dev/null 2>&1; \ - echo $$?) -ifneq ($(compdb_check),0) -override GENERATE_COMPILATION_DATABASE = no -$(warning GENERATE_COMPILATION_DATABASE is set to "yes", but your compiler does not \ -support generating compilation database entries) -endif -else -ifneq ($(GENERATE_COMPILATION_DATABASE),no) -$(error please set GENERATE_COMPILATION_DATABASE to "yes" or "no" \ -(not "$(GENERATE_COMPILATION_DATABASE)")) -endif -endif - ifdef SANE_TOOL_PATH SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH)) BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'
