* [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory @ 2021-02-08 11:28 Stephen Zhang 2021-02-08 19:54 ` Nathan Chancellor 0 siblings, 1 reply; 14+ messages in thread From: Stephen Zhang @ 2021-02-08 11:28 UTC (permalink / raw) To: ndesaulniers, natechancellor Cc: clang-built-linux, linux-kernel, Stephen Zhang The default source directory is set equal to build directory which specified by "-d".But it is designed to be set to the current working directoy by default, as the help messge says.It makes a differece when source directory and build directory are in separted directorys. Signed-off-by: Stephen Zhang <stephenzhangzsd@gmail.com> --- scripts/clang-tools/gen_compile_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py index 1996370..add9e47 100755 --- a/scripts/clang-tools/gen_compile_commands.py +++ b/scripts/clang-tools/gen_compile_commands.py @@ -64,7 +64,7 @@ def parse_arguments(): os.path.abspath(args.directory), args.output, args.ar, - args.paths if len(args.paths) > 0 else [args.directory]) + args.paths if len(args.paths) > 0 else [os.getcwd()]) def cmdfiles_in_dir(directory): -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-08 11:28 [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory Stephen Zhang @ 2021-02-08 19:54 ` Nathan Chancellor [not found] ` <CALuz2=d-ENRbWgGYaO_ESEaw5eOVSwkQmkeYBJ-w0Vb3zZ+REg@mail.gmail.com> 0 siblings, 1 reply; 14+ messages in thread From: Nathan Chancellor @ 2021-02-08 19:54 UTC (permalink / raw) To: Stephen Zhang Cc: ndesaulniers, natechancellor, clang-built-linux, linux-kernel On Mon, Feb 08, 2021 at 07:28:57PM +0800, Stephen Zhang wrote: > The default source directory is set equal to build directory which > specified by "-d".But it is designed to be set to the current working > directoy by default, as the help messge says.It makes a differece when > source directory and build directory are in separted directorys. > > Signed-off-by: Stephen Zhang <stephenzhangzsd@gmail.com> I don't think this patch makes much sense unless I am misunderstanding the description of the problem. The entire point of this script is to parse the .cmd files that kbuild generates and those are only present in the build directory, not the source directory, so we should never be looking in there, unless args.directory is its default value, which is the way the script is currently written. Your patch would appear to either make use do way more searching than necessary (if the build folder is within the source folder) or miss it altogether (if the build folder is outside the source folder). I think the help message probably needs to be updated to be a little clearer: diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py index 19963708bcf8..cf36d73dd3bf 100755 --- a/scripts/clang-tools/gen_compile_commands.py +++ b/scripts/clang-tools/gen_compile_commands.py @@ -55,7 +55,7 @@ def parse_arguments(): paths_help = ('directories to search or files to parse ' '(files should be *.o, *.a, or modules.order). ' - 'If nothing is specified, the current directory is searched') + 'If nothing is specified, the output directory is searched') parser.add_argument('paths', type=str, nargs='*', help=paths_help) args = parser.parse_args() Let me know if this makes sense or if I am completely off base :) Cheers, Nathan > --- > scripts/clang-tools/gen_compile_commands.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py > index 1996370..add9e47 100755 > --- a/scripts/clang-tools/gen_compile_commands.py > +++ b/scripts/clang-tools/gen_compile_commands.py > @@ -64,7 +64,7 @@ def parse_arguments(): > os.path.abspath(args.directory), > args.output, > args.ar, > - args.paths if len(args.paths) > 0 else [args.directory]) > + args.paths if len(args.paths) > 0 else [os.getcwd()]) > > > def cmdfiles_in_dir(directory): > -- > 1.8.3.1 > ^ permalink raw reply related [flat|nested] 14+ messages in thread
[parent not found: <CALuz2=d-ENRbWgGYaO_ESEaw5eOVSwkQmkeYBJ-w0Vb3zZ+REg@mail.gmail.com>]
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory [not found] ` <CALuz2=d-ENRbWgGYaO_ESEaw5eOVSwkQmkeYBJ-w0Vb3zZ+REg@mail.gmail.com> @ 2021-02-09 19:27 ` Nathan Chancellor [not found] ` <CALuz2=dyA_ki98t8VNe2L1UcBXrSoJT1r6j1puEmLn7WrX87XQ@mail.gmail.com> 2021-02-10 20:32 ` Tom Roeder 0 siblings, 2 replies; 14+ messages in thread From: Nathan Chancellor @ 2021-02-09 19:27 UTC (permalink / raw) To: Stephen Zhang Cc: Nick Desaulniers, natechancellor, clang-built-linux, LKML, Tom Roeder On Tue, Feb 09, 2021 at 09:56:20PM +0800, Stephen Zhang wrote: > Nathan Chancellor <nathan@kernel.org> 于2021年2月9日周二 上午3:54写道: > > > On Mon, Feb 08, 2021 at 07:28:57PM +0800, Stephen Zhang wrote: > > > The default source directory is set equal to build directory which > > > specified by "-d".But it is designed to be set to the current working > > > directoy by default, as the help messge says.It makes a differece when > > > source directory and build directory are in separted directorys. > > > > > > Signed-off-by: Stephen Zhang <stephenzhangzsd@gmail.com> > > > > I don't think this patch makes much sense unless I am misunderstanding > > the description of the problem. The entire point of this script is to > > parse the .cmd files that kbuild generates and those are only present > > in the build directory, not the source directory, so we should never be > > looking in there, unless args.directory is its default value, which is > > the way the script is currently written. Your patch would appear to > > either make use do way more searching than necessary (if the build > > folder is within the source folder) or miss it altogether (if the build > > folder is outside the source folder). > > > > Cheers, > > Nathan Just as an FYI, your email was HTML, which means it won't hit LKML. > Specifically,the souce directory is /vm/linux/tools/perf on my machine, > while the build > directory is /vm/tmpbuild/tools/perf .In the build directory , Execute the > command: > > /vm/linux/scripts/clang-tools/gen_compile_commands.py --log_level DEBUG -d . > > The resulting debugging message is: > > INFO: Could not add line from /vm/tmpbuild/tools/perf/.perf.o.cmd: File > /vm/tmpbuild/tools/perf/perf.c > not found. > > But actually what we want is : > > add line from /vm/tmpbuild/tools/perf/.perf.o.cmd: File > /vm/linux/tools/perf/perf.c. > > The " /vm/tmpbuild/tools/perf " of the "File > /vm/tmpbuild/tools/perf/perf.c not found." is passed by "-d". > > so it is the "-d" which decides the source prefix. > > Then we execute: > > /vm/linux/scripts/clang-tools/gen_compile_commands.py --log_level DEBUG > -d /vm/linux/tools/perf > > But in the oringnal code , the default build directory is the same as the > source directory: > > @@ -64,7 +64,7 @@ def parse_arguments(): > os.path.abspath(args.directory), > args.output, > args.ar, > - args.paths if len(args.paths) > 0 else [args.directory]) > + args.paths if len(args.paths) > 0 else [os.getcwd()]) > > after changing it ,we then get the right result. Okay I think I see what is going on here. Your patch does not actually fix the problem from what I can tell though: $ mkdir -p /tmp/build/perf $ make -C tools/perf -skj"$(nproc)" O=/tmp/build/perf $ cd /tmp/build/perf $ ~/cbl/src/linux/scripts/clang-tools/gen_compile_commands.py --log_level INFO -d . ... INFO: Could not add line from /tmp/build/perf/arch/x86/tests/.bp-modify.o.cmd: File /tmp/build/perf/arch/x86/tests/bp-modify.c not found INFO: Could not add line from /tmp/build/perf/arch/x86/tests/.insn-x86.o.cmd: File /tmp/build/perf/arch/x86/tests/insn-x86.c not found INFO: Could not add line from /tmp/build/perf/arch/x86/tests/.arch-tests.o.cmd: File /tmp/build/perf/arch/x86/tests/arch-tests.c not found INFO: Could not add line from /tmp/build/perf/arch/x86/tests/.intel-pt-pkt-decoder-test.o.cmd: File /tmp/build/perf/arch/x86/tests/intel-pt-pkt-decoder-test.c not found ... The script has to know where the source location is in your particular use case because the .cmd files do not record it (maybe some future improvement?) This patch appears to generate what I think the compile_commands.json should look like for the most part, I am not sure if this is proper or works correctly though. CC'ing Tom Roeder who originally wrote this. Tom, the initial patch and description of the issue is here: https://lore.kernel.org/r/1612783737-3512-1-git-send-email-stephenzhangzsd@gmail.com/ $ scripts/clang-tools/gen_compile_commands.py -d /tmp/build/perf -s tools/perf diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py index 8ddb5d099029..ba3b2dcdc3e1 100755 --- a/scripts/clang-tools/gen_compile_commands.py +++ b/scripts/clang-tools/gen_compile_commands.py @@ -27,7 +27,8 @@ def parse_arguments(): Returns: log_level: A logging level to filter log output. - directory: The work directory where the objects were built. + obj_directory: The work directory where the objects were built. + src_directory: The source directory from which the objects were built. ar: Command used for parsing .a archives. output: Where to write the compile-commands JSON file. paths: The list of files/directories to handle to find .cmd files. @@ -35,10 +36,15 @@ def parse_arguments(): usage = 'Creates a compile_commands.json database from kernel .cmd files' parser = argparse.ArgumentParser(description=usage) - directory_help = ('specify the output directory used for the kernel build ' - '(defaults to the working directory)') - parser.add_argument('-d', '--directory', type=str, default='.', - help=directory_help) + obj_directory_help = ('specify the output directory used for the kernel build ' + '(defaults to the working directory)') + parser.add_argument('-d', '--obj_directory', type=str, default='.', + help=obj_directory_help) + + src_directory_help = ('specify the source directory used for the kernel build ' + '(defaults to the working directory)') + parser.add_argument('-s', '--src_directory', type=str, default='.', + help=src_directory_help) output_help = ('path to the output command database (defaults to ' + _DEFAULT_OUTPUT + ')') @@ -55,16 +61,17 @@ def parse_arguments(): paths_help = ('directories to search or files to parse ' '(files should be *.o, *.a, or modules.order). ' - 'If nothing is specified, the current directory is searched') + 'If nothing is specified, the build directory is searched') parser.add_argument('paths', type=str, nargs='*', help=paths_help) args = parser.parse_args() return (args.log_level, - os.path.abspath(args.directory), + os.path.abspath(args.obj_directory), + os.path.abspath(args.src_directory), args.output, args.ar, - args.paths if len(args.paths) > 0 else [args.directory]) + args.paths if len(args.paths) > 0 else [args.obj_directory]) def cmdfiles_in_dir(directory): @@ -154,22 +161,23 @@ def cmdfiles_for_modorder(modorder): yield to_cmdfile(obj) -def process_line(root_directory, command_prefix, file_path): +def process_line(obj_directory, src_directory, command_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 + obj_directory: The directory that was searched for .cmd files. Usually used directly in the "directory" entry in compile_commands.json. + src_directory: The directory that was used to build the object files. command_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 root_directory, but sometimes absolute. + Usually relative to obj_directory, but sometimes absolute. Returns: An entry to append to compile_commands. Raises: ValueError: Could not find the extracted file based on file_path and - root_directory or file_directory. + src_directory or file_directory. """ # The .cmd files are intended to be included directly by Make, so they # escape the pound sign '#', either as '\#' or '$(pound)' (depending on the @@ -177,20 +185,23 @@ def process_line(root_directory, command_prefix, file_path): # by Make, so this code replaces the escaped version with '#'. prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#') - # Use os.path.abspath() to normalize the path resolving '.' and '..' . - abs_path = os.path.abspath(os.path.join(root_directory, file_path)) + if os.path.isabs(file_path): + abs_path = file_path + else: + # Use os.path.abspath() to normalize the path resolving '.' and '..' . + abs_path = os.path.abspath(os.path.join(src_directory, file_path)) if not os.path.exists(abs_path): raise ValueError('File %s not found' % abs_path) return { - 'directory': root_directory, + 'directory': obj_directory, 'file': abs_path, - 'command': prefix + file_path, + 'command': prefix + abs_path, } def main(): """Walks through the directory and finds and parses .cmd files.""" - log_level, directory, output, ar, paths = parse_arguments() + log_level, obj_directory, src_directory, output, ar, paths = parse_arguments() level = getattr(logging, log_level) logging.basicConfig(format='%(levelname)s: %(message)s', level=level) @@ -221,8 +232,8 @@ def main(): result = line_matcher.match(f.readline()) if result: try: - entry = process_line(directory, result.group(1), - result.group(2)) + entry = process_line(obj_directory, src_directory, + result.group(1), result.group(2)) compile_commands.append(entry) except ValueError as err: logging.info('Could not add line from %s: %s', ^ permalink raw reply related [flat|nested] 14+ messages in thread
[parent not found: <CALuz2=dyA_ki98t8VNe2L1UcBXrSoJT1r6j1puEmLn7WrX87XQ@mail.gmail.com>]
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory [not found] ` <CALuz2=dyA_ki98t8VNe2L1UcBXrSoJT1r6j1puEmLn7WrX87XQ@mail.gmail.com> @ 2021-02-10 18:24 ` Nathan Chancellor 2021-02-11 13:47 ` Stephen Zhang 0 siblings, 1 reply; 14+ messages in thread From: Nathan Chancellor @ 2021-02-10 18:24 UTC (permalink / raw) To: Stephen Zhang Cc: Nick Desaulniers, natechancellor, clang-built-linux, LKML, Tom Roeder On Wed, Feb 10, 2021 at 08:15:27PM +0800, Stephen Zhang wrote: > Nathan Chancellor <nathan@kernel.org> 于2021年2月10日周三 上午3:27写道: > > > Just as an FYI, your email was HTML, which means it won't hit LKML. > > > Thanks for pointing that out. The existence of a GFW makes it difficult for > me to connect > to the mail server. so I use git client to send patches only and reply to > emails with > gmail web client. You can configure your Gmail web client to send text responses by default by clicking on the three dot menu in the compose window then chose the "plain text mode" option. > $ mkdir -p /tmp/build/perf > > > > $ make -C tools/perf -skj"$(nproc)" O=/tmp/build/perf > > > > $ cd /tmp/build/perf > > > > $ ~/cbl/src/linux/scripts/clang-tools/gen_compile_commands.py --log_level > > INFO -d . > > ... > > > > According to the code logic, the source directory is specified by > parameters “-d”. Yes and no. '-d' is supposed to be the build directory but the logic of the script clearly does not work when the build and source directories are in completely separate tree paths. In other works: $ make and $ make O=build will work with '-d .' because the .cmd files are in '.' and the source files will be placed relative to '.', which is correct. Your command does not work for two reasons: 1. You are using a build directory that is not a subpath of the source directory. In other words, this script would not work for $ make O=/tmp/build because '-d /tmp/build' needs to be used to find the .cmd files but then the relative path of the source files is messed up, as you point out. 2. The source files are in tools/perf, not . > def process_line(root_directory, command_prefix, file_path): > ... > abs_path = os.path.abspath(os.path.join(root_directory, file_path)) > ... > > The "root_directory" is passed by "-d", which finally become the prefix > like "/tmp/build/perf/" > of "File /tmp/build/perf/arch/x86/tests/bp-modify.c not found".so the > command is: > > $ ~/cbl/src/linux/scripts/clang-tools/gen_compile_commands.py --log_level > INFO -d ~/cbl/src/linux/tools/perf/ > > Maybe we should make an updated version, in which the help message of "-d" > can be changed > to specify the source directory instead, or I am just misunderstanding the > code logic. > The build directory needs to be involved because that is where the .cmd files will be but the source directory needs to be known because the source files in the .cmd files are relative to the source directory, not the build directory. This happens to work in most situations like I point out above but not always. I think that my patch is most likely the way to go unless others feel differently. It would be nice if you could give it a go. Cheers, Nathan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-10 18:24 ` Nathan Chancellor @ 2021-02-11 13:47 ` Stephen Zhang 2021-02-11 14:15 ` Masahiro Yamada 0 siblings, 1 reply; 14+ messages in thread From: Stephen Zhang @ 2021-02-11 13:47 UTC (permalink / raw) To: Nathan Chancellor Cc: Nick Desaulniers, natechancellor, clang-built-linux, LKML, Tom Roeder, linux-kbuild Nathan Chancellor <nathan@kernel.org> 于2021年2月11日周四 上午2:24写道: > > On Wed, Feb 10, 2021 at 08:15:27PM +0800, Stephen Zhang wrote: > > Nathan Chancellor <nathan@kernel.org> 于2021年2月10日周三 上午3:27写道: > > > > > Just as an FYI, your email was HTML, which means it won't hit LKML. > > > > > > Thanks for pointing that out. The existence of a GFW makes it difficult for > > me to connect > > to the mail server. so I use git client to send patches only and reply to > > emails with > > gmail web client. > > You can configure your Gmail web client to send text responses by > default by clicking on the three dot menu in the compose window then > chose the "plain text mode" option. > Thanks, this has always been a problem for me. > The build directory needs to be involved because that is where the .cmd > files will be but the source directory needs to be known because the > source files in the .cmd files are relative to the source directory, not > the build directory. This happens to work in most situations like I > point out above but not always. > > I think that my patch is most likely the way to go unless others feel > differently. It would be nice if you could give it a go. > > Cheers, > Nathan Do you mean my patch's failure in some cases is because the build directoty isn't involved after using "-d" to specify the source directory? Actually, the build directory has already been involved by the "path" argument. See: def main(): for path in paths: .... if os.path.isdir(path): cmdfiles = cmdfiles_in_dir(path) ..... where the value of paths is passed by the "path" argument. Do I miss something? Cheers, Stephen ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-11 13:47 ` Stephen Zhang @ 2021-02-11 14:15 ` Masahiro Yamada 2021-02-12 11:20 ` Stephen Zhang 0 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2021-02-11 14:15 UTC (permalink / raw) To: Stephen Zhang Cc: Nathan Chancellor, Nick Desaulniers, Nathan Chancellor, clang-built-linux, LKML, Tom Roeder, Linux Kbuild mailing list On Thu, Feb 11, 2021 at 10:48 PM Stephen Zhang <stephenzhangzsd@gmail.com> wrote: > > Nathan Chancellor <nathan@kernel.org> 于2021年2月11日周四 上午2:24写道: > > > > On Wed, Feb 10, 2021 at 08:15:27PM +0800, Stephen Zhang wrote: > > > Nathan Chancellor <nathan@kernel.org> 于2021年2月10日周三 上午3:27写道: > > > > > > > Just as an FYI, your email was HTML, which means it won't hit LKML. > > > > > > > > > Thanks for pointing that out. The existence of a GFW makes it difficult for > > > me to connect > > > to the mail server. so I use git client to send patches only and reply to > > > emails with > > > gmail web client. > > > > You can configure your Gmail web client to send text responses by > > default by clicking on the three dot menu in the compose window then > > chose the "plain text mode" option. > > > > Thanks, this has always been a problem for me. > > > The build directory needs to be involved because that is where the .cmd > > files will be but the source directory needs to be known because the > > source files in the .cmd files are relative to the source directory, not > > the build directory. This happens to work in most situations like I > > point out above but not always. > > > > I think that my patch is most likely the way to go unless others feel > > differently. It would be nice if you could give it a go. > > > > Cheers, > > Nathan > > Do you mean my patch's failure in some cases is because the build > directoty isn't involved after using "-d" to specify the source directory? > > Actually, the build directory has already been involved by the "path" > argument. See: > > def main(): > for path in paths: > .... > if os.path.isdir(path): > cmdfiles = cmdfiles_in_dir(path) > ..... > > where the value of paths is passed by the "path" argument. Do I miss > something? > > Cheers, > Stephen > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CALuz2%3DeSv2N2Qp7GimLgdWjvWDwDh1Dj0Q7Czm4Br5a50rs4ew%40mail.gmail.com. Please stop. Commit 6ca4c6d25949117dc5b4845612e290b6d89e70a8 removed the tools/ support. There exist two build systems in the Linux source tree. Kbuild covers the entire tree except tools/. The tools/ directory adopts a different build system. It is a pity that the tools/ directory went in a wrong direction, and people try to fix problems in a wrong layer. You are not the first person to send to tweak obj/source trees of this script. You can not do this correctly without terribly messing up the code. Please do not try to support tools/. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-11 14:15 ` Masahiro Yamada @ 2021-02-12 11:20 ` Stephen Zhang 2021-02-13 12:45 ` Masahiro Yamada 0 siblings, 1 reply; 14+ messages in thread From: Stephen Zhang @ 2021-02-12 11:20 UTC (permalink / raw) To: Masahiro Yamada Cc: Nathan Chancellor, Nick Desaulniers, Nathan Chancellor, clang-built-linux, LKML, Tom Roeder, Linux Kbuild mailing list Masahiro Yamada <masahiroy@kernel.org> 于2021年2月11日周四 下午10:16写道: > Please stop. > > > Commit 6ca4c6d25949117dc5b4845612e290b6d89e70a8 > removed the tools/ support. > > > There exist two build systems in the Linux source tree. > Kbuild covers the entire tree except tools/. > The tools/ directory adopts a different build system. > > It is a pity that the tools/ directory > went in a wrong direction, and people > try to fix problems in a wrong layer. > > > You are not the first person to send to > tweak obj/source trees of this script. > > You can not do this correctly > without terribly messing up the code. > > Please do not try to support tools/. > > > > -- > Best Regards > Masahiro Yamada Thanks for the suggestion.But what we try to support is scripts/ instead of tools/. 'tools/' here is to help explaining the problem. Or am I just misunderstanding your words? -- Best Regards Stephen Zhang ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-12 11:20 ` Stephen Zhang @ 2021-02-13 12:45 ` Masahiro Yamada 2021-02-14 11:49 ` Stephen Zhang 0 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2021-02-13 12:45 UTC (permalink / raw) To: Stephen Zhang Cc: Nathan Chancellor, Nick Desaulniers, Nathan Chancellor, clang-built-linux, LKML, Tom Roeder, Linux Kbuild mailing list On Fri, Feb 12, 2021 at 8:20 PM Stephen Zhang <stephenzhangzsd@gmail.com> wrote: > > Masahiro Yamada <masahiroy@kernel.org> 于2021年2月11日周四 下午10:16写道: > > Please stop. > > > > > > Commit 6ca4c6d25949117dc5b4845612e290b6d89e70a8 > > removed the tools/ support. > > > > > > There exist two build systems in the Linux source tree. > > Kbuild covers the entire tree except tools/. > > The tools/ directory adopts a different build system. > > > > It is a pity that the tools/ directory > > went in a wrong direction, and people > > try to fix problems in a wrong layer. > > > > > > You are not the first person to send to > > tweak obj/source trees of this script. > > > > You can not do this correctly > > without terribly messing up the code. > > > > Please do not try to support tools/. > > > > > > > > -- > > Best Regards > > Masahiro Yamada > > Thanks for the suggestion.But what we try to support is scripts/ > instead of tools/. 'tools/' here is to help explaining the problem. > Or am I just misunderstanding your words? You took 'tools/perf' as an example, so I just thought you were trying to fix the tools/. I can get scripts/ entries without any problem. If you do O= build, you can pass that directory to the -d option of gen_compile_commands.py -d DIRECTORY, --directory DIRECTORY specify the output directory used for the kernel build (defaults to the working directory) This is the steps I tested. masahiro@oscar:~/ref/linux$ make O=build defconfig all -j24 [ snip ] masahiro@oscar:~/ref/linux$ ./scripts/clang-tools/gen_compile_commands.py -d build masahiro@oscar:~/ref/linux$ grep '"file":' compile_commands.json | grep scripts/ | head -n5 "file": "/home/masahiro/ref/linux/scripts/mod/empty.c" "file": "/home/masahiro/ref/linux/scripts/mod/sumversion.c" "file": "/home/masahiro/ref/linux/scripts/mod/file2alias.c" "file": "/home/masahiro/ref/linux/scripts/mod/modpost.c" "file": "/home/masahiro/ref/linux/build/scripts/kconfig/parser.tab.c" -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-13 12:45 ` Masahiro Yamada @ 2021-02-14 11:49 ` Stephen Zhang 2021-02-14 17:09 ` Masahiro Yamada 2021-02-14 23:28 ` Nathan Chancellor 0 siblings, 2 replies; 14+ messages in thread From: Stephen Zhang @ 2021-02-14 11:49 UTC (permalink / raw) To: Masahiro Yamada Cc: Nathan Chancellor, Nick Desaulniers, Nathan Chancellor, clang-built-linux, LKML, Tom Roeder, Linux Kbuild mailing list Masahiro Yamada <masahiroy@kernel.org> 于2021年2月13日周六 下午8:46写道: > This is the steps I tested. > > > masahiro@oscar:~/ref/linux$ make O=build defconfig all -j24 > [ snip ] > masahiro@oscar:~/ref/linux$ > ./scripts/clang-tools/gen_compile_commands.py -d build > masahiro@oscar:~/ref/linux$ grep '"file":' compile_commands.json | > grep scripts/ | head -n5 > "file": "/home/masahiro/ref/linux/scripts/mod/empty.c" > "file": "/home/masahiro/ref/linux/scripts/mod/sumversion.c" > "file": "/home/masahiro/ref/linux/scripts/mod/file2alias.c" > "file": "/home/masahiro/ref/linux/scripts/mod/modpost.c" > "file": "/home/masahiro/ref/linux/build/scripts/kconfig/parser.tab.c" > > -- > Best Regards > Masahiro Yamada Thanks. Nathan had a detailed description about this: > $ make O=build > > will work with '-d .' because the .cmd files are in '.' and the source > files will be placed relative to '.', which is correct. Your command > does not work for two reasons: > > 1. You are using a build directory that is not a subpath of the source > directory. In other words, this script would not work for > > $ make O=/tmp/build > > because '-d /tmp/build' needs to be used to find the .cmd files but then > the relative path of the source files is messed up, as you point out. This may help you reproduce the problem. So you shoud try: >masahiro@oscar:~/ref/linux$ make O=/tmp/build defconfig all -j24 where the build directory is not a subpath of the source directory. -- Best Regards Stephen Zhang ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-14 11:49 ` Stephen Zhang @ 2021-02-14 17:09 ` Masahiro Yamada 2021-02-15 11:57 ` Stephen Zhang 2021-02-14 23:28 ` Nathan Chancellor 1 sibling, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2021-02-14 17:09 UTC (permalink / raw) To: Stephen Zhang Cc: Nathan Chancellor, Nick Desaulniers, Nathan Chancellor, clang-built-linux, LKML, Tom Roeder, Linux Kbuild mailing list On Sun, Feb 14, 2021 at 8:49 PM Stephen Zhang <stephenzhangzsd@gmail.com> wrote: > > Masahiro Yamada <masahiroy@kernel.org> 于2021年2月13日周六 下午8:46写道: > > This is the steps I tested. > > > > > > masahiro@oscar:~/ref/linux$ make O=build defconfig all -j24 > > [ snip ] > > masahiro@oscar:~/ref/linux$ > > ./scripts/clang-tools/gen_compile_commands.py -d build > > masahiro@oscar:~/ref/linux$ grep '"file":' compile_commands.json | > > grep scripts/ | head -n5 > > "file": "/home/masahiro/ref/linux/scripts/mod/empty.c" > > "file": "/home/masahiro/ref/linux/scripts/mod/sumversion.c" > > "file": "/home/masahiro/ref/linux/scripts/mod/file2alias.c" > > "file": "/home/masahiro/ref/linux/scripts/mod/modpost.c" > > "file": "/home/masahiro/ref/linux/build/scripts/kconfig/parser.tab.c" > > > > -- > > Best Regards > > Masahiro Yamada > > Thanks. Nathan had a detailed description about this: > > > $ make O=build > > > > will work with '-d .' because the .cmd files are in '.' and the source > > files will be placed relative to '.', which is correct. Your command > > does not work for two reasons: > > > > 1. You are using a build directory that is not a subpath of the source > > directory. In other words, this script would not work for > > > > $ make O=/tmp/build > > > > because '-d /tmp/build' needs to be used to find the .cmd files but then > > the relative path of the source files is messed up, as you point out. > > This may help you reproduce the problem. So you shoud try: > > >masahiro@oscar:~/ref/linux$ make O=/tmp/build defconfig all -j24 > > where the build directory is not a subpath of the source directory. So, what is the problem? -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-14 17:09 ` Masahiro Yamada @ 2021-02-15 11:57 ` Stephen Zhang 0 siblings, 0 replies; 14+ messages in thread From: Stephen Zhang @ 2021-02-15 11:57 UTC (permalink / raw) To: Masahiro Yamada Cc: Nathan Chancellor, Nick Desaulniers, Nathan Chancellor, clang-built-linux, LKML, Tom Roeder, Linux Kbuild mailing list Masahiro Yamada <masahiroy@kernel.org> 于2021年2月15日周一 上午1:10写道: > > So, what is the problem? > > > > > -- > Best Regards > Masahiro Yamada Okay,it seems that I misunderstood what you said before. -- Best Regards Stephen Zhang ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-14 11:49 ` Stephen Zhang 2021-02-14 17:09 ` Masahiro Yamada @ 2021-02-14 23:28 ` Nathan Chancellor 2021-02-15 11:36 ` Stephen Zhang 1 sibling, 1 reply; 14+ messages in thread From: Nathan Chancellor @ 2021-02-14 23:28 UTC (permalink / raw) To: Stephen Zhang Cc: Masahiro Yamada, Nick Desaulniers, Nathan Chancellor, clang-built-linux, LKML, Tom Roeder, Linux Kbuild mailing list On Sun, Feb 14, 2021 at 07:49:22PM +0800, Stephen Zhang wrote: > Masahiro Yamada <masahiroy@kernel.org> 于2021年2月13日周六 下午8:46写道: > > This is the steps I tested. > > > > > > masahiro@oscar:~/ref/linux$ make O=build defconfig all -j24 > > [ snip ] > > masahiro@oscar:~/ref/linux$ > > ./scripts/clang-tools/gen_compile_commands.py -d build > > masahiro@oscar:~/ref/linux$ grep '"file":' compile_commands.json | > > grep scripts/ | head -n5 > > "file": "/home/masahiro/ref/linux/scripts/mod/empty.c" > > "file": "/home/masahiro/ref/linux/scripts/mod/sumversion.c" > > "file": "/home/masahiro/ref/linux/scripts/mod/file2alias.c" > > "file": "/home/masahiro/ref/linux/scripts/mod/modpost.c" > > "file": "/home/masahiro/ref/linux/build/scripts/kconfig/parser.tab.c" > > > > -- > > Best Regards > > Masahiro Yamada > > Thanks. Nathan had a detailed description about this: > > > $ make O=build > > > > will work with '-d .' because the .cmd files are in '.' and the source > > files will be placed relative to '.', which is correct. Your command > > does not work for two reasons: > > > > 1. You are using a build directory that is not a subpath of the source > > directory. In other words, this script would not work for > > > > $ make O=/tmp/build > > > > because '-d /tmp/build' needs to be used to find the .cmd files but then > > the relative path of the source files is messed up, as you point out. > > This may help you reproduce the problem. So you shoud try: > > >masahiro@oscar:~/ref/linux$ make O=/tmp/build defconfig all -j24 > > where the build directory is not a subpath of the source directory. > This will actually work for the regular build system as it uses the full path to the files when O= is outside of the source tree. My comment applies only to the tools/ build system, which Masahiro has explicitly said he does not want this script to support. Cheers, Nathan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-14 23:28 ` Nathan Chancellor @ 2021-02-15 11:36 ` Stephen Zhang 0 siblings, 0 replies; 14+ messages in thread From: Stephen Zhang @ 2021-02-15 11:36 UTC (permalink / raw) To: Nathan Chancellor Cc: Masahiro Yamada, Nick Desaulniers, Nathan Chancellor, clang-built-linux, LKML, Tom Roeder, Linux Kbuild mailing list Nathan Chancellor <nathan@kernel.org> 于2021年2月15日周一 上午7:28写道: > > This will actually work for the regular build system as it uses the full > path to the files when O= is outside of the source tree. My comment > applies only to the tools/ build system, which Masahiro has explicitly > said he does not want this script to support. > > Cheers, > Nathan Thanks for the clarification. I start to get what you mean. Cheers, Stephen ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory 2021-02-09 19:27 ` Nathan Chancellor [not found] ` <CALuz2=dyA_ki98t8VNe2L1UcBXrSoJT1r6j1puEmLn7WrX87XQ@mail.gmail.com> @ 2021-02-10 20:32 ` Tom Roeder 1 sibling, 0 replies; 14+ messages in thread From: Tom Roeder @ 2021-02-10 20:32 UTC (permalink / raw) To: Nathan Chancellor Cc: Stephen Zhang, Nick Desaulniers, natechancellor, clang-built-linux, LKML, linux-kbuild On Tue, Feb 09, 2021 at 12:27:29PM -0700, Nathan Chancellor wrote: >On Tue, Feb 09, 2021 at 09:56:20PM +0800, Stephen Zhang wrote: >> Nathan Chancellor <nathan@kernel.org> 于2021年2月9日周二 上午3:54写道: >> >> > On Mon, Feb 08, 2021 at 07:28:57PM +0800, Stephen Zhang wrote: >> > > The default source directory is set equal to build directory which >> > > specified by "-d".But it is designed to be set to the current working >> > > directoy by default, as the help messge says.It makes a differece when >> > > source directory and build directory are in separted directorys. >> > > >> > > Signed-off-by: Stephen Zhang <stephenzhangzsd@gmail.com> >> > >> > I don't think this patch makes much sense unless I am misunderstanding >> > the description of the problem. The entire point of this script is to >> > parse the .cmd files that kbuild generates and those are only present >> > in the build directory, not the source directory, so we should never be >> > looking in there, unless args.directory is its default value, which is >> > the way the script is currently written. Your patch would appear to >> > either make use do way more searching than necessary (if the build >> > folder is within the source folder) or miss it altogether (if the build >> > folder is outside the source folder). >> > >> > Cheers, >> > Nathan > >Just as an FYI, your email was HTML, which means it won't hit LKML. > >> Specifically,the souce directory is /vm/linux/tools/perf on my machine, >> while the build >> directory is /vm/tmpbuild/tools/perf .In the build directory , Execute the >> command: >> >> /vm/linux/scripts/clang-tools/gen_compile_commands.py --log_level DEBUG -d . >> >> The resulting debugging message is: >> >> INFO: Could not add line from /vm/tmpbuild/tools/perf/.perf.o.cmd: File >> /vm/tmpbuild/tools/perf/perf.c >> not found. >> >> But actually what we want is : >> >> add line from /vm/tmpbuild/tools/perf/.perf.o.cmd: File >> /vm/linux/tools/perf/perf.c. >> >> The " /vm/tmpbuild/tools/perf " of the "File >> /vm/tmpbuild/tools/perf/perf.c not found." is passed by "-d". >> >> so it is the "-d" which decides the source prefix. >> >> Then we execute: >> >> /vm/linux/scripts/clang-tools/gen_compile_commands.py --log_level DEBUG >> -d /vm/linux/tools/perf >> >> But in the oringnal code , the default build directory is the same as the >> source directory: >> >> @@ -64,7 +64,7 @@ def parse_arguments(): >> os.path.abspath(args.directory), >> args.output, >> args.ar, >> - args.paths if len(args.paths) > 0 else [args.directory]) >> + args.paths if len(args.paths) > 0 else [os.getcwd()]) >> >> after changing it ,we then get the right result. > >Okay I think I see what is going on here. Your patch does not actually >fix the problem from what I can tell though: > >$ mkdir -p /tmp/build/perf > >$ make -C tools/perf -skj"$(nproc)" O=/tmp/build/perf > >$ cd /tmp/build/perf > >$ ~/cbl/src/linux/scripts/clang-tools/gen_compile_commands.py --log_level INFO -d . >... >INFO: Could not add line from /tmp/build/perf/arch/x86/tests/.bp-modify.o.cmd: File /tmp/build/perf/arch/x86/tests/bp-modify.c not found >INFO: Could not add line from /tmp/build/perf/arch/x86/tests/.insn-x86.o.cmd: File /tmp/build/perf/arch/x86/tests/insn-x86.c not found >INFO: Could not add line from /tmp/build/perf/arch/x86/tests/.arch-tests.o.cmd: File /tmp/build/perf/arch/x86/tests/arch-tests.c not found >INFO: Could not add line from /tmp/build/perf/arch/x86/tests/.intel-pt-pkt-decoder-test.o.cmd: File /tmp/build/perf/arch/x86/tests/intel-pt-pkt-decoder-test.c not found >... > >The script has to know where the source location is in your particular >use case because the .cmd files do not record it (maybe some future >improvement?) > >This patch appears to generate what I think the compile_commands.json >should look like for the most part, I am not sure if this is proper or >works correctly though. CC'ing Tom Roeder who originally wrote this. >Tom, the initial patch and description of the issue is here: >https://lore.kernel.org/r/1612783737-3512-1-git-send-email-stephenzhangzsd@gmail.com/ Thanks! I'll take a look. I'm also CC'ing linux-kbuild, which is the subtree that owns the script; I haven't been very involved since I added it. My main concern is to make sure that changes don't break the simple use case: generating compile_commands.json in an in-tree build without any arguments to the script. > >$ scripts/clang-tools/gen_compile_commands.py -d /tmp/build/perf -s tools/perf > >diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py >index 8ddb5d099029..ba3b2dcdc3e1 100755 >--- a/scripts/clang-tools/gen_compile_commands.py >+++ b/scripts/clang-tools/gen_compile_commands.py >@@ -27,7 +27,8 @@ def parse_arguments(): > > Returns: > log_level: A logging level to filter log output. >- directory: The work directory where the objects were built. >+ obj_directory: The work directory where the objects were built. >+ src_directory: The source directory from which the objects were built. > ar: Command used for parsing .a archives. > output: Where to write the compile-commands JSON file. > paths: The list of files/directories to handle to find .cmd files. >@@ -35,10 +36,15 @@ def parse_arguments(): > usage = 'Creates a compile_commands.json database from kernel .cmd files' > parser = argparse.ArgumentParser(description=usage) > >- directory_help = ('specify the output directory used for the kernel build ' >- '(defaults to the working directory)') >- parser.add_argument('-d', '--directory', type=str, default='.', >- help=directory_help) >+ obj_directory_help = ('specify the output directory used for the kernel build ' >+ '(defaults to the working directory)') >+ parser.add_argument('-d', '--obj_directory', type=str, default='.', >+ help=obj_directory_help) >+ >+ src_directory_help = ('specify the source directory used for the kernel build ' >+ '(defaults to the working directory)') >+ parser.add_argument('-s', '--src_directory', type=str, default='.', >+ help=src_directory_help) > > output_help = ('path to the output command database (defaults to ' + > _DEFAULT_OUTPUT + ')') >@@ -55,16 +61,17 @@ def parse_arguments(): > > paths_help = ('directories to search or files to parse ' > '(files should be *.o, *.a, or modules.order). ' >- 'If nothing is specified, the current directory is searched') >+ 'If nothing is specified, the build directory is searched') > parser.add_argument('paths', type=str, nargs='*', help=paths_help) > > args = parser.parse_args() > > return (args.log_level, >- os.path.abspath(args.directory), >+ os.path.abspath(args.obj_directory), >+ os.path.abspath(args.src_directory), > args.output, > args.ar, >- args.paths if len(args.paths) > 0 else [args.directory]) >+ args.paths if len(args.paths) > 0 else [args.obj_directory]) > > > def cmdfiles_in_dir(directory): >@@ -154,22 +161,23 @@ def cmdfiles_for_modorder(modorder): > yield to_cmdfile(obj) > > >-def process_line(root_directory, command_prefix, file_path): >+def process_line(obj_directory, src_directory, command_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 >+ obj_directory: The directory that was searched for .cmd files. Usually > used directly in the "directory" entry in compile_commands.json. >+ src_directory: The directory that was used to build the object files. > command_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 root_directory, but sometimes absolute. >+ Usually relative to obj_directory, but sometimes absolute. > > Returns: > An entry to append to compile_commands. > > Raises: > ValueError: Could not find the extracted file based on file_path and >- root_directory or file_directory. >+ src_directory or file_directory. > """ > # The .cmd files are intended to be included directly by Make, so they > # escape the pound sign '#', either as '\#' or '$(pound)' (depending on the >@@ -177,20 +185,23 @@ def process_line(root_directory, command_prefix, file_path): > # by Make, so this code replaces the escaped version with '#'. > prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#') > >- # Use os.path.abspath() to normalize the path resolving '.' and '..' . >- abs_path = os.path.abspath(os.path.join(root_directory, file_path)) >+ if os.path.isabs(file_path): >+ abs_path = file_path >+ else: >+ # Use os.path.abspath() to normalize the path resolving '.' and '..' . >+ abs_path = os.path.abspath(os.path.join(src_directory, file_path)) > if not os.path.exists(abs_path): > raise ValueError('File %s not found' % abs_path) > return { >- 'directory': root_directory, >+ 'directory': obj_directory, > 'file': abs_path, >- 'command': prefix + file_path, >+ 'command': prefix + abs_path, > } > > > def main(): > """Walks through the directory and finds and parses .cmd files.""" >- log_level, directory, output, ar, paths = parse_arguments() >+ log_level, obj_directory, src_directory, output, ar, paths = parse_arguments() > > level = getattr(logging, log_level) > logging.basicConfig(format='%(levelname)s: %(message)s', level=level) >@@ -221,8 +232,8 @@ def main(): > result = line_matcher.match(f.readline()) > if result: > try: >- entry = process_line(directory, result.group(1), >- result.group(2)) >+ entry = process_line(obj_directory, src_directory, >+ result.group(1), result.group(2)) > compile_commands.append(entry) > except ValueError as err: > logging.info('Could not add line from %s: %s', ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-02-15 11:58 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-08 11:28 [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory Stephen Zhang 2021-02-08 19:54 ` Nathan Chancellor [not found] ` <CALuz2=d-ENRbWgGYaO_ESEaw5eOVSwkQmkeYBJ-w0Vb3zZ+REg@mail.gmail.com> 2021-02-09 19:27 ` Nathan Chancellor [not found] ` <CALuz2=dyA_ki98t8VNe2L1UcBXrSoJT1r6j1puEmLn7WrX87XQ@mail.gmail.com> 2021-02-10 18:24 ` Nathan Chancellor 2021-02-11 13:47 ` Stephen Zhang 2021-02-11 14:15 ` Masahiro Yamada 2021-02-12 11:20 ` Stephen Zhang 2021-02-13 12:45 ` Masahiro Yamada 2021-02-14 11:49 ` Stephen Zhang 2021-02-14 17:09 ` Masahiro Yamada 2021-02-15 11:57 ` Stephen Zhang 2021-02-14 23:28 ` Nathan Chancellor 2021-02-15 11:36 ` Stephen Zhang 2021-02-10 20:32 ` Tom Roeder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).