Re: [PATCH v2] gen_compile_commands: fix overlooked module files

[John Hubbard]

On 6/19/22 17:15, Masahiro Yamada wrote:
> On Thu, Jun 16, 2022 at 11:12 AM John Hubbard <jhubbard@xxxxxxxxxx> wrote:
> > scripts/clang-tools/gen_compile_commands.py incorrectly assumes that
> > each .mod file only contains one line. In fact, such files contain one
> > entry per line, and for some subsystems, there can be many, many lines.
> > For example, Nouveau has 762 entries, but only the first entry was being
> > processed. This problem causes clangd to fail to provide references and
> > definitions for valid files that are part of the current kernel
> > configuration.
>
>
> My previous comment "Can you update the commit log?"
> meant, "Can you rewrite the commit log (in a more concise way)?".
>
>
>
> I am not sure the Nouveau example is needed here, but
> more important information is that
> ecca4fea1ede4 changed the .mod file
> from "all entries at the first line" to "one entry per line".
>
>
>
>
> > This problem only occurs when using Kbuild to generate, like this:
> >
> >     make CC=clang compile_commands.json
> >
> > It does not occur if you just run gen_compile_commands.py "bare", like
> > this (below):
> >
> >     scripts/clang-tools/gen_compile_commands.py/gen_compile_commands.py .
>
> I am not sure if this is needed, but at least, the command line is wrong.
>
> scripts/clang-tools/gen_compile_commands.py/gen_compile_commands.py .
>      -->
> scripts/clang-tools/gen_compile_commands.py .

Good catch, not sure how that typo leaked in!

> > Fix this by fully processing each .mod file. This fix causes the number
> > of build commands that clangd finds in my kernel build (these numbers
> > are heavily dependent upon .config), from 2848 to 5292, which is an 85%
> > increase.
>
> I am not sure if the second sentence is needed
> because this patch is just an obvious fix.
>
>
>
> Sorry, I missed to adjust this file in ecca4fea1ede4,
> and ended up bothering you too much.

I'll have a go at rewriting the commit description, sure.

> > Fixes: 9413e7640564 ("kbuild: split the second line of *.mod into *.usyms")
> > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> > Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> > Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> > ---
> >
> > Changes since v1:
> >
> > Applied changes recommended by Masahiro Yamada (thanks!): corrected the
> > "Fixes" tag, and improved the python code for parsing .mod files.
> >
> >
> >   scripts/clang-tools/gen_compile_commands.py | 7 +++----
> >   1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
> > index 1d1bde1fd45e..acf4ec28aaf1 100755
> > --- a/scripts/clang-tools/gen_compile_commands.py
> > +++ b/scripts/clang-tools/gen_compile_commands.py
> > @@ -157,11 +157,10 @@ def cmdfiles_for_modorder(modorder):
> >               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.
> > +            # Read from *.mod, to get a list of objects that compose the module.
> >               with open(mod) as m:
> > -                for obj in m.readline().split():
> > -                    yield to_cmdfile(obj)
> > -
> > +                for mod_line in m:
> > +                    yield to_cmdfile(mod_line.rstrip())
>
>
> Please do not remove the blank line.

Sure, I didn't really intend to change the vertical whitespace. I'll restore that.

I'll send out a v3 shortly.

thanks,
--
John Hubbard
NVIDIA