# HG changeset patch # User Roger Pau Monne <roger.pau@entel.upc.edu> # Date 1324385575 -3600 # Node ID 716d6d48e647d1d4352f7206e74e693152a4f8fa # Parent f72b99fccfca694674259cc1c03c526a827b67ec libxl: add support for yajl 2.x This patch adds support for yajl versions 2.x, while retaining 1.x compatibility. This patch adds quite a lot of #ifdefs all over the json code, so I''m open to suggestions if there''s a better way to handle this. Tested with yajl 2.0.3 and 1.0.12. Changes since v1: * Check if yajl_version.h is present before trying to include it. Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu> diff -r f72b99fccfca -r 716d6d48e647 Config.mk --- a/Config.mk Tue Dec 20 08:31:40 2011 +0100 +++ b/Config.mk Tue Dec 20 13:52:55 2011 +0100 @@ -186,6 +186,11 @@ CONFIG_LIBICONV := $(shell export OS=" . $(XEN_ROOT)/tools/check/funcs.sh; \ has_lib libiconv.so && echo ''y'' || echo ''n'') +CONFIG_YAJL_VERSION := $(shell export OS="`uname -s`"; \ + export CHECK_INCLUDES="$(CHECK_INCLUDES)"; \ + . $(XEN_ROOT)/tools/check/funcs.sh; \ + has_header yajl/yajl_version.h && echo ''y'' || echo ''n'') + # Enable XSM security module (by default, Flask). XSM_ENABLE ?= n FLASK_ENABLE ?= $(XSM_ENABLE) diff -r f72b99fccfca -r 716d6d48e647 tools/check/check_yajl_lib --- a/tools/check/check_yajl_lib Tue Dec 20 08:31:40 2011 +0100 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,6 +0,0 @@ -#!/bin/sh -# CHECK-BUILD CHECK-INSTALL - -. ./funcs.sh - -has_lib libyajl.so.1 || fail "can''t find libyajl.so.1 version 1" diff -r f72b99fccfca -r 716d6d48e647 tools/libxl/Makefile --- a/tools/libxl/Makefile Tue Dec 20 08:31:40 2011 +0100 +++ b/tools/libxl/Makefile Tue Dec 20 13:52:55 2011 +0100 @@ -19,6 +19,10 @@ ifeq ($(CONFIG_Linux),y) LIBUUID_LIBS += -luuid endif +ifeq ($(CONFIG_YAJL_VERSION),y) +CFLAGS += -DHAVE_YAJL_VERSION +endif + LIBXL_LIBS LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(UTIL_LIBS) $(LIBUUID_LIBS) diff -r f72b99fccfca -r 716d6d48e647 tools/libxl/libxl_json.c --- a/tools/libxl/libxl_json.c Tue Dec 20 08:31:40 2011 +0100 +++ b/tools/libxl/libxl_json.c Tue Dec 20 13:52:55 2011 +0100 @@ -519,7 +519,11 @@ static bool is_decimal(const char *s, un return false; } +#ifdef HAVE_YAJL_V2 +static int json_callback_number(void *opaque, const char *s, size_t len) +#else static int json_callback_number(void *opaque, const char *s, unsigned int len) +#endif { libxl__yajl_ctx *ctx = opaque; libxl__json_object *obj = NULL; @@ -575,8 +579,13 @@ out: return 1; } +#ifdef HAVE_YAJL_V2 +static int json_callback_string(void *opaque, const unsigned char *str, + size_t len) +#else static int json_callback_string(void *opaque, const unsigned char *str, unsigned int len) +#endif { libxl__yajl_ctx *ctx = opaque; char *t = NULL; @@ -608,8 +617,13 @@ static int json_callback_string(void *op return 1; } +#ifdef HAVE_YAJL_V2 +static int json_callback_map_key(void *opaque, const unsigned char *str, + size_t len) +#else static int json_callback_map_key(void *opaque, const unsigned char *str, unsigned int len) +#endif { libxl__yajl_ctx *ctx = opaque; char *t = NULL; @@ -772,17 +786,26 @@ libxl__json_object *libxl__json_parse(li DEBUG_GEN_ALLOC(&yajl_ctx); if (yajl_ctx.hand == NULL) { +#ifdef HAVE_YAJL_V2 + yajl_ctx.hand = yajl_alloc(&callbacks, NULL, &yajl_ctx); +#else yajl_parser_config cfg = { .allowComments = 1, .checkUTF8 = 1, }; yajl_ctx.hand = yajl_alloc(&callbacks, &cfg, NULL, &yajl_ctx); +#endif } status = yajl_parse(yajl_ctx.hand, (const unsigned char *)s, strlen(s)); if (status != yajl_status_ok) goto out; + +#ifdef HAVE_YAJL_V2 + status = yajl_complete_parse(yajl_ctx.hand); +#else + status = yajl_parse_complete(yajl_ctx.hand); +#endif - status = yajl_parse_complete(yajl_ctx.hand); if (status != yajl_status_ok) goto out; @@ -834,14 +857,25 @@ static const char *yajl_gen_status_to_st char *libxl__object_to_json(libxl_ctx *ctx, const char *type, libxl__gen_json_callback gen, void *p) { +#ifndef HAVE_YAJL_V2 yajl_gen_config conf = { 1, " " }; +#endif const unsigned char *buf; char *ret = NULL; +#ifdef HAVE_YAJL_V2 + size_t len = 0; +#else unsigned int len = 0; +#endif yajl_gen_status s; yajl_gen hand; +#ifdef HAVE_YAJL_V2 + hand = yajl_gen_alloc(NULL); +#else hand = yajl_gen_alloc(&conf, NULL); +#endif + if (!hand) return NULL; diff -r f72b99fccfca -r 716d6d48e647 tools/libxl/libxl_json.h --- a/tools/libxl/libxl_json.h Tue Dec 20 08:31:40 2011 +0100 +++ b/tools/libxl/libxl_json.h Tue Dec 20 13:52:55 2011 +0100 @@ -17,7 +17,16 @@ #include <yajl/yajl_gen.h> +#ifdef HAVE_YAJL_VERSION +# include <yajl/yajl_version.h> +#endif + #include <_libxl_types_json.h> #include <_libxl_types_internal_json.h> +/* YAJL version check */ +#if defined(YAJL_MAJOR) && (YAJL_MAJOR > 1) +# define HAVE_YAJL_V2 1 +#endif + #endif /* LIBXL_JSON_H */ diff -r f72b99fccfca -r 716d6d48e647 tools/libxl/libxl_qmp.c --- a/tools/libxl/libxl_qmp.c Tue Dec 20 08:31:40 2011 +0100 +++ b/tools/libxl/libxl_qmp.c Tue Dec 20 13:52:55 2011 +0100 @@ -453,15 +453,26 @@ static char *qmp_send_prepare(libxl__gc qmp_callback_t callback, void *opaque, qmp_request_context *context) { +#ifndef HAVE_YAJL_V2 yajl_gen_config conf = { 0, NULL }; +#endif const unsigned char *buf = NULL; char *ret = NULL; +#ifdef HAVE_YAJL_V2 + size_t len = 0; +#else unsigned int len = 0; +#endif yajl_gen_status s; yajl_gen hand; callback_id_pair *elm = NULL; +#ifdef HAVE_YAJL_V2 + hand = yajl_gen_alloc(NULL); +#else hand = yajl_gen_alloc(&conf, NULL); +#endif + if (!hand) { return NULL; }
On Tue, 2011-12-20 at 12:53 +0000, Roger Pau Monne wrote:> # HG changeset patch > # User Roger Pau Monne <roger.pau@entel.upc.edu> > # Date 1324385575 -3600 > # Node ID 716d6d48e647d1d4352f7206e74e693152a4f8fa > # Parent f72b99fccfca694674259cc1c03c526a827b67ec > libxl: add support for yajl 2.x > > This patch adds support for yajl versions 2.x, while retaining 1.x > compatibility. This patch adds quite a lot of #ifdefs all over the > json code, so I''m open to suggestions if there''s a better way to > handle this. > > Tested with yajl 2.0.3 and 1.0.12. > > Changes since v1: > > * Check if yajl_version.h is present before trying to include it. > > Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu> > > diff -r f72b99fccfca -r 716d6d48e647 Config.mk > --- a/Config.mk Tue Dec 20 08:31:40 2011 +0100 > +++ b/Config.mk Tue Dec 20 13:52:55 2011 +0100 > @@ -186,6 +186,11 @@ CONFIG_LIBICONV := $(shell export OS=" > . $(XEN_ROOT)/tools/check/funcs.sh; \ > has_lib libiconv.so && echo ''y'' || echo ''n'') > > +CONFIG_YAJL_VERSION := $(shell export OS="`uname -s`"; \ > + export CHECK_INCLUDES="$(CHECK_INCLUDES)"; \ > + . $(XEN_ROOT)/tools/check/funcs.sh; \ > + has_header yajl/yajl_version.h && echo ''y'' || echo ''n'')We''ve ended up with a few of this sort of thing recently. I wonder if it is time to have a configuration/check phase which generates a config.h? Ian.> # Enable XSM security module (by default, Flask). > XSM_ENABLE ?= n > FLASK_ENABLE ?= $(XSM_ENABLE) > diff -r f72b99fccfca -r 716d6d48e647 tools/check/check_yajl_lib > --- a/tools/check/check_yajl_lib Tue Dec 20 08:31:40 2011 +0100 > +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 > @@ -1,6 +0,0 @@ > -#!/bin/sh > -# CHECK-BUILD CHECK-INSTALL > - > -. ./funcs.sh > - > -has_lib libyajl.so.1 || fail "can''t find libyajl.so.1 version 1" > diff -r f72b99fccfca -r 716d6d48e647 tools/libxl/Makefile > --- a/tools/libxl/Makefile Tue Dec 20 08:31:40 2011 +0100 > +++ b/tools/libxl/Makefile Tue Dec 20 13:52:55 2011 +0100 > @@ -19,6 +19,10 @@ ifeq ($(CONFIG_Linux),y) > LIBUUID_LIBS += -luuid > endif > > +ifeq ($(CONFIG_YAJL_VERSION),y) > +CFLAGS += -DHAVE_YAJL_VERSION > +endif > + > LIBXL_LIBS > LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(UTIL_LIBS) $(LIBUUID_LIBS) > > diff -r f72b99fccfca -r 716d6d48e647 tools/libxl/libxl_json.c > --- a/tools/libxl/libxl_json.c Tue Dec 20 08:31:40 2011 +0100 > +++ b/tools/libxl/libxl_json.c Tue Dec 20 13:52:55 2011 +0100 > @@ -519,7 +519,11 @@ static bool is_decimal(const char *s, un > return false; > } > > +#ifdef HAVE_YAJL_V2 > +static int json_callback_number(void *opaque, const char *s, size_t len) > +#else > static int json_callback_number(void *opaque, const char *s, unsigned int len) > +#endif > { > libxl__yajl_ctx *ctx = opaque; > libxl__json_object *obj = NULL; > @@ -575,8 +579,13 @@ out: > return 1; > } > > +#ifdef HAVE_YAJL_V2 > +static int json_callback_string(void *opaque, const unsigned char *str, > + size_t len) > +#else > static int json_callback_string(void *opaque, const unsigned char *str, > unsigned int len) > +#endif > { > libxl__yajl_ctx *ctx = opaque; > char *t = NULL; > @@ -608,8 +617,13 @@ static int json_callback_string(void *op > return 1; > } > > +#ifdef HAVE_YAJL_V2 > +static int json_callback_map_key(void *opaque, const unsigned char *str, > + size_t len) > +#else > static int json_callback_map_key(void *opaque, const unsigned char *str, > unsigned int len) > +#endif > { > libxl__yajl_ctx *ctx = opaque; > char *t = NULL; > @@ -772,17 +786,26 @@ libxl__json_object *libxl__json_parse(li > DEBUG_GEN_ALLOC(&yajl_ctx); > > if (yajl_ctx.hand == NULL) { > +#ifdef HAVE_YAJL_V2 > + yajl_ctx.hand = yajl_alloc(&callbacks, NULL, &yajl_ctx); > +#else > yajl_parser_config cfg = { > .allowComments = 1, > .checkUTF8 = 1, > }; > yajl_ctx.hand = yajl_alloc(&callbacks, &cfg, NULL, &yajl_ctx); > +#endif > } > status = yajl_parse(yajl_ctx.hand, (const unsigned char *)s, strlen(s)); > if (status != yajl_status_ok) > goto out; > + > +#ifdef HAVE_YAJL_V2 > + status = yajl_complete_parse(yajl_ctx.hand); > +#else > + status = yajl_parse_complete(yajl_ctx.hand); > +#endif > > - status = yajl_parse_complete(yajl_ctx.hand); > if (status != yajl_status_ok) > goto out; > > @@ -834,14 +857,25 @@ static const char *yajl_gen_status_to_st > char *libxl__object_to_json(libxl_ctx *ctx, const char *type, > libxl__gen_json_callback gen, void *p) > { > +#ifndef HAVE_YAJL_V2 > yajl_gen_config conf = { 1, " " }; > +#endif > const unsigned char *buf; > char *ret = NULL; > +#ifdef HAVE_YAJL_V2 > + size_t len = 0; > +#else > unsigned int len = 0; > +#endif > yajl_gen_status s; > yajl_gen hand; > > +#ifdef HAVE_YAJL_V2 > + hand = yajl_gen_alloc(NULL); > +#else > hand = yajl_gen_alloc(&conf, NULL); > +#endif > + > if (!hand) > return NULL; > > diff -r f72b99fccfca -r 716d6d48e647 tools/libxl/libxl_json.h > --- a/tools/libxl/libxl_json.h Tue Dec 20 08:31:40 2011 +0100 > +++ b/tools/libxl/libxl_json.h Tue Dec 20 13:52:55 2011 +0100 > @@ -17,7 +17,16 @@ > > #include <yajl/yajl_gen.h> > > +#ifdef HAVE_YAJL_VERSION > +# include <yajl/yajl_version.h> > +#endif > + > #include <_libxl_types_json.h> > #include <_libxl_types_internal_json.h> > > +/* YAJL version check */ > +#if defined(YAJL_MAJOR) && (YAJL_MAJOR > 1) > +# define HAVE_YAJL_V2 1 > +#endif > + > #endif /* LIBXL_JSON_H */ > diff -r f72b99fccfca -r 716d6d48e647 tools/libxl/libxl_qmp.c > --- a/tools/libxl/libxl_qmp.c Tue Dec 20 08:31:40 2011 +0100 > +++ b/tools/libxl/libxl_qmp.c Tue Dec 20 13:52:55 2011 +0100 > @@ -453,15 +453,26 @@ static char *qmp_send_prepare(libxl__gc > qmp_callback_t callback, void *opaque, > qmp_request_context *context) > { > +#ifndef HAVE_YAJL_V2 > yajl_gen_config conf = { 0, NULL }; > +#endif > const unsigned char *buf = NULL; > char *ret = NULL; > +#ifdef HAVE_YAJL_V2 > + size_t len = 0; > +#else > unsigned int len = 0; > +#endif > yajl_gen_status s; > yajl_gen hand; > callback_id_pair *elm = NULL; > > +#ifdef HAVE_YAJL_V2 > + hand = yajl_gen_alloc(NULL); > +#else > hand = yajl_gen_alloc(&conf, NULL); > +#endif > + > if (!hand) { > return NULL; > }
On 22/12/2011 15:28, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:>> diff -r f72b99fccfca -r 716d6d48e647 Config.mk >> --- a/Config.mk Tue Dec 20 08:31:40 2011 +0100 >> +++ b/Config.mk Tue Dec 20 13:52:55 2011 +0100 >> @@ -186,6 +186,11 @@ CONFIG_LIBICONV := $(shell export OS=" >> . $(XEN_ROOT)/tools/check/funcs.sh; \ >> has_lib libiconv.so && echo ''y'' || echo ''n'') >> >> +CONFIG_YAJL_VERSION := $(shell export OS="`uname -s`"; \ >> + export CHECK_INCLUDES="$(CHECK_INCLUDES)"; \ >> + . $(XEN_ROOT)/tools/check/funcs.sh; \ >> + has_header yajl/yajl_version.h && echo ''y'' || echo >> ''n'') > > We''ve ended up with a few of this sort of thing recently. I wonder if it > is time to have a configuration/check phase which generates a config.h?Perhaps there should be autotools machinery at the root of the tree, in place of Config.mk etc? It seems the popular choice. -- Keir
On Thu, 2011-12-22 at 15:48 +0000, Keir Fraser wrote:> On 22/12/2011 15:28, "Ian Campbell" <Ian.Campbell@citrix.com> wrote: > > >> diff -r f72b99fccfca -r 716d6d48e647 Config.mk > >> --- a/Config.mk Tue Dec 20 08:31:40 2011 +0100 > >> +++ b/Config.mk Tue Dec 20 13:52:55 2011 +0100 > >> @@ -186,6 +186,11 @@ CONFIG_LIBICONV := $(shell export OS=" > >> . $(XEN_ROOT)/tools/check/funcs.sh; \ > >> has_lib libiconv.so && echo ''y'' || echo ''n'') > >> > >> +CONFIG_YAJL_VERSION := $(shell export OS="`uname -s`"; \ > >> + export CHECK_INCLUDES="$(CHECK_INCLUDES)"; \ > >> + . $(XEN_ROOT)/tools/check/funcs.sh; \ > >> + has_header yajl/yajl_version.h && echo ''y'' || echo > >> ''n'') > > > > We''ve ended up with a few of this sort of thing recently. I wonder if it > > is time to have a configuration/check phase which generates a config.h? > > Perhaps there should be autotools machinery at the root of the tree, in > place of Config.mk etc? It seems the popular choice."Popular" isn''t quite the right word, "common" maybe. It''s generally pretty loathed. But, yes, something along those lines maybe. Ian.
2011/12/22 Ian Campbell <Ian.Campbell@citrix.com>:>> Perhaps there should be autotools machinery at the root of the tree, in >> place of Config.mk etc? It seems the popular choice. > > "Popular" isn''t quite the right word, "common" maybe. It''s generally > pretty loathed. But, yes, something along those lines maybe.Some autogoo stuff will be good, at least for the tools build. Can you set up the basic structure Ian?
On Thu, 2011-12-22 at 16:41 +0000, Roger Pau Monné wrote:> 2011/12/22 Ian Campbell <Ian.Campbell@citrix.com>: > >> Perhaps there should be autotools machinery at the root of the tree, in > >> place of Config.mk etc? It seems the popular choice. > > > > "Popular" isn''t quite the right word, "common" maybe. It''s generally > > pretty loathed. But, yes, something along those lines maybe. > > Some autogoo stuff will be good, at least for the tools build. Can you > set up the basic structure Ian?I''m not going to have time in the near future. Would it be sufficient to have the stuff in tools/check create a config.h as well as doing the checks? Might need a bit of Makefile magic to be sure that check is always run? Ian.
2011/12/22 Ian Campbell <Ian.Campbell@citrix.com>:> I''m not going to have time in the near future.I will try to take a look at the autogoo stuff and set something up.> Would it be sufficient to have the stuff in tools/check create a > config.h as well as doing the checks? Might need a bit of Makefile magic > to be sure that check is always run?Since we have to change things, I''m not sure if it is best to move to something more standard, like autotools.
On 23/12/2011 08:22, "Roger Pau Monné" <roger.pau@entel.upc.edu> wrote:> 2011/12/22 Ian Campbell <Ian.Campbell@citrix.com>: >> I''m not going to have time in the near future. > > I will try to take a look at the autogoo stuff and set something up. > >> Would it be sufficient to have the stuff in tools/check create a >> config.h as well as doing the checks? Might need a bit of Makefile magic >> to be sure that check is always run? > > Since we have to change things, I''m not sure if it is best to move to > something more standard, like autotools.We should probably just do autotools, if someone has the time to invest in the initial implementation work to use it. -- Keir
On Fri, 2011-12-23 at 08:22 +0000, Roger Pau Monné wrote:> 2011/12/22 Ian Campbell <Ian.Campbell@citrix.com>: > > I'm not going to have time in the near future. > > I will try to take a look at the autogoo stuff and set something up.Thanks! I'm not sure if we have any resident autotools experts but I'm sure we can muddle through. Ian.> > > Would it be sufficient to have the stuff in tools/check create a > > config.h as well as doing the checks? Might need a bit of Makefile magic > > to be sure that check is always run? > > Since we have to change things, I'm not sure if it is best to move to > something more standard, like autotools._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, 2011-12-23 at 08:40 +0000, Keir Fraser wrote:> On 23/12/2011 08:22, "Roger Pau Monné" <roger.pau@entel.upc.edu> wrote: > > > 2011/12/22 Ian Campbell <Ian.Campbell@citrix.com>: > >> I'm not going to have time in the near future. > > > > I will try to take a look at the autogoo stuff and set something up. > > > >> Would it be sufficient to have the stuff in tools/check create a > >> config.h as well as doing the checks? Might need a bit of Makefile magic > >> to be sure that check is always run? > > > > Since we have to change things, I'm not sure if it is best to move to > > something more standard, like autotools. > > We should probably just do autotools, if someone has the time to invest in > the initial implementation work to use it.You are thinking of autoconf only right? We don't want to go the whole hog and start using automake and all the other bobbins (libtool etc), do we? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 23/12/2011 10:50, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:> On Fri, 2011-12-23 at 08:40 +0000, Keir Fraser wrote: >> On 23/12/2011 08:22, "Roger Pau Monné" <roger.pau@entel.upc.edu> wrote: >> >>> 2011/12/22 Ian Campbell <Ian.Campbell@citrix.com>: >>>> I''m not going to have time in the near future. >>> >>> I will try to take a look at the autogoo stuff and set something up. >>> >>>> Would it be sufficient to have the stuff in tools/check create a >>>> config.h as well as doing the checks? Might need a bit of Makefile magic >>>> to be sure that check is always run? >>> >>> Since we have to change things, I''m not sure if it is best to move to >>> something more standard, like autotools. >> >> We should probably just do autotools, if someone has the time to invest in >> the initial implementation work to use it. > > You are thinking of autoconf only right? We don''t want to go the whole > hog and start using automake and all the other bobbins (libtool etc), do > we?You''re probably right.> Ian. >
2011/12/23 Ian Campbell <Ian.Campbell@citrix.com>:> On Fri, 2011-12-23 at 08:40 +0000, Keir Fraser wrote: >> On 23/12/2011 08:22, "Roger Pau Monné" <roger.pau@entel.upc.edu> wrote: >> >> > 2011/12/22 Ian Campbell <Ian.Campbell@citrix.com>: >> >> I'm not going to have time in the near future. >> > >> > I will try to take a look at the autogoo stuff and set something up. >> > >> >> Would it be sufficient to have the stuff in tools/check create a >> >> config.h as well as doing the checks? Might need a bit of Makefile magic >> >> to be sure that check is always run? >> > >> > Since we have to change things, I'm not sure if it is best to move to >> > something more standard, like autotools. >> >> We should probably just do autotools, if someone has the time to invest in >> the initial implementation work to use it. > > You are thinking of autoconf only right? We don't want to go the whole > hog and start using automake and all the other bobbins (libtool etc), do > we?I've never used autotools before, but what I was thinking of is a simple configure script that creates a config.h with all the needed defines that we can include when needed. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"):> On Thu, 2011-12-22 at 16:41 +0000, Roger Pau Monné wrote: > > Some autogoo stuff will be good, at least for the tools build. Can you > > set up the basic structure Ian? > > I''m not going to have time in the near future. > > Would it be sufficient to have the stuff in tools/check create a > config.h as well as doing the checks? Might need a bit of Makefile magic > to be sure that check is always run?For now, I think we should not add more stuff to the critical path for Xen 4.2. But after then we should switch to autoconf I think. (I don''t approve of automake.) Ian.
Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"):> I''m not sure if we have any resident autotools experts but I''m sure we > can muddle through.I have written autoconf macros, a long time ago. Does that count ? :-) Ian.
2012/1/3 Ian Jackson <Ian.Jackson@eu.citrix.com>:> Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"): >> I'm not sure if we have any resident autotools experts but I'm sure we >> can muddle through. > > I have written autoconf macros, a long time ago. Does that count ? :-)I've started to create a basic configure.ac, but really this autoconf thing is pissing me off. My idea was to generate a config.h and replace Config.mk (at least some parts) with the output of configure also, does this sound reasonable? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Roger Pau Monné writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"):> I''ve started to create a basic configure.ac, but really this autoconf > thing is pissing me off. My idea was to generate a config.h and > replace Config.mk (at least some parts) with the output of configure > also, does this sound reasonable?Config.mk should probably include your new autoconf-generated makefile. Ian.
On Tue, 2012-01-03 at 18:00 +0000, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"): > > On Thu, 2011-12-22 at 16:41 +0000, Roger Pau Monné wrote: > > > Some autogoo stuff will be good, at least for the tools build. Can you > > > set up the basic structure Ian? > > > > I'm not going to have time in the near future. > > > > Would it be sufficient to have the stuff in tools/check create a > > config.h as well as doing the checks? Might need a bit of Makefile magic > > to be sure that check is always run? > > For now, I think we should not add more stuff to the critical path for > Xen 4.2. But after then we should switch to autoconf I think. (I > don't approve of automake.)It seems that yajl 2.0 support is blocked on the autoconf stuff. Can we disentangle things such that we can support yajl 2.0 in Xen 4.2 and switch to autoconf afterwards or do we have/want to make the switch to autoconf for 4.2? I appreciate the concerns about critical path for 4.2. I presume autoconf would require more than just checking in the patches, specifically I'm thinking of the automated test system update and docs. On the other hand Roger posted v3 of his autoconf support patch and although I haven't got round to reviewing it yet (sorry) v2's review was generally positive/minor looking. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>:> On Tue, 2012-01-03 at 18:00 +0000, Ian Jackson wrote: >> Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"): >> > On Thu, 2011-12-22 at 16:41 +0000, Roger Pau Monné wrote: >> > > Some autogoo stuff will be good, at least for the tools build. Can you >> > > set up the basic structure Ian? >> > >> > I'm not going to have time in the near future. >> > >> > Would it be sufficient to have the stuff in tools/check create a >> > config.h as well as doing the checks? Might need a bit of Makefile magic >> > to be sure that check is always run? >> >> For now, I think we should not add more stuff to the critical path for >> Xen 4.2. But after then we should switch to autoconf I think. (I >> don't approve of automake.) > > It seems that yajl 2.0 support is blocked on the autoconf stuff. Can we > disentangle things such that we can support yajl 2.0 in Xen 4.2 and > switch to autoconf afterwards or do we have/want to make the switch to > autoconf for 4.2?The proposed autoconf patch is just a convenient replacement for tools/check scripts, which allows us to generate a makefile and a header we can include to know the system capabilities, nothing more was added.> I appreciate the concerns about critical path for 4.2. I presume > autoconf would require more than just checking in the patches, > specifically I'm thinking of the automated test system update and docs.Don't know how difficult it is to update the test bed to use the new configure script, ideally it should only require adding "./configure" before performing a "make tools". Since I don't know how the test bed works, it might be necessary to pass some options to configure execution to obtain the same result.> On the other hand Roger posted v3 of his autoconf support patch and > although I haven't got round to reviewing it yet (sorry) v2's review was > generally positive/minor looking.v3 is basically a v2 with all the mentioned fixes and the output from autoconf & automake, so we can use the configure script straight away. Anyway, this patch for adding support of yajl 2.0 needs some rework, according to the comments made by Ian Jackson, which I will try to do before the end of the week (sorry for the delay, but I'm quite busy this days). _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Mon, 2012-01-23 at 11:30 +0000, Roger Pau Monné wrote:> 2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>: > > On Tue, 2012-01-03 at 18:00 +0000, Ian Jackson wrote: > >> Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"): > >> > On Thu, 2011-12-22 at 16:41 +0000, Roger Pau Monné wrote: > >> > > Some autogoo stuff will be good, at least for the tools build. Can you > >> > > set up the basic structure Ian? > >> > > >> > I'm not going to have time in the near future. > >> > > >> > Would it be sufficient to have the stuff in tools/check create a > >> > config.h as well as doing the checks? Might need a bit of Makefile magic > >> > to be sure that check is always run? > >> > >> For now, I think we should not add more stuff to the critical path for > >> Xen 4.2. But after then we should switch to autoconf I think. (I > >> don't approve of automake.) > > > > It seems that yajl 2.0 support is blocked on the autoconf stuff. Can we > > disentangle things such that we can support yajl 2.0 in Xen 4.2 and > > switch to autoconf afterwards or do we have/want to make the switch to > > autoconf for 4.2? > > The proposed autoconf patch is just a convenient replacement for > tools/check scripts, which allows us to generate a makefile and a > header we can include to know the system capabilities, nothing more > was added. > > > I appreciate the concerns about critical path for 4.2. I presume > > autoconf would require more than just checking in the patches, > > specifically I'm thinking of the automated test system update and docs. > > Don't know how difficult it is to update the test bed to use the new > configure script, ideally it should only require adding "./configure" > before performing a "make tools". Since I don't know how the test bed > works, it might be necessary to pass some options to configure > execution to obtain the same result.Ian J would have to answer that one. Hopefully just running ./configure will get us as close as possible to the existing setup. FWIW the tester code is available at a git url which is in every posted set of results. Probably looking for anywhere that writes to .config would be a good start for deciding how far from the defaults it deviates (not far, I expect).> > On the other hand Roger posted v3 of his autoconf support patch and > > although I haven't got round to reviewing it yet (sorry) v2's review was > > generally positive/minor looking. > > v3 is basically a v2 with all the mentioned fixes and the output from > autoconf & automake, so we can use the configure script straight away.automake? I thought we agreed not to use that?> Anyway, this patch for adding support of yajl 2.0 needs some rework, > according to the comments made by Ian Jackson, which I will try to do > before the end of the week (sorry for the delay, but I'm quite busy > this days).Sure, no problem. I have put yajl2 support in the "nice to have column" and autoconf in a new "need to decide if this is 4.2 or 4.3 material" column. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>:> On Mon, 2012-01-23 at 11:30 +0000, Roger Pau Monné wrote: >> 2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>: >> > On Tue, 2012-01-03 at 18:00 +0000, Ian Jackson wrote: >> >> Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"): >> >> > On Thu, 2011-12-22 at 16:41 +0000, Roger Pau Monné wrote: >> >> > > Some autogoo stuff will be good, at least for the tools build. Can you >> >> > > set up the basic structure Ian? >> >> > >> >> > I'm not going to have time in the near future. >> >> > >> >> > Would it be sufficient to have the stuff in tools/check create a >> >> > config.h as well as doing the checks? Might need a bit of Makefile magic >> >> > to be sure that check is always run? >> >> >> >> For now, I think we should not add more stuff to the critical path for >> >> Xen 4.2. But after then we should switch to autoconf I think. (I >> >> don't approve of automake.) >> > >> > It seems that yajl 2.0 support is blocked on the autoconf stuff. Can we >> > disentangle things such that we can support yajl 2.0 in Xen 4.2 and >> > switch to autoconf afterwards or do we have/want to make the switch to >> > autoconf for 4.2? >> >> The proposed autoconf patch is just a convenient replacement for >> tools/check scripts, which allows us to generate a makefile and a >> header we can include to know the system capabilities, nothing more >> was added. >> >> > I appreciate the concerns about critical path for 4.2. I presume >> > autoconf would require more than just checking in the patches, >> > specifically I'm thinking of the automated test system update and docs. >> >> Don't know how difficult it is to update the test bed to use the new >> configure script, ideally it should only require adding "./configure" >> before performing a "make tools". Since I don't know how the test bed >> works, it might be necessary to pass some options to configure >> execution to obtain the same result. > > Ian J would have to answer that one. Hopefully just running ./configure > will get us as close as possible to the existing setup. > > FWIW the tester code is available at a git url which is in every posted > set of results. Probably looking for anywhere that writes to .config > would be a good start for deciding how far from the defaults it deviates > (not far, I expect). > >> > On the other hand Roger posted v3 of his autoconf support patch and >> > although I haven't got round to reviewing it yet (sorry) v2's review was >> > generally positive/minor looking. >> >> v3 is basically a v2 with all the mentioned fixes and the output from >> autoconf & automake, so we can use the configure script straight away. > > automake? I thought we agreed not to use that?Autotools in general is quite a mess, we don't use automake, but we need it to generate config.sub and config.guess (yes, I know I can also copy them). That's all we need automake for (don't know why autoconf can't do this automatically...)>> Anyway, this patch for adding support of yajl 2.0 needs some rework, >> according to the comments made by Ian Jackson, which I will try to do >> before the end of the week (sorry for the delay, but I'm quite busy >> this days). > > Sure, no problem. I have put yajl2 support in the "nice to have column" > and autoconf in a new "need to decide if this is 4.2 or 4.3 material" > column.Thanks, I'm sure yajl 2.0 support will get into 4.2 :) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Mon, 2012-01-23 at 11:59 +0000, Roger Pau Monné wrote:> 2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>: > > On Mon, 2012-01-23 at 11:30 +0000, Roger Pau Monné wrote: > >> 2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>: > >> > On Tue, 2012-01-03 at 18:00 +0000, Ian Jackson wrote: > >> >> Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"): > >> >> > On Thu, 2011-12-22 at 16:41 +0000, Roger Pau Monné wrote: > >> >> > > Some autogoo stuff will be good, at least for the tools build. Can you > >> >> > > set up the basic structure Ian? > >> >> > > >> >> > I'm not going to have time in the near future. > >> >> > > >> >> > Would it be sufficient to have the stuff in tools/check create a > >> >> > config.h as well as doing the checks? Might need a bit of Makefile magic > >> >> > to be sure that check is always run? > >> >> > >> >> For now, I think we should not add more stuff to the critical path for > >> >> Xen 4.2. But after then we should switch to autoconf I think. (I > >> >> don't approve of automake.) > >> > > >> > It seems that yajl 2.0 support is blocked on the autoconf stuff. Can we > >> > disentangle things such that we can support yajl 2.0 in Xen 4.2 and > >> > switch to autoconf afterwards or do we have/want to make the switch to > >> > autoconf for 4.2? > >> > >> The proposed autoconf patch is just a convenient replacement for > >> tools/check scripts, which allows us to generate a makefile and a > >> header we can include to know the system capabilities, nothing more > >> was added. > >> > >> > I appreciate the concerns about critical path for 4.2. I presume > >> > autoconf would require more than just checking in the patches, > >> > specifically I'm thinking of the automated test system update and docs. > >> > >> Don't know how difficult it is to update the test bed to use the new > >> configure script, ideally it should only require adding "./configure" > >> before performing a "make tools". Since I don't know how the test bed > >> works, it might be necessary to pass some options to configure > >> execution to obtain the same result. > > > > Ian J would have to answer that one. Hopefully just running ./configure > > will get us as close as possible to the existing setup. > > > > FWIW the tester code is available at a git url which is in every posted > > set of results. Probably looking for anywhere that writes to .config > > would be a good start for deciding how far from the defaults it deviates > > (not far, I expect). > > > >> > On the other hand Roger posted v3 of his autoconf support patch and > >> > although I haven't got round to reviewing it yet (sorry) v2's review was > >> > generally positive/minor looking. > >> > >> v3 is basically a v2 with all the mentioned fixes and the output from > >> autoconf & automake, so we can use the configure script straight away. > > > > automake? I thought we agreed not to use that? > > Autotools in general is quite a mess, we don't use automake, but we > need it to generate config.sub and config.guess (yes, I know I can > also copy them). That's all we need automake for (don't know why > autoconf can't do this automatically...)As IanJ (I think) said we should just copy and commit them.> >> Anyway, this patch for adding support of yajl 2.0 needs some rework, > >> according to the comments made by Ian Jackson, which I will try to do > >> before the end of the week (sorry for the delay, but I'm quite busy > >> this days). > > > > Sure, no problem. I have put yajl2 support in the "nice to have column" > > and autoconf in a new "need to decide if this is 4.2 or 4.3 material" > > column. > > Thanks, I'm sure yajl 2.0 support will get into 4.2 :)_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>:> On Mon, 2012-01-23 at 11:59 +0000, Roger Pau Monné wrote: >> 2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>: >> > On Mon, 2012-01-23 at 11:30 +0000, Roger Pau Monné wrote: >> >> 2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>: >> >> > On Tue, 2012-01-03 at 18:00 +0000, Ian Jackson wrote: >> >> >> Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"): >> >> >> > On Thu, 2011-12-22 at 16:41 +0000, Roger Pau Monné wrote: >> >> >> > > Some autogoo stuff will be good, at least for the tools build. Can you >> >> >> > > set up the basic structure Ian? >> >> >> > >> >> >> > I'm not going to have time in the near future. >> >> >> > >> >> >> > Would it be sufficient to have the stuff in tools/check create a >> >> >> > config.h as well as doing the checks? Might need a bit of Makefile magic >> >> >> > to be sure that check is always run? >> >> >> >> >> >> For now, I think we should not add more stuff to the critical path for >> >> >> Xen 4.2. But after then we should switch to autoconf I think. (I >> >> >> don't approve of automake.) >> >> > >> >> > It seems that yajl 2.0 support is blocked on the autoconf stuff. Can we >> >> > disentangle things such that we can support yajl 2.0 in Xen 4.2 and >> >> > switch to autoconf afterwards or do we have/want to make the switch to >> >> > autoconf for 4.2? >> >> >> >> The proposed autoconf patch is just a convenient replacement for >> >> tools/check scripts, which allows us to generate a makefile and a >> >> header we can include to know the system capabilities, nothing more >> >> was added. >> >> >> >> > I appreciate the concerns about critical path for 4.2. I presume >> >> > autoconf would require more than just checking in the patches, >> >> > specifically I'm thinking of the automated test system update and docs. >> >> >> >> Don't know how difficult it is to update the test bed to use the new >> >> configure script, ideally it should only require adding "./configure" >> >> before performing a "make tools". Since I don't know how the test bed >> >> works, it might be necessary to pass some options to configure >> >> execution to obtain the same result. >> > >> > Ian J would have to answer that one. Hopefully just running ./configure >> > will get us as close as possible to the existing setup. >> > >> > FWIW the tester code is available at a git url which is in every posted >> > set of results. Probably looking for anywhere that writes to .config >> > would be a good start for deciding how far from the defaults it deviates >> > (not far, I expect). >> > >> >> > On the other hand Roger posted v3 of his autoconf support patch and >> >> > although I haven't got round to reviewing it yet (sorry) v2's review was >> >> > generally positive/minor looking. >> >> >> >> v3 is basically a v2 with all the mentioned fixes and the output from >> >> autoconf & automake, so we can use the configure script straight away. >> > >> > automake? I thought we agreed not to use that? >> >> Autotools in general is quite a mess, we don't use automake, but we >> need it to generate config.sub and config.guess (yes, I know I can >> also copy them). That's all we need automake for (don't know why >> autoconf can't do this automatically...) > > As IanJ (I think) said we should just copy and commit them.That's what automake does, it just copies them into the current folder, anyway forget about the automake stuff, I just used it to copy config.sub and config.guess because I didn't know exactly where they where (I guess /usr/share...) and I didn't want to waste time searching for them.>> >> Anyway, this patch for adding support of yajl 2.0 needs some rework, >> >> according to the comments made by Ian Jackson, which I will try to do >> >> before the end of the week (sorry for the delay, but I'm quite busy >> >> this days). >> > >> > Sure, no problem. I have put yajl2 support in the "nice to have column" >> > and autoconf in a new "need to decide if this is 4.2 or 4.3 material" >> > column. >> >> Thanks, I'm sure yajl 2.0 support will get into 4.2 :)_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Roger Pau Monné writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"):> Don''t know how difficult it is to update the test bed to use the new > configure script, ideally it should only require adding "./configure" > before performing a "make tools". Since I don''t know how the test bed > works, it might be necessary to pass some options to configure > execution to obtain the same result.Currently the tester drops a .config in the root directory containing various tags and urls of the subtrees that the xen tree clones. Is that stuff affected by your autoconf series ? I can easily enough have it test whether ./configure exists and make it run it if it does. Ian.
Roger Pau Monné writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"):> Autotools in general is quite a mess, we don''t use automake, but we > need it to generate config.sub and config.guess (yes, I know I can > also copy them). That''s all we need automake for (don''t know why > autoconf can''t do this automatically...)Please just copy them, and don''t run automake. That''s what was done before automake existed and it''s what we should do. Ian.
2012/1/24 Ian Jackson <Ian.Jackson@eu.citrix.com>:> Roger Pau Monné writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"): >> Don't know how difficult it is to update the test bed to use the new >> configure script, ideally it should only require adding "./configure" >> before performing a "make tools". Since I don't know how the test bed >> works, it might be necessary to pass some options to configure >> execution to obtain the same result. > > Currently the tester drops a .config in the root directory containing > various tags and urls of the subtrees that the xen tree clones. Is > that stuff affected by your autoconf series ?Don't think so, the only thing that might change is the prefix path, if you where using PREFIX, you should pass --prefix=/... to configure script instead of setting it on .config or env variable. If you want, you can take a look at the makefile variables defined from configure, the template is in config/Tools.mk.in. If you where setting any of the variables defined in tools/Tools.mk.in, check for the appropriate way of setting it from configure (either by passing --enable-foo or by setting the correct environment variable when executing configure script).> I can easily enough have it test whether ./configure exists and make > it run it if it does.That sounds ok, could you test it before pushing the change? I don't want to break your test bed. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>:> On Mon, 2012-01-23 at 11:30 +0000, Roger Pau Monné wrote: >> 2012/1/23 Ian Campbell <Ian.Campbell@citrix.com>: >> > On Tue, 2012-01-03 at 18:00 +0000, Ian Jackson wrote: >> >> Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"): >> >> > On Thu, 2011-12-22 at 16:41 +0000, Roger Pau Monné wrote: >> >> > > Some autogoo stuff will be good, at least for the tools build. Can you >> >> > > set up the basic structure Ian? >> >> > >> >> > I'm not going to have time in the near future. >> >> > >> >> > Would it be sufficient to have the stuff in tools/check create a >> >> > config.h as well as doing the checks? Might need a bit of Makefile magic >> >> > to be sure that check is always run? >> >> >> >> For now, I think we should not add more stuff to the critical path for >> >> Xen 4.2. But after then we should switch to autoconf I think. (I >> >> don't approve of automake.) >> > >> > It seems that yajl 2.0 support is blocked on the autoconf stuff. Can we >> > disentangle things such that we can support yajl 2.0 in Xen 4.2 and >> > switch to autoconf afterwards or do we have/want to make the switch to >> > autoconf for 4.2? >> >> The proposed autoconf patch is just a convenient replacement for >> tools/check scripts, which allows us to generate a makefile and a >> header we can include to know the system capabilities, nothing more >> was added. >> >> > I appreciate the concerns about critical path for 4.2. I presume >> > autoconf would require more than just checking in the patches, >> > specifically I'm thinking of the automated test system update and docs. >> >> Don't know how difficult it is to update the test bed to use the new >> configure script, ideally it should only require adding "./configure" >> before performing a "make tools". Since I don't know how the test bed >> works, it might be necessary to pass some options to configure >> execution to obtain the same result. > > Ian J would have to answer that one. Hopefully just running ./configure > will get us as close as possible to the existing setup. > > FWIW the tester code is available at a git url which is in every posted > set of results. Probably looking for anywhere that writes to .config > would be a good start for deciding how far from the defaults it deviates > (not far, I expect). > >> > On the other hand Roger posted v3 of his autoconf support patch and >> > although I haven't got round to reviewing it yet (sorry) v2's review was >> > generally positive/minor looking. >> >> v3 is basically a v2 with all the mentioned fixes and the output from >> autoconf & automake, so we can use the configure script straight away. > > automake? I thought we agreed not to use that? > >> Anyway, this patch for adding support of yajl 2.0 needs some rework, >> according to the comments made by Ian Jackson, which I will try to do >> before the end of the week (sorry for the delay, but I'm quite busy >> this days). > > Sure, no problem. I have put yajl2 support in the "nice to have column" > and autoconf in a new "need to decide if this is 4.2 or 4.3 material" > column.I'm going to re-work on the yajl 2.0 support, to try to fix the remaining issues with this patch. Just to be clear, should I rebase this patch based on the autoconf stuff? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Roger Pau Monné writes ("Re: [Xen-devel] [PATCH v2] libxl: add support for yajl 2.x"):> I''m going to re-work on the yajl 2.0 support, to try to fix the > remaining issues with this patch. Just to be clear, should I rebase > this patch based on the autoconf stuff?I''m happy either way. We''re probably going to end up with both in 4.2 but I think the yajl 2.0 support is important. Ian.