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@xxxxxxxxxx> 于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@xxxxxxxxx>
> >
> > 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@xxxxxxxxx/

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