Re: [PATCH] scripts/tags.sh: allow only selected directories to be indexed

[Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>]

On Sat, Dec 10, 2022 at 12:31:41PM -0800, Vipin Sharma wrote:
> On Fri, Dec 9, 2022 at 11:18 AM 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 want to include as part of the find_other_sources function if a
> > makefile variable OTHERSRCDIRS is present, otherwise the original
> > behaviour is kept.
> >
> > Example:
> >         make ARCH=x86 OTHERSRCDIRS=drivers/gpu/drm/radeon,tools cscope
> >
> 
> It is better to make the opposite option i.e. ignore directories. By
> default, cscope is all inclusive and it is more beneficial to have
> more code indexed than less. Default indexed
> directories will be different with and without OTHERSRCDIRS.
> 
> For example,
> 
> make ARCH=x86 cscope
> 
> # This includes all of the kernel code except non-x86 arch code.
> 
> make ARCH=x86 OTHERSRCSDIRS=drivers/gpu/drm/radeon/tools,tools cscope
> 
> # This includes only arch/x86, include/, tools/ and
> driver/gpu/drm/radeon/tools. It removes kernel/, block/, lib/,
> crypto/, virt/, etc. These are important kernel source code
> directories.
> 
> My vote is to make something like:
> make ARCH=x86 IGNOREDIRS=drivers/gpu/drm/amdgpu cscope
> 
> Parse IGNOREDIRS in the scripts/tags.sh and append to $ignore variable.
> 
> Also you should write this option in /Documentation/kbuild/kbuild.rst
> similar to ALLSOURCE_ARCHS
> 
> Thanks

Hi Vipin,

Thanks for taking the time to review this patch. I agree with you that
keeping cscope in its all inclusive approach is a better move. I will
make the requested changes and send a new patch.

Thanks!

- Paulo A.

> 
> 
> > Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
> > ---
> >  scripts/tags.sh | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/tags.sh b/scripts/tags.sh
> > index e137cf15aae9..958c07c4ac4a 100755
> > --- a/scripts/tags.sh
> > +++ b/scripts/tags.sh
> > @@ -59,12 +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 \
> > -            \( -path ${tree}include -o -path ${tree}arch -o -name '.tmp_*' \) -prune -o \
> > -              -name "$1" -not -type l -print;
> > +       find_def_params="-name $1 -not -type l -print"
> > +       if [ -n "${OTHERSRCDIRS}" ]; then
> > +               exp_src_dirs=$(sed 's/,/ /g' <<< ${OTHERSRCDIRS})
> > +               find ${exp_src_dirs} ${ignore} ${find_def_params};
> > +       else
> > +               find ${tree}* ${ignore} \
> > +                    \( -path ${tree}include -o -path ${tree}arch -o -name '.tmp_*' \) \
> > +                    -prune -o ${find_def_params};
> > +       fi
> >  }
> >
> >  find_sources()
> > --
> > 2.38.1
> >