[PATCH] git-compat-util: prefer poll.h to sys/poll.h

[PATCH] git-compat-util: prefer poll.h to sys/poll.h

[Đoàn Trần Công Danh]

POSIX specifies that <poll.h> is the correct header for poll(2)
whereas <sys/poll.h> is only needed for some old libc.

Let's follow the POSIX way by default.

This effectively eliminates musl's warning:

    warning redirecting incorrect #include <sys/poll.h> to <poll.h>

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
t0028, t1308.23, t3900.34 is failing under musl,
Those test cases in question also fails without this patch.

- t0028 is failing because musl `iconv` output UTF-16 without BOM.
I'm not sure if my installation is broken, or it's musl's default behavior.
But, I think RFC2781, section 4.3 allows the missing BOM

- t1308.23 is failing because musl `fopen` is success when open directory
in readonly mode. POSIX allows this behavior:
http://pubs.opengroup.org/onlinepubs/7908799/xsh/fopen.html
[EISDIR]
The named file is a directory and mode requires write access.

- t3900.34: from what I understand, musl haven't supported ISO-2022-JP, yet.
https://wiki.musl-libc.org/functional-differences-from-glibc.html#iconv

 Makefile          | 8 +++++++-
 configure.ac      | 6 ++++++
 git-compat-util.h | 5 ++++-
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index bbfbb4292..5734efe74 100644
--- a/Makefile
+++ b/Makefile
@@ -207,10 +207,12 @@ all::
 # Define MMAP_PREVENTS_DELETE if a file that is currently mmapped cannot be
 # deleted or cannot be replaced using rename().
 #
+# Define NO_POLL_H if you don't have poll.h.
+#
 # Define NO_SYS_POLL_H if you don't have sys/poll.h.
 #
 # Define NO_POLL if you do not have or don't want to use poll().
-# This also implies NO_SYS_POLL_H.
+# This also implies NO_POLL_H and NO_SYS_POLL_H.
 #
 # Define NEEDS_SYS_PARAM_H if you need to include sys/param.h to compile,
 # *PLEASE* REPORT to git@vger.kernel.org if your platform needs this;
@@ -1459,6 +1461,7 @@ ifdef NO_GETTEXT
 	USE_GETTEXT_SCHEME ?= fallthrough
 endif
 ifdef NO_POLL
+	NO_POLL_H = YesPlease
 	NO_SYS_POLL_H = YesPlease
 	COMPAT_CFLAGS += -DNO_POLL -Icompat/poll
 	COMPAT_OBJS += compat/poll/poll.o
@@ -1497,6 +1500,9 @@ endif
 ifdef NO_SYS_SELECT_H
 	BASIC_CFLAGS += -DNO_SYS_SELECT_H
 endif
+ifdef NO_POLL_H
+	BASIC_CFLAGS += -DNO_POLL_H
+endif
 ifdef NO_SYS_POLL_H
 	BASIC_CFLAGS += -DNO_SYS_POLL_H
 endif
diff --git a/configure.ac b/configure.ac
index e11b7976a..908e66a97 100644
--- a/configure.ac
+++ b/configure.ac
@@ -792,6 +792,12 @@ AC_CHECK_HEADER([sys/select.h],
 [NO_SYS_SELECT_H=UnfortunatelyYes])
 GIT_CONF_SUBST([NO_SYS_SELECT_H])
 #
+# Define NO_POLL_H if you don't have poll.h
+AC_CHECK_HEADER([poll.h],
+[NO_POLL_H=],
+[NO_POLL_H=UnfortunatelyYes])
+GIT_CONF_SUBST([NO_POLL_H])
+#
 # Define NO_SYS_POLL_H if you don't have sys/poll.h
 AC_CHECK_HEADER([sys/poll.h],
 [NO_SYS_POLL_H=],
diff --git a/git-compat-util.h b/git-compat-util.h
index 96a3f86d8..65f229f10 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -180,9 +180,12 @@
 #include <regex.h>
 #include <utime.h>
 #include <syslog.h>
-#ifndef NO_SYS_POLL_H
+#if !defined(NO_POLL_H)
+#include <poll.h>
+#elif !defined(NO_SYS_POLL_H)
 #include <sys/poll.h>
 #else
+/* Pull the compat stuff */
 #include <poll.h>
 #endif
 #ifdef HAVE_BSD_SYSCTL
-- 
2.19.1.856.g8858448bb.dirty

Re: [PATCH] git-compat-util: prefer poll.h to sys/poll.h

[brian m. carlson]

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

On Wed, Nov 14, 2018 at 08:10:43AM +0700, Đoàn Trần Công Danh wrote:
> POSIX specifies that <poll.h> is the correct header for poll(2)
> whereas <sys/poll.h> is only needed for some old libc.
> 
> Let's follow the POSIX way by default.
> 
> This effectively eliminates musl's warning:
> 
>     warning redirecting incorrect #include <sys/poll.h> to <poll.h>
> 
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>

I think this patch is fine.  This was in SUSv2, and I don't feel bad
about siding with a spec that's at least 17 years old.

> t0028, t1308.23, t3900.34 is failing under musl,
> Those test cases in question also fails without this patch.
> 
> - t0028 is failing because musl `iconv` output UTF-16 without BOM.
> I'm not sure if my installation is broken, or it's musl's default behavior.
> But, I think RFC2781, section 4.3 allows the missing BOM

While the spec may allow this, we cannot for practical reasons.  There
are a large number of broken Windows programs that don't honor the spec
when it says to interpret UTF-16 byte sequences without a BOM as
big-endian, and instead use little-endian.  Since we cannot fix all the
broken Windows programs people use, we need to emit the BOM in UTF-16
mode.

> - t1308.23 is failing because musl `fopen` is success when open directory
> in readonly mode. POSIX allows this behavior:
> http://pubs.opengroup.org/onlinepubs/7908799/xsh/fopen.html
> [EISDIR]
> The named file is a directory and mode requires write access.

Does setting FREAD_READS_DIRECTORIES fix this?
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH] git-compat-util: prefer poll.h to sys/poll.h

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On Wed, Nov 14, 2018 at 08:10:43AM +0700, Đoàn Trần Công Danh wrote:
>> POSIX specifies that <poll.h> is the correct header for poll(2)
>> whereas <sys/poll.h> is only needed for some old libc.
>> 
>> Let's follow the POSIX way by default.
>> 
>> This effectively eliminates musl's warning:
>> 
>>     warning redirecting incorrect #include <sys/poll.h> to <poll.h>
>> 
>> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
>
> I think this patch is fine.  This was in SUSv2, and I don't feel bad
> about siding with a spec that's at least 17 years old.

Yup, I agree.  Thanks, both.

Re: [PATCH] git-compat-util: prefer poll.h to sys/poll.h

[Danh Doan]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

>> - t1308.23 is failing because musl `fopen` is success when open directory
>> in readonly mode. POSIX allows this behavior:
>> http://pubs.opengroup.org/onlinepubs/7908799/xsh/fopen.html
>> [EISDIR]
>> The named file is a directory and mode requires write access.
>
> Does setting FREAD_READS_DIRECTORIES fix this?

Yes, setting FREAD_READS_DIRECTORIES fixes this.
And, `configure` can set that itself.

I blindly followed the Void's build template which explicitly unset it
in the configure script without investigating it.
My bad!

I'll send them a patch to fix this downstream.

> -- 
> brian m. carlson: Houston, Texas, US
> OpenPGP: https://keybase.io/bk2204

-- 
Danh

Re: [PATCH] git-compat-util: prefer poll.h to sys/poll.h

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> -#ifndef NO_SYS_POLL_H
> +#if !defined(NO_POLL_H)
> +#include <poll.h>
> +#elif !defined(NO_SYS_POLL_H)
>  #include <sys/poll.h>
>  #else
> +/* Pull the compat stuff */
>  #include <poll.h>
>  #endif

The last comment would help readers who got "Huh?  When NO_POLL_H
and NO_SYS_POLL_H is given, we still include <poll.h>" without it.

Looks good.  Thanks.