git-compat-util: prefer poll.h to sys/poll.h
PATCH 1/1 git-compat-util: prefer poll.h to sys/poll.h
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>
---
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(-)
Makefile +7 -1
@@ -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 [email protected] 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
configure.ac +6 -0
@@ -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=],
git-compat-util.h +4 -1
@@ -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
Signature · 1 line
2.19.1.856.g8858448bb.dirty
Quoted 10 lines · Đ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 <[email protected]>
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.
Quoted 6 lines
t0028, t1308.23, t3900.34 is failing under musl, Those test cases in question also fails without this patch.
- t0028 is failing because musl
iconvoutput 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.
Quoted 5 lines
- t1308.23 is failing because musl
fopenis 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?
Signature · 2 lines
brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204
Quoted 6 lines · brian m. carlson
Quoted 5 lines · Đ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.
Quoted 8 lines
Quoted 5 lines
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 <[email protected]>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.
Quoted 7 lines · brian m. carlson
Quoted 5 lines
- t1308.23 is failing because musl
fopenis 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_DIRECTORIESfix 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.
Quoted 3 lines
Signature · 2 lines
brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204
Signature · 1 line
Danh
git-compat-util.h line 190
Quoted 9 lines · Đoàn Trần Công Danh
git-compat-util.h +4
-1-#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.