[PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Nathan Chancellor]

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
> 

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Nathan Chancellor]

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',

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Nathan Chancellor]

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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Stephen Zhang]

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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Masahiro Yamada]

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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Stephen Zhang]

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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Masahiro Yamada]

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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Stephen Zhang]

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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Masahiro Yamada]

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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Stephen Zhang]

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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Nathan Chancellor]

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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Stephen Zhang]

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

Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

[Tom Roeder]

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',