Re: [PATCH 4/4] kernelshark: add ftrace gui viewer

[Darren Hart <dvhart@linux.intel.com>]

On 01/03/2011 11:04 AM, Richard Purdie wrote:
> On Tue, 2010-12-28 at 17:10 -0800, Darren Hart wrote:
>> trace-cmd/kernelshark_git.bb b/meta/recipes-kernel/trace-cmd/kernelshark_git.bb
>> new file mode 100644
>> index 0000000..781f983
>> --- /dev/null
>> +++ b/meta/recipes-kernel/trace-cmd/kernelshark_git.bb
>> @@ -0,0 +1,22 @@
>> +DESCRIPTION = "Graphical trace viewer for Ftrace"
>> +LICENSE = "GPLv2"
>> +LIC_FILES_CHKSUM = "file://COPYING;md5=751419260aa954499f7abaabaa882bbe \
>> +                    file://kernelshark.c;beginline=6;endline=8;md5=2c22c965a649ddd7973d7913c5634a5e"
>> +
>> +PR = r0
>> +PV = "1.0.4+git${SRCPV}"
>> +
>> +DEPENDS = "gtk+"
>> +RDEPENDS = "trace-cmd"
>
> This should be RDEPENDS_${PN} I suspect. We need to be clear about which
> package has the runtime dependency.

Hi Richard, thanks for the review.

I'm missing something about how these variables are used I guess. I had 
thought that being assigned inside the kernelshark_git.bb recipe would 
have been sufficient to make it clear it applied to kernelshark. How 
does this bleed over into other recipes?

>
>> +inherit pkgconfig
>> +
>> +SRC_URI = "git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/trace-cmd.git;protocol=git"
>> +SRCREV = "${SRCREV_pn-trace-cmd}"
>
> I'd suggest declaring that alongside the trace-cmd SRCREV definition,
> something like:
>
> TRACECMDREV ?= "xxx"
> SRCREV_pn-trace-cmd ?= "${TRACECMDREV}"
> SRCREV_pn-kernelshark ?= "${TRACECMDREV}"

If I do this, I think a trace-cmd.inc would be the right way to go. 
Having kernelshark specific bits in the base trace-cmd recipe seems 
backwards to me: kernelshark builds on trace-cmd, so it makes sense to 
me for the extension to made in the kernelshark recipe.

-- 
Darren Hart
Yocto Linux Kernel