Re: [PATCH kvm-unit-tests] cscope: fix database generation

[Bandan Das <bsd@redhat.com>]

Radim Krčmář <rkrcmar@redhat.com> writes:

> 2016-04-19 14:58-0400, Bandan Das:
>> Radim Krčmář <rkrcmar@redhat.com> writes:
>>> 2016-04-19 13:08-0400, Bandan Das:
>>>> Andrew Jones <drjones@redhat.com> writes:
>>>>> On Mon, Apr 18, 2016 at 07:04:55PM -0400, Bandan Das wrote:
>>>>>> 
>>>>>> The cscope.files that we generate doesn't include all
>>>>>> source files that are potentially interesting. We should
>>>>>> include all $(ARCH)es  and not the just the one configure
>>>>>> finds. Moreover, $(ARCH) expands to x86_64 which is not the
>>>>>> correct path for x86 sources. Generate cscope.files by searching
>>>>>> for all files starting from root.
>>>>>
>>>>> No thanks :-) I'd rather not get hits for x86 and powerpc when
>>>> 
>>>> I don't think that's a good idea. Just because you don't like to
>>>> see x86 bits when jumping around in arm code doesn't mean you should
>>>> hide all references to a given function. And if the arm code is
>>>> segregated enough that there are no common functions, you won't
>>>> see them anyway. 
>>>> [...]                                               But please let's
>>>> not make it difficult to write arm/powerpc tests just because someone's on
>>>> x86 :)
>>>
>>> In the kernel, environment can override configuration, so you'd do
>>>
>>>   % ARCH=arm make cscope
>>>
>>> and it doesn't matter what arch is being compiled.  I think this would
>>> be a better solution for kvm-unit-tests too.
>>>
>>> I agree with Drew that other arches would mostly clutter searches if
>>> they were included by default, but we can also add a rule in case
>> 
>> Since you mentioned the kernel, it looks like both the kernel/qemu
>> include all by default which makes sense to me.
>
> qemu does, kernel doesn't (e.g. search for sys_sigreturn).

Not exactly, arch specific(arch/*) files are excluded but there are
others such as sound/* include/* that are not all x86 specific. The
arch segregation is helpful since there would be functions with similar
names (and similar functionality). That's not the case for every file
in x86/ and arm/ of kvm-unit-tests.

>
>>                                                 As I mentioned above,
>> you definitely don't want to modify a common library function only to
>> realize that you forgot to modify all sites. Looking at all sites adds
>> visibility, not introduce clutter.
>> 
>>> indexing all arches is desired (like when changing the prototype of some
>>> generic function?).
>> 
>> kvm-unit-tests is small enough that this can be the default.
>
> I don't see a reason to add results that we'd then have to mentally
> filter regardless of their amount.
>
>>                                                              That said, this
>> is a trivial change and I am happy to live with it in my local branch. (Unless
>> ofcourse both Linux and Qemu decide to make this default behavior too!)
>
> I think we'll agree on something that can be merged -- the present
> situation is bad.

It's not that bad, actually. We are simply arguing over personal
preferences here. Changing my workflow a bit is not hard. :)

> For me, "include all" is a matter of `cscope -bk **/*.[chS]` and getting
> only arch-specific cscope is harder, so having an option to generate
> cscope just for target arch would be nice.

Yes, the option is definitely nice but making it the default, not so much.

> (I don't care about the default much, `ARCH=$sth make cscope` is easy
>  to type if the default included all.)
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html