linux-kernel - Re: [PATCH] kbuild: add Kbuild bash completion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAK7LNATdpJ9uprUTowFjA8XFWk1_wttWPcG11-X+A3b-CwLZ5g@mail.gmail.com>
Date: Thu, 6 Mar 2025 23:02:48 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Nicolas Schier <n.schier@....de>
Cc: linux-kbuild@...r.kernel.org, Nathan Chancellor <nathan@...nel.org>, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kbuild: add Kbuild bash completion

On Mon, Feb 24, 2025 at 11:25 PM Nicolas Schier <n.schier@....de> wrote:

> > +     O | KBUILD_OUTPUT | M | KBUILD_EXTMOD | MO | KBUILD_EXTMOD_OUTPUT | *_PATH)
> > +             # variables that take a directory.
> > +             _filedir -d
>
> Would it make sense to temporarily switch cwd to ${srctree} to allow
> completion of KBUILD_OUTPUT and friends with relative paths from
> ${srctree}?

I do not think so.
The completion should work against the current working directory

This usually matches to ${srctree}, as many people work
at the top of the kernel source tree. However, it is not necessary
to start the kernel build at the top of the kernel source tree.
You can start Kbuild at any arbitrary directory by using
the -f option to specify the top-level Makefile.


> A simple approach (probably with some bad side-effects), works for me:
>
>                 local opwd=$OLDPWD
>                 cd ${srctree} && _filedir -d && cd $OLDPWD
>                 OLDPWD=$opwd
>
> But this is quite a step further, if there is no clean solution right
> now, I'd prefer leaving it as it is for now.







> > +             return
> > +             ;;
> > +     KBUILD_EXTRA_SYMBOL | KBUILD_KCONFIG | KCONFIG_CONFIG)
> > +             # variables that take a file.
> > +             _filedir
> > +             return
> > +     esac
> > +
> > +     COMPREPLY+=($(compgen -W "${keywords[*]}" -- "${cur}"))
> > +}
> > +
> > +# Check the -C, -f options and 'source' symlink. Return the source tree we are
> > +# working in.
> > +__kbuild_get_srctree()
> > +{
> > +     local words=("$@")
> > +     local cwd makef_dir
> > +
> > +        # see if a path was specified with -C/--directory
> > +        for ((i = 1; i < ${#words[@]}; i++)); do
>
> indent ^^: spaces instead of tab

Thanks. I fixed all indentation errors.

> > +             if [[ ${words[i]} == -@(f|-?(make)file) ]]; then
>
> (I am really impressed: everytime I am reviewing one of your new shell
> scripts, I learn something new.  TIL: '-@()' and '?()'.)

This line was copy-pasted from the bash-completion project.  :-)

You can find the same code in
/usr/share/bash-completion/completions/make




> > +     KBUILD_*)
> > +             # There are many variables prefixed with 'KBUILD_'.
> > +             # Display them only when 'KBUILD_' is entered.
> > +             # shellcheck disable=SC2191 # '=' is appended for variables
> > +             keywords+=(
> > +                     KBUILD_{CHECKSRC,EXTMOD,EXTMOD_OUTPUT,VERBOSE,EXTRA_WARN,CLIPPY}=
> > +                     KBUILD_BUILD_{USER,HOST,TIMESTAMP}=
> > +                     KBUILD_MODPOST_{NOFINAL,WARN}=
> > +                     KBUILD_{ABS_SRCTREE,EXTRA_SYMBOLS,KCONFIG}=
>
> Did you leave-out KBUILD_OUTPUT by intention?  I think it would be nice
> to complete it here, too.

I just forgot to add it.
Will fix it.


>
> I am unsure, if I like the mix of quoted and unquoted use of variables.
> But as they all look correct to me, this is just a question of personal
> style.

I added quotes where required (i.e. shellcheck warns).
Otherwise, I do not add quotes.




>
>
> Thanks, I really appreciate this kbuild bash-completion.
>
> Reviewed-by: Nicolas Schier <n.schier@....de>
> Tested-by: Nicolas Schier <n.schier@....de>

Thanks for the review.

--
Best Regards
Masahiro Yamada