xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] libxl: add support for yajl 2.x
@ 2011-12-20 12:53 Roger Pau Monne
  2011-12-22 15:28 ` Ian Campbell
  0 siblings, 1 reply; 27+ messages in thread
From: Roger Pau Monne @ 2011-12-20 12:53 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian.Campbell

# 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;
     }

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-20 12:53 [PATCH v2] libxl: add support for yajl 2.x Roger Pau Monne
@ 2011-12-22 15:28 ` Ian Campbell
  2011-12-22 15:48   ` Keir Fraser
  0 siblings, 1 reply; 27+ messages in thread
From: Ian Campbell @ 2011-12-22 15:28 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel@lists.xensource.com

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;
>      }

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-22 15:28 ` Ian Campbell
@ 2011-12-22 15:48   ` Keir Fraser
  2011-12-22 15:52     ` Ian Campbell
  0 siblings, 1 reply; 27+ messages in thread
From: Keir Fraser @ 2011-12-22 15:48 UTC (permalink / raw)
  To: Ian Campbell, Roger Pau Monne; +Cc: xen-devel@lists.xensource.com

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-22 15:48   ` Keir Fraser
@ 2011-12-22 15:52     ` Ian Campbell
  2011-12-22 16:41       ` Roger Pau Monné
  0 siblings, 1 reply; 27+ messages in thread
From: Ian Campbell @ 2011-12-22 15:52 UTC (permalink / raw)
  To: Keir Fraser; +Cc: Roger Pau Monne, xen-devel@lists.xensource.com

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.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-22 15:52     ` Ian Campbell
@ 2011-12-22 16:41       ` Roger Pau Monné
  2011-12-22 17:26         ` Ian Campbell
  0 siblings, 1 reply; 27+ messages in thread
From: Roger Pau Monné @ 2011-12-22 16:41 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel@lists.xensource.com, Keir Fraser

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?

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-22 16:41       ` Roger Pau Monné
@ 2011-12-22 17:26         ` Ian Campbell
  2011-12-23  8:22           ` Roger Pau Monné
  2012-01-03 18:00           ` Ian Jackson
  0 siblings, 2 replies; 27+ messages in thread
From: Ian Campbell @ 2011-12-22 17:26 UTC (permalink / raw)
  To: Roger Pau Monné; +Cc: xen-devel@lists.xensource.com, Keir (Xen.org)

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.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-22 17:26         ` Ian Campbell
@ 2011-12-23  8:22           ` Roger Pau Monné
  2011-12-23  8:40             ` Keir Fraser
  2011-12-23 10:45             ` Ian Campbell
  2012-01-03 18:00           ` Ian Jackson
  1 sibling, 2 replies; 27+ messages in thread
From: Roger Pau Monné @ 2011-12-23  8:22 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel@lists.xensource.com, Keir (Xen.org)

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.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-23  8:22           ` Roger Pau Monné
@ 2011-12-23  8:40             ` Keir Fraser
  2011-12-23 10:50               ` Ian Campbell
  2011-12-23 10:45             ` Ian Campbell
  1 sibling, 1 reply; 27+ messages in thread
From: Keir Fraser @ 2011-12-23  8:40 UTC (permalink / raw)
  To: Roger Pau Monné, Ian Campbell
  Cc: xen-devel@lists.xensource.com, Keir (Xen.org)

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-23  8:22           ` Roger Pau Monné
  2011-12-23  8:40             ` Keir Fraser
@ 2011-12-23 10:45             ` Ian Campbell
  2012-01-03 18:01               ` Ian Jackson
  1 sibling, 1 reply; 27+ messages in thread
From: Ian Campbell @ 2011-12-23 10:45 UTC (permalink / raw)
  To: Roger Pau Monné; +Cc: xen-devel@lists.xensource.com, Keir (Xen.org)

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-23  8:40             ` Keir Fraser
@ 2011-12-23 10:50               ` Ian Campbell
  2011-12-23 10:58                 ` Keir Fraser
  2011-12-23 11:18                 ` Roger Pau Monné
  0 siblings, 2 replies; 27+ messages in thread
From: Ian Campbell @ 2011-12-23 10:50 UTC (permalink / raw)
  To: Keir Fraser
  Cc: Roger Pau Monné, xen-devel@lists.xensource.com,
	Keir (Xen.org)

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-23 10:50               ` Ian Campbell
@ 2011-12-23 10:58                 ` Keir Fraser
  2011-12-23 11:18                 ` Roger Pau Monné
  1 sibling, 0 replies; 27+ messages in thread
From: Keir Fraser @ 2011-12-23 10:58 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Roger Pau Monné, xen-devel@lists.xensource.com

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-23 10:50               ` Ian Campbell
  2011-12-23 10:58                 ` Keir Fraser
@ 2011-12-23 11:18                 ` Roger Pau Monné
  1 sibling, 0 replies; 27+ messages in thread
From: Roger Pau Monné @ 2011-12-23 11:18 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Keir Fraser, Keir (Xen.org), xen-devel@lists.xensource.com

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-22 17:26         ` Ian Campbell
  2011-12-23  8:22           ` Roger Pau Monné
@ 2012-01-03 18:00           ` Ian Jackson
  2012-01-23 10:47             ` Ian Campbell
  1 sibling, 1 reply; 27+ messages in thread
From: Ian Jackson @ 2012-01-03 18:00 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel@lists.xensource.com, Keir (Xen.org)

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.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2011-12-23 10:45             ` Ian Campbell
@ 2012-01-03 18:01               ` Ian Jackson
  2012-01-03 18:14                 ` Roger Pau Monné
  0 siblings, 1 reply; 27+ messages in thread
From: Ian Jackson @ 2012-01-03 18:01 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel@lists.xensource.com, Keir (Xen.org)

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.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-03 18:01               ` Ian Jackson
@ 2012-01-03 18:14                 ` Roger Pau Monné
  2012-01-04 11:22                   ` Ian Jackson
  0 siblings, 1 reply; 27+ messages in thread
From: Roger Pau Monné @ 2012-01-03 18:14 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel@lists.xensource.com, Keir (Xen.org), Ian Campbell

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-03 18:14                 ` Roger Pau Monné
@ 2012-01-04 11:22                   ` Ian Jackson
  0 siblings, 0 replies; 27+ messages in thread
From: Ian Jackson @ 2012-01-04 11:22 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: xen-devel@lists.xensource.com, Keir (Xen.org), Ian Campbell

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.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-03 18:00           ` Ian Jackson
@ 2012-01-23 10:47             ` Ian Campbell
  2012-01-23 11:30               ` Roger Pau Monné
  0 siblings, 1 reply; 27+ messages in thread
From: Ian Campbell @ 2012-01-23 10:47 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Roger Pau Monné, xen-devel@lists.xensource.com,
	Keir (Xen.org)

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-23 10:47             ` Ian Campbell
@ 2012-01-23 11:30               ` Roger Pau Monné
  2012-01-23 11:39                 ` Ian Campbell
  2012-01-24 18:23                 ` Ian Jackson
  0 siblings, 2 replies; 27+ messages in thread
From: Roger Pau Monné @ 2012-01-23 11:30 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Keir (Xen.org), xen-devel@lists.xensource.com, Ian Jackson

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-23 11:30               ` Roger Pau Monné
@ 2012-01-23 11:39                 ` Ian Campbell
  2012-01-23 11:59                   ` Roger Pau Monné
  2012-01-25 10:11                   ` Roger Pau Monné
  2012-01-24 18:23                 ` Ian Jackson
  1 sibling, 2 replies; 27+ messages in thread
From: Ian Campbell @ 2012-01-23 11:39 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Keir (Xen.org), xen-devel@lists.xensource.com, Ian Jackson

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-23 11:39                 ` Ian Campbell
@ 2012-01-23 11:59                   ` Roger Pau Monné
  2012-01-23 13:14                     ` Ian Campbell
  2012-01-24 18:24                     ` Ian Jackson
  2012-01-25 10:11                   ` Roger Pau Monné
  1 sibling, 2 replies; 27+ messages in thread
From: Roger Pau Monné @ 2012-01-23 11:59 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Keir (Xen.org), xen-devel@lists.xensource.com, Ian Jackson

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-23 11:59                   ` Roger Pau Monné
@ 2012-01-23 13:14                     ` Ian Campbell
  2012-01-23 13:21                       ` Roger Pau Monné
  2012-01-24 18:24                     ` Ian Jackson
  1 sibling, 1 reply; 27+ messages in thread
From: Ian Campbell @ 2012-01-23 13:14 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Keir (Xen.org), xen-devel@lists.xensource.com, Ian Jackson

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-23 13:14                     ` Ian Campbell
@ 2012-01-23 13:21                       ` Roger Pau Monné
  0 siblings, 0 replies; 27+ messages in thread
From: Roger Pau Monné @ 2012-01-23 13:21 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Keir (Xen.org), xen-devel@lists.xensource.com, Ian Jackson

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-23 11:30               ` Roger Pau Monné
  2012-01-23 11:39                 ` Ian Campbell
@ 2012-01-24 18:23                 ` Ian Jackson
  2012-01-25 10:02                   ` Roger Pau Monné
  1 sibling, 1 reply; 27+ messages in thread
From: Ian Jackson @ 2012-01-24 18:23 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: xen-devel@lists.xensource.com, Keir (Xen.org), Ian Campbell

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.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-23 11:59                   ` Roger Pau Monné
  2012-01-23 13:14                     ` Ian Campbell
@ 2012-01-24 18:24                     ` Ian Jackson
  1 sibling, 0 replies; 27+ messages in thread
From: Ian Jackson @ 2012-01-24 18:24 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: xen-devel@lists.xensource.com, Keir (Xen.org), Ian Campbell

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.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-24 18:23                 ` Ian Jackson
@ 2012-01-25 10:02                   ` Roger Pau Monné
  0 siblings, 0 replies; 27+ messages in thread
From: Roger Pau Monné @ 2012-01-25 10:02 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel@lists.xensource.com, Keir (Xen.org), Ian Campbell

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-23 11:39                 ` Ian Campbell
  2012-01-23 11:59                   ` Roger Pau Monné
@ 2012-01-25 10:11                   ` Roger Pau Monné
  2012-01-25 12:09                     ` Ian Jackson
  1 sibling, 1 reply; 27+ messages in thread
From: Roger Pau Monné @ 2012-01-25 10:11 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Keir (Xen.org), xen-devel@lists.xensource.com, Ian Jackson

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] libxl: add support for yajl 2.x
  2012-01-25 10:11                   ` Roger Pau Monné
@ 2012-01-25 12:09                     ` Ian Jackson
  0 siblings, 0 replies; 27+ messages in thread
From: Ian Jackson @ 2012-01-25 12:09 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: xen-devel@lists.xensource.com, Keir (Xen.org), Ian Campbell

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.

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2012-01-25 12:09 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-20 12:53 [PATCH v2] libxl: add support for yajl 2.x Roger Pau Monne
2011-12-22 15:28 ` Ian Campbell
2011-12-22 15:48   ` Keir Fraser
2011-12-22 15:52     ` Ian Campbell
2011-12-22 16:41       ` Roger Pau Monné
2011-12-22 17:26         ` Ian Campbell
2011-12-23  8:22           ` Roger Pau Monné
2011-12-23  8:40             ` Keir Fraser
2011-12-23 10:50               ` Ian Campbell
2011-12-23 10:58                 ` Keir Fraser
2011-12-23 11:18                 ` Roger Pau Monné
2011-12-23 10:45             ` Ian Campbell
2012-01-03 18:01               ` Ian Jackson
2012-01-03 18:14                 ` Roger Pau Monné
2012-01-04 11:22                   ` Ian Jackson
2012-01-03 18:00           ` Ian Jackson
2012-01-23 10:47             ` Ian Campbell
2012-01-23 11:30               ` Roger Pau Monné
2012-01-23 11:39                 ` Ian Campbell
2012-01-23 11:59                   ` Roger Pau Monné
2012-01-23 13:14                     ` Ian Campbell
2012-01-23 13:21                       ` Roger Pau Monné
2012-01-24 18:24                     ` Ian Jackson
2012-01-25 10:11                   ` Roger Pau Monné
2012-01-25 12:09                     ` Ian Jackson
2012-01-24 18:23                 ` Ian Jackson
2012-01-25 10:02                   ` Roger Pau Monné

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).