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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 20, 2016 at 10:31:24AM -0400, Bandan Das wrote:
> Andrew Jones <drjones@xxxxxxxxxx> writes:
> 
> > On Tue, Apr 19, 2016 at 03:44:50PM -0400, Bandan Das wrote:
> >> Radim Krčmář <rkrcmar@xxxxxxxxxx> writes:
> >> 
> >> > 2016-04-19 14:58-0400, Bandan Das:
> >> >> Radim Krčmář <rkrcmar@xxxxxxxxxx> writes:
> >> >>> 2016-04-19 13:08-0400, Bandan Das:
> >> >>>> Andrew Jones <drjones@xxxxxxxxxx> 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 :)
> >
> > How does it make it more difficult to write arm/powerpc tests on x86?
> > I do it all the time. Of course you need to run ./configure --arch=<ARCH>
> > first, but you need to do that to compile anyway.
> >
> >> >>>
> >> >>> 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.
> >
> > This is what kvm-unit-tests does, except it takes ARCH from config.mak,
> > instead of making you specify it, and there is no 'everything' option,
> > default or otherwise right now.
> >
> >> >>>
> >> >>> 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.
> >
> > kvm-unit-tests' cscope find collects all common files and all arch
> > specific files, just as you've described about the kernel. Please give
> > an example of where you couldn't find something you needed with this
> > find, so that I know what the problem is. Also, if there is a problem,
> 
> Sorry, you are right. Looks like I was looking at stale files (iotable.c/h)
> which don't exist anymore. So, I think the current setup works for me
> too since I mostly work on x86. For other times, when I do have to certain
> things in other arches, I will just run cscope by hand.

Why run it by hand? Configure the arch you want to focus on, and then do
'make cscope'. Or, if you really need to view all, then submit a patch
adding a new makefile target, and then run that.

> 
> > we can likely extend the find just a bit, rather than changing it to a
> > wholesale search.
> >
> >> 
> ...
> >> 
> >> Yes, the option is definitely nice but making it the default, not so much.
> >
> > As should be clear by now, IMHO, the default is not only nice, but correct.
> > Feel free to add a cscope-all target though.
> 
> Yeah, I can live with this too. IMHO, the default is a matter of preference, there's
> no question of correctness.

Selection of defaults is hard. Somebody always feels it wrong. However
if you look at what works best for the largest audience, then I believe
that selection is the _correct_ default. I could be wrong about this
one, but I don't think so ;-)

> 
> >> 
> >> > (I don't care about the default much, `ARCH=$sth make cscope` is easy
> >> >  to type if the default included all.)
> >
> > I can live with that too, but I do care about defaults, and defaults
> > should target the common cases. I haven't yet seen enough justification
> > as to why seeing everything (whether you want to or not) is the best
> > common case for kvm-unit-tests' cscope generation.
> 
> Me neither, I am not convinced yet how including all files obstructs your workflow
> but I am certain living with the defaults won't obstruct mine.

It doesn't obstruct it. It adds useless overhead. More mental filtering,
as Radim described it.

drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux