{"thread":{"id":"49829","subject":"[PATCH] git-compat-util: prefer poll.h to sys/poll.h","startedAt":"2018-11-14T01:13:14Z","lastAt":"2018-11-14T07:32:25Z","messageCount":5,"participants":["Đoàn Trần Công Danh","brian m. carlson","Junio C Hamano","Danh Doan"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"363279","messageId":"20181114011043.27419-1-congdanhqx@gmail.com","threadId":"49829","inReplyTo":null,"subject":"[PATCH] git-compat-util: prefer poll.h to sys/poll.h","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2018-11-14T01:10:43Z","receivedAt":"2018-11-14T01:13:14Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"POSIX specifies that <poll.h> is the correct header for poll(2)\nwhereas <sys/poll.h> is only needed for some old libc.\n\nLet's follow the POSIX way by default.\n\nThis effectively eliminates musl's warning:\n\n    warning redirecting incorrect #include <sys/poll.h> to <poll.h>\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\nt0028, t1308.23, t3900.34 is failing under musl,\nThose test cases in question also fails without this patch.\n\n- t0028 is failing because musl `iconv` output UTF-16 without BOM.\nI'm not sure if my installation is broken, or it's musl's default behavior.\nBut, I think RFC2781, section 4.3 allows the missing BOM\n\n- t1308.23 is failing because musl `fopen` is success when open directory\nin readonly mode. POSIX allows this behavior:\nhttp://pubs.opengroup.org/onlinepubs/7908799/xsh/fopen.html\n[EISDIR]\nThe named file is a directory and mode requires write access.\n\n- t3900.34: from what I understand, musl haven't supported ISO-2022-JP, yet.\nhttps://wiki.musl-libc.org/functional-differences-from-glibc.html#iconv\n\n Makefile          | 8 +++++++-\n configure.ac      | 6 ++++++\n git-compat-util.h | 5 ++++-\n 3 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex bbfbb4292..5734efe74 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -207,10 +207,12 @@ all::\n # Define MMAP_PREVENTS_DELETE if a file that is currently mmapped cannot be\n # deleted or cannot be replaced using rename().\n #\n+# Define NO_POLL_H if you don't have poll.h.\n+#\n # Define NO_SYS_POLL_H if you don't have sys/poll.h.\n #\n # Define NO_POLL if you do not have or don't want to use poll().\n-# This also implies NO_SYS_POLL_H.\n+# This also implies NO_POLL_H and NO_SYS_POLL_H.\n #\n # Define NEEDS_SYS_PARAM_H if you need to include sys/param.h to compile,\n # *PLEASE* REPORT to git@vger.kernel.org if your platform needs this;\n@@ -1459,6 +1461,7 @@ ifdef NO_GETTEXT\n \tUSE_GETTEXT_SCHEME ?= fallthrough\n endif\n ifdef NO_POLL\n+\tNO_POLL_H = YesPlease\n \tNO_SYS_POLL_H = YesPlease\n \tCOMPAT_CFLAGS += -DNO_POLL -Icompat/poll\n \tCOMPAT_OBJS += compat/poll/poll.o\n@@ -1497,6 +1500,9 @@ endif\n ifdef NO_SYS_SELECT_H\n \tBASIC_CFLAGS += -DNO_SYS_SELECT_H\n endif\n+ifdef NO_POLL_H\n+\tBASIC_CFLAGS += -DNO_POLL_H\n+endif\n ifdef NO_SYS_POLL_H\n \tBASIC_CFLAGS += -DNO_SYS_POLL_H\n endif\ndiff --git a/configure.ac b/configure.ac\nindex e11b7976a..908e66a97 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -792,6 +792,12 @@ AC_CHECK_HEADER([sys/select.h],\n [NO_SYS_SELECT_H=UnfortunatelyYes])\n GIT_CONF_SUBST([NO_SYS_SELECT_H])\n #\n+# Define NO_POLL_H if you don't have poll.h\n+AC_CHECK_HEADER([poll.h],\n+[NO_POLL_H=],\n+[NO_POLL_H=UnfortunatelyYes])\n+GIT_CONF_SUBST([NO_POLL_H])\n+#\n # Define NO_SYS_POLL_H if you don't have sys/poll.h\n AC_CHECK_HEADER([sys/poll.h],\n [NO_SYS_POLL_H=],\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 96a3f86d8..65f229f10 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -180,9 +180,12 @@\n #include <regex.h>\n #include <utime.h>\n #include <syslog.h>\n-#ifndef NO_SYS_POLL_H\n+#if !defined(NO_POLL_H)\n+#include <poll.h>\n+#elif !defined(NO_SYS_POLL_H)\n #include <sys/poll.h>\n #else\n+/* Pull the compat stuff */\n #include <poll.h>\n #endif\n #ifdef HAVE_BSD_SYSCTL\n-- \n2.19.1.856.g8858448bb.dirty\n\n"},{"id":"363281","messageId":"20181114014307.GM890086@genre.crustytoothpaste.net","threadId":"49829","inReplyTo":"20181114011043.27419-1-congdanhqx@gmail.com","subject":"Re: [PATCH] git-compat-util: prefer poll.h to sys/poll.h","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-11-14T01:43:07Z","receivedAt":"2018-11-14T01:43:17Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Nov 14, 2018 at 08:10:43AM +0700, Đoàn Trần Công Danh wrote:\n> POSIX specifies that <poll.h> is the correct header for poll(2)\n> whereas <sys/poll.h> is only needed for some old libc.\n> \n> Let's follow the POSIX way by default.\n> \n> This effectively eliminates musl's warning:\n> \n>     warning redirecting incorrect #include <sys/poll.h> to <poll.h>\n> \n> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n\nI think this patch is fine.  This was in SUSv2, and I don't feel bad\nabout siding with a spec that's at least 17 years old.\n\n> t0028, t1308.23, t3900.34 is failing under musl,\n> Those test cases in question also fails without this patch.\n> \n> - t0028 is failing because musl `iconv` output UTF-16 without BOM.\n> I'm not sure if my installation is broken, or it's musl's default behavior.\n> But, I think RFC2781, section 4.3 allows the missing BOM\n\nWhile the spec may allow this, we cannot for practical reasons.  There\nare a large number of broken Windows programs that don't honor the spec\nwhen it says to interpret UTF-16 byte sequences without a BOM as\nbig-endian, and instead use little-endian.  Since we cannot fix all the\nbroken Windows programs people use, we need to emit the BOM in UTF-16\nmode.\n\n> - t1308.23 is failing because musl `fopen` is success when open directory\n> in readonly mode. POSIX allows this behavior:\n> http://pubs.opengroup.org/onlinepubs/7908799/xsh/fopen.html\n> [EISDIR]\n> The named file is a directory and mode requires write access.\n\nDoes setting FREAD_READS_DIRECTORIES fix this?\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"363309","messageId":"xmqqr2fop8lr.fsf@gitster-ct.c.googlers.com","threadId":"49829","inReplyTo":"20181114014307.GM890086@genre.crustytoothpaste.net","subject":"Re: [PATCH] git-compat-util: prefer poll.h to sys/poll.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T04:48:48Z","receivedAt":"2018-11-14T04:48:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> On Wed, Nov 14, 2018 at 08:10:43AM +0700, Đoàn Trần Công Danh wrote:\n>> POSIX specifies that <poll.h> is the correct header for poll(2)\n>> whereas <sys/poll.h> is only needed for some old libc.\n>> \n>> Let's follow the POSIX way by default.\n>> \n>> This effectively eliminates musl's warning:\n>> \n>>     warning redirecting incorrect #include <sys/poll.h> to <poll.h>\n>> \n>> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n>\n> I think this patch is fine.  This was in SUSv2, and I don't feel bad\n> about siding with a spec that's at least 17 years old.\n\nYup, I agree.  Thanks, both.\n"},{"id":"363318","messageId":"87h8gkrzpu.fsf@congdanhqx.xyz","threadId":"49829","inReplyTo":"20181114014307.GM890086@genre.crustytoothpaste.net","subject":"Re: [PATCH] git-compat-util: prefer poll.h to sys/poll.h","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2018-11-14T05:32:29Z","receivedAt":"2018-11-14T05:32:40Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n>> - t1308.23 is failing because musl `fopen` is success when open directory\n>> in readonly mode. POSIX allows this behavior:\n>> http://pubs.opengroup.org/onlinepubs/7908799/xsh/fopen.html\n>> [EISDIR]\n>> The named file is a directory and mode requires write access.\n>\n> Does setting FREAD_READS_DIRECTORIES fix this?\n\nYes, setting FREAD_READS_DIRECTORIES fixes this.\nAnd, `configure` can set that itself.\n\nI blindly followed the Void's build template which explicitly unset it\nin the configure script without investigating it.\nMy bad!\n\nI'll send them a patch to fix this downstream.\n\n> -- \n> brian m. carlson: Houston, Texas, US\n> OpenPGP: https://keybase.io/bk2204\n\n-- \nDanh\n"},{"id":"363331","messageId":"xmqq5zx0nmgv.fsf@gitster-ct.c.googlers.com","threadId":"49829","inReplyTo":"20181114011043.27419-1-congdanhqx@gmail.com","subject":"Re: [PATCH] git-compat-util: prefer poll.h to sys/poll.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T07:32:16Z","receivedAt":"2018-11-14T07:32:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n> -#ifndef NO_SYS_POLL_H\n> +#if !defined(NO_POLL_H)\n> +#include <poll.h>\n> +#elif !defined(NO_SYS_POLL_H)\n>  #include <sys/poll.h>\n>  #else\n> +/* Pull the compat stuff */\n>  #include <poll.h>\n>  #endif\n\nThe last comment would help readers who got \"Huh?  When NO_POLL_H\nand NO_SYS_POLL_H is given, we still include <poll.h>\" without it.\n\nLooks good.  Thanks.\n"}]}