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

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

On 6/30/22 09:42, Greg KH wrote:
> 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@linuxfoundation.org> 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@google.com> 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@google.com>
>>>>>> ---
>>>>>>
>>>>>> 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:"?

Acked-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>