{"thread":{"id":"54160","subject":"[PATCH] Makefile: add support for generating JSON compilation database","startedAt":"2020-08-30T19:28:34Z","lastAt":"2020-09-03T22:13:44Z","messageCount":15,"participants":["Philippe Blain via GitGitGadget","brian m. carlson","Philippe Blain","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"404770","messageId":"pull.714.git.1598815707540.gitgitgadget@gmail.com","threadId":"54160","inReplyTo":null,"subject":"[PATCH] Makefile: add support for generating JSON compilation database","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-08-30T19:28:27Z","receivedAt":"2020-08-30T19:28:34Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain <levraiphilippeblain@gmail.com>\n\nTools based on LibClang [1] can make use of a 'JSON Compilation\nDatabase' [2] that keeps track of the exact options used to compile a set\nof source files.\n\nThe Clang compiler can generate JSON fragments when compiling [3],\nusing the `-MJ` flag. These JSON fragments (one per compiled source\nfile) can then be concatenated to create the compilation database,\ncommonly called 'compile_commands.json'.\n\nAdd support to the Makefile for generating these JSON fragments as well\nas the compilation database itself, if the environment variable\n'GENERATE_COMPILATION_DATABASE' is set.\n\nIf this variable is set, check that $(CC) indeed supports the `-MJ`\nflag, following what is done for automatic dependencies.\n\nAll JSON fragments are placed in the 'compile_commands/' directory, and\nthe compilation database 'compile_commands.json' is generated as a\ndependency of the 'all' target using a `sed` invocation.\n\n[1] https://clang.llvm.org/docs/Tooling.html\n[2] https://clang.llvm.org/docs/JSONCompilationDatabase.html\n[3] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mj-arg\n\nSigned-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n---\n    Add support for generating JSON compilation database\n    \n    I don't have a lot of knowledge of Make double-colon rules, or insight\n    into why they are used for the 'all' target, but I think the approach I\n    chose makes sense. In particular, I do not list any prerequisite for the\n    'compile_commands.json' file, but from what I tested it is still rebuilt\n    anytime the 'all' target is rebuilt, which is what we want.\n    \n    Note: CMakeLists.txt in contrib/buildsystems does not need to be updated\n    to also support this feature because CMake supports it out-of-the-box\n    [1].\n    \n    [1] \n    https://cmake.org/cmake/help/latest/variable/CMAKE_EXPORT_COMPILE_COMMANDS.html\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-714%2Fphil-blain%2Fcompiledb-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-714/phil-blain/compiledb-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/714\n\n .gitignore |  2 ++\n Makefile   | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 49 insertions(+), 5 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex ee509a2ad2..f4c51300e0 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -197,6 +197,7 @@\n /git.spec\n *.exe\n *.[aos]\n+*.o.json\n *.py[co]\n .depend/\n *.gcda\n@@ -218,6 +219,7 @@\n /tags\n /TAGS\n /cscope*\n+/compile_commands.json\n *.hcc\n *.obj\n *.lib\ndiff --git a/Makefile b/Makefile\nindex 65f8cfb236..954bd2aa47 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -462,6 +462,12 @@ all::\n # the global variable _wpgmptr containing the absolute path of the current\n # executable (this is the case on Windows).\n #\n+# Define GENERATE_COMPILATION_DATABASE to generate JSON compilation database\n+# entries during compilation if your compiler supports it, using the `-MJ` flag.\n+# The JSON entries will be placed in the `compile_commands/` directory,\n+# and the JSON compilation database can be created afterwards with\n+# `make compile_commands.json`.\n+#\n # Define DEVELOPER to enable more compiler warnings. Compiler version\n # and family are auto detected, but could be overridden by defining\n # COMPILER_FEATURES (see config.mak.dev). You can still set\n@@ -1258,6 +1264,20 @@ $(error please set COMPUTE_HEADER_DEPENDENCIES to yes, no, or auto \\\n endif\n endif\n \n+ifdef GENERATE_COMPILATION_DATABASE\n+compdb_check = $(shell $(CC) $(ALL_CFLAGS) \\\n+\t-c -MJ /dev/null \\\n+\t-x c /dev/null -o /dev/null 2>&1; \\\n+\techo $$?)\n+ifeq ($(compdb_check),0)\n+override GENERATE_COMPILATION_DATABASE = yes\n+else\n+override GENERATE_COMPILATION_DATABASE = no\n+$(warning GENERATE_COMPILATION_DATABASE is set, but your compiler does not \\\n+support generating compilation database entries)\n+endif\n+endif\n+\n ifdef SANE_TOOL_PATH\n SANE_TOOL_PATH_SQ = $(subst ','\\'',$(SANE_TOOL_PATH))\n BROKEN_PATH_FIX = 's|^\\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix \"$(SANE_TOOL_PATH_SQ)\"|'\n@@ -2381,16 +2401,30 @@ missing_dep_dirs =\n dep_args =\n endif\n \n+compdb_dir = compile_commands/\n+\n+ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n+missing_compdb_dir = $(compdb_dir)\n+$(missing_compdb_dir):\n+\t@mkdir -p $@\n+\n+compdb_file = $(compdb_dir)$(subst .-,,$(subst /,-,$(dir $@)))$(notdir $@).json\n+compdb_args = -MJ $(compdb_file)\n+else\n+missing_compdb_dir =\n+compdb_args =\n+endif\n+\n ASM_SRC := $(wildcard $(OBJECTS:o=S))\n ASM_OBJ := $(ASM_SRC:S=o)\n C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))\n \n .SUFFIXES:\n \n-$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)\n-\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n-$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)\n-\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n+$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)\n+\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n+$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)\n+\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n \n %.s: %.c GIT-CFLAGS FORCE\n \t$(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n@@ -2413,6 +2447,14 @@ else\n $(OBJECTS): $(LIB_H) $(GENERATED_H)\n endif\n \n+ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n+all:: compile_commands.json\n+compile_commands.json:\n+\t@$(RM) $@\n+\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)*.o.json > $@+\n+\t@if test -s $@+; then mv $@+ $@; else $(RM) $@+; fi\n+endif\n+\n exec-cmd.sp exec-cmd.s exec-cmd.o: GIT-PREFIX\n exec-cmd.sp exec-cmd.s exec-cmd.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n@@ -3117,7 +3159,7 @@ clean: profile-clean coverage-clean cocciclean\n \t$(RM) $(TEST_PROGRAMS)\n \t$(RM) $(FUZZ_PROGRAMS)\n \t$(RM) $(HCC)\n-\t$(RM) -r bin-wrappers $(dep_dirs)\n+\t$(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json\n \t$(RM) -r po/build/\n \t$(RM) *.pyc *.pyo */*.pyc */*.pyo $(GENERATED_H) $(ETAGS_TARGET) tags cscope*\n \t$(RM) -r $(GIT_TARNAME) .doc-tmp-dir\n\nbase-commit: d9cd4331470f4d9d78677f12dc79063dab832f53\n-- \ngitgitgadget\n"},{"id":"404776","messageId":"20200830221004.GB241078@camp.crustytoothpaste.net","threadId":"54160","inReplyTo":"pull.714.git.1598815707540.gitgitgadget@gmail.com","subject":"Re: [PATCH] Makefile: add support for generating JSON compilation database","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-08-30T22:10:04Z","receivedAt":"2020-08-30T22:10:13Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-08-30 at 19:28:27, Philippe Blain via GitGitGadget wrote:\n> From: Philippe Blain <levraiphilippeblain@gmail.com>\n> \n> Tools based on LibClang [1] can make use of a 'JSON Compilation\n> Database' [2] that keeps track of the exact options used to compile a set\n> of source files.\n\nFor additional context why this is valuable, clangd, which is a C\nlanguage server protocol implementation, can use these files to\ndetermine the flags needed to compile a file so it can provide proper\neditor integration.  As a result, editors supporting the language server\nprotocol (such as VS Code, or Vim with a suitable plugin) can provide\nbetter searching, integration, and refactoring tools.\n\nSo I'm very much in favor of a change like this.\n\n> +ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n> +all:: compile_commands.json\n> +compile_commands.json:\n> +\t@$(RM) $@\n> +\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)*.o.json > $@+\n> +\t@if test -s $@+; then mv $@+ $@; else $(RM) $@+; fi\n> +endif\n\nHow are those commas at the end of the line added?  Are they natively\npart of the files?  If so, this seems reasonable.\n-- \nbrian m. carlson: Houston, Texas, US\n"},{"id":"404777","messageId":"FC95CFF7-F9DA-4CDA-9923-99C3432DCAD5@gmail.com","threadId":"54160","inReplyTo":"pull.714.git.1598815707540.gitgitgadget@gmail.com","subject":"Re: [PATCH] Makefile: add support for generating JSON compilation database","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-08-30T22:17:55Z","receivedAt":"2020-08-30T22:18:05Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"\n> #\n> +# Define GENERATE_COMPILATION_DATABASE to generate JSON compilation database\n> +# entries during compilation if your compiler supports it, using the `-MJ` flag.\n> +# The JSON entries will be placed in the `compile_commands/` directory,\n> +# and the JSON compilation database can be created afterwards with\n> +# `make compile_commands.json`.\n> +#\n\nI'm realizing that the way I'm describing how it works here is wrong: there is no\nseparate 'make compile_commands.json' step needed (it was needed in my first draft).\n\nI'll fix that for v2.\n\n"},{"id":"404779","messageId":"194CC2CB-8C74-446E-940D-DC4582AF3EB3@gmail.com","threadId":"54160","inReplyTo":"20200830221004.GB241078@camp.crustytoothpaste.net","subject":"Re: [PATCH] Makefile: add support for generating JSON compilation database","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-08-31T02:37:45Z","receivedAt":"2020-08-31T02:37:55Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Brian, \n\n> Le 30 août 2020 à 18:10, brian m. carlson <sandals@crustytoothpaste.net> a écrit :\n> \n> On 2020-08-30 at 19:28:27, Philippe Blain via GitGitGadget wrote:\n>> From: Philippe Blain <levraiphilippeblain@gmail.com>\n>> \n>> Tools based on LibClang [1] can make use of a 'JSON Compilation\n>> Database' [2] that keeps track of the exact options used to compile a set\n>> of source files.\n> \n> For additional context why this is valuable, clangd, which is a C\n> language server protocol implementation, can use these files to\n> determine the flags needed to compile a file so it can provide proper\n> editor integration.  As a result, editors supporting the language server\n> protocol (such as VS Code, or Vim with a suitable plugin) can provide\n> better searching, integration, and refactoring tools.\n> \n> So I'm very much in favor of a change like this.\n\nThanks!\n\n> \n>> +ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n>> +all:: compile_commands.json\n>> +compile_commands.json:\n>> +\t@$(RM) $@\n>> +\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)*.o.json > $@+\n>> +\t@if test -s $@+; then mv $@+ $@; else $(RM) $@+; fi\n>> +endif\n> \n> How are those commas at the end of the line added?  Are they natively\n> part of the files?  If so, this seems reasonable.\n\nYes: the '*.o.json' files generated by the compiler contain one JSON object per file, \nwith a trailing comma.\nThis 'sed' invocation turns these files into a proper JSON array by:\n- adding a '[' at the beginning and a ']' at the end of the list of objects\n- removing the comma after the last entry (before the closing ']')\n\n"},{"id":"404781","messageId":"xmqqsgc3h28s.fsf@gitster.c.googlers.com","threadId":"54160","inReplyTo":"20200830221004.GB241078@camp.crustytoothpaste.net","subject":"Re: [PATCH] Makefile: add support for generating JSON compilation database","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-31T04:24:03Z","receivedAt":"2020-08-31T04:26:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> On 2020-08-30 at 19:28:27, Philippe Blain via GitGitGadget wrote:\n>> From: Philippe Blain <levraiphilippeblain@gmail.com>\n>> \n>> Tools based on LibClang [1] can make use of a 'JSON Compilation\n>> Database' [2] that keeps track of the exact options used to compile a set\n>> of source files.\n>\n> For additional context why this is valuable, clangd, which is a C\n> language server protocol implementation, can use these files to\n> determine the flags needed to compile a file so it can provide proper\n> editor integration.  As a result, editors supporting the language server\n> protocol (such as VS Code, or Vim with a suitable plugin) can provide\n> better searching, integration, and refactoring tools.\n\nI found that the proposed commit log was very weak to sell the\nchange; some of what you gave above should definitely help strenthen\nit.\n\nThanks.\n"},{"id":"404837","messageId":"20200901073827.GA3967005@coredump.intra.peff.net","threadId":"54160","inReplyTo":"xmqqsgc3h28s.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] Makefile: add support for generating JSON compilation database","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-01T07:38:27Z","receivedAt":"2020-09-01T07:38:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 30, 2020 at 09:24:03PM -0700, Junio C Hamano wrote:\n\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n> > On 2020-08-30 at 19:28:27, Philippe Blain via GitGitGadget wrote:\n> >> From: Philippe Blain <levraiphilippeblain@gmail.com>\n> >> \n> >> Tools based on LibClang [1] can make use of a 'JSON Compilation\n> >> Database' [2] that keeps track of the exact options used to compile a set\n> >> of source files.\n> >\n> > For additional context why this is valuable, clangd, which is a C\n> > language server protocol implementation, can use these files to\n> > determine the flags needed to compile a file so it can provide proper\n> > editor integration.  As a result, editors supporting the language server\n> > protocol (such as VS Code, or Vim with a suitable plugin) can provide\n> > better searching, integration, and refactoring tools.\n> \n> I found that the proposed commit log was very weak to sell the\n> change; some of what you gave above should definitely help strenthen\n> it.\n\nLikewise. Looking at the output, I'm confused how it would help with\nthings like searching and refactoring. It might be nice to spell it out\nfor those of us exposed to it for the first time (I tried following the\nlinks but remained unenlightened).\n\nI'd also be curious to hear what advantages it gives to add a new\nMakefile knob rather than just letting interested parties add -MJ to\ntheir CFLAGS. Is it just a convenience to create the concatenated form?\nIt seems weird that projects would need to do so themselves with sed\nhackery (i.e., I'd expect whatever consumes this json to be able to\nhandle multiple files).\n\n-Peff\n"},{"id":"404862","messageId":"867D6718-E377-47B8-BD32-0800872B3DE0@gmail.com","threadId":"54160","inReplyTo":"20200901073827.GA3967005@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: add support for generating JSON compilation database","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-09-01T13:18:01Z","receivedAt":"2020-09-01T13:27:44Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"\n> Le 1 sept. 2020 à 03:38, Jeff King <peff@peff.net> a écrit :\n> \n> On Sun, Aug 30, 2020 at 09:24:03PM -0700, Junio C Hamano wrote:\n> \n>> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n>> \n>>> On 2020-08-30 at 19:28:27, Philippe Blain via GitGitGadget wrote:\n>>>> From: Philippe Blain <levraiphilippeblain@gmail.com>\n>>>> \n>>>> Tools based on LibClang [1] can make use of a 'JSON Compilation\n>>>> Database' [2] that keeps track of the exact options used to compile a set\n>>>> of source files.\n>>> \n>>> For additional context why this is valuable, clangd, which is a C\n>>> language server protocol implementation, can use these files to\n>>> determine the flags needed to compile a file so it can provide proper\n>>> editor integration.  As a result, editors supporting the language server\n>>> protocol (such as VS Code, or Vim with a suitable plugin) can provide\n>>> better searching, integration, and refactoring tools.\n>> \n>> I found that the proposed commit log was very weak to sell the\n>> change; some of what you gave above should definitely help strenthen\n>> it.\n> \n> Likewise. Looking at the output, I'm confused how it would help with\n> things like searching and refactoring. It might be nice to spell it out\n> for those of us exposed to it for the first time (I tried following the\n> links but remained unenlightened).\n\nOK, I'll improve the commit message. I'm not at all an expert in this subject,\nI just had to generate a compilation database myself to use the Sourcetrail source\nexplorer [1] with Git so I figured I'd share what I had done. Further exploration of the \ntopic are in [2] and [3]. Note that I did try some of the tools listed in [2] before resorting\nto modifying the Makefile, but these tools either did not work at all or produced wrong \noutput (ex. strings in the JSON were not properly quoted, etc.)\n\n> I'd also be curious to hear what advantages it gives to add a new\n> Makefile knob rather than just letting interested parties add -MJ to\n> their CFLAGS. Is it just a convenience to create the concatenated form?\n\nUnfortunately this would not work because the '-MJ' flag needs a file name\nto know where to put the JSON fragment.\n\nThanks,\nPhilippe.\n\n\n[1] www.sourcetrail.com\n[2] https://sarcasm.github.io/notes/dev/compilation-database.html\n[3] https://eli.thegreenplace.net/2014/05/21/compilation-databases-for-clang-based-tools\n\n"},{"id":"404892","messageId":"pull.714.v2.git.1599001759548.gitgitgadget@gmail.com","threadId":"54160","inReplyTo":"pull.714.git.1598815707540.gitgitgadget@gmail.com","subject":"[PATCH v2] Makefile: add support for generating JSON compilation database","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-01T23:09:19Z","receivedAt":"2020-09-01T23:09:27Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain <levraiphilippeblain@gmail.com>\n\nTools based on LibClang [1] can make use of a 'JSON Compilation\nDatabase' [2] that keeps track of the exact options used to compile a set\nof source files.\n\nFor example, clangd [3], which is a C language server protocol\nimplementation, can use a JSON compilation database to determine the\nflags needed to compile a file so it can provide proper editor\nintegration.  As a result, editors supporting the language server\nprotocol (such as VS Code, Emacs, or Vim, with suitable plugins) can\nprovide better searching, integration, and refactoring tools.\n\nThe Clang compiler can generate JSON fragments when compiling [4],\nusing the `-MJ` flag. These JSON fragments (one per compiled source\nfile) can then be concatenated to create the compilation database,\ncommonly called 'compile_commands.json'.\n\nAdd support to the Makefile for generating these JSON fragments as well\nas the compilation database itself, if the environment variable\n'GENERATE_COMPILATION_DATABASE' is set.\n\nIf this variable is set, check that $(CC) indeed supports the `-MJ`\nflag, following what is done for automatic dependencies.\n\nAll JSON fragments are placed in the 'compile_commands/' directory, and\nthe compilation database 'compile_commands.json' is generated as a\ndependency of the 'all' target using a `sed` invocation.\n\n[1] https://clang.llvm.org/docs/Tooling.html\n[2] https://clang.llvm.org/docs/JSONCompilationDatabase.html\n[3] https://clangd.llvm.org/\n[4] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mj-arg\n\nHelped-by: brian m. carlson <sandals@crustytoothpaste.net>\nSigned-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n---\n    Add support for generating JSON compilation database\n    \n    Changes since v1:\n    \n     * Added a paragraph to the commit message to better explain why this\n       feature would help Git developers (Thanks Brian!)\n     * Corrected the description of the new GENERATE_COMPILATION_DATABASE \n       Makefile knob to make it reflect its actual behavior (cf. \n       https://lore.kernel.org/git/FC95CFF7-F9DA-4CDA-9923-99C3432DCAD5@gmail.com/\n       )\n    \n    v1: I don't have a lot of knowledge of Make double-colon rules, or\n    insight into why they are used for the 'all' target, but I think the\n    approach I chose makes sense. In particular, I do not list any\n    prerequisite for the 'compile_commands.json' file, but from what I\n    tested it is still rebuilt anytime the 'all' target is rebuilt, which is\n    what we want.\n    \n    Note: CMakeLists.txt in contrib/buildsystems does not need to be updated\n    to also support this feature because CMake supports it out-of-the-box\n    [1].\n    \n    [1] \n    https://cmake.org/cmake/help/latest/variable/CMAKE_EXPORT_COMPILE_COMMANDS.html\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-714%2Fphil-blain%2Fcompiledb-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-714/phil-blain/compiledb-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/714\n\nRange-diff vs v1:\n\n 1:  411fefaafb ! 1:  da9544e4aa Makefile: add support for generating JSON compilation database\n     @@ Commit message\n          Database' [2] that keeps track of the exact options used to compile a set\n          of source files.\n      \n     -    The Clang compiler can generate JSON fragments when compiling [3],\n     +    For example, clangd [3], which is a C language server protocol\n     +    implementation, can use a JSON compilation database to determine the\n     +    flags needed to compile a file so it can provide proper editor\n     +    integration.  As a result, editors supporting the language server\n     +    protocol (such as VS Code, Emacs, or Vim, with suitable plugins) can\n     +    provide better searching, integration, and refactoring tools.\n     +\n     +    The Clang compiler can generate JSON fragments when compiling [4],\n          using the `-MJ` flag. These JSON fragments (one per compiled source\n          file) can then be concatenated to create the compilation database,\n          commonly called 'compile_commands.json'.\n     @@ Commit message\n      \n          [1] https://clang.llvm.org/docs/Tooling.html\n          [2] https://clang.llvm.org/docs/JSONCompilationDatabase.html\n     -    [3] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mj-arg\n     +    [3] https://clangd.llvm.org/\n     +    [4] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mj-arg\n      \n     +    Helped-by: brian m. carlson <sandals@crustytoothpaste.net>\n          Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n      \n       ## .gitignore ##\n     @@ Makefile: all::\n      +# Define GENERATE_COMPILATION_DATABASE to generate JSON compilation database\n      +# entries during compilation if your compiler supports it, using the `-MJ` flag.\n      +# The JSON entries will be placed in the `compile_commands/` directory,\n     -+# and the JSON compilation database can be created afterwards with\n     -+# `make compile_commands.json`.\n     ++# and the JSON compilation database 'compile_commands.json' will be created \n     ++# at the root of the repository. \n      +#\n       # Define DEVELOPER to enable more compiler warnings. Compiler version\n       # and family are auto detected, but could be overridden by defining\n\n\n .gitignore |  2 ++\n Makefile   | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 49 insertions(+), 5 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex ee509a2ad2..f4c51300e0 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -197,6 +197,7 @@\n /git.spec\n *.exe\n *.[aos]\n+*.o.json\n *.py[co]\n .depend/\n *.gcda\n@@ -218,6 +219,7 @@\n /tags\n /TAGS\n /cscope*\n+/compile_commands.json\n *.hcc\n *.obj\n *.lib\ndiff --git a/Makefile b/Makefile\nindex 65f8cfb236..51cd0f302b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -462,6 +462,12 @@ all::\n # the global variable _wpgmptr containing the absolute path of the current\n # executable (this is the case on Windows).\n #\n+# Define GENERATE_COMPILATION_DATABASE to generate JSON compilation database\n+# entries during compilation if your compiler supports it, using the `-MJ` flag.\n+# The JSON entries will be placed in the `compile_commands/` directory,\n+# and the JSON compilation database 'compile_commands.json' will be created \n+# at the root of the repository. \n+#\n # Define DEVELOPER to enable more compiler warnings. Compiler version\n # and family are auto detected, but could be overridden by defining\n # COMPILER_FEATURES (see config.mak.dev). You can still set\n@@ -1258,6 +1264,20 @@ $(error please set COMPUTE_HEADER_DEPENDENCIES to yes, no, or auto \\\n endif\n endif\n \n+ifdef GENERATE_COMPILATION_DATABASE\n+compdb_check = $(shell $(CC) $(ALL_CFLAGS) \\\n+\t-c -MJ /dev/null \\\n+\t-x c /dev/null -o /dev/null 2>&1; \\\n+\techo $$?)\n+ifeq ($(compdb_check),0)\n+override GENERATE_COMPILATION_DATABASE = yes\n+else\n+override GENERATE_COMPILATION_DATABASE = no\n+$(warning GENERATE_COMPILATION_DATABASE is set, but your compiler does not \\\n+support generating compilation database entries)\n+endif\n+endif\n+\n ifdef SANE_TOOL_PATH\n SANE_TOOL_PATH_SQ = $(subst ','\\'',$(SANE_TOOL_PATH))\n BROKEN_PATH_FIX = 's|^\\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix \"$(SANE_TOOL_PATH_SQ)\"|'\n@@ -2381,16 +2401,30 @@ missing_dep_dirs =\n dep_args =\n endif\n \n+compdb_dir = compile_commands/\n+\n+ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n+missing_compdb_dir = $(compdb_dir)\n+$(missing_compdb_dir):\n+\t@mkdir -p $@\n+\n+compdb_file = $(compdb_dir)$(subst .-,,$(subst /,-,$(dir $@)))$(notdir $@).json\n+compdb_args = -MJ $(compdb_file)\n+else\n+missing_compdb_dir =\n+compdb_args =\n+endif\n+\n ASM_SRC := $(wildcard $(OBJECTS:o=S))\n ASM_OBJ := $(ASM_SRC:S=o)\n C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))\n \n .SUFFIXES:\n \n-$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)\n-\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n-$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)\n-\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n+$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)\n+\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n+$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)\n+\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n \n %.s: %.c GIT-CFLAGS FORCE\n \t$(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n@@ -2413,6 +2447,14 @@ else\n $(OBJECTS): $(LIB_H) $(GENERATED_H)\n endif\n \n+ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n+all:: compile_commands.json\n+compile_commands.json:\n+\t@$(RM) $@\n+\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)*.o.json > $@+\n+\t@if test -s $@+; then mv $@+ $@; else $(RM) $@+; fi\n+endif\n+\n exec-cmd.sp exec-cmd.s exec-cmd.o: GIT-PREFIX\n exec-cmd.sp exec-cmd.s exec-cmd.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n@@ -3117,7 +3159,7 @@ clean: profile-clean coverage-clean cocciclean\n \t$(RM) $(TEST_PROGRAMS)\n \t$(RM) $(FUZZ_PROGRAMS)\n \t$(RM) $(HCC)\n-\t$(RM) -r bin-wrappers $(dep_dirs)\n+\t$(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json\n \t$(RM) -r po/build/\n \t$(RM) *.pyc *.pyo */*.pyc */*.pyo $(GENERATED_H) $(ETAGS_TARGET) tags cscope*\n \t$(RM) -r $(GIT_TARNAME) .doc-tmp-dir\n\nbase-commit: d9cd4331470f4d9d78677f12dc79063dab832f53\n-- \ngitgitgadget\n"},{"id":"404897","messageId":"20200902013351.GD241078@camp.crustytoothpaste.net","threadId":"54160","inReplyTo":"20200901073827.GA3967005@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: add support for generating JSON compilation database","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-09-02T01:33:51Z","receivedAt":"2020-09-02T01:33:59Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-09-01 at 07:38:27, Jeff King wrote:\n> Likewise. Looking at the output, I'm confused how it would help with\n> things like searching and refactoring. It might be nice to spell it out\n> for those of us exposed to it for the first time (I tried following the\n> links but remained unenlightened).\n\nTraditionally, editors had to learn about every language if they wanted\nto add special functionality like refactoring (e.g., renaming \"struct\nfoo\" to \"struct bar\"), finding all the instances of a type, finding\nwhere a type or function was declared, or similar IDE features.  When\nMicrosoft developed Visual Studio Code, they decided that they did not\nwant to implement this functionality for every language under the sun,\nand instead developed the Language Server Protocol[0].\n\nWith LSP, each editor needs functionality to speak its portion (either\nnatively, as with VS Code, or with a plugin, such as Vim's ALE) and each\nlanguage implements a language server to implement its part of the\nfunctionality.  The protocol is capability based, so implementations can\nsupport those features which make sense for their editor or language and\nomit those which don't.  This way, all editors can benefit and language\ncommunities can implement one program to provide features, and the\nproblem becomes an O(M + N) problem instead of an O(M * N) problem.\n\nIn some languages, like Rust, it's pretty obvious how to compile your\nproject: you use cargo, the built-in build tool.  There is also a\nstandard layout to find and enumerate files within a project.  However,\nC is not so standardized, so clangd, which is a clang-based C and C++\nLSP implementation, needs help to find out which flags are needed to\ncompile, and therefore find the header files to make sense of parsing\nthe C code and implementing its side of the protocol.  That's what this\npatch implements.\n\nI use Vim and ALE extensively, and it pretty much just works for most\nlanguages, including Go and Rust, once you install the LSP server.  Git\nis one of the few projects I work on which is still C and therefore\nneeds help here.\n\nHopefully this is at least more enlightening about the functionality\nthat clangd provides, why it's interesting, how it works, and why it's\nvaluable.\n\n> I'd also be curious to hear what advantages it gives to add a new\n> Makefile knob rather than just letting interested parties add -MJ to\n> their CFLAGS. Is it just a convenience to create the concatenated form?\n> It seems weird that projects would need to do so themselves with sed\n> hackery (i.e., I'd expect whatever consumes this json to be able to\n> handle multiple files).\n\nI believe clangd does need the concatenated form, and at least the ALE\nplugin for Vim uses that specific file name to detect whether clangd\nshould be used.  The problem is that clangd doesn't know where your\nsource code is actually located and it's very expensive to traverse an\nentire repository which might contain literally millions of files if\nyou're only really interested in a handful.\n\n[0] https://microsoft.github.io/language-server-protocol/\n-- \nbrian m. carlson: Houston, Texas, US\n"},{"id":"404911","messageId":"20200902080432.GB855335@coredump.intra.peff.net","threadId":"54160","inReplyTo":"20200902013351.GD241078@camp.crustytoothpaste.net","subject":"Re: [PATCH] Makefile: add support for generating JSON compilation database","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-02T08:04:32Z","receivedAt":"2020-09-02T08:04:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 02, 2020 at 01:33:51AM +0000, brian m. carlson wrote:\n\n> Traditionally, editors had to learn about every language if they wanted\n> to add special functionality like refactoring (e.g., renaming \"struct\n> foo\" to \"struct bar\"), finding all the instances of a type, finding\n> where a type or function was declared, or similar IDE features.  When\n> Microsoft developed Visual Studio Code, they decided that they did not\n> want to implement this functionality for every language under the sun,\n> and instead developed the Language Server Protocol[0].\n> [...]\n\nThanks for the explanation. I understand what LSP does, but the missing\nlink for me was how \"here are the command-line flags to the compiler\"\nturned into something useful like \"here's a list of identifiers\". And\nclangd fills in that gap (presumably re-running the front-end bits of\nclang on the fly to pull out that kind of information).\n\n-Peff\n"},{"id":"404923","messageId":"xmqq1rjkccw8.fsf@gitster.c.googlers.com","threadId":"54160","inReplyTo":"pull.714.v2.git.1599001759548.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] Makefile: add support for generating JSON compilation database","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-02T17:21:59Z","receivedAt":"2020-09-02T17:22:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philippe Blain via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> diff --git a/.gitignore b/.gitignore\n> index ee509a2ad2..f4c51300e0 100644\n> --- a/.gitignore\n> +++ b/.gitignore\n> @@ -197,6 +197,7 @@\n>  /git.spec\n>  *.exe\n>  *.[aos]\n> +*.o.json\n\nDoes this naming/suffix follow the common practice followed among\nthose projects that use the -MJ option?  Even though I may have\nheard of it, I am not familiar with the use of the feature at all,\nand to such a person, naming a file after what format it is written\nin (i.e. 'json') feels a lot less useful than what it contains\n(i.e. compilation database entries).  \n\nIt's like naming our source files with .txt suffix ;-)\n\n> +# Define GENERATE_COMPILATION_DATABASE to generate JSON compilation database\n> +# entries during compilation if your compiler supports it, using the `-MJ` flag.\n> +# The JSON entries will be placed in the `compile_commands/` directory,\n> +# and the JSON compilation database 'compile_commands.json' will be created \n> +# at the root of the repository. \n\nLikewise for the name of the directory and the resulting single\ndatabase file.  I just want to make sure that we are following the\ncommon convention, so people familiar with the use of the feature\nwould feel at home, so a simple answer \"yes\" would suffice.\n\n> +ifdef GENERATE_COMPILATION_DATABASE\n> +compdb_check = $(shell $(CC) $(ALL_CFLAGS) \\\n> +\t-c -MJ /dev/null \\\n> +\t-x c /dev/null -o /dev/null 2>&1; \\\n> +\techo $$?)\n> +ifeq ($(compdb_check),0)\n> +override GENERATE_COMPILATION_DATABASE = yes\n\nThis feels strange.  If the end user said to GENERATE and we find we\nare capable, we still override to 'yes'?  What if the end user set\n'no' to the GENERATE_COMPILATION_DATABASE macro?  Shouldn't we be\nhonoring that wish?\n\n> +else\n> +override GENERATE_COMPILATION_DATABASE = no\n> +$(warning GENERATE_COMPILATION_DATABASE is set, but your compiler does not \\\n> +support generating compilation database entries)\n\nThis side is perfectly understandable and I think it is a valid use\nof override.  But I do not understand the other side.\n\n> @@ -2381,16 +2401,30 @@ missing_dep_dirs =\n>  dep_args =\n>  endif\n>  \n> +compdb_dir = compile_commands/\n> +\n> +ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n> +missing_compdb_dir = $(compdb_dir)\n> +$(missing_compdb_dir):\n> +\t@mkdir -p $@\n> +\n> +compdb_file = $(compdb_dir)$(subst .-,,$(subst /,-,$(dir $@)))$(notdir $@).json\n\nThis detail does not matter as long as the end result ensures unique\noutput for all source files, but I am having trouble guessing what\nthe outermost subst, which removes \".-\" sequence, is trying to make\nprettier.  Care to explain?\n\n> +compdb_args = -MJ $(compdb_file)\n> +else\n> +missing_compdb_dir =\n> +compdb_args =\n\nThese are unfortunate.  I briefly wondered if we can make GIT-CFLAGS\ndepend on these two variables so that ASM_OBJ and C_OBJ do not have\nto depend on them, but the actual rules need to be updated anyway,\nso it cannot be helped.\n\nI do wonder if we can unify dep_args and compdb_args into a single\nextra_args (and similarly dep_dirs and compdb_dir to extra_dirs) so\nthat future similar options can all piggyback on it, but this can do\nfor now.\n\n> @@ -2413,6 +2447,14 @@ else\n>  $(OBJECTS): $(LIB_H) $(GENERATED_H)\n>  endif\n>  \n> +ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n> +all:: compile_commands.json\n> +compile_commands.json:\n> +\t@$(RM) $@\n> +\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)*.o.json > $@+\n\nOK.  The entire thing is concatenated and enclosed by a single pair\nof '[' and ']'.\n\nIf we are planning to allow developers customize compdb_dir,\nrequiring trailing slash in the value of $(compdb_dir) is not very\nfriendly.\n\n\nThanks.\n"},{"id":"404989","messageId":"41D647C5-BC9E-431F-BEF9-C0040F4E0C94@gmail.com","threadId":"54160","inReplyTo":"xmqq1rjkccw8.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] Makefile: add support for generating JSON compilation database","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-09-03T21:17:23Z","receivedAt":"2020-09-03T21:17:31Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Junio, \n\n> Le 2 sept. 2020 à 13:21, Junio C Hamano <gitster@pobox.com> a écrit :\n> \n> \"Philippe Blain via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n>> diff --git a/.gitignore b/.gitignore\n>> index ee509a2ad2..f4c51300e0 100644\n>> --- a/.gitignore\n>> +++ b/.gitignore\n>> @@ -197,6 +197,7 @@\n>> /git.spec\n>> *.exe\n>> *.[aos]\n>> +*.o.json\n> \n> Does this naming/suffix follow the common practice followed among\n> those projects that use the -MJ option?  Even though I may have\n> heard of it, I am not familiar with the use of the feature at all,\n> and to such a person, naming a file after what format it is written\n> in (i.e. 'json') feels a lot less useful than what it contains\n> (i.e. compilation database entries).  \n> \n> It's like naming our source files with .txt suffix ;-)\n\nThis addition to the .gitignore is for the individual JSON files (one per source file), \nthat are placed in the $(compdb_dir). \nI think naming \"rebase.o.json\" the JSON file that describes how to compile \"rebase.c\"\ninto \"rebase.o\" makes sense. I don't know what is the convention for other projects.\n\n>> +# Define GENERATE_COMPILATION_DATABASE to generate JSON compilation database\n>> +# entries during compilation if your compiler supports it, using the `-MJ` flag.\n>> +# The JSON entries will be placed in the `compile_commands/` directory,\n>> +# and the JSON compilation database 'compile_commands.json' will be created \n>> +# at the root of the repository. \n> \n> Likewise for the name of the directory and the resulting single\n> database file.  I just want to make sure that we are following the\n> common convention, so people familiar with the use of the feature\n> would feel at home, so a simple answer \"yes\" would suffice.\n\nThe name `compile_commands.json` for the database itself is standard. \nThe name of the directory where the '*.o.json' files are placed is a name\nI chose, and I don't feel strongly about it. I thought it made sense to name\nit like that, then its purpose is clear.  We could make it a hidden directory \nif we don't want to add a new folder to the root of the repo when using this feature.\n\n>> +ifdef GENERATE_COMPILATION_DATABASE\n>> +compdb_check = $(shell $(CC) $(ALL_CFLAGS) \\\n>> +\t-c -MJ /dev/null \\\n>> +\t-x c /dev/null -o /dev/null 2>&1; \\\n>> +\techo $$?)\n>> +ifeq ($(compdb_check),0)\n>> +override GENERATE_COMPILATION_DATABASE = yes\n> \n> This feels strange.  If the end user said to GENERATE and we find we\n> are capable, we still override to 'yes'?  What if the end user set\n> 'no' to the GENERATE_COMPILATION_DATABASE macro?  Shouldn't we be\n> honoring that wish?\n\nWe should. I'll tweak (and simplify) that for v3.\n\n>> @@ -2381,16 +2401,30 @@ missing_dep_dirs =\n>> dep_args =\n>> endif\n>> \n>> +compdb_dir = compile_commands/\n>> +\n>> +ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n>> +missing_compdb_dir = $(compdb_dir)\n>> +$(missing_compdb_dir):\n>> +\t@mkdir -p $@\n>> +\n>> +compdb_file = $(compdb_dir)$(subst .-,,$(subst /,-,$(dir $@)))$(notdir $@).json\n> \n> This detail does not matter as long as the end result ensures unique\n> output for all source files, but I am having trouble guessing what\n> the outermost subst, which removes \".-\" sequence, is trying to make\n> prettier.  Care to explain?\n\nYes, it is because the `$(dir $@)` Makefile function will return `./` for source files \nat the base of the repo, so the JSON files get named eg. `.-rebase.o.json` and then they are \nhidden. So it's just to make them non-hidden, so as not to confuse someone that would\ncount the number of source files and compare with the number of (non-hidden)\n '*.o.json' files in $(comdb_dir) and get a different number.\n\n>> @@ -2413,6 +2447,14 @@ else\n>> $(OBJECTS): $(LIB_H) $(GENERATED_H)\n>> endif\n>> \n>> +ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n>> +all:: compile_commands.json\n>> +compile_commands.json:\n>> +\t@$(RM) $@\n>> +\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)*.o.json > $@+\n> \n> OK.  The entire thing is concatenated and enclosed by a single pair\n> of '[' and ']'.\n\nYes, and the comma after the last entry removed for JSON compliance.\n\n> If we are planning to allow developers customize compdb_dir,\n> requiring trailing slash in the value of $(compdb_dir) is not very\n> friendly.\n\nOK I'll change that.\n\n> Thanks.\n\nThank you for your comments!\n\nPhilippe.\n\n"},{"id":"404991","messageId":"xmqqpn728s3f.fsf@gitster.c.googlers.com","threadId":"54160","inReplyTo":"41D647C5-BC9E-431F-BEF9-C0040F4E0C94@gmail.com","subject":"Re: [PATCH v2] Makefile: add support for generating JSON compilation database","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-03T21:31:48Z","receivedAt":"2020-09-03T21:31:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain <levraiphilippeblain@gmail.com> writes:\n\n> This addition to the .gitignore is for the individual JSON files (one per source file), \n> that are placed in the $(compdb_dir). \n> I think naming \"rebase.o.json\" the JSON file that describes how to compile \"rebase.c\"\n> into \"rebase.o\" makes sense. I don't know what is the convention for other projects.\n\nI agree rebase.o.$somesuffix does make sense, but I do not know\n'json' is a great value for $somesuffix.  I wouldn't be surprised if\n'cdb' or some other silly abbreviation for \"compilation database\" is\nhow other people use this feature.\n\nThose watching from the sidelines.  Does anybody know if there is an\nestablished convention used by other projects?  If we hear nothing\nby early next week, let's declare 'json' is good enough and move on.\n\n> The name `compile_commands.json` for the database itself is standard. \n> The name of the directory where the '*.o.json' files are placed is a name\n> I chose, and I don't feel strongly about it. I thought it made sense to name\n> it like that, then its purpose is clear.  We could make it a hidden directory \n> if we don't want to add a new folder to the root of the repo when using this feature.\n\nI think both of these are sensible.  Again if we hear nothing about\ncommon practice, let's move on with these constants as-is.\n\n>>> +ifdef GENERATE_COMPILATION_DATABASE\n>>> +compdb_check = $(shell $(CC) $(ALL_CFLAGS) \\\n>>> +\t-c -MJ /dev/null \\\n>>> +\t-x c /dev/null -o /dev/null 2>&1; \\\n>>> +\techo $$?)\n>>> +ifeq ($(compdb_check),0)\n>>> +override GENERATE_COMPILATION_DATABASE = yes\n>> \n>> This feels strange.  If the end user said to GENERATE and we find we\n>> are capable, we still override to 'yes'?  What if the end user set\n>> 'no' to the GENERATE_COMPILATION_DATABASE macro?  Shouldn't we be\n>> honoring that wish?\n>\n> We should. I'll tweak (and simplify) that for v3.\n\nI think\n\n - GENERATE_COMPILATION_DATABASE is set to 'no': don't even probe\n\n - GENERATE_COMPILATION_DATABASE is set to 'yes': probe and turn it\n   to 'no' if unavailable.\n\n - GENERATE_COMPILATION_DATABASE is set to anything else: either\n   error out, or turn it into 'no' (I have no preference between\n   them).\n\nwould cover all the cases.\n\n>>> +compdb_file = $(compdb_dir)$(subst .-,,$(subst /,-,$(dir $@)))$(notdir $@).json\n>> \n>> This detail does not matter as long as the end result ensures unique\n>> output for all source files, but I am having trouble guessing what\n>> the outermost subst, which removes \".-\" sequence, is trying to make\n>> prettier.  Care to explain?\n>\n> Yes, it is because the `$(dir $@)` Makefile function will return `./` for source files \n> at the base of the repo, so the JSON files get named eg. `.-rebase.o.json` and then they are \n> hidden. So it's just to make them non-hidden, so as not to confuse someone that would\n> count the number of source files and compare with the number of (non-hidden)\n>  '*.o.json' files in $(comdb_dir) and get a different number.\n\nHmph.  Would $(subst /,-,$@) instead of \"only substitute leading\ndirectory part, and concatenate the basename part unmolested\" work\nbetter then?  After all, by definition the basename part would not\nhave a slash in it, so substituting all '/' to '-' in the whole\npathname should do the same thing and we won't have to worry about\nthe spurious './', no?\n\n"},{"id":"404997","messageId":"98711D92-AE89-46B4-A749-89F85A9A87A7@gmail.com","threadId":"54160","inReplyTo":"xmqqpn728s3f.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] Makefile: add support for generating JSON compilation database","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-09-03T22:04:04Z","receivedAt":"2020-09-03T22:04:11Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"\n> Le 3 sept. 2020 à 17:31, Junio C Hamano <gitster@pobox.com> a écrit :\n> \n> Philippe Blain <levraiphilippeblain@gmail.com> writes:\n> \n>> This addition to the .gitignore is for the individual JSON files (one per source file), \n>> that are placed in the $(compdb_dir). \n>> I think naming \"rebase.o.json\" the JSON file that describes how to compile \"rebase.c\"\n>> into \"rebase.o\" makes sense. I don't know what is the convention for other projects.\n> \n> I agree rebase.o.$somesuffix does make sense, but I do not know\n> 'json' is a great value for $somesuffix.  I wouldn't be surprised if\n> 'cdb' or some other silly abbreviation for \"compilation database\" is\n> how other people use this feature.\n> \n> Those watching from the sidelines.  Does anybody know if there is an\n> established convention used by other projects?  If we hear nothing\n> by early next week, let's declare 'json' is good enough and move on.\n> \n>> The name `compile_commands.json` for the database itself is standard. \n>> The name of the directory where the '*.o.json' files are placed is a name\n>> I chose, and I don't feel strongly about it. I thought it made sense to name\n>> it like that, then its purpose is clear.  We could make it a hidden directory \n>> if we don't want to add a new folder to the root of the repo when using this feature.\n> \n> I think both of these are sensible.  Again if we hear nothing about\n> common practice, let's move on with these constants as-is.\n\nOK. \n\n> \n>>>> +ifdef GENERATE_COMPILATION_DATABASE\n>>>> +compdb_check = $(shell $(CC) $(ALL_CFLAGS) \\\n>>>> +\t-c -MJ /dev/null \\\n>>>> +\t-x c /dev/null -o /dev/null 2>&1; \\\n>>>> +\techo $$?)\n>>>> +ifeq ($(compdb_check),0)\n>>>> +override GENERATE_COMPILATION_DATABASE = yes\n>>> \n>>> This feels strange.  If the end user said to GENERATE and we find we\n>>> are capable, we still override to 'yes'?  What if the end user set\n>>> 'no' to the GENERATE_COMPILATION_DATABASE macro?  Shouldn't we be\n>>> honoring that wish?\n>> \n>> We should. I'll tweak (and simplify) that for v3.\n> \n> I think\n> \n> - GENERATE_COMPILATION_DATABASE is set to 'no': don't even probe\n> \n> - GENERATE_COMPILATION_DATABASE is set to 'yes': probe and turn it\n>   to 'no' if unavailable.\n> \n> - GENERATE_COMPILATION_DATABASE is set to anything else: either\n>   error out, or turn it into 'no' (I have no preference between\n>   them).\n> \n> would cover all the cases.\n\nI agree. I'll do that.\n\n> \n>>>> +compdb_file = $(compdb_dir)$(subst .-,,$(subst /,-,$(dir $@)))$(notdir $@).json\n>>> \n>>> This detail does not matter as long as the end result ensures unique\n>>> output for all source files, but I am having trouble guessing what\n>>> the outermost subst, which removes \".-\" sequence, is trying to make\n>>> prettier.  Care to explain?\n>> \n>> Yes, it is because the `$(dir $@)` Makefile function will return `./` for source files \n>> at the base of the repo, so the JSON files get named eg. `.-rebase.o.json` and then they are \n>> hidden. So it's just to make them non-hidden, so as not to confuse someone that would\n>> count the number of source files and compare with the number of (non-hidden)\n>> '*.o.json' files in $(comdb_dir) and get a different number.\n> \n> Hmph.  Would $(subst /,-,$@) instead of \"only substitute leading\n> directory part, and concatenate the basename part unmolested\" work\n> better then?  After all, by definition the basename part would not\n> have a slash in it, so substituting all '/' to '-' in the whole\n> pathname should do the same thing and we won't have to worry about\n> the spurious './', no?\n\nThis indeed works, and reads better. Thanks!\n\n"},{"id":"404998","messageId":"pull.714.v3.git.1599171218675.gitgitgadget@gmail.com","threadId":"54160","inReplyTo":"pull.714.v2.git.1599001759548.gitgitgadget@gmail.com","subject":"[PATCH v3] Makefile: add support for generating JSON compilation database","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-03T22:13:38Z","receivedAt":"2020-09-03T22:13:44Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain <levraiphilippeblain@gmail.com>\n\nTools based on LibClang [1] can make use of a 'JSON Compilation\nDatabase' [2] that keeps track of the exact options used to compile a set\nof source files.\n\nFor example, clangd [3], which is a C language server protocol\nimplementation, can use a JSON compilation database to determine the\nflags needed to compile a file so it can provide proper editor\nintegration.  As a result, editors supporting the language server\nprotocol (such as VS Code, Emacs, or Vim, with suitable plugins) can\nprovide better searching, integration, and refactoring tools.\n\nThe Clang compiler can generate JSON fragments when compiling [4],\nusing the `-MJ` flag. These JSON fragments (one per compiled source\nfile) can then be concatenated to create the compilation database,\ncommonly called 'compile_commands.json'.\n\nAdd support to the Makefile for generating these JSON fragments as well\nas the compilation database itself, if the environment variable\n'GENERATE_COMPILATION_DATABASE' is set.\n\nIf this variable is set, check that $(CC) indeed supports the `-MJ`\nflag, following what is done for automatic dependencies.\n\nAll JSON fragments are placed in the 'compile_commands/' directory, and\nthe compilation database 'compile_commands.json' is generated as a\ndependency of the 'all' target using a `sed` invocation.\n\n[1] https://clang.llvm.org/docs/Tooling.html\n[2] https://clang.llvm.org/docs/JSONCompilationDatabase.html\n[3] https://clangd.llvm.org/\n[4] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mj-arg\n\nHelped-by: brian m. carlson <sandals@crustytoothpaste.net>\nSigned-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n---\n    Add support for generating JSON compilation database\n    \n    Changes since v2: Incorporated Junio's suggestions:\n    \n     * improved the logic around values of the new \n       GENERATE_COMPILATION_DATABASE macro\n     * simplified the Makefile functions used to generate the path of the\n       JSON fragments\n     * removed the trailing slash after the default value of compdb_dir\n    \n    Changes since v1:\n    \n     * Added a paragraph to the commit message to better explain why this\n       feature would help Git developers (Thanks Brian!)\n     * Corrected the description of the new GENERATE_COMPILATION_DATABASE \n       Makefile knob to make it reflect its actual behavior (cf. \n       https://lore.kernel.org/git/FC95CFF7-F9DA-4CDA-9923-99C3432DCAD5@gmail.com/\n       )\n    \n    v1: I don't have a lot of knowledge of Make double-colon rules, or\n    insight into why they are used for the 'all' target, but I think the\n    approach I chose makes sense. In particular, I do not list any\n    prerequisite for the 'compile_commands.json' file, but from what I\n    tested it is still rebuilt anytime the 'all' target is rebuilt, which is\n    what we want.\n    \n    Note: CMakeLists.txt in contrib/buildsystems does not need to be updated\n    to also support this feature because CMake supports it out-of-the-box\n    [1].\n    \n    [1] \n    https://cmake.org/cmake/help/latest/variable/CMAKE_EXPORT_COMPILE_COMMANDS.html\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-714%2Fphil-blain%2Fcompiledb-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-714/phil-blain/compiledb-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/714\n\nRange-diff vs v2:\n\n 1:  da9544e4aa ! 1:  1955b6b390 Makefile: add support for generating JSON compilation database\n     @@ Makefile: all::\n       # the global variable _wpgmptr containing the absolute path of the current\n       # executable (this is the case on Windows).\n       #\n     -+# Define GENERATE_COMPILATION_DATABASE to generate JSON compilation database\n     -+# entries during compilation if your compiler supports it, using the `-MJ` flag.\n     -+# The JSON entries will be placed in the `compile_commands/` directory,\n     -+# and the JSON compilation database 'compile_commands.json' will be created \n     -+# at the root of the repository. \n     ++# Define GENERATE_COMPILATION_DATABASE to \"yes\" to generate JSON compilation\n     ++# database entries during compilation if your compiler supports it, using the\n     ++# `-MJ` flag. The JSON entries will be placed in the `compile_commands/`\n     ++# directory, and the JSON compilation database 'compile_commands.json' will be\n     ++# created at the root of the repository.\n      +#\n       # Define DEVELOPER to enable more compiler warnings. Compiler version\n       # and family are auto detected, but could be overridden by defining\n     @@ Makefile: $(error please set COMPUTE_HEADER_DEPENDENCIES to yes, no, or auto \\\n       endif\n       endif\n       \n     -+ifdef GENERATE_COMPILATION_DATABASE\n     ++ifndef GENERATE_COMPILATION_DATABASE\n     ++GENERATE_COMPILATION_DATABASE = no\n     ++endif\n     ++\n     ++ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n      +compdb_check = $(shell $(CC) $(ALL_CFLAGS) \\\n      +\t-c -MJ /dev/null \\\n      +\t-x c /dev/null -o /dev/null 2>&1; \\\n      +\techo $$?)\n     -+ifeq ($(compdb_check),0)\n     -+override GENERATE_COMPILATION_DATABASE = yes\n     -+else\n     ++ifneq ($(compdb_check),0)\n      +override GENERATE_COMPILATION_DATABASE = no\n     -+$(warning GENERATE_COMPILATION_DATABASE is set, but your compiler does not \\\n     ++$(warning GENERATE_COMPILATION_DATABASE is set to \"yes\", but your compiler does not \\\n      +support generating compilation database entries)\n      +endif\n     ++else\n     ++ifneq ($(GENERATE_COMPILATION_DATABASE),no)\n     ++$(error please set GENERATE_COMPILATION_DATABASE to \"yes\" or \"no\" \\\n     ++(not \"$(GENERATE_COMPILATION_DATABASE)\"))\n     ++endif\n      +endif\n      +\n       ifdef SANE_TOOL_PATH\n     @@ Makefile: missing_dep_dirs =\n       dep_args =\n       endif\n       \n     -+compdb_dir = compile_commands/\n     ++compdb_dir = compile_commands\n      +\n      +ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n      +missing_compdb_dir = $(compdb_dir)\n      +$(missing_compdb_dir):\n      +\t@mkdir -p $@\n      +\n     -+compdb_file = $(compdb_dir)$(subst .-,,$(subst /,-,$(dir $@)))$(notdir $@).json\n     ++compdb_file = $(compdb_dir)/$(subst /,-,$@.json)\n      +compdb_args = -MJ $(compdb_file)\n      +else\n      +missing_compdb_dir =\n     @@ Makefile: else\n      +all:: compile_commands.json\n      +compile_commands.json:\n      +\t@$(RM) $@\n     -+\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)*.o.json > $@+\n     ++\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)/*.o.json > $@+\n      +\t@if test -s $@+; then mv $@+ $@; else $(RM) $@+; fi\n      +endif\n      +\n\n\n .gitignore |  2 ++\n Makefile   | 59 +++++++++++++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 56 insertions(+), 5 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex ee509a2ad2..f4c51300e0 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -197,6 +197,7 @@\n /git.spec\n *.exe\n *.[aos]\n+*.o.json\n *.py[co]\n .depend/\n *.gcda\n@@ -218,6 +219,7 @@\n /tags\n /TAGS\n /cscope*\n+/compile_commands.json\n *.hcc\n *.obj\n *.lib\ndiff --git a/Makefile b/Makefile\nindex 65f8cfb236..39ca667689 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -462,6 +462,12 @@ all::\n # the global variable _wpgmptr containing the absolute path of the current\n # executable (this is the case on Windows).\n #\n+# Define GENERATE_COMPILATION_DATABASE to \"yes\" to generate JSON compilation\n+# database entries during compilation if your compiler supports it, using the\n+# `-MJ` flag. The JSON entries will be placed in the `compile_commands/`\n+# directory, and the JSON compilation database 'compile_commands.json' will be\n+# created at the root of the repository.\n+#\n # Define DEVELOPER to enable more compiler warnings. Compiler version\n # and family are auto detected, but could be overridden by defining\n # COMPILER_FEATURES (see config.mak.dev). You can still set\n@@ -1258,6 +1264,27 @@ $(error please set COMPUTE_HEADER_DEPENDENCIES to yes, no, or auto \\\n endif\n endif\n \n+ifndef GENERATE_COMPILATION_DATABASE\n+GENERATE_COMPILATION_DATABASE = no\n+endif\n+\n+ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n+compdb_check = $(shell $(CC) $(ALL_CFLAGS) \\\n+\t-c -MJ /dev/null \\\n+\t-x c /dev/null -o /dev/null 2>&1; \\\n+\techo $$?)\n+ifneq ($(compdb_check),0)\n+override GENERATE_COMPILATION_DATABASE = no\n+$(warning GENERATE_COMPILATION_DATABASE is set to \"yes\", but your compiler does not \\\n+support generating compilation database entries)\n+endif\n+else\n+ifneq ($(GENERATE_COMPILATION_DATABASE),no)\n+$(error please set GENERATE_COMPILATION_DATABASE to \"yes\" or \"no\" \\\n+(not \"$(GENERATE_COMPILATION_DATABASE)\"))\n+endif\n+endif\n+\n ifdef SANE_TOOL_PATH\n SANE_TOOL_PATH_SQ = $(subst ','\\'',$(SANE_TOOL_PATH))\n BROKEN_PATH_FIX = 's|^\\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix \"$(SANE_TOOL_PATH_SQ)\"|'\n@@ -2381,16 +2408,30 @@ missing_dep_dirs =\n dep_args =\n endif\n \n+compdb_dir = compile_commands\n+\n+ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n+missing_compdb_dir = $(compdb_dir)\n+$(missing_compdb_dir):\n+\t@mkdir -p $@\n+\n+compdb_file = $(compdb_dir)/$(subst /,-,$@.json)\n+compdb_args = -MJ $(compdb_file)\n+else\n+missing_compdb_dir =\n+compdb_args =\n+endif\n+\n ASM_SRC := $(wildcard $(OBJECTS:o=S))\n ASM_OBJ := $(ASM_SRC:S=o)\n C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))\n \n .SUFFIXES:\n \n-$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)\n-\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n-$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)\n-\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n+$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)\n+\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n+$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)\n+\t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n \n %.s: %.c GIT-CFLAGS FORCE\n \t$(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n@@ -2413,6 +2454,14 @@ else\n $(OBJECTS): $(LIB_H) $(GENERATED_H)\n endif\n \n+ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n+all:: compile_commands.json\n+compile_commands.json:\n+\t@$(RM) $@\n+\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)/*.o.json > $@+\n+\t@if test -s $@+; then mv $@+ $@; else $(RM) $@+; fi\n+endif\n+\n exec-cmd.sp exec-cmd.s exec-cmd.o: GIT-PREFIX\n exec-cmd.sp exec-cmd.s exec-cmd.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n@@ -3117,7 +3166,7 @@ clean: profile-clean coverage-clean cocciclean\n \t$(RM) $(TEST_PROGRAMS)\n \t$(RM) $(FUZZ_PROGRAMS)\n \t$(RM) $(HCC)\n-\t$(RM) -r bin-wrappers $(dep_dirs)\n+\t$(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json\n \t$(RM) -r po/build/\n \t$(RM) *.pyc *.pyo */*.pyc */*.pyo $(GENERATED_H) $(ETAGS_TARGET) tags cscope*\n \t$(RM) -r $(GIT_TARNAME) .doc-tmp-dir\n\nbase-commit: d9cd4331470f4d9d78677f12dc79063dab832f53\n-- \ngitgitgadget\n"}]}