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

[Bandan Das <bsd@redhat.com>]

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. 

> jumping around in arm code. It's true that $(ARCH) expands to
> x86_64, but $(TEST_DIR) expands to x86, so the current find
> does find all x86 source.

Yep, you are right. It still prints a warning message about non-existent
dirs though which needs to be fixed.

>> 
>> While we are there, remove the unnecessary sed substitution
>> and modify find to include a few other file name extensions.
>
> I can't remember why the sed was necessary, so it could maybe go
> away. Does it hurt anything though?

No, it doesn't hurt anything. Looks like it was meant to replace
"./" with "". So, I really don't know what it was meant to do
because that substitution doesn't seem right either.

> Looks like you've added .cc, .sh, and .bash. I don't want to mix
> scripts with C source. As for api/*.{cc|hh}, we could change the
> API configure variable to a path ('./api') instead of a boolean,
> and then use it in the current cscope line, along with adding
> both .cc and .hh to the name extension list. (On a side note, we
> should probably create ./api its own makefile and look into
> getting it to work with other architectures at some point).

Ok, this I agree with. We can separate the scripts from the c sources
and probably have a variable that sets adding them. But please let's
not make it difficult to write arm/powerpc tests just because someone's on
x86 :)

> Thanks,
> drew
>
>> 
>> Signed-off-by: Bandan Das <bsd@redhat.com>
>> ---
>>  Makefile  | 9 +++------
>>  configure | 2 ++
>>  2 files changed, 5 insertions(+), 6 deletions(-)
>> 
>> diff --git a/Makefile b/Makefile
>> index 5d7506e..458d0f0 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -92,11 +92,8 @@ distclean: clean libfdt_clean
>>  	$(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.*
>>  	$(RM) -r tests
>>  
>> -cscope: cscope_dirs = lib lib/libfdt lib/linux
>> -cscope: cscope_dirs += lib/$(ARCH)/asm lib/$(TEST_DIR)/asm lib/asm-generic
>> -cscope: cscope_dirs += $(TEST_DIR) lib/$(TEST_DIR) lib/$(ARCH)
>>  cscope:
>> -	$(RM) ./cscope.*
>> -	find -L $(cscope_dirs) -maxdepth 1 \
>> -		-name '*.[chsS]' -print | sed 's,^\./,,' | sort -u > ./cscope.files
>> +	$(RM) $(SRCDIR)/cscope.*
>> +	find -L $(SRCDIR) -maxdepth 3 \
>> +		-regex '.*\.\(c\|h\|cc\|S\|sh\|bash\|s\)$$' -print | sort -u > ./cscope.files
>>  	cscope -bk
>> diff --git a/configure b/configure
>> index ba6c55b..3153fb9 100755
>> --- a/configure
>> +++ b/configure
>> @@ -12,6 +12,7 @@ host=$arch
>>  cross_prefix=
>>  endian=""
>>  pretty_print_stacks=yes
>> +srcdir=`pwd`
>>  
>>  usage() {
>>      cat <<-EOF
>> @@ -159,4 +160,5 @@ TEST_DIR=$testdir
>>  FIRMWARE=$firmware
>>  ENDIAN=$endian
>>  PRETTY_PRINT_STACKS=$pretty_print_stacks
>> +SRCDIR=$srcdir
>>  EOF
>> -- 
>> 2.5.5
>> 
>> --
>> 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
> --
> 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