[PATCH] gen_compile_command: Add support for separate KBUILD_OUTPUT directory

[PATCH] gen_compile_command: Add support for separate KBUILD_OUTPUT directory

[Matthias Kaehlcke]

gen_compile_command.py currently assumes that the .cmd files and the
source code live in the same directory, which is not the case when
a separate KBUILD_OUTPUT directory is used.

Add a new option to specify the kbuild output directory. If the
option is not set the kernel source directory is used.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Feel free to bikeshed about the option names ;-)

 scripts/gen_compile_commands.py | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
index 7915823b92a5..5a738ec66cc7 100755
--- a/scripts/gen_compile_commands.py
+++ b/scripts/gen_compile_commands.py
@@ -31,15 +31,21 @@ def parse_arguments():
 
     Returns:
         log_level: A logging level to filter log output.
-        directory: The directory to search for .cmd files.
+        source_directory: The kernel source directory.
+        kbuild_output_directory: The directory to search for .cmd files.
         output: Where to write the compile-commands JSON file.
     """
     usage = 'Creates a compile_commands.json database from kernel .cmd files'
     parser = argparse.ArgumentParser(description=usage)
 
-    directory_help = ('Path to the kernel source directory to search '
+    directory_help = ('Path to the kernel source directory'
                       '(defaults to the working directory)')
     parser.add_argument('-d', '--directory', type=str, help=directory_help)
+    kbuild_output_directory_help = ('Path to the directory to search for '
+                                    '.cmd files'
+                      '(defaults to the kernel source directory)')
+    parser.add_argument('-k', '--kbuild-output-directory', type=str,
+                        help=kbuild_output_directory_help)
 
     output_help = ('The location to write compile_commands.json (defaults to '
                    'compile_commands.json in the search directory)')
@@ -58,11 +64,14 @@ def parse_arguments():
     if log_level not in _VALID_LOG_LEVELS:
         raise ValueError('%s is not a valid log level' % log_level)
 
-    directory = args.directory or os.getcwd()
-    output = args.output or os.path.join(directory, _DEFAULT_OUTPUT)
-    directory = os.path.abspath(directory)
+    source_directory = args.directory or os.getcwd()
+    output = args.output or os.path.join(source_directory, _DEFAULT_OUTPUT)
+    source_directory = os.path.abspath(source_directory)
 
-    return log_level, directory, output
+    kbuild_output_directory = args.kbuild_output_directory or source_directory
+    kbuild_output_directory = os.path.abspath(kbuild_output_directory)
+
+    return log_level, source_directory, kbuild_output_directory, output
 
 
 def process_line(root_directory, file_directory, command_prefix, relative_path):
@@ -109,7 +118,8 @@ def process_line(root_directory, file_directory, command_prefix, relative_path):
 
 def main():
     """Walks through the directory and finds and parses .cmd files."""
-    log_level, directory, output = parse_arguments()
+    log_level, source_directory, kbuild_output_directory, output = \
+        parse_arguments()
 
     level = getattr(logging, log_level)
     logging.basicConfig(format='%(levelname)s: %(message)s', level=level)
@@ -118,7 +128,7 @@ def main():
     line_matcher = re.compile(_LINE_PATTERN)
 
     compile_commands = []
-    for dirpath, _, filenames in os.walk(directory):
+    for dirpath, _, filenames in os.walk(kbuild_output_directory):
         for filename in filenames:
             if not filename_matcher.match(filename):
                 continue
@@ -131,7 +141,7 @@ def main():
                         continue
 
                     try:
-                        entry = process_line(directory, dirpath,
+                        entry = process_line(source_directory, dirpath,
                                              result.group(1), result.group(2))
                         compile_commands.append(entry)
                     except ValueError as err:
-- 
2.22.0.410.gd8fdbe21b5-goog

Re: [PATCH] gen_compile_command: Add support for separate KBUILD_OUTPUT directory

[Tom Roeder]

I can confirm that I can still run clang-tidy on the kernel using this
new version of the script; it generates a version of
compile_commands.json that works in my case.

On Thu, Jun 20, 2019 at 11:45:23AM -0700, Matthias Kaehlcke wrote:
> gen_compile_command.py currently assumes that the .cmd files and the
> source code live in the same directory, which is not the case when
> a separate KBUILD_OUTPUT directory is used.
> 
> Add a new option to specify the kbuild output directory. If the
> option is not set the kernel source directory is used.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Tom Roeder <tmroeder@google.com>
Tested-by: Tom Roeder <tmroeder@google.com>

> ---
> Feel free to bikeshed about the option names ;-)
> 
>  scripts/gen_compile_commands.py | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> index 7915823b92a5..5a738ec66cc7 100755
> --- a/scripts/gen_compile_commands.py
> +++ b/scripts/gen_compile_commands.py
> @@ -31,15 +31,21 @@ def parse_arguments():
>  
>      Returns:
>          log_level: A logging level to filter log output.
> -        directory: The directory to search for .cmd files.
> +        source_directory: The kernel source directory.
> +        kbuild_output_directory: The directory to search for .cmd files.
>          output: Where to write the compile-commands JSON file.
>      """
>      usage = 'Creates a compile_commands.json database from kernel .cmd files'
>      parser = argparse.ArgumentParser(description=usage)
>  
> -    directory_help = ('Path to the kernel source directory to search '
> +    directory_help = ('Path to the kernel source directory'
Minor detail: this needs a space after "directory" so that it reads
"directory '". Otherwise, the output doesn't have a space before the
parenthesis.

>                        '(defaults to the working directory)')
>      parser.add_argument('-d', '--directory', type=str, help=directory_help)
> +    kbuild_output_directory_help = ('Path to the directory to search for '
> +                                    '.cmd files'
Same comment here: this should be "files '", with a space before the
ending quote character.

Re: [PATCH] gen_compile_command: Add support for separate KBUILD_OUTPUT directory

[Nick Desaulniers]

On Thu, Jun 20, 2019 at 11:45 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> gen_compile_command.py currently assumes that the .cmd files and the
> source code live in the same directory, which is not the case when
> a separate KBUILD_OUTPUT directory is used.

Great point; android builds the kernel outside of the source dir
(`make O=/non-source/path ...`). Thanks for the patch! BTW if CrOS is
doing cool stuff with compile_commands.json; I'd like to know!
Particularly; I'm curious if it's possible to generate Ninja build
files from compile_commands.json; I do miss Doug's Kbuild caching
patches' speedup.
Acked-by: Nick Desaulniers <ndesaulniers@google.com>

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] gen_compile_command: Add support for separate KBUILD_OUTPUT directory

[Doug Anderson]

Hi,

On Thu, Jun 20, 2019 at 12:53 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> I do miss Doug's Kbuild caching patches' speedup.

You actually get quite a bit of this by grabbing a new version of
ccache (assuming you use ccache).  :-P  You still have to pay the
penalty (twice) for all the options that are tested that the compiler
_doesn't_ support, but at least you get the cache for the commands
that the compiler does support.

Specifically, make sure you have a ccache with:

    * https://github.com/ccache/ccache/pull/365
    * https://github.com/ccache/ccache/pull/370

I still have it in my thoughts to avoid the penalty for options that
the compiler doesn't support but haven't had time to work on it
recently.


-Doug

Re: [PATCH] gen_compile_command: Add support for separate KBUILD_OUTPUT directory

[Nick Desaulniers]

On Thu, Jun 20, 2019 at 1:13 PM Doug Anderson <dianders@google.com> wrote:
> On Thu, Jun 20, 2019 at 12:53 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > I do miss Doug's Kbuild caching patches' speedup.
>
> You actually get quite a bit of this by grabbing a new version of
> ccache (assuming you use ccache).  :-P  You still have to pay the
> penalty (twice) for all the options that are tested that the compiler
> _doesn't_ support, but at least you get the cache for the commands
> that the compiler does support.

Hello darkness my old friend:
https://nickdesaulniers.github.io/blog/2018/06/02/speeding-up-linux-kernel-builds-with-ccache/
Man, that post has not aged well.  Here's what we do now:
https://github.com/ClangBuiltLinux/continuous-integration/blob/45ab5842a69cb0c72d27d34e73b0599ec2a0e2ed/driver.sh#L227-L245

> Specifically, make sure you have a ccache with:
>
>     * https://github.com/ccache/ccache/pull/365
>     * https://github.com/ccache/ccache/pull/370

Oh! Interesting finds and thanks for the pointers.  Did these make it
into a release version of ccache, yet? If so, do you know which
version?

> I still have it in my thoughts to avoid the penalty for options that
> the compiler doesn't support but haven't had time to work on it
> recently.

It had better not be autoconf! (Hopefully yet-to-be-written GNU C
extensions can support feature detection via C preprocessor)
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] gen_compile_command: Add support for separate KBUILD_OUTPUT directory

[Nathan Chancellor]

On Thu, Jun 20, 2019 at 01:25:36PM -0700, Nick Desaulniers wrote:
> On Thu, Jun 20, 2019 at 1:13 PM Doug Anderson <dianders@google.com> wrote:
> > On Thu, Jun 20, 2019 at 12:53 PM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > I do miss Doug's Kbuild caching patches' speedup.
> >
> > You actually get quite a bit of this by grabbing a new version of
> > ccache (assuming you use ccache).  :-P  You still have to pay the
> > penalty (twice) for all the options that are tested that the compiler
> > _doesn't_ support, but at least you get the cache for the commands
> > that the compiler does support.
> 
> Hello darkness my old friend:
> https://nickdesaulniers.github.io/blog/2018/06/02/speeding-up-linux-kernel-builds-with-ccache/
> Man, that post has not aged well.  Here's what we do now:
> https://github.com/ClangBuiltLinux/continuous-integration/blob/45ab5842a69cb0c72d27d34e73b0599ec2a0e2ed/driver.sh#L227-L245
> 
> > Specifically, make sure you have a ccache with:
> >
> >     * https://github.com/ccache/ccache/pull/365
> >     * https://github.com/ccache/ccache/pull/370
> 
> Oh! Interesting finds and thanks for the pointers.  Did these make it
> into a release version of ccache, yet? If so, do you know which
> version?
>

It should be available in 3.7 if I am reading git history right.

Cheers,
Nathan

> > I still have it in my thoughts to avoid the penalty for options that
> > the compiler doesn't support but haven't had time to work on it
> > recently.
> 
> It had better not be autoconf! (Hopefully yet-to-be-written GNU C
> extensions can support feature detection via C preprocessor)
> -- 
> Thanks,
> ~Nick Desaulniers

Re: [PATCH] gen_compile_command: Add support for separate KBUILD_OUTPUT directory

[Matthias Kaehlcke]

On Thu, Jun 20, 2019 at 12:23:45PM -0700, Tom Roeder wrote:
> I can confirm that I can still run clang-tidy on the kernel using this
> new version of the script; it generates a version of
> compile_commands.json that works in my case.
> 
> On Thu, Jun 20, 2019 at 11:45:23AM -0700, Matthias Kaehlcke wrote:
> > gen_compile_command.py currently assumes that the .cmd files and the
> > source code live in the same directory, which is not the case when
> > a separate KBUILD_OUTPUT directory is used.
> > 
> > Add a new option to specify the kbuild output directory. If the
> > option is not set the kernel source directory is used.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> Reviewed-by: Tom Roeder <tmroeder@google.com>
> Tested-by: Tom Roeder <tmroeder@google.com>

Thanks!

> >  scripts/gen_compile_commands.py | 28 +++++++++++++++++++---------
> >  1 file changed, 19 insertions(+), 9 deletions(-)
> > 
> > diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> > index 7915823b92a5..5a738ec66cc7 100755
> > --- a/scripts/gen_compile_commands.py
> > +++ b/scripts/gen_compile_commands.py
> > @@ -31,15 +31,21 @@ def parse_arguments():
> >  
> >      Returns:
> >          log_level: A logging level to filter log output.
> > -        directory: The directory to search for .cmd files.
> > +        source_directory: The kernel source directory.
> > +        kbuild_output_directory: The directory to search for .cmd files.
> >          output: Where to write the compile-commands JSON file.
> >      """
> >      usage = 'Creates a compile_commands.json database from kernel .cmd files'
> >      parser = argparse.ArgumentParser(description=usage)
> >  
> > -    directory_help = ('Path to the kernel source directory to search '
> > +    directory_help = ('Path to the kernel source directory'
> Minor detail: this needs a space after "directory" so that it reads
> "directory '". Otherwise, the output doesn't have a space before the
> parenthesis.
> 
> >                        '(defaults to the working directory)')
> >      parser.add_argument('-d', '--directory', type=str, help=directory_help)
> > +    kbuild_output_directory_help = ('Path to the directory to search for '
> > +                                    '.cmd files'
> Same comment here: this should be "files '", with a space before the
> ending quote character.

Ok, I'll wait a bit for it there are other comments and send out a new
version with the spaces added.

Re: [PATCH] gen_compile_command: Add support for separate KBUILD_OUTPUT directory

[Matthias Kaehlcke]

On Thu, Jun 20, 2019 at 12:53:22PM -0700, Nick Desaulniers wrote:
> On Thu, Jun 20, 2019 at 11:45 AM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > gen_compile_command.py currently assumes that the .cmd files and the
> > source code live in the same directory, which is not the case when
> > a separate KBUILD_OUTPUT directory is used.
> 
> Great point; android builds the kernel outside of the source dir
> (`make O=/non-source/path ...`). Thanks for the patch! BTW if CrOS is
> doing cool stuff with compile_commands.json; I'd like to know!
> Particularly; I'm curious if it's possible to generate Ninja build
> files from compile_commands.json; I do miss Doug's Kbuild caching
> patches' speedup.
> Acked-by: Nick Desaulniers <ndesaulniers@google.com>

At this point Chrome OS doesn't do anything with compile_commands.json
for the kernel. I was just toying around a bit after a presentation
from Tom Hughes about IDE integration and encountered this limitation.

Re: [PATCH] gen_compile_command: Add support for separate KBUILD_OUTPUT directory

[Doug Anderson]

Hi,

On Thu, Jun 20, 2019 at 1:25 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Thu, Jun 20, 2019 at 1:13 PM Doug Anderson <dianders@google.com> wrote:
> > On Thu, Jun 20, 2019 at 12:53 PM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > I do miss Doug's Kbuild caching patches' speedup.
> >
> > You actually get quite a bit of this by grabbing a new version of
> > ccache (assuming you use ccache).  :-P  You still have to pay the
> > penalty (twice) for all the options that are tested that the compiler
> > _doesn't_ support, but at least you get the cache for the commands
> > that the compiler does support.
>
> Hello darkness my old friend:
> https://nickdesaulniers.github.io/blog/2018/06/02/speeding-up-linux-kernel-builds-with-ccache/
> Man, that post has not aged well.  Here's what we do now:
> https://github.com/ClangBuiltLinux/continuous-integration/blob/45ab5842a69cb0c72d27d34e73b0599ec2a0e2ed/driver.sh#L227-L245
>
> > Specifically, make sure you have a ccache with:
> >
> >     * https://github.com/ccache/ccache/pull/365
> >     * https://github.com/ccache/ccache/pull/370
>
> Oh! Interesting finds and thanks for the pointers.  Did these make it
> into a release version of ccache, yet? If so, do you know which
> version?
>
> > I still have it in my thoughts to avoid the penalty for options that
> > the compiler doesn't support but haven't had time to work on it
> > recently.
>
> It had better not be autoconf! (Hopefully yet-to-be-written GNU C
> extensions can support feature detection via C preprocessor)

I've had a few ideas.  I won't object if you wanted to steal any of
them and implement them yourself.  :-P

1. Lamest but easiest (and best speedup for me personally) is to just
cheat and hack our toolchain wrapper (which is invoked _before_
ccache) to immediately reject flags we know our toolchain doesn't
support.  I don't love our toolchain wrapper since it adds ~15 ms to
every compiler invocation, but if it's there anyway might as well use
it.

2. Part of the slowness in testing for unsupported options is that
ccache runs twice.  After validating that it doesn't have a cache hit,
it first tries to produce a preprocessed version of the file.  With an
unsupported option that fails.  ...so ccache tries again _without_ the
preprocessor.  So you call the compiler twice.  We could either make
ccache skip this double-step when the target is /dev/null (why bother
trying to preprocess /dev/null?) or we could add a CCACHE directive
into the kernel build when testing options.  Probably the ccache
option makes the most sense.

3. In theory we could teach ccache how to cache these "tests for
unsupported options".  That might be hard, though.

-Doug