Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
On Mon, Feb 11, 2013 at 12:59:55PM -0800, Matt Kraai wrote: From: Matt Kraai matt.kr...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
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.kr...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
Matt Kraai kr...@ftbfs.org writes: From: Matt Kraai matt.kr...@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.kr...@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; -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
Jeff King p...@peff.net writes: On Mon, Feb 11, 2013 at 12:59:55PM -0800, Matt Kraai wrote: From: Matt Kraai matt.kr...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
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? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Include xmlparse.h instead of expat.h on QNX
Matt Kraai kr...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
