Re: [PATCH] Improve QNX support in GIT

Re: [PATCH] Improve QNX support in GIT

[Matt Kraai]

Hi,

Please include me in the Cc field, since I'm not subscribed to the
list.

Mike Gorchak wrote:
> Do you have a testcase for this (without using git codebase)? I wrote
> numerous resource managers since QNX 6.0 using threads and fork()s for
> daemonization in different order and never experienced a problems.
> There can be issues with pipes in case of external command run.

I just created the following one:


#include <pthread.h>
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <sys/types.h>
#include <process.h>

static void *
start_routine (void *arg)
{
  return NULL;
}

int
main (int argc, char **argv)
{
  int err;

  if ((err = pthread_create (NULL, NULL, start_routine, NULL)))
    {
      fprintf (stderr, "foo: pthread_create failed: %s\n", strerror (errno));
      return 1;
    }

  if (fork () == -1)
    {
      fprintf (stderr, "foo: fork failed: %s\n", strerror (errno));
      return 1;
    }

  return 0;
}


When I compile and run it on either QNX 6.3.2 or QNX 6.5.0, it
produces the following output:

 foo: fork failed: Function not implemented

If I remove the call to pthread_create, it doesn't output anything and
exits successfully.

Re: [PATCH] Improve QNX support in GIT

[Mike Gorchak]

> If I remove the call to pthread_create, it doesn't output anything and
> exits successfully.

I see. Most resource managers use procmgr_daemon(), which has no such
limitation. Anyway, as far as I can see current git sources do not use
fork together with pthread, except for daemonize() function.

Re: [PATCH] Improve QNX support in GIT

[Johannes Sixt]

Am 26.02.2013 21:32, schrieb Mike Gorchak:
>> If I remove the call to pthread_create, it doesn't output anything and
>> exits successfully.
> 
> I see. Most resource managers use procmgr_daemon(), which has no such
> limitation. Anyway, as far as I can see current git sources do not use
> fork together with pthread, except for daemonize() function.

Not true: When a clean or smudge filter is configured, a thread is
created, and the thread runs the external program via fork().

-- Hannes

Re: [PATCH] Improve QNX support in GIT

[Matt Kraai]

On Tue, Feb 26, 2013 at 10:32:16PM +0200, Mike Gorchak wrote:
> I see. Most resource managers use procmgr_daemon(), which has no such
> limitation. Anyway, as far as I can see current git sources do not use
> fork together with pthread, except for daemonize() function.

If threads are enabled, git does use fork after calling pthread_create:

 * When I run "git grep foo".
 * When I run "git fetch" to update my git working copy, which is
   using an HTTPS remote.

Both of these commands fail as a result.

Re: [PATCH] Improve QNX support in GIT

[Matt Kraai]

Hi Mike,

Mike Gorchak wrote:
> 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

Is there a point to the version checking?  I don't know that anyone
has tried to build Git on QNX 4, so adding a case for it seems
misleading.

I didn't realize that QNX 6.3.2 provided getpagesize.  Its header
files don't provide a prototype, so when I saw the warning, I assumed
it wasn't available.  Since NO_GETPAGESIZE is only used by QNX, if
it's OK to reintroduce the warning, NO_GETPAGESIZE might as well be
removed entirely.

I don't think it's a good idea to just enable thread support.  On QNX,
once a process creates a thread, fork stops working.  This breaks
commands that create threads and then try to run other programs, such
as "git fetch" with an https remote.  If threads are enabled, I think
that the uses of fork need to be audited and, if they can be called
after a thread is created, fixed.

Re: [PATCH] Improve QNX support in GIT

[David Michael]

Hi,

On Tue, Feb 26, 2013 at 12:25 PM, Matt Kraai <kraai@ftbfs.org> wrote:
> I didn't realize that QNX 6.3.2 provided getpagesize.  Its header
> files don't provide a prototype, so when I saw the warning, I assumed
> it wasn't available.  Since NO_GETPAGESIZE is only used by QNX, if
> it's OK to reintroduce the warning, NO_GETPAGESIZE might as well be
> removed entirely.

I have been using this feature locally for building on z/OS USS.  IBM
decided to drop the getpagesize definition by default, as it was
removed from SUSv3.  There is a way to re-enable the withdrawn legacy
functions, but I'd rather this option for using sysconf(_SC_PAGESIZE)
since that is apparently the preferred method now anyway.

So, if it's not being too much trouble, I'd vote for keeping this feature.

Thanks.

David

Re: [PATCH] Improve QNX support in GIT

[Mike Gorchak]

> Is there a point to the version checking?  I don't know that anyone
> has tried to build Git on QNX 4, so adding a case for it seems
> misleading.

getpagesize() was introduced in QNX 6.4.1, it is present in QNX 6.5.0
also. So at least for this version checking is requied.

> I didn't realize that QNX 6.3.2 provided getpagesize.  Its header
> files don't provide a prototype, so when I saw the warning, I assumed
> it wasn't available.  Since NO_GETPAGESIZE is only used by QNX, if
> it's OK to reintroduce the warning, NO_GETPAGESIZE might as well be
> removed entirely.

David asked to leave NO_GETPAGESIZE for other platform.

> I don't think it's a good idea to just enable thread support.  On QNX,
> once a process creates a thread, fork stops working.  This breaks
> commands that create threads and then try to run other programs, such
> as "git fetch" with an https remote.  If threads are enabled, I think
> that the uses of fork need to be audited and, if they can be called
> after a thread is created, fixed.

Do you have a testcase for this (without using git codebase)? I wrote
numerous resource managers since QNX 6.0 using threads and fork()s for
daemonization in different order and never experienced a problems.
There can be issues with pipes in case of external command run.

Re: [PATCH] Improve QNX support in GIT

[Mike Gorchak]

> I don't think it's a good idea to just enable thread support.  On QNX,
> once a process creates a thread, fork stops working.  This breaks
> commands that create threads and then try to run other programs, such
> as "git fetch" with an https remote.  If threads are enabled, I think
> that the uses of fork need to be audited and, if they can be called
> after a thread is created, fixed.

I did a quick look into the run-command.c and transport-helper.c
modules, they use pthread OR fork for external command spawning
depending on NO_PTHREAD declaration. Another fork() occurence in the
module daemon.c for daemonization and I know that it works.

[PATCH] Improve QNX support in GIT

[Mike Gorchak]

[-- Attachment #1: Type: text/plain, Size: 978 bytes --]

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!

[-- Attachment #2: git-qnx.diff --]
[-- Type: application/octet-stream, Size: 3742 bytes --]

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

Re: [PATCH] Improve QNX support in GIT

[Mike Gorchak]

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.

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

Re: [PATCH] Improve QNX support in GIT

[Junio C Hamano]

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

> 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.

Re: [PATCH] Improve QNX support in GIT

[Mike Gorchak]

>>> 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.

Re: [PATCH] Improve QNX support in GIT

[Junio C Hamano]

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.

Re: [PATCH] Improve QNX support in GIT

[Junio C Hamano]

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

> 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.

Re: [PATCH] Improve QNX support in GIT

[Mike Gorchak]

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.

> 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.