# [PATCH] Include xmlparse.h instead of expat.h on QNX

11 messages from 2013-02-11 to 2013-02-11. Participants: Matt Kraai, Jeff King, Junio C Hamano.
Thread: https://gitlist.dev/t/32880

## Matt Kraai, 2013-02-11 20:59

Subject: [PATCH] Include xmlparse.h instead of expat.h on QNX
Message-ID: <1360616395-18912-1-git-send-email-kraai@ftbfs.org>
URL: https://gitlist.dev/e/1360616395-18912-1-git-send-email-kraai%40ftbfs.org

```
From: Matt Kraai <matt.kraai@amo.abbott.com>

QNX 6.3.2 through 6.5.0 include Expat 1.1, which provides xmlparse.h
instead of expat.h, so include the former on QNX systems.

Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
---
 http-push.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/http-push.c b/http-push.c
index 9923441..55c575e 100644
--- a/http-push.c
+++ b/http-push.c
@@ -11,7 +11,11 @@
 #include "list-objects.h"
 #include "sigchain.h"
 
+#ifndef __QNX__
 #include <expat.h>
+#else
+#include <xmlparse.h>
+#endif
 
 static const char http_push_usage[] =
 "git http-push [--all] [--dry-run] [--force] [--verbose] <remote> [<head>...]\n";
-- 
1.8.1.2.547.g7ce9def

```

## Jeff King, 2013-02-11 21:06

Subject: Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
Message-ID: <20130211210621.GC32740@sigill.intra.peff.net>
URL: https://gitlist.dev/e/20130211210621.GC32740%40sigill.intra.peff.net
In-Reply-To: <1360616395-18912-1-git-send-email-kraai@ftbfs.org>

```
On Mon, Feb 11, 2013 at 12:59:55PM -0800, Matt Kraai wrote:

> From: Matt Kraai <matt.kraai@amo.abbott.com>
> 
> QNX 6.3.2 through 6.5.0 include Expat 1.1, which provides xmlparse.h
> instead of expat.h, so include the former on QNX systems.

So it is not just QNX, but rather older versions of expat?

> diff --git a/http-push.c b/http-push.c
> index 9923441..55c575e 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -11,7 +11,11 @@
>  #include "list-objects.h"
>  #include "sigchain.h"
>  
> +#ifndef __QNX__
>  #include <expat.h>
> +#else
> +#include <xmlparse.h>
> +#endif

If that is the case, should this #ifdef look for EXPAT_NEEDS_XMLPARSE_H,
and that macro triggered externally? Either in the QNX section of the
Makefile, or potentially by an autoconf macro?

-Peff

```

## Matt Kraai, 2013-02-11 21:24

Subject: Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
Message-ID: <20130211212411.GA19113@ftbfs.org>
URL: https://gitlist.dev/e/20130211212411.GA19113%40ftbfs.org
In-Reply-To: <20130211210621.GC32740@sigill.intra.peff.net>

```
On Mon, Feb 11, 2013 at 04:06:21PM -0500, Jeff King wrote:
> On Mon, Feb 11, 2013 at 12:59:55PM -0800, Matt Kraai wrote:
> 
> > From: Matt Kraai <matt.kraai@amo.abbott.com>
> > 
> > QNX 6.3.2 through 6.5.0 include Expat 1.1, which provides xmlparse.h
> > instead of expat.h, so include the former on QNX systems.
> 
> So it is not just QNX, but rather older versions of expat?

Yes, Expat 1.1 and 1.2 provide xmlparse.h, whereas 1.95.0 and later
provide expat.h.

> > diff --git a/http-push.c b/http-push.c
> > index 9923441..55c575e 100644
> > --- a/http-push.c
> > +++ b/http-push.c
> > @@ -11,7 +11,11 @@
> >  #include "list-objects.h"
> >  #include "sigchain.h"
> >  
> > +#ifndef __QNX__
> >  #include <expat.h>
> > +#else
> > +#include <xmlparse.h>
> > +#endif
> 
> If that is the case, should this #ifdef look for EXPAT_NEEDS_XMLPARSE_H,
> and that macro triggered externally? Either in the QNX section of the
> Makefile, or potentially by an autoconf macro?

I'll submit another patch shortly that does so, defining the variable
in the QNX section of config.mak.uname.

```

## Junio C Hamano, 2013-02-11 21:34

Subject: Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
Message-ID: <7vip5ylekj.fsf@alter.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vip5ylekj.fsf%40alter.siamese.dyndns.org
In-Reply-To: <1360616395-18912-1-git-send-email-kraai@ftbfs.org>

```
Matt Kraai <kraai@ftbfs.org> writes:

> From: Matt Kraai <matt.kraai@amo.abbott.com>
>
> QNX 6.3.2 through 6.5.0 include Expat 1.1, which provides xmlparse.h
> instead of expat.h, so include the former on QNX systems.
>
> Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
> ---

Two points and a possibly irrelevant half:

 - If a fix is platform specific (i.e. tempts to use #ifdef
   PLATFORM_NAME), we would prefer to see a patch that that is
   isolated to platform-specific compatibility layer, which would
   involve:

   . add compat/qnx/expat.h file that #include <xmlparse.h>
   . to Makefile, add -Icompat/qnx/ to CFLAGS

 - Is this really a fix for a problem specific to QNX?  It looks
   like this is for any platform with expat 1, no?

 - What happens to people with QNX older than 6.3.2 or newer than
   6.5.0 (assuming they will eventually start shipping expat 2) with
   your patch?

Assuming that this change is about building with expat1, it would
probably be better to do something like this instead, I would think.


 Makefile         | 5 +++++
 config.mak.uname | 1 +
 http-push.c      | 4 ++++
 3 files changed, 10 insertions(+)

diff --git a/Makefile b/Makefile
index 5a2e02d..57032cc 100644
--- a/Makefile
+++ b/Makefile
@@ -43,6 +43,8 @@ all::
 # Define EXPATDIR=/foo/bar if your expat header and library files are in
 # /foo/bar/include and /foo/bar/lib directories.
 #
+# Define EXPAT_VERSION=1 if you are trying to build with expat 1.x (e.g. QNX).
+#
 # Define NO_GETTEXT if you don't want Git output to be translated.
 # A translated Git requires GNU libintl or another gettext implementation,
 # plus libintl-perl at runtime.
@@ -1089,6 +1091,9 @@ else
 		else
 			EXPAT_LIBEXPAT = -lexpat
 		endif
+		ifdef EXPAT_VERSION
+		BASIC_CFLAGS += -DEXPAT_VERSION=$(EXPAT_VERSION)
+		endif
 	endif
 endif
 
diff --git a/config.mak.uname b/config.mak.uname
index bea34f0..281d834 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -536,4 +536,5 @@ ifeq ($(uname_S),QNX)
 	NO_R_TO_GCC_LINKER = YesPlease
 	NO_STRCASESTR = YesPlease
 	NO_STRLCPY = YesPlease
+	EXPAT_VERSION = 1
 endif
diff --git a/http-push.c b/http-push.c
index 3e72e84..2fdb0cd 100644
--- a/http-push.c
+++ b/http-push.c
@@ -11,7 +11,11 @@
 #include "list-objects.h"
 #include "sigchain.h"
 
+#if EXPAT_VERSION == 1
+#include <xmlparse.h>
+#else
 #include <expat.h>
+#endif
 
 static const char http_push_usage[] =
 "git http-push [--all] [--dry-run] [--force] [--verbose] <remote> [<head>...]\n";

```

## Junio C Hamano, 2013-02-11 21:41

Subject: Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
Message-ID: <7vehgmle8z.fsf@alter.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vehgmle8z.fsf%40alter.siamese.dyndns.org
In-Reply-To: <20130211210621.GC32740@sigill.intra.peff.net>

```
Jeff King <peff@peff.net> writes:

> On Mon, Feb 11, 2013 at 12:59:55PM -0800, Matt Kraai wrote:
>
>> From: Matt Kraai <matt.kraai@amo.abbott.com>
>> 
>> QNX 6.3.2 through 6.5.0 include Expat 1.1, which provides xmlparse.h
>> instead of expat.h, so include the former on QNX systems.
>
> So it is not just QNX, but rather older versions of expat?
>
>> diff --git a/http-push.c b/http-push.c
>> index 9923441..55c575e 100644
>> --- a/http-push.c
>> +++ b/http-push.c
>> @@ -11,7 +11,11 @@
>>  #include "list-objects.h"
>>  #include "sigchain.h"
>>  
>> +#ifndef __QNX__
>>  #include <expat.h>
>> +#else
>> +#include <xmlparse.h>
>> +#endif
>
> If that is the case, should this #ifdef look for EXPAT_NEEDS_XMLPARSE_H,
> and that macro triggered externally?

Heh, our mails crossed.  Another thing neither of us mentioned is
how compatible the subset of libexpat our codebase uses to what was
offered by the older versions of expat.  I would not be surprised if
nobody has tried running the resulting binary linked with expat 1.

```

## Matt Kraai, 2013-02-11 21:49

Subject: Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
Message-ID: <20130211214948.GB19113@ftbfs.org>
URL: https://gitlist.dev/e/20130211214948.GB19113%40ftbfs.org
In-Reply-To: <7vip5ylekj.fsf@alter.siamese.dyndns.org>

```
On Mon, Feb 11, 2013 at 01:34:52PM -0800, Junio C Hamano wrote:
> Two points and a possibly irrelevant half:
> 
>  - If a fix is platform specific (i.e. tempts to use #ifdef
>    PLATFORM_NAME), we would prefer to see a patch that that is
>    isolated to platform-specific compatibility layer, which would
>    involve:
> 
>    . add compat/qnx/expat.h file that #include <xmlparse.h>
>    . to Makefile, add -Icompat/qnx/ to CFLAGS
> 
>  - Is this really a fix for a problem specific to QNX?  It looks
>    like this is for any platform with expat 1, no?

It should apply to anyone trying to build with expat 1.1 or 1.2, but
not with 1.95.0 or later.

>  - What happens to people with QNX older than 6.3.2 or newer than
>    6.5.0 (assuming they will eventually start shipping expat 2) with
>    your patch?

Git will fail to build http-push.c.  I don't know if QNX will ever
update expat, though.  expat 1.95.0 was released in 2000, expat 2.0.0
was released in 2006, and QNX 6.5.0 was released in 2010.

> Assuming that this change is about building with expat1, it would
> probably be better to do something like this instead, I would think.

expat 1.95.0 through 1.95.8 used expat.h; should I still use
EXPAT_VERSION = 1 to signify that it should use xmlparse.h, use
EXPAT_NEEDS_XMLPARSE_H as Jeff suggested, or something else entirely?

```

## Junio C Hamano, 2013-02-11 21:53

Subject: Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
Message-ID: <7v38x2ldow.fsf@alter.siamese.dyndns.org>
URL: https://gitlist.dev/e/7v38x2ldow.fsf%40alter.siamese.dyndns.org
In-Reply-To: <20130211214948.GB19113@ftbfs.org>

```
Matt Kraai <kraai@ftbfs.org> writes:

>> Assuming that this change is about building with expat1, it would
>> probably be better to do something like this instead, I would think.
>
> expat 1.95.0 through 1.95.8 used expat.h; should I still use
> EXPAT_VERSION = 1 to signify that it should use xmlparse.h, use
> EXPAT_NEEDS_XMLPARSE_H as Jeff suggested, or something else entirely?

Oh, please do not take it as a request to use that exact name (in
case you didn't know, I am bad at naming things).  It was merely an
illustration to show the direction, written without knowing that
Peff was essentially giving the same suggestion.

Thanks.

Oh, by the way, please do not deflect an attempt to directly send a
response to you with a Mail-Followup-To header.

```

## Matt Kraai, 2013-02-11 22:03

Subject: [PATCH] Allow building with xmlparse.h
Message-ID: <1360620225-19587-1-git-send-email-kraai@ftbfs.org>
URL: https://gitlist.dev/e/1360620225-19587-1-git-send-email-kraai%40ftbfs.org
In-Reply-To: <20130211212411.GA19113@ftbfs.org>

```
From: Matt Kraai <matt.kraai@amo.abbott.com>

expat 1.1 and 1.2 provide xmlparse.h instead of expat.h.  Include the
former on systems that define the EXPAT_NEEDS_XMLPARSE_H variable and
define that variable on QNX systems, which ship with expat 1.1.

Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
---
 Makefile         | 6 ++++++
 config.mak.uname | 1 +
 http-push.c      | 4 ++++
 3 files changed, 11 insertions(+)

diff --git a/Makefile b/Makefile
index 5a2e02d..720fc18 100644
--- a/Makefile
+++ b/Makefile
@@ -43,6 +43,9 @@ all::
 # Define EXPATDIR=/foo/bar if your expat header and library files are in
 # /foo/bar/include and /foo/bar/lib directories.
 #
+# Define EXPAT_NEEDS_XMLPARSE_H if you have an old version of expat (e.g.,
+# 1.1 or 1.2) that provides xmlparse.h instead of expat.h.
+#
 # Define NO_GETTEXT if you don't want Git output to be translated.
 # A translated Git requires GNU libintl or another gettext implementation,
 # plus libintl-perl at runtime.
@@ -1089,6 +1092,9 @@ else
 		else
 			EXPAT_LIBEXPAT = -lexpat
 		endif
+		ifdef EXPAT_NEEDS_XMLPARSE_H
+			BASIC_CFLAGS += -DEXPAT_NEEDS_XMLPARSE_H
+		endif
 	endif
 endif
 
diff --git a/config.mak.uname b/config.mak.uname
index bea34f0..8743a6d 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -523,6 +523,7 @@ endif
 endif
 ifeq ($(uname_S),QNX)
 	COMPAT_CFLAGS += -DSA_RESTART=0
+	EXPAT_NEEDS_XMLPARSE_H = YesPlease
 	HAVE_STRINGS_H = YesPlease
 	NEEDS_SOCKET = YesPlease
 	NO_FNMATCH_CASEFOLD = YesPlease
diff --git a/http-push.c b/http-push.c
index 9923441..7202e2d 100644
--- a/http-push.c
+++ b/http-push.c
@@ -11,7 +11,11 @@
 #include "list-objects.h"
 #include "sigchain.h"
 
+#ifndef EXPAT_NEEDS_XMLPARSE_H
 #include <expat.h>
+#else
+#include <xmlparse.h>
+#endif
 
 static const char http_push_usage[] =
 "git http-push [--all] [--dry-run] [--force] [--verbose] <remote> [<head>...]\n";
-- 
1.8.1.2.547.g7ce9def

```

## Junio C Hamano, 2013-02-11 22:11

Subject: Re: [PATCH] Allow building with xmlparse.h
Message-ID: <7vy5eujybf.fsf@alter.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vy5eujybf.fsf%40alter.siamese.dyndns.org
In-Reply-To: <1360620225-19587-1-git-send-email-kraai@ftbfs.org>

```
Matt Kraai <kraai@ftbfs.org> writes:

> From: Matt Kraai <matt.kraai@amo.abbott.com>
>
> expat 1.1 and 1.2 provide xmlparse.h instead of expat.h.  Include the
> former on systems that define the EXPAT_NEEDS_XMLPARSE_H variable and
> define that variable on QNX systems, which ship with expat 1.1.
>
> Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
> ---
> ...
> diff --git a/http-push.c b/http-push.c
> index 9923441..7202e2d 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -11,7 +11,11 @@
>  #include "list-objects.h"
>  #include "sigchain.h"
>  
> +#ifndef EXPAT_NEEDS_XMLPARSE_H
>  #include <expat.h>
> +#else
> +#include <xmlparse.h>
> +#endif

Thanks for a quick re-roll.

Is it just me who finds the above hard to read and find the below
much more natural?

	#ifdef NEEDS_FOO_H
        #include <foo.h>
        #else
        #include <bar.h>
        #endif

```

## Matt Kraai, 2013-02-11 22:30

Subject: [PATCH] Allow building with xmlparse.h
Message-ID: <1360621855-19863-1-git-send-email-kraai@ftbfs.org>
URL: https://gitlist.dev/e/1360621855-19863-1-git-send-email-kraai%40ftbfs.org
In-Reply-To: <7vy5eujybf.fsf@alter.siamese.dyndns.org>

```
From: Matt Kraai <matt.kraai@amo.abbott.com>

expat 1.1 and 1.2 provide xmlparse.h instead of expat.h.  Include the
former on systems that define the EXPAT_NEEDS_XMLPARSE_H variable and
define that variable on QNX systems, which ship with expat 1.1.

Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
---
 Makefile         | 6 ++++++
 config.mak.uname | 1 +
 http-push.c      | 4 ++++
 3 files changed, 11 insertions(+)

I've changed #ifndef to #ifdef and changed the order of the branches
in http-push.c.  If you'd also like me to rename the variable (e.g.,
to NEEDS_XMLPARSE_H), please let me know.

diff --git a/Makefile b/Makefile
index 5a2e02d..720fc18 100644
--- a/Makefile
+++ b/Makefile
@@ -43,6 +43,9 @@ all::
 # Define EXPATDIR=/foo/bar if your expat header and library files are in
 # /foo/bar/include and /foo/bar/lib directories.
 #
+# Define EXPAT_NEEDS_XMLPARSE_H if you have an old version of expat (e.g.,
+# 1.1 or 1.2) that provides xmlparse.h instead of expat.h.
+#
 # Define NO_GETTEXT if you don't want Git output to be translated.
 # A translated Git requires GNU libintl or another gettext implementation,
 # plus libintl-perl at runtime.
@@ -1089,6 +1092,9 @@ else
 		else
 			EXPAT_LIBEXPAT = -lexpat
 		endif
+		ifdef EXPAT_NEEDS_XMLPARSE_H
+			BASIC_CFLAGS += -DEXPAT_NEEDS_XMLPARSE_H
+		endif
 	endif
 endif
 
diff --git a/config.mak.uname b/config.mak.uname
index bea34f0..8743a6d 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -523,6 +523,7 @@ endif
 endif
 ifeq ($(uname_S),QNX)
 	COMPAT_CFLAGS += -DSA_RESTART=0
+	EXPAT_NEEDS_XMLPARSE_H = YesPlease
 	HAVE_STRINGS_H = YesPlease
 	NEEDS_SOCKET = YesPlease
 	NO_FNMATCH_CASEFOLD = YesPlease
diff --git a/http-push.c b/http-push.c
index 9923441..9fa47a7 100644
--- a/http-push.c
+++ b/http-push.c
@@ -11,7 +11,11 @@
 #include "list-objects.h"
 #include "sigchain.h"
 
+#ifdef EXPAT_NEEDS_XMLPARSE_H
+#include <xmlparse.h>
+#else
 #include <expat.h>
+#endif
 
 static const char http_push_usage[] =
 "git http-push [--all] [--dry-run] [--force] [--verbose] <remote> [<head>...]\n";
-- 
1.8.1.2.547.g7ce9def

```

## Junio C Hamano, 2013-02-11 22:35

Subject: Re: [PATCH] Allow building with xmlparse.h
Message-ID: <7vtxpijx7r.fsf@alter.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vtxpijx7r.fsf%40alter.siamese.dyndns.org
In-Reply-To: <1360621855-19863-1-git-send-email-kraai@ftbfs.org>

```
Matt Kraai <kraai@ftbfs.org> writes:

> From: Matt Kraai <matt.kraai@amo.abbott.com>
>
> expat 1.1 and 1.2 provide xmlparse.h instead of expat.h.  Include the
> former on systems that define the EXPAT_NEEDS_XMLPARSE_H variable and
> define that variable on QNX systems, which ship with expat 1.1.
>
> Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
> ---
>  Makefile         | 6 ++++++
>  config.mak.uname | 1 +
>  http-push.c      | 4 ++++
>  3 files changed, 11 insertions(+)
>
> I've changed #ifndef to #ifdef and changed the order of the branches
> in http-push.c.  If you'd also like me to rename the variable (e.g.,
> to NEEDS_XMLPARSE_H), please let me know.

I do not think renaming is necessary (the name you used in the
original and this patch is better than NEEDS_XMLPARSE_H).

I take that you also think the updated order is easier to read;
thanks for sanity-checking ;-).

```