From: Nick Desaulniers <ndesaulniers@google.com>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
Nathan Huckleberry <nhuck@google.com>,
Tom Roeder <tmroeder@google.com>,
clang-built-linux <clang-built-linux@googlegroups.com>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 7/9] gen_compile_commands: support *.o, *.a, modules.order in positional argument
Date: Fri, 21 Aug 2020 17:59:33 -0700 [thread overview]
Message-ID: <CAKwvOdmR=VeR0=LUgXCwnpK9LH90_itzv627wBEK4hCroBEW9Q@mail.gmail.com> (raw)
In-Reply-To: <20200821190159.1033740-8-masahiroy@kernel.org>
On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> This script currently searches the specified directory for .cmd files.
> One drawback is it may contain stale .cmd files after you rebuild the
> kernel several times without 'make clean'.
>
> This commit supports *.o, *.a, and modules.order as positional
> parameters. If such files are given, they are parsed to collect
> associated .cmd files. I added a generator helper for each of them.
>
> This feature is useful to get the list of active .cmd files from the
> last build, and will be used by the next commit to wire up the
> compile_commands.json rule to the Makefile.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> Changes in v2:
> - Separate the file parser into generator functions
> - Use 'obj' instead of 'object' because 'object' is a built-in function
> - I think using 'file' is OK because it is not a built-in function in Python3
> (https://docs.python.org/3/library/functions.html)
> Anyway, the variable 'file' is no longer used in this version
> - Keep the previous work-flow to allow to search the given directory
>
> scripts/gen_compile_commands.py | 100 ++++++++++++++++++++++++++++++--
> 1 file changed, 96 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> index 6dec7e2c4098..65859e6044b5 100755
> --- a/scripts/gen_compile_commands.py
> +++ b/scripts/gen_compile_commands.py
> @@ -12,6 +12,7 @@ import json
> import logging
> import os
> import re
> +import subprocess
>
> _DEFAULT_OUTPUT = 'compile_commands.json'
> _DEFAULT_LOG_LEVEL = 'WARNING'
> @@ -32,8 +33,9 @@ def parse_arguments():
> Returns:
> log_level: A logging level to filter log output.
> directory: The work directory where the objects were built
> + ar: Command used for parsing .a archives
> output: Where to write the compile-commands JSON file.
> - paths: The list of directories to handle to find .cmd files
> + paths: The list of files/directories to handle to find .cmd files
> """
> usage = 'Creates a compile_commands.json database from kernel .cmd files'
> parser = argparse.ArgumentParser(description=usage)
> @@ -53,12 +55,21 @@ def parse_arguments():
> parser.add_argument('--log_level', choices=_VALID_LOG_LEVELS,
> default=_DEFAULT_LOG_LEVEL, help=log_level_help)
>
> + ar_help = 'command used for parsing .a archives'
> + parser.add_argument('-a', '--ar', type=str, default='ar', help=ar_help)
If there's a default, doesn't that mean it's no longer required? I
think it should be required. For a clang specific tool, we'd prefer
the default to be llvm-ar anyways.
> +
> + 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')
> + parser.add_argument('paths', type=str, nargs='*', help=paths_help)
> +
> args = parser.parse_args()
>
> return (args.log_level,
> os.path.abspath(args.directory),
> args.output,
> - [args.directory])
> + args.ar,
> + args.paths if len(args.paths) > 0 else [args.directory])
>
>
> def cmdfiles_in_dir(directory):
> @@ -81,6 +92,73 @@ def cmdfiles_in_dir(directory):
> yield os.path.join(dirpath, filename)
>
>
> +def to_cmdfile(path):
> + """Return the path of .cmd file used for the given build artifact
> +
> + Args:
> + Path: file path
> +
> + Returns:
> + The path to .cmd file
> + """
> + dir, base = os.path.split(path)
> + return os.path.join(dir, '.' + base + '.cmd')
> +
> +
> +def cmdfiles_for_o(obj):
> + """Generate the iterator of .cmd files associated with the object
> +
> + Yield the .cmd file used to build the given object
> +
> + Args:
> + obj: The object path
> +
> + Yields:
> + The path to .cmd file
> + """
> + yield to_cmdfile(obj)
> +
> +
> +def cmdfiles_for_a(archive, ar):
> + """Generate the iterator of .cmd files associated with the archive.
> +
> + Parse the given archive, and yield every .cmd file used to build it.
> +
> + Args:
> + archive: The archive to parse
> +
> + Yields:
> + The path to every .cmd file found
> + """
> + for obj in subprocess.check_output([ar, '-t', archive]).decode().split():
> + yield to_cmdfile(obj)
> +
> +
> +def cmdfiles_for_modorder(modorder):
> + """Generate the iterator of .cmd files associated with the modules.order.
> +
> + Parse the given modules.order, and yield every .cmd file used to build the
> + contained modules.
> +
> + Args:
> + modorder: The modules.order file to parse
> +
> + Yields:
> + The path to every .cmd file found
> + """
> + with open(modorder) as f:
> + for line in f:
> + ko = line.rstrip()
> + base, ext = os.path.splitext(ko)
below in main() you check the file extension with endswith(). Would
it be good to be consistent between the two?
> + if ext != '.ko':
> + sys.exit('{}: module path must end with .ko'.format(ko))
> + mod = base + '.mod'
> + # The first line of *.mod lists the objects that compose the module.
> + with open(mod) as m:
> + for obj in m.readline().split():
> + yield to_cmdfile(obj)
> +
> +
> def process_line(root_directory, command_prefix, file_path):
> """Extracts information from a .cmd line and creates an entry from it.
>
> @@ -116,7 +194,7 @@ def process_line(root_directory, command_prefix, file_path):
>
> def main():
> """Walks through the directory and finds and parses .cmd files."""
> - log_level, directory, output, paths = parse_arguments()
> + log_level, directory, output, ar, paths = parse_arguments()
>
> level = getattr(logging, log_level)
> logging.basicConfig(format='%(levelname)s: %(message)s', level=level)
> @@ -126,7 +204,21 @@ def main():
> compile_commands = []
>
> for path in paths:
> - cmdfiles = cmdfiles_in_dir(path)
> + # If 'path' is a directory, handle all .cmd files under it.
> + # Otherwise, handle .cmd files associated with the file.
> + # Most of built-in objects are linked via archives (built-in.a or lib.a)
> + # but some are linked to vmlinux directly.
> + # Modules are lis
^ was this comment cut off?
> + if os.path.isdir(path):
> + cmdfiles = cmdfiles_in_dir(path)
> + elif path.endswith('.o'):
> + cmdfiles = cmdfiles_for_o(path)
> + elif path.endswith('.a'):
> + cmdfiles = cmdfiles_for_a(path, ar)
> + elif path.endswith('modules.order'):
> + cmdfiles = cmdfiles_for_modorder(path)
> + else:
> + sys.exit('{}: unknown file type'.format(path))
>
> for cmdfile in cmdfiles:
> with open(cmdfile, 'rt') as f:
> --
> 2.25.1
>
--
Thanks,
~Nick Desaulniers
next prev parent reply other threads:[~2020-08-22 0:59 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-21 19:01 [PATCH v2 0/9] kbuild: clang-tidy Masahiro Yamada
2020-08-21 19:01 ` [PATCH v2 1/9] gen_compile_commands: parse only the first line of .*.cmd files Masahiro Yamada
2020-08-21 19:01 ` [PATCH v2 2/9] gen_compile_commands: use choices for --log_levels option Masahiro Yamada
2020-08-22 0:23 ` Nick Desaulniers
2020-08-21 19:01 ` [PATCH v2 3/9] gen_compile_commands: do not support .cmd files under tools/ directory Masahiro Yamada
2020-08-22 0:27 ` Nick Desaulniers
2020-08-21 19:01 ` [PATCH v2 4/9] gen_compile_commands: reword the help message of -d option Masahiro Yamada
2020-08-22 0:29 ` Nick Desaulniers
2020-08-22 1:55 ` Masahiro Yamada
2020-08-22 2:05 ` Nick Desaulniers
2020-08-21 19:01 ` [PATCH v2 5/9] gen_compile_commands: make -o option independent " Masahiro Yamada
2020-08-22 0:35 ` Nick Desaulniers
2020-08-21 19:01 ` [PATCH v2 6/9] gen_compile_commands: move directory walk to a generator function Masahiro Yamada
2020-08-22 0:41 ` Nick Desaulniers
2020-08-22 4:35 ` Masahiro Yamada
2020-08-21 19:01 ` [PATCH v2 7/9] gen_compile_commands: support *.o, *.a, modules.order in positional argument Masahiro Yamada
2020-08-22 0:59 ` Nick Desaulniers [this message]
2020-08-22 3:11 ` Masahiro Yamada
2020-08-21 19:01 ` [PATCH v2 8/9] kbuild: wire up the build rule of compile_commands.json to Makefile Masahiro Yamada
2020-08-22 0:45 ` Nick Desaulniers
2020-08-21 19:01 ` [PATCH v2 9/9] Makefile: Add clang-tidy and static analyzer support to makefile Masahiro Yamada
2020-08-22 1:06 ` [PATCH v2 0/9] kbuild: clang-tidy Nick Desaulniers
2020-08-22 1:12 ` Sedat Dilek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKwvOdmR=VeR0=LUgXCwnpK9LH90_itzv627wBEK4hCroBEW9Q@mail.gmail.com' \
--to=ndesaulniers@google.com \
--cc=clang-built-linux@googlegroups.com \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=masahiroy@kernel.org \
--cc=nhuck@google.com \
--cc=tmroeder@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).