Re: [PATCH] Improve QNX support in GIT
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
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
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
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
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
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
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
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
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
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
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
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
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
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