* [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
* 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: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 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
* [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
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).