Re: [PATCH v2] Makefile: add support for generating JSON compilation database

[Junio C Hamano <gitster@pobox.com>]

Philippe Blain <levraiphilippeblain@gmail.com> writes:

> This addition to the .gitignore is for the individual JSON files (one per source file), 
> that are placed in the $(compdb_dir). 
> I think naming "rebase.o.json" the JSON file that describes how to compile "rebase.c"
> into "rebase.o" makes sense. I don't know what is the convention for other projects.

I agree rebase.o.$somesuffix does make sense, but I do not know
'json' is a great value for $somesuffix.  I wouldn't be surprised if
'cdb' or some other silly abbreviation for "compilation database" is
how other people use this feature.

Those watching from the sidelines.  Does anybody know if there is an
established convention used by other projects?  If we hear nothing
by early next week, let's declare 'json' is good enough and move on.

> The name `compile_commands.json` for the database itself is standard. 
> The name of the directory where the '*.o.json' files are placed is a name
> I chose, and I don't feel strongly about it. I thought it made sense to name
> it like that, then its purpose is clear.  We could make it a hidden directory 
> if we don't want to add a new folder to the root of the repo when using this feature.

I think both of these are sensible.  Again if we hear nothing about
common practice, let's move on with these constants as-is.

>>> +ifdef GENERATE_COMPILATION_DATABASE
>>> +compdb_check = $(shell $(CC) $(ALL_CFLAGS) \
>>> +	-c -MJ /dev/null \
>>> +	-x c /dev/null -o /dev/null 2>&1; \
>>> +	echo $$?)
>>> +ifeq ($(compdb_check),0)
>>> +override GENERATE_COMPILATION_DATABASE = yes
>> 
>> This feels strange.  If the end user said to GENERATE and we find we
>> are capable, we still override to 'yes'?  What if the end user set
>> 'no' to the GENERATE_COMPILATION_DATABASE macro?  Shouldn't we be
>> honoring that wish?
>
> We should. I'll tweak (and simplify) that for v3.

I think

 - GENERATE_COMPILATION_DATABASE is set to 'no': don't even probe

 - GENERATE_COMPILATION_DATABASE is set to 'yes': probe and turn it
   to 'no' if unavailable.

 - GENERATE_COMPILATION_DATABASE is set to anything else: either
   error out, or turn it into 'no' (I have no preference between
   them).

would cover all the cases.

>>> +compdb_file = $(compdb_dir)$(subst .-,,$(subst /,-,$(dir $@)))$(notdir $@).json
>> 
>> This detail does not matter as long as the end result ensures unique
>> output for all source files, but I am having trouble guessing what
>> the outermost subst, which removes ".-" sequence, is trying to make
>> prettier.  Care to explain?
>
> Yes, it is because the `$(dir $@)` Makefile function will return `./` for source files 
> at the base of the repo, so the JSON files get named eg. `.-rebase.o.json` and then they are 
> hidden. So it's just to make them non-hidden, so as not to confuse someone that would
> count the number of source files and compare with the number of (non-hidden)
>  '*.o.json' files in $(comdb_dir) and get a different number.

Hmph.  Would $(subst /,-,$@) instead of "only substitute leading
directory part, and concatenate the basename part unmolested" work
better then?  After all, by definition the basename part would not
have a slash in it, so substituting all '/' to '-' in the whole
pathname should do the same thing and we won't have to worry about
the spurious './', no?