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

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Mon, 2011-01-03 at 11:23 -0800, Darren Hart wrote:
> 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?

Other packages (.rpm/ipk), not recipes.

RDEPENDS = "XXX" means apply this RDEPENDS to every package the
kernelshark recipe generates. This as standard would be:

kernelshark
kernelshark-dbg
kernelshark-dev
kernelshark-doc

and it clearly doesn't apply for the last three, only the first (in
general principle, the kernelshark recipe probably doesn't generate a
-doc at all but you get the idea).



> >> +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.

I meant in the common .inc file in the distro directiory where all the
other SRCREVs are set.

Cheers,

Richard