* [PATCH] Include xmlparse.h instead of expat.h on QNX @ 2013-02-11 20:59 Matt Kraai 2013-02-11 21:06 ` Jeff King 2013-02-11 21:34 ` Junio C Hamano 0 siblings, 2 replies; 11+ messages in thread From: Matt Kraai @ 2013-02-11 20:59 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Matt Kraai 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Include xmlparse.h instead of expat.h on QNX 2013-02-11 20:59 [PATCH] Include xmlparse.h instead of expat.h on QNX Matt Kraai @ 2013-02-11 21:06 ` Jeff King 2013-02-11 21:24 ` Matt Kraai 2013-02-11 21:41 ` [PATCH] Include xmlparse.h instead of expat.h on QNX Junio C Hamano 2013-02-11 21:34 ` Junio C Hamano 1 sibling, 2 replies; 11+ messages in thread From: Jeff King @ 2013-02-11 21:06 UTC (permalink / raw) To: Matt Kraai; +Cc: git, Junio C Hamano, Matt Kraai 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Include xmlparse.h instead of expat.h on QNX 2013-02-11 21:06 ` Jeff King @ 2013-02-11 21:24 ` Matt Kraai 2013-02-11 22:03 ` [PATCH] Allow building with xmlparse.h Matt Kraai 2013-02-11 21:41 ` [PATCH] Include xmlparse.h instead of expat.h on QNX Junio C Hamano 1 sibling, 1 reply; 11+ messages in thread From: Matt Kraai @ 2013-02-11 21:24 UTC (permalink / raw) To: Jeff King; +Cc: git, Junio C Hamano, Matt Kraai 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Allow building with xmlparse.h 2013-02-11 21:24 ` Matt Kraai @ 2013-02-11 22:03 ` Matt Kraai 2013-02-11 22:11 ` Junio C Hamano 0 siblings, 1 reply; 11+ messages in thread From: Matt Kraai @ 2013-02-11 22:03 UTC (permalink / raw) To: git, Junio C Hamano, Jeff King; +Cc: Matt Kraai 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Allow building with xmlparse.h 2013-02-11 22:03 ` [PATCH] Allow building with xmlparse.h Matt Kraai @ 2013-02-11 22:11 ` Junio C Hamano 2013-02-11 22:30 ` Matt Kraai 0 siblings, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2013-02-11 22:11 UTC (permalink / raw) To: Matt Kraai; +Cc: git, Jeff King, Matt Kraai 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Allow building with xmlparse.h 2013-02-11 22:11 ` Junio C Hamano @ 2013-02-11 22:30 ` Matt Kraai 2013-02-11 22:35 ` Junio C Hamano 0 siblings, 1 reply; 11+ messages in thread From: Matt Kraai @ 2013-02-11 22:30 UTC (permalink / raw) To: git, Junio C Hamano, Jeff King; +Cc: Matt Kraai 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Allow building with xmlparse.h 2013-02-11 22:30 ` Matt Kraai @ 2013-02-11 22:35 ` Junio C Hamano 0 siblings, 0 replies; 11+ messages in thread From: Junio C Hamano @ 2013-02-11 22:35 UTC (permalink / raw) To: Matt Kraai; +Cc: git, Jeff King, Matt Kraai 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 ;-). ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Include xmlparse.h instead of expat.h on QNX 2013-02-11 21:06 ` Jeff King 2013-02-11 21:24 ` Matt Kraai @ 2013-02-11 21:41 ` Junio C Hamano 1 sibling, 0 replies; 11+ messages in thread From: Junio C Hamano @ 2013-02-11 21:41 UTC (permalink / raw) To: Jeff King; +Cc: Matt Kraai, git, Matt Kraai 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Include xmlparse.h instead of expat.h on QNX 2013-02-11 20:59 [PATCH] Include xmlparse.h instead of expat.h on QNX Matt Kraai 2013-02-11 21:06 ` Jeff King @ 2013-02-11 21:34 ` Junio C Hamano 2013-02-11 21:49 ` Matt Kraai 1 sibling, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2013-02-11 21:34 UTC (permalink / raw) To: Matt Kraai; +Cc: git, Matt Kraai 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"; ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Include xmlparse.h instead of expat.h on QNX 2013-02-11 21:34 ` Junio C Hamano @ 2013-02-11 21:49 ` Matt Kraai 2013-02-11 21:53 ` Junio C Hamano 0 siblings, 1 reply; 11+ messages in thread From: Matt Kraai @ 2013-02-11 21:49 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jeff King 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? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Include xmlparse.h instead of expat.h on QNX 2013-02-11 21:49 ` Matt Kraai @ 2013-02-11 21:53 ` Junio C Hamano 0 siblings, 0 replies; 11+ messages in thread From: Junio C Hamano @ 2013-02-11 21:53 UTC (permalink / raw) To: Matt Kraai; +Cc: git, Jeff King 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-02-11 22:35 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-02-11 20:59 [PATCH] Include xmlparse.h instead of expat.h on QNX Matt Kraai 2013-02-11 21:06 ` Jeff King 2013-02-11 21:24 ` Matt Kraai 2013-02-11 22:03 ` [PATCH] Allow building with xmlparse.h Matt Kraai 2013-02-11 22:11 ` Junio C Hamano 2013-02-11 22:30 ` Matt Kraai 2013-02-11 22:35 ` Junio C Hamano 2013-02-11 21:41 ` [PATCH] Include xmlparse.h instead of expat.h on QNX Junio C Hamano 2013-02-11 21:34 ` Junio C Hamano 2013-02-11 21:49 ` Matt Kraai 2013-02-11 21:53 ` Junio C Hamano
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).