threads / patch / 33021

patchRe: [PATCH] Improve QNX support in GIT

Subject: Re: [PATCH] Improve QNX support in GIT

## tl;dr

4 messages between Feb 26, 2013 and Feb 26, 2013. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Matt Kraai· Feb 26, 2013, 17:25 UTC · lore

Hi Mike,

Mike Gorchak wrote:

Show 31 quoted lines

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

David Michael· Feb 26, 2013, 18:09 UTC · re: Matt Kraai · lore

Hi,

On Tue, Feb 26, 2013 at 12:25 PM, Matt Kraai <kraai@ftbfs.org> wrote:

Show 5 quoted lines

> 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

Mike Gorchak· Feb 26, 2013, 18:36 UTC · re: Matt Kraai · lore

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

Show 5 quoted lines

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

Show 6 quoted lines

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

Mike Gorchak· Feb 26, 2013, 18:54 UTC · re: Matt Kraai · lore

Show 6 quoted lines

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