build system: add compile-commands target to generate compile_commands.json

[maribu]

Contribution description

By running make compile-commands a compile_commands.json in the RIOT base
directory. With the environment variable COMPILE_COMMANDS the path of
this file can be changed to a custom location.

The compile_commands.json will contain the exact compile command, but
as additional flag -I/usr/$(TARGET)/include is added to work around
clangd not being able to locate the newlib system headers. The
additional includes can be overwritten using the environment variable
COMPILE_COMMANDS_EXTRA_INCLUDES.

Testing procedure

make BOARD=<SOME_BOARD> -C examples/default compile-commands

Now, a compile_commands.json should be placed in $(RIOTBASE).

Issues/PRs references

None

[kaspar030]

I'm using this:

#!/bin/sh

PROJECT_ROOT="$(git rev-parse --show-toplevel 2>/dev/null)"
if [ -z "$PROJECT_ROOT" ]; then
    make "$@"
else
    bear --output "${PROJECT_ROOT}/compile_commands.json" -- make "$@"
fi

... saved as "bmake" in the path, which makes use of bear to generate the compile commands. Works like a charm, just call "bmake ..." instead of "make ...".

[maribu]

I also tried bear, but I'm not really happy with it. clangd doesn't find any of the includes of newlib and bear is useless if the source doesn't compile yet. And the latter is a real pain point: Most of the time I'm writing new code, so I'm constantly in a state no compile_commands.json can be produced with bear until after I have use for them.

[jnohlgard]

Personally I don't like that this is writing files in the source directories instead of the binary build dir. I tend to want to avoid having any generated files mixed in with the sources. Also, this also implies that running builds for multiple platforms (native + board) will cause the compile commands to change.
My preferred solution would be to have one compile_commands per target platform, ideally in the bin/$BOARD dir.

[jnohlgard]

some comments on the implementation.
I did not test this, but I have a feeling that the word-splitting in Makefile approach will fail on quoted spaces, e.g. CFLAGS=-DMY_APPLICATION_DESCRIPTION="My cool IoT project"

[maribu]

Personally I don't like that this is writing files in the source directories instead of the binary build dir. I tend to want to avoid having any generated files mixed in with the sources. Also, this also implies that running builds for multiple platforms (native + board) will cause the compile commands to change.
My preferred solution would be to have one compile_commands per target platform, ideally in the bin/$BOARD dir.

I partially agree. But note that placing compile_commands.json outside of the sources will result in having to point your editor/IDO to it, while just placing it in the sources will allow just work (tm). But I could make the folder configurable via environment variables and default to the binary dir.

I originally intended to build them completely independent of the build process with a fake board that provides all features and performing dependency resolution for each module separately. That would result in only the include paths actually needed to be added - and to uncover missing dependencies. And it would also allow to quickly generate a compile_commands.json for a module without navigating to an application. But I think @chrysn has a use case where he needs to correct build commands - not some that are good enough for linting and completion.

With bundling the generation with the compile process anyway, it should now be possible to generate a singe compile_commands.json.

I did not test this, but I have a feeling that the word-splitting in Makefile approach will fail on quoted spaces, e.g. CFLAGS=-DMY_APPLICATION_DESCRIPTION="My cool IoT project"

You're right. There are two easy solutions to it:

a) Don't use spaces in values defined via the command line
b) use CFLAGS_WITH_MACROS += -DMY_APPLICATION_DESCRIPTION="My cool IoT project". This will result in the define being stripped from the command line and instead being added to bin/<BOARD>/riotbuild/riotbuild.h as a #define.

I don't think I want to handle this in a Makefile. There is IMO not really a use case that justifies the effort and option b) can be used to work around this limitation, if one really wants spaces.

[maribu]

@jnohlgard: It was impossible to generate a single compile_commands.json using Makefiles only without reworking the architecture of the build system, as I couldn't find a way to pass variables up from child make to its parent.

So I began from scratch and now the Makefiles generate some simple text files per module containing the build details. A python scripts converts this to a single JSON. As side effect, I believe that now parameters with spaces and quotation marks should just work.

The generation is no longer intermixed with the build. Running make compile-commands should work no matter how broken the C / C++ files are. The generation takes a whooping amount of 1.3 seconds on my machine. But a no-op build also takes 1.4 seconds, so I guess there is nothing (other than fixing the build system) to do about this.

[maribu]

For the intended use case of code completion and linting, make compile-commands now works like a charm for me with clangd + nvim + ALE. The changes to the build system are minimal, but the python script grew a bit more complex in order to detect the built-in include search paths.

[maribu]

I think this is now functionally complete and reasonably well documented. For me, it works like a charm.

May I squash?

[benpicco]

The Python linter in static tests still has some open comments.

[maribu]

The Python linter in static tests still has some open comments.

Addressed and rebased to fix the merge conflict in .gitignore. May I squash?

[chrysn]

I think that my use case is also fine with something that's good enough for linting and completing, but I'm experimenting with this PR to get more precision on what I actually need.

Given that all on-code comments are addressed, I wouldn't mind this being squashed. (And if something steals the time I like to take for reviewing this for practical applications, don't let a lack of follow-up from me stop this PR's progress).

[chrysn]

What I found I miss from the compile commands are all the -DMODULE_... definitions. I see them in CFLAGS, but don't find them in the compile-commands.json file.

Are they missed, or excluded for a reason?

[kaspar030]

Are they missed, or excluded for a reason?

I guess they're in the generated riotbuild.h?

[chrysn]

They are there, but I also pluck them out to make flag decisions on the Rust side. No biggie if I have to peek into riotbuild.h for them if that's where they are going on the long run, but it's a difference in the CFLAGS I get from the environment and those I can pick from the compile-commands.

[chrysn]

Whoopsy! (Did this to confirm that defines with spaces now work properly)

Speaking of which: CFLAGS += '-DTHREAD_STACKSIZE_MAIN=(THREAD_STACKSIZE_DEFAULT + 1400)' causes shell errors for me which I haven't managed to track down, could be related.

[maribu]

I think the reason is that flags are moved from CLFAGS to riotbuild.h at a point in time later than you check. The code dumps the exact CFLAGS into $(BINDIR)/$(MODULE)/cflags.txt. Only when called with --clangd, the python script some post processing of the flags to also add search paths built into gcc explicitly via -isystem and removes some incompatible flags.

But since the make target by default (unless overwritten e.g. via environment variables) adds the --clangd flag to the script, you will not have the exact compile commands. Out of the 1.5 seconds the generation of the compile commands takes, like 99% of the time is spent on make. I think it might make sense to just invoke the python script a second time for rust without the --clangd for fetching the exact compile commands.

Back to actual question: Since the -DMODULE_FOO wasn't really part of the compilation command, I think it is actually better to peek into riotbuild.h.

[chrysn]

I've done some experimentation with what can be passed around how in make, I find it impossible-so-far to run something like TOOLCHAIN=llvm make compile-commands from inside the build system, and that's not even accounting yet for packages with the toolchain blacklisted or other reasons why clang is not fully supported.

For those reasons it's probably worth having one place in RIOT that produces clang-equivalent CFLAGS for a build. If, on the long run, we manage to do better (dunno, maybe pass ${CFLAGS} ${CFLAGS_${PLATFORM}} to the compiler?), we can still do that, and until then, a --clangd is a good place to do that.

[chrysn]

As they haven't been cross-linked yet: Closes #16002

[benpicco]

This only needs squashing?

[maribu]

This only needs squashing?

I actually wanted to re-write this and heavily modify the build system. But since soft feature freeze is now, I think I really favor a sub-optimal version of this rather than no version at all.

But I think it makes sense to have all the info dumped by the build system in a single file, e.g. an entire file just for the compile name seems a bit wasteful and raises the bar to add more details.

[maribu]

The last updated changes the following:

• with --clangd, the python script uses clang / clang++ for $CC / $CXX instead of the original value
• with --clangd, -target <ARCH_TRIPLE> is added to the compiler flags
• the compilation details are now dumped into a single text file with a simple <KEY>: <VALUE>\n syntax. This should ease debugging / reading the raw dump a bit

It is still minimally invasive into the build system, so it should be IMO possible to get this in even during soft feature freeze.

[maribu]

May I squash?

[chrysn]

Please squash.

[chrysn]

I've run some tests with the squashed version and ran into one last issue:

On native with GCC as toolchain, the compile commands contain ["-target", ""], which at least bindgen doesn't like (that's the clang based tool I'm using this for; didn't test applicability with others but assume it could cause the same problem). The error message is a nondescript list of options, but "Host vs. target architecture mismatch" is one of them.

The bin/native/application_saul_example/compile_cmds.txt file in that situation shows:

SRC: main.c
SRC_NO_LTO: 
SRCXX: 
CURDIR: /home/chrysn/git/RIOT-doc-fixes/examples/saul
CFLAGS: -DDEVELHELP -Werror -Wall -Wextra -pedantic -g3 -Og -U_FORTIFY_SOURCE -std=gnu11 -m32 -fstack-protector-all -ffunction-sections -fdata-sections -DDEBUG_ASSERT_VERBOSE -DRIOT_APPLICATION=\"saul_example\" -DBOARD_NATIVE=\"native\" -DRIOT_BOARD=BOARD_NATIVE -DCPU_NATIVE=\"native\" -DRIOT_CPU=CPU_NATIVE -DMCU_NATIVE=\"native\" -DRIOT_MCU=MCU_NATIVE -fwrapv -Wstrict-overflow -fno-common -ffunction-sections -fdata-sections -Wall -Wextra -Wmissing-include-dirs -fno-delete-null-pointer-checks -fdiagnostics-color -Wstrict-prototypes -Wold-style-definition -gz -Wformat=2 -Wformat-overflow -Wformat-truncation -include '/home/chrysn/git/RIOT-doc-fixes/examples/saul/bin/native/riotbuild/riotbuild.h'
LTOFLAGS: 
INCLUDES: -I/home/chrysn/git/RIOT-doc-fixes/core/include -I/home/chrysn/git/RIOT-doc-fixes/drivers/include -I/home/chrysn/git/RIOT-doc-fixes/sys/include -I/home/chrysn/git/RIOT-doc-fixes/boards/native/include -DNATIVE_INCLUDES -I/home/chrysn/git/RIOT-doc-fixes/boards/native/include/ -I/home/chrysn/git/RIOT-doc-fixes/core/include/ -I/home/chrysn/git/RIOT-doc-fixes/drivers/include/ -I/home/chrysn/git/RIOT-doc-fixes/cpu/native/include -I/home/chrysn/git/RIOT-doc-fixes/sys/include -I/home/chrysn/git/RIOT-doc-fixes/cpu/native/include 
CXXFLAGS: -DDEVELHELP -Werror -Wall -Wextra -pedantic -g3 -Og -U_FORTIFY_SOURCE -m32 -fstack-protector-all -ffunction-sections -fdata-sections -DDEBUG_ASSERT_VERBOSE -DRIOT_APPLICATION=\"saul_example\" -DBOARD_NATIVE=\"native\" -DRIOT_BOARD=BOARD_NATIVE -DCPU_NATIVE=\"native\" -DRIOT_CPU=CPU_NATIVE -DMCU_NATIVE=\"native\" -DRIOT_MCU=MCU_NATIVE -fwrapv -Wstrict-overflow -fno-common -ffunction-sections -fdata-sections -Wall -Wextra -Wmissing-include-dirs -fno-delete-null-pointer-checks -fdiagnostics-color -gz -Wformat=2 -Wformat-overflow -Wformat-truncation -include '/home/chrysn/git/RIOT-doc-fixes/examples/saul/bin/native/riotbuild/riotbuild.h' 
CXXINCLUDES: 
CC: gcc
CXX: g++
TARGET_ARCH: 
TARGET_ARCH_LLVM: 

In contrast, when building for an nrf52840, it shows:

TARGET_ARCH: arm-none-eabi
TARGET_ARCH_LLVM: 

[fjmolinas]

@chrysn does your latest comments mean this PR is not ready to merge?

[chrysn]

Yes; things worked before but now (after latest fixes and squashing) don't. I tried before to set my status to "still has open comments" but that didn't work, trying again after having self-requested a review...

[maribu]

Sorry, I didn't test on native. I think this should work now. I don't have 32/64 bit multlib installed, so cannot really test, tough.

[chrysn]

Running the tests again I ran into another issue (sorry, could have run through ignoring the first one right away): Whereas on a clean checkout make BOARD=sltb001a -C examples/saul compile-commands runs through alright, a make BOARD=sltb001a -C examples/saul all compile-commands doesn't:

make[3]: Nothing to be done for 'compile-commands'.
Traceback (most recent call last):
  File "/home/chrysn/git/RIOT-doc-fixes/dist/tools/compile_commands/compile_commands.py", line 292, in <module>
    generate_compile_commands(_args)
  File "/home/chrysn/git/RIOT-doc-fixes/dist/tools/compile_commands/compile_commands.py", line 267, in generate_compile_commands
    generate_module_compile_commands(module.path, state, args)
  File "/home/chrysn/git/RIOT-doc-fixes/dist/tools/compile_commands/compile_commands.py", line 213, in generate_module_compile_commands
    cdetails = CompilationDetails(path)
  File "/home/chrysn/git/RIOT-doc-fixes/dist/tools/compile_commands/compile_commands.py", line 107, in __init__
    with open(os.path.join(path, "compile_cmds.txt"), "r") as file:
FileNotFoundError: [Errno 2] No such file or directory: '/home/chrysn/git/RIOT-doc-fixes/examples/saul/bin/sltb001a/gecko_sdk/compile_cmds.txt'
make: *** [/home/chrysn/git/RIOT-doc-fixes/examples/saul/../../Makefile.include:643: compile-commands] Error 1
make: Leaving directory '/home/chrysn/git/RIOT-doc-fixes/examples/saul'

(Same for make compile-commands once all was made). It seems that the generate_compile_commands just encounters directories created by the gecko_sdk package (bin/sltb001a/gecko_sdk{,_emlib,_emlib_extra}) that compile commands were not created for. That's not specific to the gecko pkg; something similar can be seen around make BOARD=microbit-v2 -C tests/pkg_littlefs2.

For my use cases it doesn't make much of a difference whether pkgs are in there or not (so just continuing on absence is fine, as the Makefile would have complained if files should be created but couldn't); if that works for the IDE use cases too (well, no help for pkg includes I guess, can still be added later) that'd be the easiest way around.

[maribu]

Fixed. The script now will only consider folders in bin/<BOARD>/ as module, if the contain a compile_cmds.txt file.

[chrysn]

Usage tests with bindgen pass now for all architectures I can currently test. Thanks, please squash.

[maribu]

All green. And I think this shouldn't interfere with any other stuff, so IMO this could be considered for merge during soft feature freeze.

[chrysn]

With Kaspar having announced the soft freeze and having thumbs-upped this, merging. [edit: and agreeing on "This does not interfere"]

[maribu]

Thanks! :-)

[benpicco]

Is it possible to place compile_commands.json in the application folder too or does it always have to be the root of the project?

[maribu]

The destination can actually be controlled via a Makefile variable.

Typically this is used for clangd and typically one doesn't specify the path to compile_commands.json explicitly, but let clangd search for it. And clangd will try the current folder, the parent folder, the grandparent folder and so on. So when opening the editor with an application as working dir, it will work fine to have it stored there, even for editing RIOT code that is outside of that folder. However, if you have your editors working dir set to the RIOT base dir, it will only work if compile_commands.json is placed there.

Since placing it in RIOT's base dir works out of the box for all scenarios, this is the current default. It may actually be better for application developers (who are likely to set their application as working dir for the editor) to place it in the app dir by default, as the don't have to run make compile-commands every time the open a different app in their editor.