[PATCH] some systems don't have (and need) sys/select.h

[PATCH] some systems don't have (and need) sys/select.h

[Robert Schiele]

The select stuff is already in sys/time.h on on some systems like HP-UX
thus we should not include sys/select.h in that case.

Signed-off-by: Robert Schiele <rschiele@gmail.com>
---
This patch replaces my previously sent patch
"HP-UX traditionally has no sys/select.h".

 Makefile          |    5 +++++
 git-compat-util.h |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 5aac0c0..c9e54b1 100644
--- a/Makefile
+++ b/Makefile
@@ -42,6 +42,8 @@ all::
 #
 # Define NO_MKDTEMP if you don't have mkdtemp in the C library.
 #
+# Define NO_SYS_SELECT_H if you don't have sys/select.h.
+#
 # Define NO_SYMLINK_HEAD if you never want .git/HEAD to be a symbolic link.
 # Enable it on Windows.  By default, symrefs are still used.
 #
@@ -635,6 +637,9 @@ ifdef NO_UNSETENV
 	COMPAT_CFLAGS += -DNO_UNSETENV
 	COMPAT_OBJS += compat/unsetenv.o
 endif
+ifdef NO_SYS_SELECT_H
+	BASIC_CFLAGS += -DNO_SYS_SELECT_H
+endif
 ifdef NO_MMAP
 	COMPAT_CFLAGS += -DNO_MMAP
 	COMPAT_OBJS += compat/mmap.o
diff --git a/git-compat-util.h b/git-compat-util.h
index b6ef544..4df90cb 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -68,7 +68,9 @@
 #include <sys/poll.h>
 #include <sys/socket.h>
 #include <sys/ioctl.h>
+#ifndef NO_SYS_SELECT_H
 #include <sys/select.h>
+#endif
 #include <assert.h>
 #include <regex.h>
 #include <netinet/in.h>
-- 
1.5.2.4

Re: [PATCH] some systems don't have (and need) sys/select.h

[Johannes Schindelin]

Hi,

On Thu, 24 Jan 2008, Robert Schiele wrote:

> The select stuff is already in sys/time.h on on some systems like HP-UX 
> thus we should not include sys/select.h in that case.

Thank you very much,
Dscho

Re: [PATCH] some systems don't have (and need) sys/select.h

[Junio C Hamano]

Robert Schiele <rschiele@gmail.com> writes:

> The select stuff is already in sys/time.h on on some systems like HP-UX
> thus we should not include sys/select.h in that case.
>
> Signed-off-by: Robert Schiele <rschiele@gmail.com>

The patch looks good.  I'd rather try to be a bit more helpful
to people who want to port git to their system that is not
HP-UX, though.

How about wording it like this?

Subject: pre-POSIX.1-2001 systems do not have <sys/select.h>

POSIX.1-2001 has declaration of select(2) in <sys/select.h>, but
in previous version of SUS, it is declared in <sys/time.h>,
which is already included in git-compat-util.h.

This introduces NO_SYS_SELECT_H macro in the Makefile to be set
on older systems, to skip inclusion of <sys/select.h> that does
not exist on them.

We could check _POSIX_VERSION with 200112L and do this
automatically, but earlier it was reported that the approach
does not work well on some vintage of HP-UX.  Other systems may
get _POSIX_VERSION itself wrong.  At least for now, this manual
configuration is safer.

Re: [PATCH] some systems don't have (and need) sys/select.h

[Robert Schiele]

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

On Thu, Jan 24, 2008 at 01:17:14PM -0800, Junio C Hamano wrote:
> Robert Schiele <rschiele@gmail.com> writes:
> 
> > The select stuff is already in sys/time.h on on some systems like HP-UX
> > thus we should not include sys/select.h in that case.
> >
> > Signed-off-by: Robert Schiele <rschiele@gmail.com>
> 
> The patch looks good.  I'd rather try to be a bit more helpful
> to people who want to port git to their system that is not
> HP-UX, though.
> 
> How about wording it like this?
> 
> Subject: pre-POSIX.1-2001 systems do not have <sys/select.h>
> 
> POSIX.1-2001 has declaration of select(2) in <sys/select.h>, but
> in previous version of SUS, it is declared in <sys/time.h>,
> which is already included in git-compat-util.h.
> 
> This introduces NO_SYS_SELECT_H macro in the Makefile to be set
> on older systems, to skip inclusion of <sys/select.h> that does
> not exist on them.
> 
> We could check _POSIX_VERSION with 200112L and do this
> automatically, but earlier it was reported that the approach
> does not work well on some vintage of HP-UX.  Other systems may
> get _POSIX_VERSION itself wrong.  At least for now, this manual
> configuration is safer.

I have no strong opinion on the wording.  Sounds reasonable to me.

Robert

-- 
Robert Schiele
Dipl.-Wirtsch.informatiker	mailto:rschiele@gmail.com

"Quidquid latine dictum sit, altum sonatur."

[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

[PATCH] autoconf: Add test for sys/select.h header file

[Jakub Narebski]

Some systems like HP-UX don't have sys/select.h; the select stuff
is already present in some other headef file (e.g. sys/time.h for
HP-UX).

Companion to
  "some systems don't have (and need) sys/select.h"

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It could be alternately just squashed together with
"some systems don't have (and need) sys/select.h" by Robert Schiele.

Robert, could you please check this patch on HP-UX? It does work
correctly on Linux (which has sys/select.h).

 config.mak.in |    1 +
 configure.ac  |    6 ++++++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/config.mak.in b/config.mak.in
index 40b14d9..ee6c33d 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -30,6 +30,7 @@ NO_CURL=@NO_CURL@
 NO_EXPAT=@NO_EXPAT@
 NEEDS_LIBICONV=@NEEDS_LIBICONV@
 NEEDS_SOCKET=@NEEDS_SOCKET@
+NO_SYS_SELECT_H=@NO_SYS_SELECT_H@
 NO_D_INO_IN_DIRENT=@NO_D_INO_IN_DIRENT@
 NO_D_TYPE_IN_DIRENT=@NO_D_TYPE_IN_DIRENT@
 NO_SOCKADDR_STORAGE=@NO_SOCKADDR_STORAGE@
diff --git a/configure.ac b/configure.ac
index af177fd..85d7ef5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -235,6 +235,12 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
 ## Checks for header files.
 AC_MSG_NOTICE([CHECKS for header files])
 #
+# Define NO_SYS_SELECT_H if you don't have sys/select.h.
+AC_CHECK_HEADER([sys/select.h],
+[NO_SYS_SELECT_H=],
+[NO_SYS_SELECT_H=UnfortunatelyYes])
+AC_SUBST(NO_SYS_SELECT_H)
+#
 # Define OLD_ICONV if your library has an old iconv(), where the second
 # (input buffer pointer) parameter is declared with type (const char **).
 AC_DEFUN([OLDICONVTEST_SRC], [[
-- 
1.5.3.7

Re: [PATCH] autoconf: Add test for sys/select.h header file

[Johannes Schindelin]

Hi,

On Fri, 25 Jan 2008, Jakub Narebski wrote:

> diff --git a/configure.ac b/configure.ac
> index af177fd..85d7ef5 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -235,6 +235,12 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
>  ## Checks for header files.
>  AC_MSG_NOTICE([CHECKS for header files])
>  #
> +# Define NO_SYS_SELECT_H if you don't have sys/select.h.
> +AC_CHECK_HEADER([sys/select.h],
> +[NO_SYS_SELECT_H=],
> +[NO_SYS_SELECT_H=UnfortunatelyYes])
> +AC_SUBST(NO_SYS_SELECT_H)
> +#

Just because I am curious: would that not define "NO_SYS_SELECT_H" in both 
cases?  IOW would the "ifdef NO_SYS_SELECT_H" not be triggered all the 
time?

Thanks,
Dscho

Re: [PATCH] autoconf: Add test for sys/select.h header file

[Robert Schiele]

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

On Fri, Jan 25, 2008 at 12:30:27PM +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 25 Jan 2008, Jakub Narebski wrote:
> 
> > diff --git a/configure.ac b/configure.ac
> > index af177fd..85d7ef5 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -235,6 +235,12 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
> >  ## Checks for header files.
> >  AC_MSG_NOTICE([CHECKS for header files])
> >  #
> > +# Define NO_SYS_SELECT_H if you don't have sys/select.h.
> > +AC_CHECK_HEADER([sys/select.h],
> > +[NO_SYS_SELECT_H=],
> > +[NO_SYS_SELECT_H=UnfortunatelyYes])
> > +AC_SUBST(NO_SYS_SELECT_H)
> > +#
> 
> Just because I am curious: would that not define "NO_SYS_SELECT_H" in both 
> cases?  IOW would the "ifdef NO_SYS_SELECT_H" not be triggered all the 
> time?

No, in make the empty string is equal to not being defined.

Robert

-- 
Robert Schiele
Dipl.-Wirtsch.informatiker	mailto:rschiele@gmail.com

"Quidquid latine dictum sit, altum sonatur."

[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

Re: [PATCH] autoconf: Add test for sys/select.h header file

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Some systems like HP-UX don't have sys/select.h; the select stuff
> is already present in some other headef file (e.g. sys/time.h for
> HP-UX).
>
> Companion to
>   "some systems don't have (and need) sys/select.h"
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> It could be alternately just squashed together with
> "some systems don't have (and need) sys/select.h" by Robert Schiele.
>
> Robert, could you please check this patch on HP-UX? It does work
> correctly on Linux (which has sys/select.h).

The issue is not really about "do we have <sys/select.h>?" but
is about "do we need to include <sys/select.h> in order to
obtain declaration of select(2)?" so this is covering only a
half of the issue.  But I guess we will find out if the system
does not offer select(2) whether <sys/select.h> is included or
not, so this is not too bad.

Will apply.  I do not think it would have problems but even if
it does it is small and easy to revert before the final.

Thanks.

Re: [PATCH] autoconf: Add test for sys/select.h header file

[Robert Schiele]

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

On Fri, Jan 25, 2008 at 10:25:33PM -0800, Junio C Hamano wrote:
> The issue is not really about "do we have <sys/select.h>?" but
> is about "do we need to include <sys/select.h> in order to
> obtain declaration of select(2)?" so this is covering only a

This is true.  Actually HP-UX 11.23 for instance has sys/select.h but this is
not intended to be included to get the select call but this is intended to be
included by kernel drivers that implement the interface the kernel needs to
_provide_ the select call.  In that case the test determines that sys/select.h
exists but it does not notice that it is not the header to include.  It does
not hurt though since including sys/select.h outside of a kernel driver is a
no-op and we include sys/time.h which is really needed anyway.

> half of the issue.  But I guess we will find out if the system
> does not offer select(2) whether <sys/select.h> is included or
> not, so this is not too bad.
> 
> Will apply.  I do not think it would have problems but even if
> it does it is small and easy to revert before the final.

At least on the systems I know it does not hurt even when it does not always
exactly what it is intended to do.

Robert

-- 
Robert Schiele
Dipl.-Wirtsch.informatiker	mailto:rschiele@gmail.com

"Quidquid latine dictum sit, altum sonatur."

[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]