You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ni...@apache.org on 2013/09/06 12:06:19 UTC

[1/2] git commit: Introduce build-time option to select libxml2 in place of expat as build-time parser.

Updated Branches:
  refs/heads/master 194d2454d -> 57fcef17e


Introduce build-time option to select libxml2 in place of expat
as build-time parser.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/2184cc8a
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/2184cc8a
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/2184cc8a

Branch: refs/heads/master
Commit: 2184cc8ace81675d0c4167a380e743c8b96dd808
Parents: 194d245
Author: Nick Kew <nk...@qualys.com>
Authored: Fri Sep 6 10:59:34 2013 +0100
Committer: Nick Kew <nk...@qualys.com>
Committed: Fri Sep 6 10:59:34 2013 +0100

----------------------------------------------------------------------
 build/xml.m4                | 78 ++++++++++++++++++++++++++++++++++++++--
 mgmt/stats/StatProcessor.cc | 30 +++++++++++++---
 mgmt/stats/StatProcessor.h  |  8 +++++
 mgmt/utils/XmlUtils.cc      | 61 ++++++++++++++++++++++++++-----
 4 files changed, 161 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2184cc8a/build/xml.m4
----------------------------------------------------------------------
diff --git a/build/xml.m4 b/build/xml.m4
index 03c59ff..d342814 100644
--- a/build/xml.m4
+++ b/build/xml.m4
@@ -23,12 +23,84 @@ dnl TS_CHECK_XML: look for xml libraries and headers
 dnl
 AC_DEFUN([TS_CHECK_XML], [
   enable_xml=no
-
-  TS_CHECK_XML_EXPAT
-  dnl add checks for other varieties of xml here
+  AC_MSG_CHECKING(["For XML parser"])
+  AC_ARG_WITH(xml, [AC_HELP_STRING([--with-xml=(expat|libxml2)],[select XML parser])],
+  [
+    if test "$withval" = "expat" ; then
+      TS_CHECK_XML_EXPAT
+    elif test "$withval" = "libxml2" ; then
+      TS_CHECK_XML_LIBXML2
+    elif test "x$withval" = "x" ; then
+      TS_CHECK_XML_EXPAT
+      if test "$enable_xml" = "no"; then
+        TS_CHECK_XML_LIBXML2
+      fi
+    else
+      AC_MSG_ERROR([Unrecognised --with-xml option])
+    fi
+  ])
+  if test "$enable_xml" = "no"; then
+    AC_MSG_ERROR([An XML parser (expat or libxml2) is required.])
+  fi
 ])
 dnl
 
+AC_DEFUN([TS_CHECK_XML_LIBXML2], [
+  enable_libxml2=no
+  libxml2_include=""
+  libxml2_ldflags=""
+  AC_ARG_WITH(libxml2, [AC_HELP_STRING([--with-libxml2=DIR],[use a specific libxml2 library])],
+  [
+    if test "x$withval" != "xyes" && test "x$withval" != "x"; then
+      if test "$withval" = "yes"; then
+        enable_libxml2=yes
+        libxml2_include="/usr/include/libxml2"
+      elif test "$withval" != "no"; then
+        enable_libxml2=yes
+        libxml2_include="$withval/include/libxml2"
+        libxml2_ldflags="$withval/lib"
+      fi
+    fi
+  ])
+  if test ${enable_libxml2} = "no"; then
+    enable_libxml2=yes
+    libxml2_include="/usr/include/libxml2"
+  fi
+  if test ${enable_libxml2} != "no"; then
+    AC_CACHE_CHECK([libxml2], [ts_cv_libxml2], [
+      ts_libxml2_CPPFLAGS=$CPPFLAGS
+      ts_libxml2_LIBS="$LIBS"
+      ts_libxml2_LDFLAGS="$LDFLAGS"
+      CPPFLAGS="$CPPFLAGS -I$libxml2_include"
+      LDFLAGS="$LDFLAGS $libxml2_ldflags"
+      LIBS="$LIBS -lxml2"
+      AC_TRY_LINK(
+        [#include <libxml/parser.h>],
+        [xmlSAXHandler sax; xmlCreatePushParserCtxt(&sax, NULL, NULL, 0, NULL);],
+        [ts_cv_libxml2=yes],
+        [ts_cv_libxml2=no],
+      )
+      CPPFLAGS=$ts_libxml2_CPPFLAGS
+      LIBS=$ts_libxml2_LIBS
+      LDFLAGS=$ts_libxml2_LDFLAGS
+    ])
+    if test $ts_cv_libxml2 = yes ; then
+      AC_DEFINE([HAVE_LIBXML2], 1, [Using libxml2])
+      if test -d "$libxml2_include" ; then
+        TS_ADDTO(CPPFLAGS, [-I${libxml2_include}])
+      fi
+      if test -d "$libxml2_ldflags" ; then
+        TS_ADDTO(LDFLAGS, [-L${libxml2_ldflags}])
+        TS_ADDTO(LIBTOOL_LINK_FLAGS, [-R${libxml2_ldflags}])
+      fi
+      TS_ADDTO(LIBS, -lxml2)
+      enable_xml=yes
+    else
+      AC_MSG_ERROR(["Failed to find libxml2"])
+    fi
+  fi
+])
+
 AC_DEFUN([TS_CHECK_XML_EXPAT], [
 enable_expat=no
 AC_ARG_WITH(expat, [AC_HELP_STRING([--with-expat=DIR],[use a specific Expat library])],

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2184cc8a/mgmt/stats/StatProcessor.cc
----------------------------------------------------------------------
diff --git a/mgmt/stats/StatProcessor.cc b/mgmt/stats/StatProcessor.cc
index 268dc29..93a3f05 100644
--- a/mgmt/stats/StatProcessor.cc
+++ b/mgmt/stats/StatProcessor.cc
@@ -65,7 +65,8 @@ startElement(void * /* userData ATS_UNUSED */, const char *name, const char **at
     statObject = NEW(new StatObject(++statCount));
     Debug(MODULE_INIT, "\nStat #: ----------------------- %d -----------------------\n", statCount);
 
-    for (i = 0; atts[i]; i += 2) {
+    if (atts)
+     for (i = 0; atts[i]; i += 2) {
       ink_assert(atts[i + 1]);    // Attribute comes in pairs, hopefully.
 
       if (!strcmp(atts[i], "minimum")) {
@@ -93,7 +94,8 @@ startElement(void * /* userData ATS_UNUSED */, const char *name, const char **at
     nodeVar = true;
     sumClusterVar = true;       // Should only be used with cluster variable
 
-    for (i = 0; atts[i]; i += 2) {
+    if (atts)
+     for (i = 0; atts[i]; i += 2) {
       ink_assert(atts[i + 1]);    // Attribute comes in pairs, hopefully.
       if (!strcmp(atts[i], "scope")) {
         nodeVar = (!strcmp(atts[i + 1], "node") ? true : false);
@@ -137,14 +139,14 @@ endElement(void * /* userData ATS_UNUSED */, const char */* name ATS_UNUSED */)
 
 
 void
-charDataHandler(void * /* userData ATS_UNUSED */, const XML_Char * name, int /* len ATS_UNUSED */)
+charDataHandler(void * /* userData ATS_UNUSED */, const xmlchar * name, int /* len ATS_UNUSED */)
 {
   if (currentTag != EXPR_TAG && currentTag != DST_TAG) {
     return;
   }
 
   char content[BUFSIZ * 10];
-  if (XML_extractContent(name, content, BUFSIZ * 10) == 0) {
+  if (XML_extractContent((const char*)name, content, BUFSIZ * 10) == 0) {
     return;
   }
 
@@ -184,6 +186,7 @@ StatProcessor::rereadConfig()
   fileBuffer = fileContent->bufPtr();
   fileLen = strlen(fileBuffer);
 
+#if HAVE_LIBEXPAT
   /*
    * Start the XML Praser -- the package used is EXPAT
    */
@@ -214,6 +217,25 @@ StatProcessor::rereadConfig()
    * Cleaning upt
    */
   XML_ParserFree(parser);
+#else
+  /* Parse XML with libxml2 */
+  xmlSAXHandler sax;
+  memset(&sax, 0, sizeof(xmlSAXHandler));
+  sax.startElement = startElement;
+  sax.endElement = endElement;
+  sax.characters = charDataHandler;
+  sax.initialized = 1;
+  xmlParserCtxtPtr parser = xmlCreatePushParserCtxt(&sax, NULL, NULL, 0, NULL);
+
+  int status = xmlParseChunk(parser, fileBuffer, fileLen, 1);
+  if (status != 0) {
+    xmlErrorPtr errptr = xmlCtxtGetLastError(parser);
+    mgmt_log(stderr, "%s at %s:%d\n", errptr->message, errptr->file, errptr->line);
+  }
+  xmlFreeParserCtxt(parser);
+#endif
+
+
   delete fileContent;
 
   Debug(MODULE_INIT, "\n\n---------- END OF PARSING & INITIALIZING ---------\n\n");

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2184cc8a/mgmt/stats/StatProcessor.h
----------------------------------------------------------------------
diff --git a/mgmt/stats/StatProcessor.h b/mgmt/stats/StatProcessor.h
index 8e6b0c6..e543e6c 100644
--- a/mgmt/stats/StatProcessor.h
+++ b/mgmt/stats/StatProcessor.h
@@ -46,7 +46,15 @@
 #define _FOOTER
 #include "DynamicStats.h"
 #include "StatType.h"
+#if HAVE_LIBEXPAT
 #include "expat.h"
+typedef XML_Char xmlchar;
+#elif HAVE_LIBXML2
+#include <libxml/parser.h>
+typedef xmlChar xmlchar;
+#else
+# error "No XML parser - please configure expat or libxml2"
+#endif
 #include <string.h>
 #include <stdlib.h>
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2184cc8a/mgmt/utils/XmlUtils.cc
----------------------------------------------------------------------
diff --git a/mgmt/utils/XmlUtils.cc b/mgmt/utils/XmlUtils.cc
index 528d40c..317703f 100644
--- a/mgmt/utils/XmlUtils.cc
+++ b/mgmt/utils/XmlUtils.cc
@@ -21,7 +21,16 @@
   limitations under the License.
  */
 
+#include "ink_autoconf.h"
+#if HAVE_LIBEXPAT
 #include "expat.h"
+typedef XML_Char xmlchar;
+#elif HAVE_LIBXML2
+#include <libxml/parser.h>
+typedef xmlChar xmlchar;
+#else
+# error "No XML parser.  Please configure expat or libxml2"
+#endif
 #include <stdio.h>
 #include <stdlib.h>
 #include "ink_platform.h"
@@ -378,7 +387,7 @@ XMLNode::getAttributeValueByName(const char *pAName)
 }
 
 void /*XMLDom:: */
-elementStart(void *pObj, const char *el, const char **attr)
+elementStart(void *pObj, const xmlchar *el, const xmlchar **attr)
 {
   XMLDom *pDom = (XMLDom *) pObj;
   int nTagLen;
@@ -392,12 +401,15 @@ elementStart(void *pObj, const char *el, const char **attr)
     pDom->m_pCur = p;
   }
 
-  nTagLen = strlen(el);
+  nTagLen = strlen((const char*)el);
   pTag = new char[nTagLen + 1];
   pDom->m_pCur->m_pNodeName = pTag;
   memcpy(pTag, el, nTagLen);
   pTag[nTagLen] = 0;
 
+  if (!attr)
+    return;
+
   int i;
   int nCount = 0;
   for (i = 0; attr[i]; i += 2)
@@ -410,13 +422,13 @@ elementStart(void *pObj, const char *el, const char **attr)
   pDom->m_pCur->m_pAList = new Attribute[nCount];
   for (i = 0; i < nCount; i++) {
     /* Attribute Name: attr[2*i], Value: attr[2*i+1]; */
-    alloc_and_copy(&pDom->m_pCur->m_pAList[i].pAName, attr[2 * i]);
-    alloc_and_copy(&pDom->m_pCur->m_pAList[i].pAValue, attr[2 * i + 1]);
+    alloc_and_copy(&pDom->m_pCur->m_pAList[i].pAName, (const char*)attr[2 * i]);
+    alloc_and_copy(&pDom->m_pCur->m_pAList[i].pAValue, (const char*)attr[2 * i + 1]);
   }
 }
 
 void /*XMLDom:: */
-elementEnd(void *pObj, const char * /* el ATS_UNUSED */)
+elementEnd(void *pObj, const xmlchar * /* el ATS_UNUSED */)
 {
   /*ASSERT(strcmp(el, pCur->pNodeName) == 0); */
   XMLDom *pDom = (XMLDom *) pObj;
@@ -424,7 +436,7 @@ elementEnd(void *pObj, const char * /* el ATS_UNUSED */)
 }
 
 void /*XMLDom:: */
-charHandler(void *pObj, const char *s, int len)
+charHandler(void *pObj, const xmlchar *s, int len)
 {
   XMLNode *pNode = ((XMLDom *) pObj)->m_pCur;
   int oldlen;
@@ -450,6 +462,9 @@ charHandler(void *pObj, const char *s, int len)
 int
 XMLDom::LoadXML(const char *pXml)
 {
+  int rv = 0;
+#if HAVE_LIBEXPAT
+  /* Old (expat) xml parsing */
   XML_Parser p = XML_ParserCreate(NULL);
   if (!p)                       /* no Memory. */
     return 1;
@@ -466,11 +481,39 @@ XMLDom::LoadXML(const char *pXml)
                               /*(void (*)(void *, const char *, int)) */ charHandler
     );
 
-  if (!XML_Parse(p, pXml, strlen(pXml), 0)) {
-    /*return 2;     Parse Error: bad xml format. */
+  if (!XML_Parse(p, pXml, strlen(pXml), 1)) {
+    rv = 2; /*return 2;     Parse Error: bad xml format. */
   }
+  XML_ParserFree(p);
+
+#else
+  /* Alternative (libxml2) xml parsing */
+  xmlParserCtxtPtr p;
+  xmlSAXHandler sax;
+  memset(&sax, 0, sizeof(xmlSAXHandler));
+  sax.startElement = elementStart;
+  sax.endElement = elementEnd;
+  sax.characters = charHandler;
+  sax.initialized = 1;
+  p = xmlCreatePushParserCtxt(&sax, this, NULL, 0, NULL);
+  if (!p)                       /* no Memory. */
+    return 1;
 
-  return 0;
+  /* Looks as if this is actually the whole xml and expat version's
+   * use of 0 for is_final is a bug.  We'll use 1.
+   */
+  if (xmlParseChunk(p, pXml, strlen(pXml), 1) != 0) {
+    rv = 2; /* expat version ignores error, so we'll do likewise */
+  }
+  xmlFreeParserCtxt(p);
+#endif
+
+  /* I don't know why the "return 2" in the old (expat) version was
+   * commented out.  Possibly because the call to XML_Parse incorrectly
+   * used 0 for is_final last arg, in which case we should restore it.
+   * If it's a bug in the caller then we'll just have to return 0 always.
+   */
+  return rv;
 }
 
 int


Re: [2/2] git commit: Make libxml2 the configuration default XML parser (so it should get a bit of test-driving).

Posted by Bryan Call <bc...@yahoo-inc.com>.
It looks like some of the m4 files incorrectly implement the AC_ARG_WITH when there is no argument.  Looking at the autoconf manual the 4th argument should be the block to run if there is no action set (e.g. no --with-xml  on the configure).  This was included inside of the 3rd argument previously and this was the part that was failing on the builds, because it was not being executed.

The second problem was using AC_MSG_ERROR instead of AC_MSG_WARN for the xml2 check and it would fail/stop if xml2 was not found.

autoconf section 15.2:
http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/External-Software.html#External-Software

-Bryan

On Sep 7, 2013, at 8:25 PM, James Peach <jp...@apache.org> wrote:

> On Sep 7, 2013, at 5:00 PM, Nick Kew <ni...@apache.org> wrote:
> 
>> 
>> On 7 Sep 2013, at 01:35, Nick Kew wrote:
>> 
>>>> Something in these commits breaks the builds completely. I have both expat and libxml2 installed (with dev packages), and it doesn't detect either. Same problem (I think) on all build bots (they all fail as far as I can tell?).
>> 
>> Somewhat to my surprise, I've failed to find any platform where
>> the build doesn't work out-of-the-box[1].
> 
> Bryan made 2 commits to this yesterday. Maybe that's the complete fix?
> 
> At any rate, the build farm is healthy now :)
> 
>> It also correctly
>> finds and uses expat when I manually move the libxml2
>> headers so configure failed to find them.
>> 
>> Are you working from a clean repo, did you run autoreconf,
>> could there be something lurking from outside /trunk/?
>> If not, what platform, and how is your libxml2 laid out?
>> 
>> I'd be reluctant to commit a lot of extra debug to trunk!
>> 
>> [1] Except for needing to feed it a working PCRE path on Mac.
>> 
>> -- 
>> Nick Kew
> 


Re: [2/2] git commit: Make libxml2 the configuration default XML parser (so it should get a bit of test-driving).

Posted by James Peach <jp...@apache.org>.
On Sep 7, 2013, at 5:00 PM, Nick Kew <ni...@apache.org> wrote:

> 
> On 7 Sep 2013, at 01:35, Nick Kew wrote:
> 
>>> Something in these commits breaks the builds completely. I have both expat and libxml2 installed (with dev packages), and it doesn't detect either. Same problem (I think) on all build bots (they all fail as far as I can tell?).
> 
> Somewhat to my surprise, I've failed to find any platform where
> the build doesn't work out-of-the-box[1].

Bryan made 2 commits to this yesterday. Maybe that's the complete fix?

At any rate, the build farm is healthy now :)

>  It also correctly
> finds and uses expat when I manually move the libxml2
> headers so configure failed to find them.
> 
> Are you working from a clean repo, did you run autoreconf,
> could there be something lurking from outside /trunk/?
> If not, what platform, and how is your libxml2 laid out?
> 
> I'd be reluctant to commit a lot of extra debug to trunk!
> 
> [1] Except for needing to feed it a working PCRE path on Mac.
> 
> -- 
> Nick Kew


Re: [2/2] git commit: Make libxml2 the configuration default XML parser (so it should get a bit of test-driving).

Posted by Nick Kew <ni...@apache.org>.
On 7 Sep 2013, at 01:35, Nick Kew wrote:

>> Something in these commits breaks the builds completely. I have both expat and libxml2 installed (with dev packages), and it doesn't detect either. Same problem (I think) on all build bots (they all fail as far as I can tell?).

Somewhat to my surprise, I've failed to find any platform where
the build doesn't work out-of-the-box[1].  It also correctly
finds and uses expat when I manually move the libxml2
headers so configure failed to find them.

Are you working from a clean repo, did you run autoreconf,
could there be something lurking from outside /trunk/?
If not, what platform, and how is your libxml2 laid out?

I'd be reluctant to commit a lot of extra debug to trunk!

[1] Except for needing to feed it a working PCRE path on Mac.

-- 
Nick Kew

Re: [2/2] git commit: Make libxml2 the configuration default XML parser (so it should get a bit of test-driving).

Posted by Nick Kew <ni...@apache.org>.
On Fri, 6 Sep 2013 14:51:56 -0600
Leif Hedstrom <zw...@apache.org> wrote:

> On Sep 6, 2013, at 4:06 AM, niq@apache.org wrote:
> 
> > Make libxml2 the configuration default XML parser
> > (so it should get a bit of test-driving).
> > 
> > Separate micro-commit so it's an easy distinction we can confine
> > to trunk even if the new code makes it into a release as an option
> > before it's well-tested.
> 
> 
> Something in these commits breaks the builds completely. I have both expat and libxml2 installed (with dev packages), and it doesn't detect either. Same problem (I think) on all build bots (they all fail as far as I can tell?).

Heh.  I guess I should've left the expat default, at least
for a while.  If you specify
  --with-xml=expat
it'll call the old expat-detection code, which I haven't changed.

But if you don't specify any --with-xml option, it should try
both before giving that error.  If it doesn't, it's a bug in my M4.

> checking for existence of /usr/lib64/tclConfig.sh... loading
> checking "For XML parser"... configure: error: An XML parser (expat or libxml2) is required.

Damn.  Guess we could use some temporary debug so you get
a trace of what it tried.

I should find time to revisit it over the weekend.

-- 
Nick Kew

Re: [2/2] git commit: Make libxml2 the configuration default XML parser (so it should get a bit of test-driving).

Posted by Leif Hedstrom <zw...@apache.org>.
On Sep 6, 2013, at 4:06 AM, niq@apache.org wrote:

> Make libxml2 the configuration default XML parser
> (so it should get a bit of test-driving).
> 
> Separate micro-commit so it's an easy distinction we can confine
> to trunk even if the new code makes it into a release as an option
> before it's well-tested.


Something in these commits breaks the builds completely. I have both expat and libxml2 installed (with dev packages), and it doesn't detect either. Same problem (I think) on all build bots (they all fail as far as I can tell?).

-- Leif

checking for existence of /usr/lib64/tclConfig.sh... loading
checking "For XML parser"... configure: error: An XML parser (expat or libxml2) is required.

Re: [2/2] git commit: Make libxml2 the configuration default XML parser (so it should get a bit of test-driving).

Posted by Leif Hedstrom <zw...@apache.org>.
On Sep 6, 2013, at 4:06 AM, niq@apache.org wrote:

> Make libxml2 the configuration default XML parser
> (so it should get a bit of test-driving).
> 
> Separate micro-commit so it's an easy distinction we can confine
> to trunk even if the new code makes it into a release as an option
> before it's well-tested.


Something in these commits breaks the builds completely. I have both expat and libxml2 installed (with dev packages), and it doesn't detect either. Same problem (I think) on all build bots (they all fail as far as I can tell?).

-- Leif

checking for existence of /usr/lib64/tclConfig.sh... loading
checking "For XML parser"... configure: error: An XML parser (expat or libxml2) is required.

[2/2] git commit: Make libxml2 the configuration default XML parser (so it should get a bit of test-driving).

Posted by ni...@apache.org.
Make libxml2 the configuration default XML parser
(so it should get a bit of test-driving).

Separate micro-commit so it's an easy distinction we can confine
to trunk even if the new code makes it into a release as an option
before it's well-tested.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/57fcef17
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/57fcef17
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/57fcef17

Branch: refs/heads/master
Commit: 57fcef17e1ef5020d475d3c8c71d71c6a7c7c337
Parents: 2184cc8
Author: Nick Kew <nk...@qualys.com>
Authored: Fri Sep 6 11:01:36 2013 +0100
Committer: Nick Kew <nk...@qualys.com>
Committed: Fri Sep 6 11:01:36 2013 +0100

----------------------------------------------------------------------
 build/xml.m4 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/57fcef17/build/xml.m4
----------------------------------------------------------------------
diff --git a/build/xml.m4 b/build/xml.m4
index d342814..fc342a9 100644
--- a/build/xml.m4
+++ b/build/xml.m4
@@ -31,9 +31,9 @@ AC_DEFUN([TS_CHECK_XML], [
     elif test "$withval" = "libxml2" ; then
       TS_CHECK_XML_LIBXML2
     elif test "x$withval" = "x" ; then
-      TS_CHECK_XML_EXPAT
+      TS_CHECK_XML_LIBXML2
       if test "$enable_xml" = "no"; then
-        TS_CHECK_XML_LIBXML2
+        TS_CHECK_XML_EXPAT
       fi
     else
       AC_MSG_ERROR([Unrecognised --with-xml option])