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

2018-11-13 Thread Junio C Hamano
Đoàn Trần Công Danh   writes:

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

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

Looks good.  Thanks.


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

2018-11-13 Thread Danh Doan
"brian m. carlson"  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

2018-11-13 Thread Junio C Hamano
"brian m. carlson"  writes:

> On Wed, Nov 14, 2018 at 08:10:43AM +0700, Đoàn Trần Công Danh wrote:
>> POSIX specifies that  is the correct header for poll(2)
>> whereas  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  to 
>> 
>> Signed-off-by: Đoàn Trần Công Danh 
>
> 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

2018-11-13 Thread brian m. carlson
On Wed, Nov 14, 2018 at 08:10:43AM +0700, Đoàn Trần Công Danh wrote:
> POSIX specifies that  is the correct header for poll(2)
> whereas  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  to 
> 
> Signed-off-by: Đoàn Trần Công Danh 

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


signature.asc
Description: PGP signature


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

2018-11-13 Thread Đoàn Trần Công Danh
POSIX specifies that  is the correct header for poll(2)
whereas  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  to 

Signed-off-by: Đoàn Trần Công Danh 
---
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 
 #include 
 #include 
-#ifndef NO_SYS_POLL_H
+#if !defined(NO_POLL_H)
+#include 
+#elif !defined(NO_SYS_POLL_H)
 #include 
 #else
+/* Pull the compat stuff */
 #include 
 #endif
 #ifdef HAVE_BSD_SYSCTL
-- 
2.19.1.856.g8858448bb.dirty