Re: [Xen-devel] [PATCH] tools/check: check for yajl (needed by libxl)

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Tue, 2011-10-04 at 15:48 +0100, Andre Przywara wrote:
> On 09/30/2011 12:39 PM, Ian Campbell wrote:
> > On Fri, 2011-09-30 at 11:35 +0100, Anthony PERARD wrote:
> >> On Fri, Sep 30, 2011 at 08:36, Ian Campbell<ian.campbell@xxxxxxxxxx>  
> >> wrote:
> >>  ...
> >> You probably want to check the yajl headers as well, no ?
> >> #include<yajl/yajl_parse.h>
> >> #include<yajl/yajl_gen.h>
> >
> > tools/check seems a bit inconsistent and I'd expect them both to be in
> > the -dev package but I guess it can't hurt.
> >
> > # HG changeset patch
> > # User Ian Campbell<ian.campbell@xxxxxxxxxx>
> > # Date 1317379175 -3600
> > # Node ID de602616358b7def9351850f518e453c68141c4f
> > # Parent  302b7556edd91a7506f2215bed5302b4b5eaa52a
> > tools/check: check for yajl (needed by libxl)
> >
> > Signed-off-by: Ian Campbell<ian.campbell@xxxxxxxxxx>
> > ---
> > (note to committer, ensure the new files are executable)
> >
> > diff -r 302b7556edd9 -r de602616358b tools/check/check_yajl_devel
> > --- /dev/null       Thu Jan 01 00:00:00 1970 +0000
> > +++ b/tools/check/check_yajl_devel  Fri Sep 30 11:39:35 2011 +0100
> > @@ -0,0 +1,7 @@
> > +#!/bin/sh
> > +# CHECK-BUILD CHECK-INSTALL
> 
> Shouldn't you be able to omit the CHECK_INSTALL on this one?
> The header files are not a runtime requirement, right?

Correct, it was a cut'n'paste-o.

> Also wouldn't it be nicer to link this small library statically? This 
> would omit the runtime requirement at all.
> The test team just spelled yajl as "yet another library to get Xen 
> running"...

I don't think this is necessary, it's just a library you need to install
and for most users the packaging system will transparently cover this.
All the usual reasons for not statically linking stuff apply here.
There's nothing special about that wrt Xen and/or the size of the
library.

> Also be advised that it looks like this is relying on version 1 of yajl. 
> It looks like the new version 2.0.1 is not compatible, I got compile 
> errors with the newer version. If the stable version 1.x is required, 
> this should be stated somewhere or at least checked properly in check_yajl.

Any idea what we could test for here?

> 
> Regards,
> Andre.
> 
> > +
> > +. ./funcs.sh
> > +
> > +has_header yajl/yajl_parse.h || fail "can't find yajl/yajl_parse.h"
> > +has_header yajl/yajl_gen.h || fail "can't find yajl/yajl_gen.h"
> > diff -r 302b7556edd9 -r de602616358b tools/check/check_yajl_lib
> > --- /dev/null       Thu Jan 01 00:00:00 1970 +0000
> > +++ b/tools/check/check_yajl_lib    Fri Sep 30 11:39:35 2011 +0100
> > @@ -0,0 +1,6 @@
> > +#!/bin/sh
> > +# CHECK-BUILD CHECK-INSTALL
> > +
> > +. ./funcs.sh
> > +
> > +has_lib libyajl.so || fail "can't find libyajl.so"
> >
> >
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel