threads / patch / 32990

patchImprove QNX support in GIT

Subject: [PATCH] Improve QNX support in GIT

## tl;dr

7 messages between Feb 23, 2013 and Feb 25, 2013. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Mike Gorchak· Feb 23, 2013, 22:02 UTC · lore

Hi,

Here is a small patch with QNX build improvements:

1) Rename tar.h to tar_git.h. Latest QNX versions have system tar.h
header according to
http://pubs.opengroup.org/onlinepubs/009696699/basedefs/tar.h.html ,
to avoid inclusion of another tar.h, original header was renamed.
2) Rename fnmatch.h to fnmatch_gnu.h and fnmatch.c to fnmatch_gnu.c to
avoid inclusion of system fnmatch.h header in case if -I/usr/include
path is specified before -Icompat/fnmatch. Which is common situation.
3) pager.c - default "less" invocation flags were changed for QNX 6,x
platform, since QNX has incompatible with GNU coreutils version of
"less" utility.
4) config.mak.uname - a) do not override mkdtemp/mkstemps/strcasestr
detection, since newer QNX version could contain such functions. Let
to configure decide what is present in the system. b) getpagesize()
function is existing under QNX, c) QNX has pthread functions in the
libc, so do not define NO_PTHREAD macro.

Thanks in advance!

Show changes to 10 files +26 −11

Makefile, archive-tar.c, builtin/tar-tree.c, compat/fnmatch/fnmatch.c, compat/fnmatch/fnmatch_gnu.c, compat/fnmatch/fnmatch.h, config.mak.uname, git-compat-util.h, pager.c, tar.h

diff --git a/Makefile b/Makefile
index ba8e243..f6dd2eb 100644
--- a/Makefile
+++ b/Makefile
@@ -726,7 +726,7 @@ LIB_H += streaming.h
 LIB_H += string-list.h
 LIB_H += submodule.h
 LIB_H += tag.h
-LIB_H += tar.h
+LIB_H += tar_git.h
 LIB_H += thread-utils.h
 LIB_H += transport.h
 LIB_H += tree-walk.h
@@ -1256,12 +1256,12 @@ endif
 ifdef NO_FNMATCH
 	COMPAT_CFLAGS += -Icompat/fnmatch
 	COMPAT_CFLAGS += -DNO_FNMATCH
-	COMPAT_OBJS += compat/fnmatch/fnmatch.o
+	COMPAT_OBJS += compat/fnmatch/fnmatch_gnu.o
 else
 ifdef NO_FNMATCH_CASEFOLD
 	COMPAT_CFLAGS += -Icompat/fnmatch
 	COMPAT_CFLAGS += -DNO_FNMATCH_CASEFOLD
-	COMPAT_OBJS += compat/fnmatch/fnmatch.o
+	COMPAT_OBJS += compat/fnmatch/fnmatch_gnu.o
 endif
 endif
 ifdef USE_WILDMATCH
diff --git a/archive-tar.c b/archive-tar.c
index 719b629..8e24336 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -2,7 +2,7 @@
  * Copyright (c) 2005, 2006 Rene Scharfe
  */
 #include "cache.h"
-#include "tar.h"
+#include "tar_git.h"
 #include "archive.h"
 #include "streaming.h"
 #include "run-command.h"
diff --git a/builtin/tar-tree.c b/builtin/tar-tree.c
index 3f1e701..b0e4551 100644
--- a/builtin/tar-tree.c
+++ b/builtin/tar-tree.c
@@ -3,7 +3,7 @@
  */
 #include "cache.h"
 #include "commit.h"
-#include "tar.h"
+#include "tar_git.h"
 #include "builtin.h"
 #include "quote.h"
 
diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch_gnu.c
similarity index 99%
rename from compat/fnmatch/fnmatch.c
rename to compat/fnmatch/fnmatch_gnu.c
index 5ef0685..f9a5e5b 100644
--- a/compat/fnmatch/fnmatch.c
+++ b/compat/fnmatch/fnmatch_gnu.c
@@ -26,7 +26,7 @@
 #endif
 
 #include <errno.h>
-#include <fnmatch.h>
+#include <fnmatch_gnu.h>
 #include <ctype.h>
 
 #if HAVE_STRING_H || defined _LIBC
diff --git a/compat/fnmatch/fnmatch.h b/compat/fnmatch/fnmatch_gnu.h
similarity index 100%
rename from compat/fnmatch/fnmatch.h
rename to compat/fnmatch/fnmatch_gnu.h
diff --git a/config.mak.uname b/config.mak.uname
index 8743a6d..2d42ffe 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -527,14 +527,21 @@ ifeq ($(uname_S),QNX)
 	HAVE_STRINGS_H = YesPlease
 	NEEDS_SOCKET = YesPlease
 	NO_FNMATCH_CASEFOLD = YesPlease
-	NO_GETPAGESIZE = YesPlease
 	NO_ICONV = YesPlease
 	NO_MEMMEM = YesPlease
-	NO_MKDTEMP = YesPlease
-	NO_MKSTEMPS = YesPlease
 	NO_NSEC = YesPlease
-	NO_PTHREADS = YesPlease
 	NO_R_TO_GCC_LINKER = YesPlease
-	NO_STRCASESTR = YesPlease
 	NO_STRLCPY = YesPlease
+	# All QNX 6.x versions have pthread functions in libc
+	# and getpagesize. Leave mkstemps/mkdtemp/strcasestr for
+	# autodetection.
+	ifeq ($(shell expr "$(uname_R)" : '6\.[0-9]\.[0-9]'),5)
+		PTHREAD_LIBS = ""
+	else
+		NO_PTHREADS = YesPlease
+		NO_GETPAGESIZE = YesPlease
+		NO_STRCASESTR = YesPlease
+		NO_MKSTEMPS = YesPlease
+		NO_MKDTEMP = YesPlease
+	endif
 endif
diff --git a/git-compat-util.h b/git-compat-util.h
index b7eaaa9..f59d696 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -113,7 +113,11 @@
 #include <time.h>
 #include <signal.h>
 #ifndef USE_WILDMATCH
+#if defined(NO_FNMATCH) || defined(NO_FNMATCH_CASEFOLD)
+#include <fnmatch_gnu.h>
+#else
 #include <fnmatch.h>
+#endif /* NO_FNMATCH */
 #endif
 #include <assert.h>
 #include <regex.h>
diff --git a/pager.c b/pager.c
index c1ecf65..bed627a 100644
--- a/pager.c
+++ b/pager.c
@@ -81,7 +81,11 @@ void setup_pager(void)
 	pager_process.argv = pager_argv;
 	pager_process.in = -1;
 	if (!getenv("LESS")) {
+	#if !defined(__QNXNTO__)
 		static const char *env[] = { "LESS=FRSX", NULL };
+	#else
+		static const char *env[] = { "LESS=rS", NULL };
+	#endif /* __QNXNTO__ */
 		pager_process.env = env;
 	}
 	if (start_command(&pager_process))
diff --git a/tar.h b/tar_git.h
similarity index 100%
rename from tar.h
rename to tar_git.h

Mike Gorchak· Feb 24, 2013, 07:36 UTC · re: Mike Gorchak · lore

Re: [PATCH] Improve QNX support in GIT

Hello,

Show 17 quoted lines

> Here is a small patch with QNX build improvements:
>
> 1) Rename tar.h to tar_git.h. Latest QNX versions have system tar.h
> header according to
> http://pubs.opengroup.org/onlinepubs/009696699/basedefs/tar.h.html ,
> to avoid inclusion of another tar.h, original header was renamed.
> 2) Rename fnmatch.h to fnmatch_gnu.h and fnmatch.c to fnmatch_gnu.c to
> avoid inclusion of system fnmatch.h header in case if -I/usr/include
> path is specified before -Icompat/fnmatch. Which is common situation.
> 3) pager.c - default "less" invocation flags were changed for QNX 6,x
> platform, since QNX has incompatible with GNU coreutils version of
> "less" utility.
> 4) config.mak.uname - a) do not override mkdtemp/mkstemps/strcasestr
> detection, since newer QNX version could contain such functions. Let
> to configure decide what is present in the system. b) getpagesize()
> function is existing under QNX, c) QNX has pthread functions in the
> libc, so do not define NO_PTHREAD macro.

Sorry, in the previous post the patch was not inlined.

Show changes to 10 files +26 −11

Makefile, archive-tar.c, builtin/tar-tree.c, compat/fnmatch/fnmatch.c, compat/fnmatch/fnmatch_gnu.c, compat/fnmatch/fnmatch.h, config.mak.uname, git-compat-util.h, pager.c, tar.h

diff --git a/Makefile b/Makefile
index ba8e243..f6dd2eb 100644
--- a/Makefile
+++ b/Makefile
@@ -726,7 +726,7 @@ LIB_H += streaming.h
 LIB_H += string-list.h
 LIB_H += submodule.h
 LIB_H += tag.h
-LIB_H += tar.h
+LIB_H += tar_git.h
 LIB_H += thread-utils.h
 LIB_H += transport.h
 LIB_H += tree-walk.h
@@ -1256,12 +1256,12 @@ endif
 ifdef NO_FNMATCH
 	COMPAT_CFLAGS += -Icompat/fnmatch
 	COMPAT_CFLAGS += -DNO_FNMATCH
-	COMPAT_OBJS += compat/fnmatch/fnmatch.o
+	COMPAT_OBJS += compat/fnmatch/fnmatch_gnu.o
 else
 ifdef NO_FNMATCH_CASEFOLD
 	COMPAT_CFLAGS += -Icompat/fnmatch
 	COMPAT_CFLAGS += -DNO_FNMATCH_CASEFOLD
-	COMPAT_OBJS += compat/fnmatch/fnmatch.o
+	COMPAT_OBJS += compat/fnmatch/fnmatch_gnu.o
 endif
 endif
 ifdef USE_WILDMATCH
diff --git a/archive-tar.c b/archive-tar.c
index 719b629..8e24336 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -2,7 +2,7 @@
  * Copyright (c) 2005, 2006 Rene Scharfe
  */
 #include "cache.h"
-#include "tar.h"
+#include "tar_git.h"
 #include "archive.h"
 #include "streaming.h"
 #include "run-command.h"
diff --git a/builtin/tar-tree.c b/builtin/tar-tree.c
index 3f1e701..b0e4551 100644
--- a/builtin/tar-tree.c
+++ b/builtin/tar-tree.c
@@ -3,7 +3,7 @@
  */
 #include "cache.h"
 #include "commit.h"
-#include "tar.h"
+#include "tar_git.h"
 #include "builtin.h"
 #include "quote.h"

diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch_gnu.c
similarity index 99%
rename from compat/fnmatch/fnmatch.c
rename to compat/fnmatch/fnmatch_gnu.c
index 5ef0685..f9a5e5b 100644
--- a/compat/fnmatch/fnmatch.c
+++ b/compat/fnmatch/fnmatch_gnu.c
@@ -26,7 +26,7 @@
 #endif

 #include <errno.h>
-#include <fnmatch.h>
+#include <fnmatch_gnu.h>
 #include <ctype.h>

 #if HAVE_STRING_H || defined _LIBC
diff --git a/compat/fnmatch/fnmatch.h b/compat/fnmatch/fnmatch_gnu.h
similarity index 100%
rename from compat/fnmatch/fnmatch.h
rename to compat/fnmatch/fnmatch_gnu.h
diff --git a/config.mak.uname b/config.mak.uname
index 8743a6d..2d42ffe 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -527,14 +527,21 @@ ifeq ($(uname_S),QNX)
 	HAVE_STRINGS_H = YesPlease
 	NEEDS_SOCKET = YesPlease
 	NO_FNMATCH_CASEFOLD = YesPlease
-	NO_GETPAGESIZE = YesPlease
 	NO_ICONV = YesPlease
 	NO_MEMMEM = YesPlease
-	NO_MKDTEMP = YesPlease
-	NO_MKSTEMPS = YesPlease
 	NO_NSEC = YesPlease
-	NO_PTHREADS = YesPlease
 	NO_R_TO_GCC_LINKER = YesPlease
-	NO_STRCASESTR = YesPlease
 	NO_STRLCPY = YesPlease
+	# All QNX 6.x versions have pthread functions in libc
+	# and getpagesize. Leave mkstemps/mkdtemp/strcasestr for
+	# autodetection.
+	ifeq ($(shell expr "$(uname_R)" : '6\.[0-9]\.[0-9]'),5)
+		PTHREAD_LIBS = ""
+	else
+		NO_PTHREADS = YesPlease
+		NO_GETPAGESIZE = YesPlease
+		NO_STRCASESTR = YesPlease
+		NO_MKSTEMPS = YesPlease
+		NO_MKDTEMP = YesPlease
+	endif
 endif
diff --git a/git-compat-util.h b/git-compat-util.h
index b7eaaa9..f59d696 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -113,7 +113,11 @@
 #include <time.h>
 #include <signal.h>
 #ifndef USE_WILDMATCH
+#if defined(NO_FNMATCH) || defined(NO_FNMATCH_CASEFOLD)
+#include <fnmatch_gnu.h>
+#else
 #include <fnmatch.h>
+#endif /* NO_FNMATCH */
 #endif
 #include <assert.h>
 #include <regex.h>
diff --git a/pager.c b/pager.c
index c1ecf65..bed627a 100644
--- a/pager.c
+++ b/pager.c
@@ -81,7 +81,11 @@ void setup_pager(void)
 	pager_process.argv = pager_argv;
 	pager_process.in = -1;
 	if (!getenv("LESS")) {
+	#if !defined(__QNXNTO__)
 		static const char *env[] = { "LESS=FRSX", NULL };
+	#else
+		static const char *env[] = { "LESS=rS", NULL };
+	#endif /* __QNXNTO__ */
 		pager_process.env = env;
 	}
 	if (start_command(&pager_process))
diff --git a/tar.h b/tar_git.h
similarity index 100%
rename from tar.h
rename to tar_git.h

Junio C Hamano· Feb 24, 2013, 08:46 UTC · re: Mike Gorchak · lore

Re: [PATCH] Improve QNX support in GIT

Mike Gorchak <mike.gorchak.qnx@gmail.com> writes:

Show 21 quoted lines

> Hello,
>
>> Here is a small patch with QNX build improvements:
>>
>> 1) Rename tar.h to tar_git.h. Latest QNX versions have system tar.h
>> header according to
>> http://pubs.opengroup.org/onlinepubs/009696699/basedefs/tar.h.html ,
>> to avoid inclusion of another tar.h, original header was renamed.
>> 2) Rename fnmatch.h to fnmatch_gnu.h and fnmatch.c to fnmatch_gnu.c to
>> avoid inclusion of system fnmatch.h header in case if -I/usr/include
>> path is specified before -Icompat/fnmatch. Which is common situation.
>> 3) pager.c - default "less" invocation flags were changed for QNX 6,x
>> platform, since QNX has incompatible with GNU coreutils version of
>> "less" utility.
>> 4) config.mak.uname - a) do not override mkdtemp/mkstemps/strcasestr
>> detection, since newer QNX version could contain such functions. Let
>> to configure decide what is present in the system. b) getpagesize()
>> function is existing under QNX, c) QNX has pthread functions in the
>> libc, so do not define NO_PTHREAD macro.
>
> Sorry, in the previous post the patch was not inlined.

First on the form. The message lacks a proper commit log message and a sign-off. Please check Documentation/SubmittingPatches and also compare the message I am responding to with recent patch submission messages from other people on the list.

As to the substance, I am fairly negative about the approach this patch takes, especially the rationale it uses for #2 above. It goes directly against the spirit of having compat/ directory in the first place to have -I/usr/include _before_ -Icompat/anything and that, not the names of header files in compat/ directory, is the root cause of the problem you are seeing, I think.

Mike Gorchak· Feb 24, 2013, 14:12 UTC · re: Junio C Hamano · lore

Re: [PATCH] Improve QNX support in GIT

Show 9 quoted lines

>>> 2) Rename fnmatch.h to fnmatch_gnu.h and fnmatch.c to fnmatch_gnu.c to
>>> avoid inclusion of system fnmatch.h header in case if -I/usr/include
>>> path is specified before -Icompat/fnmatch. Which is common situation.
> As to the substance, I am fairly negative about the approach this
> patch takes, especially the rationale it uses for #2 above.  It goes
> directly against the spirit of having compat/ directory in the first
> place to have -I/usr/include _before_ -Icompat/anything and that,
> not the names of header files in compat/ directory, is the root
> cause of the problem you are seeing, I think.

It is quite common to pass CPPFLAGS/CFLAGS/CXXFLAGS before configure script to make a custom build. For example, I have specific set of headers which belong to another version of libc, so I pass directory where these headers are located right before configure script:

CFLAGS="-I/usr/qnxVVV/include" LDFLAGS="-I/usr/qnxVVV/lib" ./configure --prefix=/usr

About this you can read by typing ./configure --help. This approach works for every autoconf-based project, except for GIT due to headers collision. I do not know anything about "spirit of compat/ directory", but if it interferes with the normal build process, it means something is wrong. But it is up to you.

Thanks.

Junio C Hamano· Feb 24, 2013, 21:24 UTC · re: Mike Gorchak · lore

Re: [PATCH] Improve QNX support in GIT

Mike Gorchak <mike.gorchak.qnx@gmail.com> writes:

> CFLAGS="-I/usr/qnxVVV/include" LDFLAGS="-I/usr/qnxVVV/lib" ./configure
> --prefix=/usr

Oh, I didn't notice that, but the definition of ALL_CFLAGS may be what is wrong. It allows CFLAGS to come before BASIC_CFLAGS that adds -Icompat/, which goes against the whole point of having replacement headers in compat/ directory.

Junio C Hamano· Feb 25, 2013, 07:14 UTC · re: Junio C Hamano · lore

Re: [PATCH] Improve QNX support in GIT

Junio C Hamano <gitster@pobox.com> writes:

Show 9 quoted lines

> Mike Gorchak <mike.gorchak.qnx@gmail.com> writes:
>
>> CFLAGS="-I/usr/qnxVVV/include" LDFLAGS="-I/usr/qnxVVV/lib" ./configure
>> --prefix=/usr
>
> Oh, I didn't notice that, but the definition of ALL_CFLAGS may be
> what is wrong.  It allows CFLAGS to come before BASIC_CFLAGS that
> adds -Icompat/, which goes against the whole point of having
> replacement headers in compat/ directory.

Also, in general, as the end-user input, we would want to make it take the precedence, so that CFLAGS can be used to override the default command line; e.g. we may have -DMACRO=value on BASIC_CFLAGS or others on ALL_CFLAGS, and let the users who know what they are doing use CFLAGS=-DMACRO=anothervalue to override it.

Swapping the order between CFLAGS and BASIC_CFLAGS in ALL_CFLAGS may be a good change for that reason as well.

In any case, I won't take a patch to rename source files left and right only to work around name collisions with random system header files we do not even use ourselves, unless/until I know we have tried all the other saner approaches first. That's a workaround, not a solution.

Mike Gorchak· Feb 25, 2013, 08:30 UTC · re: Junio C Hamano · lore

Re: [PATCH] Improve QNX support in GIT

Hi Junio,

> Swapping the order between CFLAGS and BASIC_CFLAGS in ALL_CFLAGS may
> be a good change for that reason as well.

This sounds very reasonable.

Show 5 quoted lines

> In any case, I won't take a patch to rename source files left and
> right only to work around name collisions with random system header
> files we do not even use ourselves, unless/until I know we have
> tried all the other saner approaches first.  That's a workaround,
> not a solution.

Ok, no problem, I will create another patch which alter CFLAGS order.