Re: [PATCH] Improve QNX support in GIT

2013-02-26 Thread 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.
--
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] Improve QNX support in GIT

2013-02-26 Thread David Michael
Hi,

On Tue, Feb 26, 2013 at 12:25 PM, Matt Kraai kr...@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
--
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] Improve QNX support in GIT

2013-02-26 Thread 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.
--
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] Improve QNX support in GIT

2013-02-26 Thread 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.
--
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] Improve QNX support in GIT

2013-02-26 Thread 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.
--
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] Improve QNX support in GIT

2013-02-26 Thread 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.
--
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] Improve QNX support in GIT

2013-02-26 Thread 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

--
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] Improve QNX support in GIT

2013-02-25 Thread 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.
--
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] Improve QNX support in GIT

2013-02-24 Thread Junio C Hamano
Mike Gorchak mike.gorchak@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.
--
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] Improve QNX support in GIT

2013-02-24 Thread 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.
--
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] Improve QNX support in GIT

2013-02-24 Thread Junio C Hamano
Mike Gorchak mike.gorchak@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.


--
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] Improve QNX support in GIT

2013-02-24 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Mike Gorchak mike.gorchak@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.
--
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


[PATCH] Improve QNX support in GIT

2013-02-23 Thread Mike Gorchak
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!


git-qnx.diff
Description: Binary data


Re: [PATCH] Improve QNX support in GIT

2013-02-23 Thread 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