Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <Y5T66yWNVAZNIaJ0@mail.google.com> <Y5UP+tnnxNgoi6A2@mail.google.com>
In-Reply-To: <Y5UP+tnnxNgoi6A2@mail.google.com>
From:   Vipin Sharma <vipinsh@google.com>
Date:   Mon, 12 Dec 2022 13:27:37 -0800
Message-ID: <CAHVum0eOzd8MgP0FGObHWvqG_oPVoTmk_5gkEB0sAJK9JgCsFg@mail.gmail.com>
Subject: Re: [PATCH v2] scripts/tags.sh: choose which directories to exclude
 from being indexed
To:     Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
Cc:     Masahiro Yamada <masahiroy@kernel.org>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Nicolas Schier <nicolas@fjasle.eu>,
        Jonathan Corbet <corbet@lwn.net>,
        Cristian Ciocaltea <cristian.ciocaltea@collabora.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kbuild@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Dec 10, 2022 at 3:02 PM Paulo Miguel Almeida
<paulo.miguel.almeida.rodenas@gmail.com> wrote:
>
> It's common for drivers that share same physical components to also
> duplicate source code (or at least portions of it). A good example is
> both drivers/gpu/drm/amdgpu/* and drivers/gpu/drm/radeon/* have a header
> file called atombios.h.
>
> While their contents aren't the same, a lot of their structs have
> the exact same names which makes navigating through the code base a bit
> messy as cscope will show up 'references' across drivers which aren't
> exactly correct.
>
> This patch makes it possible for the devs to specify which folders
> they don't want to include into database as part of the
> find_other_sources func if a makefile variable IGNOREDIRS is present,
> otherwise the original behaviour is kept.
>
> Example:
>         make ARCH=x86 IGNOREDIRS=drivers/gpu/drm/radeon,tools cscope
>
> Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
> ---
> Changelog:
>
> - v2: change approach to include everything unless specified by the
>   IGNOREDIRS variable: (Req: Vipin Sharma)
> - v1: https://lore.kernel.org/lkml/Y5OKDvbGk4Kro6MK@mail.google.com/
> ---
>  Documentation/kbuild/kbuild.rst |  7 +++++++
>  scripts/tags.sh                 | 11 +++++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
> index 08f575e6236c..5f99f30e20d8 100644
> --- a/Documentation/kbuild/kbuild.rst
> +++ b/Documentation/kbuild/kbuild.rst
> @@ -278,6 +278,13 @@ To get all available archs you can also specify all. E.g.::
>
>      $ make ALLSOURCE_ARCHS=all tags
>
> +IGNOREDIRS
> +---------------
> +For tags/TAGS/cscope targets, you can choose which directories won't
> +be included in the databases, separated by comma. E.g.:
> +
> +    $ make IGNOREDIRS=drivers/gpu/drm/radeon,tools cscope
> +
>  KBUILD_BUILD_TIMESTAMP
>  ----------------------
>  Setting this to a date string overrides the timestamp used in the
> diff --git a/scripts/tags.sh b/scripts/tags.sh
> index e137cf15aae9..554721e9cad2 100755
> --- a/scripts/tags.sh
> +++ b/scripts/tags.sh
> @@ -59,10 +59,17 @@ find_include_sources()
>  }
>
>  # find sources in rest of tree
> -# we could benefit from a list of dirs to search in here
>  find_other_sources()
>  {
> -       find ${tree}* $ignore \
> +       local loc_ignore=${ignore}
> +       if [ -n "${IGNOREDIRS}" ]; then
> +               exp_ignored_dirs=$(sed 's/,/ /g' <<< ${IGNOREDIRS})
> +               for i in ${exp_ignored_dirs}; do
> +                       loc_ignore="${loc_ignore} ( -path $i ) -prune -o"
> +               done
> +       fi
> +

This should be global overwrite instead of just in this function.
Before find_other_sources() is executed, this script finds files in
arch directories. So, if you keep it local then those files cannot be
excluded which makes execution of the command incorrect:

make IGNOREDIRS=arch/x86 cscope

Above command will still index all of the code in arch/x86. Something
like this will be better.

--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -17,6 +17,13 @@ ignore="$(echo "$RCS_FIND_IGNORE" | sed 's|\\||g' )"
 # tags and cscope files should also ignore MODVERSION *.mod.c files
 ignore="$ignore ( -name *.mod.c ) -prune -o"

+if [ -n "${IGNOREDIRS}" ]; then
+       exp_ignored_dirs=$(sed 's/,/ /g' <<< ${IGNOREDIRS})
+       for i in ${exp_ignored_dirs}; do
+               ignore="${ignore} ( -path $i ) -prune -o"
+       done
+fi
+
 # Use make KBUILD_ABS_SRCTREE=1 {tags|cscope}
 # to force full paths for a non-O= build
 if [ "${srctree}" = "." -o -z "${srctree}" ]; then
@@ -62,9 +69,9 @@ find_include_sources()
 # we could benefit from a list of dirs to search in here
 find_other_sources()
 {
-       find ${tree}* $ignore \
-            \( -path ${tree}include -o -path ${tree}arch -o -name
'.tmp_*' \) -prune -o \
-              -name "$1" -not -type l -print;
+       find ${tree}* ${ignore} \
+               \( -path ${tree}include -o -path ${tree}arch -o -name
'.tmp_*' \) -prune -o \
+               -name "$1" -not -type l -print;
 }

We will still have to specify arch/x86 and arch/x86/include but this
works and keeps the definition of IGNOREDIRS relatively correct.


> +       find ${tree}* ${loc_ignore} \
>              \( -path ${tree}include -o -path ${tree}arch -o -name '.tmp_*' \) -prune -o \
>                -name "$1" -not -type l -print;
>  }
> --
> 2.38.1
>