Messages in this thread | | | From | Masahiro Yamada <> | Date | Wed, 12 Aug 2020 21:28:32 +0900 | Subject | Re: [PATCH] gen_compile_commands: Add support for separate kbuild output directory |
| |
(+cc clang-built-linux)
On Sat, Aug 1, 2020 at 6:21 AM Peter Kalauskas <peskal@google.com> wrote: > > Add support for builds that use an output directory different than the > kernel source tree (e.g. make O=/tmp/kernel-obj). This also introduces > support for .cmd files that use absolute paths. > > Previously, gen_compile_commands.py only supported builds where the > kernel source tree and the output directory were the same: > $ make defconfig > $ make -j32 > $ ./scripts/gen_compile_commands.py > > gen_compile_commands.py had flags to support out of tree use, but the > generated compile_commands.json file still assumed that the source tree > and kbuild output directory were the same.
Thanks Tom for CCing Kbuild ML. Otherwise, I would not have noticed this patch.
I think the current code _mostly_ supports the out of tree use. The key is to specify '-p <build-dir>' when you use compile_commands.json from clang-check, clang-tidy, etc.
> Now, the following cases are supported as well: > > - Absolute output path: > $ mkdir /tmp/kernel-obj > $ make O=/tmp/kernel-obj defconfig > $ make O=/tmp/kernel-obj -j32 > $ ./scripts/gen_compile_commands.py -k /tmp/kernel-obj > > - Relative output path: > $ mkdir kernel-obj > $ make O=kernel-obj/ defconfig > $ make O=kernel-obj/ -j32 > $ ./scripts/gen_compile_commands.py -k kernel-obj
In the current script, I would do like follows:
- Absolute output path: $ export CC=clang $ make O=/tmp/kernel-obj defconfig $ make O=/tmp/kernel-obj -j32 $ ./scripts/gen_compile_commands.py -d /tmp/kernel-obj $ clang-tidy '--checks=linuxkernel*' -p /tmp/kernel-obj kernel/fork.c
- Relative output path: $ export CC=clang $ make O=kernel-obj defconfig $ make O=kernel-obj -j32 $ ./scripts/gen_compile_commands.py -d kernel-obj $ clang-tidy '--checks=linuxkernel*' -p kernel-obj kernel/fork.c
With your patch and the -k option use, compile_commands.json is always created in the source tree whether O= is given or not. Then, you no longer need to pass '-p <build-path>' from clang tools.
However, I like the workflow to create any build artifact in the output tree for O= use-case, and keep the source tree completely clean. This is because the source tree might be read-only. Perhaps, it is located under /usr/src/, or the source code might be provided by DVD-ROM, etc.
In my understanding, the flaw of the -d option is, it cannot handle objtool.
'--log_level DEBUG' emits error log for objtool.
masahiro@oscar:~/ref/linux$ ./scripts/gen_compile_commands.py -d /tmp/kernel-obj --log_level DEBUG INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.elf.o.cmd: File elf.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.builtin-orc.o.cmd: File builtin-orc.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.builtin-check.o.cmd: File builtin-check.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.orc_dump.o.cmd: File orc_dump.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.libstring.o.cmd: File ../lib/string.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.libctype.o.cmd: File ../lib/ctype.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.parse-options.o.cmd: File parse-options.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.objtool.o.cmd: File objtool.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.fixdep.o.cmd: File fixdep.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.pager.o.cmd: File pager.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.str_error_r.o.cmd: File ../lib/str_error_r.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.special.o.cmd: File special.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.orc_gen.o.cmd: File orc_gen.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.help.o.cmd: File help.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.librbtree.o.cmd: File ../lib/rbtree.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.run-command.o.cmd: File run-command.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.exec-cmd.o.cmd: File exec-cmd.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.weak.o.cmd: File weak.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.subcmd-config.o.cmd: File subcmd-config.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.check.o.cmd: File check.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/.sigchain.o.cmd: File sigchain.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool INFO: Could not add line from /tmp/kernel-obj/tools/objtool/arch/x86/.decode.o.cmd: File arch/x86/decode.c not in /tmp/kernel-obj or /tmp/kernel-obj/tools/objtool/arch/x86
Your patch solves it, but I wonder if it is worth lots of code addition. objtool is the only unfortunate case because it did not join Kbuild.
> The new argument, -k, is introduced in a way that makes the script > backward compatible with how -d was previously used. > > Signed-off-by: Peter Kalauskas <peskal@google.com> > ---
> -def process_line(root_directory, file_directory, command_prefix, relative_path): > +def process_line(src_dir, kbuild_out_dir, file_dir, cmd_prefix, file_path): > """Extracts information from a .cmd line and creates an entry from it. > > Args: > - root_directory: The directory that was searched for .cmd files. Usually > + src_dir: The directory of the kernel source tree. > + kbuild_out_dir: The directory that was searched for .cmd files. Usually > used directly in the "directory" entry in compile_commands.json. > - file_directory: The path to the directory the .cmd file was found in. > - command_prefix: The extracted command line, up to the last element. > - relative_path: The .c file from the end of the extracted command. > - Usually relative to root_directory, but sometimes relative to > - file_directory and sometimes neither. > + file_dir: The path to the directory the .cmd file was found in. > + cmd_prefix: The extracted command line, up to the last element. > + file_path: The .c file from the end of the extracted command. > + Usually relative to kbuild_out_dir, but sometimes relative to > + src_dir and sometimes neither. > > Returns: > An entry to append to compile_commands. > > Raises: > - ValueError: Could not find the extracted file based on relative_path and > - root_directory or file_directory. > + ValueError: Could not find the extracted file. > """ > # The .cmd files are intended to be included directly by Make, so they > # escape the pound sign '#', either as '\#' or '$(pound)' (depending on the > - # kernel version). The compile_commands.json file is not interepreted > + # kernel version). The compile_commands.json file is not interpreted > # by Make, so this code replaces the escaped version with '#'. > - prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#') > - > - cur_dir = root_directory > - expected_path = os.path.join(cur_dir, relative_path) > - if not os.path.exists(expected_path): > - # Try using file_directory instead. Some of the tools have a different > - # style of .cmd file than the kernel. > - cur_dir = file_directory > - expected_path = os.path.join(cur_dir, relative_path) > + prefix = cmd_prefix.replace('\#', '#').replace('$(pound)', '#') > + > + # Compile commands are usually run in the top level of the kbuild output > + # directory > + working_dir = kbuild_out_dir > + > + if os.path.isabs(file_path):
I might be misreading the code, but is this if-else switch needed?
os.path.join() returns the last parameter as-is if it is already absolute, right?
This is my quick experiment:
masahiro@oscar:~$ python3 Python 3.8.2 (default, Jul 16 2020, 14:00:26) [GCC 9.3.0] on linux Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.path.join("/foo/bar/", "a/b/c") '/foo/bar/a/b/c' >>> os.path.join("/foo/bar/", "/a/b/c") '/a/b/c'
So, the current code:
expected_path = os.path.join(cur_dir, relative_path)
works whether 'relative_path' is relative or not.
I was also thinking this variable name was misleading since 'relative_path' may be actually absolute.
-- Best Regards
Masahiro Yamada
| |