| Message ID | 20181114011043.27419-1-congdanhqx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-compat-util: prefer poll.h to sys/poll.h | expand |
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" <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.
"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
Đ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.
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
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(-)