Re: [PATCH] scripts/tags.sh: Include tools directory in tags generation

[Greg KH]

On Thu, Jun 30, 2022 at 01:54:00AM +0300, Cristian Ciocaltea wrote:
> 
> On 6/30/22 01:18, Vipin Sharma wrote:
> > On Mon, Jun 27, 2022 at 11:05 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > > 
> > > On Mon, Jun 27, 2022 at 10:47:35AM -0700, Vipin Sharma wrote:
> > > > On Fri, Jun 17, 2022 at 5:55 PM Vipin Sharma <vipinsh@xxxxxxxxxx> wrote:
> > > > > 
> > > > > Add tools directory in generating tags and quiet the "No such file or
> > > > > directory" warnings.
> > > > > 
> > > > > It reverts the changes introduced in commit 162343a876f1
> > > > > ("scripts/tags.sh: exclude tools directory from tags generation") while
> > > > > maintainig the original intent of the patch to get rid of the warnings.
> > > > > This allows the root level cscope files to include tools source code
> > > > > besides kernel and a single place to browse the code for both.
> > > > > 
> > > > > Signed-off-by: Vipin Sharma <vipinsh@xxxxxxxxxx>
> > > > > ---
> > > > > 
> > > > > I have found myself many times to browse tools and other part of the
> > > > > kernel code together. Excluding tools from the root level cscope makes
> > > > > it difficult to efficiently move between files and find user api
> > > > > definitions.
> > > > > 
> > > > > Root cause of these warning is due to generated .cmd files which use
> > > > > relative paths in some files, I am not sure how to make them absolute
> > > > > file paths which can satisfy realpath warnings. Also, not sure if those
> > > > > warnings are helpful and should be kept. Passing "-q" to realpath seems
> > > > > easier solution. Please, let me know if there is a better alternative.
> > > > > 
> > > > > Thanks
> > > > > 
> > > > >   scripts/tags.sh | 9 +--------
> > > > >   1 file changed, 1 insertion(+), 8 deletions(-)
> > > > > 
> > > > > diff --git a/scripts/tags.sh b/scripts/tags.sh
> > > > > index 01fab3d4f90b5..e137cf15aae9d 100755
> > > > > --- a/scripts/tags.sh
> > > > > +++ b/scripts/tags.sh
> > > > > @@ -25,13 +25,6 @@ else
> > > > >          tree=${srctree}/
> > > > >   fi
> > > > > 
> > > > > -# ignore userspace tools
> > > > > -if [ -n "$COMPILED_SOURCE" ]; then
> > > > > -       ignore="$ignore ( -path ./tools ) -prune -o"
> > > > > -else
> > > > > -       ignore="$ignore ( -path ${tree}tools ) -prune -o"
> > > > > -fi
> > > > > -
> > > > >   # Detect if ALLSOURCE_ARCHS is set. If not, we assume SRCARCH
> > > > >   if [ "${ALLSOURCE_ARCHS}" = "" ]; then
> > > > >          ALLSOURCE_ARCHS=${SRCARCH}
> > > > > @@ -100,7 +93,7 @@ all_compiled_sources()
> > > > >                  find $ignore -name "*.cmd" -exec \
> > > > >                          grep -Poh '(?(?=^source_.* \K).*|(?=^  \K\S).*(?= \\))' {} \+ |
> > > > >                  awk '!a[$0]++'
> > > > > -       } | xargs realpath -es $([ -z "$KBUILD_ABS_SRCTREE" ] && echo --relative-to=.) |
> > > > > +       } | xargs realpath -esq $([ -z "$KBUILD_ABS_SRCTREE" ] && echo --relative-to=.) |
> > > > >          sort -u
> > > > >   }
> > > > > 
> > > > > --
> > > > > 2.37.0.rc0.104.g0611611a94-goog
> > > > > 
> > > > 
> > > > Hi Greg,
> > > > 
> > > > Any update on the patch?
> > > 
> > > Nope!
> > > 
> > > I don't really think we should add back in the tools to this, as if you
> > > want to search them, then can't you just generate the needed tags for
> > > the tools directory?
> > > 
> > 
> > Some folders in the tools directory do provide cscope rules. However,
> > those tags can only be used when I open the vim in those directories.
> > For example, if I am writing a KVM selftest and I want to explore code
> > related to certain ioctl in kernel as well as some code in KVM
> > selftest library, I cannot use two cscope files (one in the kernel
> > root dir and another in tools/testing/selftests/kvm) in a single VIM
> > instance. It starts having issues with the file paths. If the root
> > level cscope file includes tools directory then all of the tags will
> > be at one place and makes it very easy to browse tools code along with
> > the rest of the kernel.
> > 
> > > But as I don't even use this script ever, it feels odd for me to be the
> > > one "owning" it, so it would be great if others could chime in who
> > > actually use it.
> > > 
> 
> Since the tools directory has been excluded just to get rid of those
> warnings, I think there is no obvious reason to not add it back - at least
> the use case described above is perfectly valid.

So is that an "Acked-by:"?