{"thread":{"id":"47644","subject":"[PATCH v2 1/6] Bypass GCC attributes on NonStop platform where used.","startedAt":"2018-01-19T17:34:26Z","lastAt":"2018-01-25T03:43:12Z","messageCount":32,"participants":["randall.s.becker@rogers.com","Stefan Beller","Ramsay Jones","Jeff King","Randall S. Becker","Torsten Bögershausen","René Scharfe","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":6},"messages":[{"id":"336902","messageId":"20180119173406.13324-3-randall.s.becker@rogers.com","threadId":"47644","inReplyTo":"20180119173406.13324-1-randall.s.becker@rogers.com","subject":"[PATCH v2 1/6] Bypass GCC attributes on NonStop platform where used.","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2018-01-19T17:34:01Z","receivedAt":"2018-01-19T17:34:26Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n\n* remote.c: force ignoring of GCC __attribute construct not supported\n  by c99 by defining it as an empty CPP macro.\n\nSigned-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n---\n remote.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/remote.c b/remote.c\nindex 4e93753e1..c18f9de7f 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -11,6 +11,10 @@\n #include \"mergesort.h\"\n #include \"argv-array.h\"\n \n+#if defined (__TANDEM)\n+#define __attribute(a)\n+#endif\n+\n enum map_direction { FROM_SRC, FROM_DST };\n \n static struct refspec s_tag_refspec = {\n-- \n2.16.0.31.gf1a482c\n\n"},{"id":"336903","messageId":"20180119173406.13324-2-randall.s.becker@rogers.com","threadId":"47644","inReplyTo":"20180119173406.13324-1-randall.s.becker@rogers.com","subject":"[PATCH v2 0/6] Cover Letter for NonStop Port Patches","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2018-01-19T17:34:00Z","receivedAt":"2018-01-19T17:34:30Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n\nThis is the second attempt at submission of the NonStop port to\nthe git team. This package is split by file but should be applied\natomically.\n\nI am not happy with the change in compat/regex/regcomp.c and figure\nthis might change with feedback.\n\nSincerely,\nRandall\n\nRandall S. Becker (7):\n  Force pipes to flush immediately on NonStop platform\n  Bypass GCC attributes on NonStop platform where used.\n  Add tar extract install options override in installation processing.\n  Define config options required for the HPE NonStop NSX and NSE\n    platforms\n  Force test suite traps to be cleared for NonStop ksh\n  Bring NonStop platform definitions up to date in git-compat-util.h\n  Add intptr_t and uintptr_t to regcomp.c for NonStop platform.\n\n Makefile               |  6 +++++-\n compat/regex/regcomp.c |  8 ++++++++\n config.mak.uname       | 29 +++++++++++++++++++++--------\n git-compat-util.h      | 15 +++++++++++++++\n remote.c               |  4 ++++\n t/lib-git-daemon.sh    |  3 +++\n wrapper.c              |  3 +++\n 7 files changed, 59 insertions(+), 9 deletions(-)\n\n-- \n2.16.0.31.gf1a482c\n\n"},{"id":"336904","messageId":"20180119173406.13324-1-randall.s.becker@rogers.com","threadId":"47644","inReplyTo":null,"subject":"[PATCH v2 0/6] Force pipes to flush immediately on NonStop platform","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2018-01-19T17:33:59Z","receivedAt":"2018-01-19T17:34:33Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n\n* wrapper.c: called setbuf(stream,0) to force pipe flushes not enabled by\n  default on the NonStop platform.\n\nSigned-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n---\n wrapper.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex d20356a77..671cbb4b4 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -403,6 +403,9 @@ FILE *xfdopen(int fd, const char *mode)\n \tFILE *stream = fdopen(fd, mode);\n \tif (stream == NULL)\n \t\tdie_errno(\"Out of memory? fdopen failed\");\n+#ifdef __TANDEM\n+\tsetbuf(stream,0);\n+#endif\n \treturn stream;\n }\n \n-- \n2.16.0.31.gf1a482c\n\n"},{"id":"336905","messageId":"20180119173406.13324-6-randall.s.becker@rogers.com","threadId":"47644","inReplyTo":"20180119173406.13324-1-randall.s.becker@rogers.com","subject":"[PATCH v2 4/6] Force test suite traps to be cleared for NonStop ksh","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2018-01-19T17:34:04Z","receivedAt":"2018-01-19T17:34:42Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n\n* t/lib-git-daemon.sh: fix incompatibilities with ksh traps not being\n  cleared automatically on platform. This caused tests to seem to fail\n  while actually succeeding.\n\nSigned-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n---\n t/lib-git-daemon.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\nindex 987d40680..955beecd9 100644\n--- a/t/lib-git-daemon.sh\n+++ b/t/lib-git-daemon.sh\n@@ -68,6 +68,7 @@ start_git_daemon() {\n \t\ttest_skip_or_die $GIT_TEST_GIT_DAEMON \\\n \t\t\t\"git daemon failed to start\"\n \tfi\n+\ttrap '' EXIT\n }\n \n stop_git_daemon() {\n@@ -89,4 +90,6 @@ stop_git_daemon() {\n \tfi\n \tGIT_DAEMON_PID=\n \trm -f git_daemon_output\n+\n+\ttrap '' EXIT\n }\n-- \n2.16.0.31.gf1a482c\n\n"},{"id":"336906","messageId":"20180119173406.13324-8-randall.s.becker@rogers.com","threadId":"47644","inReplyTo":"20180119173406.13324-1-randall.s.becker@rogers.com","subject":"[PATCH v2 6/6] Add intptr_t and uintptr_t to regcomp.c for NonStop platform.","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2018-01-19T17:34:06Z","receivedAt":"2018-01-19T17:34:45Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n\n* compat/regex/regcomp.c: fix missing intptr_t on NonStop. This is\n  done because git-compat-util.h cannot be cleanly included into\n  this file without additional compile errors.\n\nSigned-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n---\n compat/regex/regcomp.c | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c\nindex 51cd60baa..8bb4c966d 100644\n--- a/compat/regex/regcomp.c\n+++ b/compat/regex/regcomp.c\n@@ -17,6 +17,14 @@\n    License along with the GNU C Library; if not, see\n    <http://www.gnu.org/licenses/>.  */\n \n+#if defined __TANDEM\n+/* This is currently duplicated from git-compat-utils.h */\n+#ifdef NO_INTPTR_T\n+typedef long intptr_t;\n+typedef unsigned long uintptr_t;\n+#endif\n+#endif\n+\n static reg_errcode_t re_compile_internal (regex_t *preg, const char * pattern,\n \t\t\t\t\t  size_t length, reg_syntax_t syntax);\n static void re_compile_fastmap_iter (regex_t *bufp,\n-- \n2.16.0.31.gf1a482c\n\n"},{"id":"336907","messageId":"20180119173406.13324-7-randall.s.becker@rogers.com","threadId":"47644","inReplyTo":"20180119173406.13324-1-randall.s.becker@rogers.com","subject":"[PATCH v2 5/6] Bring NonStop platform definitions up to date in git-compat-util.h","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2018-01-19T17:34:05Z","receivedAt":"2018-01-19T17:34:52Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n\n* git-compat-util.h: add correct FLOSS definitions to allow correct\n  emulation of non-platform behaviour. Add NSIG definition that is\n  not explicitly supplied in signal.h on platform.\n\nSigned-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n---\n git-compat-util.h | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 68b2ad531..1e13f050a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -378,6 +378,21 @@ static inline char *git_find_last_dir_sep(const char *path)\n #define find_last_dir_sep git_find_last_dir_sep\n #endif\n \n+#ifdef __TANDEM\n+#if !defined(_THREAD_SUPPORT_FUNCTIONS) && !defined(_PUT_MODEL_)\n+/* #include <floss.h(floss_read,floss_write,floss_fsync,floss_fork)> */\n+/* #include <floss.h(floss_fork)> */\n+#endif\n+#include <floss.h(floss_execl,floss_execlp,floss_execv,floss_execvp)>\n+#include <floss.h(floss_getpwuid)>\n+#if ! defined NSIG\n+/* NonStop NSE and NSX do not provide NSIG. SIGGUARDIAN(99) is the highest\n+   known, by detective work using kill -l as a list of all signals\n+   instead of signal.h where it should be. */\n+# define NSIG 100\n+#endif\n+#endif\n+\n #if defined(__HP_cc) && (__HP_cc >= 61000)\n #define NORETURN __attribute__((noreturn))\n #define NORETURN_PTR\n-- \n2.16.0.31.gf1a482c\n\n"},{"id":"336908","messageId":"20180119173406.13324-4-randall.s.becker@rogers.com","threadId":"47644","inReplyTo":"20180119173406.13324-1-randall.s.becker@rogers.com","subject":"[PATCH v2 2/6] Add tar extract install options override in installation processing.","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2018-01-19T17:34:02Z","receivedAt":"2018-01-19T17:34:57Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n\n* Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to be\n  specified if needed. The default is xof.\n\nSigned-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n---\n Makefile | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 1a9b23b67..040e9eacd 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -429,6 +429,9 @@ all::\n # running the test scripts (e.g., bash has better support for \"set -x\"\n # tracing).\n #\n+# Define TAR_EXTRACT_OPTIONS if you want to change the default behaviour\n+# from xvf to something else during installation.\n+#\n # When cross-compiling, define HOST_CPU as the canonical name of the CPU on\n # which the built Git will run (for instance \"x86_64\").\n \n@@ -452,6 +455,7 @@ LDFLAGS =\n ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n STRIP ?= strip\n+TAR_EXTRACT_OPTIONS = xof\n \n # Create as necessary, replace existing, make ranlib unneeded.\n ARFLAGS = rcs\n@@ -2569,7 +2573,7 @@ install: all\n ifndef NO_GETTEXT\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'\n \t(cd po/build/locale && $(TAR) cf - .) | \\\n-\t(cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -)\n+\t(cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) $(TAR_EXTRACT_OPTIONS) -)\n endif\n ifndef NO_PERL\n \t$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install\n-- \n2.16.0.31.gf1a482c\n\n"},{"id":"336909","messageId":"20180119173406.13324-5-randall.s.becker@rogers.com","threadId":"47644","inReplyTo":"20180119173406.13324-1-randall.s.becker@rogers.com","subject":"[PATCH v2 3/6] Define config options required for the HPE NonStop NSX and NSE platforms","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2018-01-19T17:34:03Z","receivedAt":"2018-01-19T17:35:12Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n\n* config.mak.uname: upgrade old options to currently supported\n  NonStop operating system versions (J06.21 and L17.xx).\n\nSigned-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n---\n config.mak.uname | 29 +++++++++++++++++++++--------\n 1 file changed, 21 insertions(+), 8 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 685a80d13..d9f8d57e3 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -428,27 +428,37 @@ ifeq ($(uname_S),NONSTOP_KERNEL)\n \t# INLINE='' would just replace one set of warnings with another and\n \t# still not compile in c89 mode, due to non-const array initializations.\n \tCC = cc -c99\n+\t# Build down-rev compatible objects that don't use our new getopt_long.\n+\tifeq ($(uname_R).$(uname_V),J06.21)\n+\t\tCC += -WRVU=J06.20\n+\tendif\n+\tifeq ($(uname_R).$(uname_V),L17.02)\n+\t\tCC += -WRVU=L16.05\n+\tendif\n \t# Disable all optimization, seems to result in bad code, with -O or -O2\n \t# or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects\n \t# abends on \"git push\". Needs more investigation.\n-\tCFLAGS = -g -O0\n+\tCFLAGS = -g -O0 -Winline\n \t# We'd want it to be here.\n \tprefix = /usr/local\n \t# Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl).\n-\tPERL_PATH = ${prefix}/bin/perl\n-\tPYTHON_PATH = ${prefix}/bin/python\n-\n+\tPERL_PATH = /usr/bin/perl\n+\tPYTHON_PATH = /usr/bin/python\n+\tRM = /bin/rm -f\n \t# As detected by './configure'.\n \t# Missdetected, hence commented out, see below.\n \t#NO_CURL = YesPlease\n \t# Added manually, see above.\n \tNEEDS_SSL_WITH_CURL = YesPlease\n+\tNEEDS_CRYPTO_WITH_SSL = YesPlease\n+\tHAVE_DEV_TTY = YesPlease\n \tHAVE_LIBCHARSET_H = YesPlease\n \tHAVE_STRINGS_H = YesPlease\n \tNEEDS_LIBICONV = YesPlease\n \tNEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease\n \tNO_SYS_SELECT_H = UnfortunatelyYes\n \tNO_D_TYPE_IN_DIRENT = YesPlease\n+\tNO_GETTEXT = YesPlease\n \tNO_HSTRERROR = YesPlease\n \tNO_STRCASESTR = YesPlease\n \tNO_MEMMEM = YesPlease\n@@ -458,8 +468,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL)\n \tNO_MKDTEMP = YesPlease\n \t# Currently libiconv-1.9.1.\n \tOLD_ICONV = UnfortunatelyYes\n-\tNO_REGEX = YesPlease\n+\tNO_REGEX=NeedsStartEnd\n \tNO_PTHREADS = UnfortunatelyYes\n+\tifdef NO_PTHREADS\n+\telse # WIP, use Posix User Threads\n+\t\tPTHREAD_CFLAGS = -D_PUT_MODEL_ -I/usr/include\n+\t\tPTHREAD_LIBS = -lput\n+\tendif\n \n \t# Not detected (nor checked for) by './configure'.\n \t# We don't have SA_RESTART on NonStop, unfortunalety.\n@@ -477,9 +492,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)\n \t# RFE 10-120912-4693 submitted to HP NonStop development.\n \tNO_SETITIMER = UnfortunatelyYes\n \tSANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin\n-\tSHELL_PATH = /usr/local/bin/bash\n-\t# as of H06.25/J06.14, we might better use this\n-\t#SHELL_PATH = /usr/coreutils/bin/bash\n+\tSHELL_PATH = /usr/coreutils/bin/bash\n endif\n ifneq (,$(findstring MINGW,$(uname_S)))\n \tpathsep = ;\n-- \n2.16.0.31.gf1a482c\n\n"},{"id":"336927","messageId":"CAGZ79kY4HzL8ed23RmoCU0izjsRKsZ0Mkzcmpm3+ugu7qZtcLA@mail.gmail.com","threadId":"47644","inReplyTo":"20180119173406.13324-3-randall.s.becker@rogers.com","subject":"Re: [PATCH v2 1/6] Bypass GCC attributes on NonStop platform where used.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-01-19T19:25:28Z","receivedAt":"2018-01-19T19:25:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jan 19, 2018 at 9:34 AM,  <randall.s.becker@rogers.com> wrote:\n> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n>\n> * remote.c: force ignoring of GCC __attribute construct not supported\n>   by c99 by defining it as an empty CPP macro.\n>\n> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n\nI do not know about the __TANDEM symbol, but some research[1]\nindicates it is a macro specifically for case of cross platform support:\n\n    You can use the __TANDEM symbol to increase the portability\n    of your programs. Enclose system-dependent source text in an\n    if section that uses #ifdef or #ifndef to test for the existence of the\n    __TANDEM symbol.\n\nIt seems to not be used outside of the NonStop port[2], so the code seems\nok. (I would have used #ifdef, as I thought this is more prevalent in our\ncode base and for consistency we'd rather want to use one thing only, bug\n\"git grep '#if defined'\" proves me wrong)\n\nHowever the commit message is hard to understand (say for somebody who\nwill find this commit in 2 years using git-blame).\n\nMaybe:\n\n    In NonStop HPE (version X, all versions?) there is no support for the\n    __attribute macro. By redefining the __attribute to an empty macro, the\n    code compiles on NonStop HPE. Use the system specific __TANDEM\n    macro to guard it for just this platform.\n\nas that will help those people in the future not having to do the research?\n\n\n[1] http://h20628.www2.hp.com/km-ext/kmcsdirect/emr_na-c02121175-1.pdf\n[2] https://sourceforge.net/p/predef/wiki/OperatingSystems/?version=44\n\nThanks,\nStefan\n\n\n> ---\n>  remote.c | 4 ++++\n>  1 file changed, 4 insertions(+)\n>\n> diff --git a/remote.c b/remote.c\n> index 4e93753e1..c18f9de7f 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -11,6 +11,10 @@\n>  #include \"mergesort.h\"\n>  #include \"argv-array.h\"\n>\n> +#if defined (__TANDEM)\n> +#define __attribute(a)\n> +#endif\n> +\n>  enum map_direction { FROM_SRC, FROM_DST };\n>\n>  static struct refspec s_tag_refspec = {\n> --\n> 2.16.0.31.gf1a482c\n>\n"},{"id":"336928","messageId":"CAGZ79kYUBRueisw2exRC7E-mpHZYE0Y07nnT4CNF_daAYMna9g@mail.gmail.com","threadId":"47644","inReplyTo":"20180119173406.13324-1-randall.s.becker@rogers.com","subject":"Re: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-01-19T19:34:34Z","receivedAt":"2018-01-19T19:34:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jan 19, 2018 at 9:33 AM,  <randall.s.becker@rogers.com> wrote:\n> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n>\n> * wrapper.c: called setbuf(stream,0) to force pipe flushes not enabled by\n>   default on the NonStop platform.\n>\n\nDue to my review of a previous patch, I now know about the __TANDEM\ndirective and why we use this to guard this line. :)\n\nThe setbuf(0) is easier than sprinkling (guarded) flushes all over the code,\nso that seems like a clean solution, which we currently don't use.\n(it occurred twice in history, see eac14f8909 (Win32: Thread-safe\nwindows console output, 2012-01-14) for its introduction and fcd428f4a9\n(Win32: fix broken pipe detection, 2012-03-01) for its deletion).\n\n> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> ---\n>  wrapper.c | 3 +++\n>  1 file changed, 3 insertions(+)\n>\n> diff --git a/wrapper.c b/wrapper.c\n> index d20356a77..671cbb4b4 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -403,6 +403,9 @@ FILE *xfdopen(int fd, const char *mode)\n>         FILE *stream = fdopen(fd, mode);\n>         if (stream == NULL)\n>                 die_errno(\"Out of memory? fdopen failed\");\n> +#ifdef __TANDEM\n> +       setbuf(stream,0);\n\nMy man page tells me the second arg is a pointer,\nso we'd prefer NULL instead?\n\nThanks,\nStefan\n\n> +#endif\n>         return stream;\n>  }\n>\n> --\n> 2.16.0.31.gf1a482c\n>\n"},{"id":"336929","messageId":"CAGZ79kYft+6k3YjyMn32-gJwFEwCpEYKJ=WnksqJdQaozm5PQw@mail.gmail.com","threadId":"47644","inReplyTo":"20180119173406.13324-6-randall.s.becker@rogers.com","subject":"Re: [PATCH v2 4/6] Force test suite traps to be cleared for NonStop ksh","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-01-19T19:55:15Z","receivedAt":"2018-01-19T19:55:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jan 19, 2018 at 9:34 AM,  <randall.s.becker@rogers.com> wrote:\n> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n>\n> * t/lib-git-daemon.sh: fix incompatibilities with ksh traps not being\n>   cleared automatically on platform.\n\nWhich platform? Do we need to guard it for other platforms?\n(I guess we don't we have traps in some other places and it is\nPOSIX)\n\n>  This caused tests to seem to fail\n>   while actually succeeding.\n>\n> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> ---\n>  t/lib-git-daemon.sh | 3 +++\n>  1 file changed, 3 insertions(+)\n>\n> diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\n> index 987d40680..955beecd9 100644\n> --- a/t/lib-git-daemon.sh\n> +++ b/t/lib-git-daemon.sh\n> @@ -68,6 +68,7 @@ start_git_daemon() {\n>                 test_skip_or_die $GIT_TEST_GIT_DAEMON \\\n>                         \"git daemon failed to start\"\n>         fi\n> +       trap '' EXIT\n>  }\n>\n>  stop_git_daemon() {\n> @@ -89,4 +90,6 @@ stop_git_daemon() {\n>         fi\n>         GIT_DAEMON_PID=\n>         rm -f git_daemon_output\n> +\n> +       trap '' EXIT\n>  }\n> --\n> 2.16.0.31.gf1a482c\n>\n"},{"id":"336932","messageId":"86c7345f-814d-8d81-bb55-e91fe630e118@ramsayjones.plus.com","threadId":"47644","inReplyTo":"20180119173406.13324-3-randall.s.becker@rogers.com","subject":"Re: [PATCH v2 1/6] Bypass GCC attributes on NonStop platform where used.","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-01-19T20:28:48Z","receivedAt":"2018-01-19T20:28:55Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 19/01/18 17:34, randall.s.becker@rogers.com wrote:\n> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> \n> * remote.c: force ignoring of GCC __attribute construct not supported\n>   by c99 by defining it as an empty CPP macro.\n> \n> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> ---\n>  remote.c | 4 ++++\n>  1 file changed, 4 insertions(+)\n> \n> diff --git a/remote.c b/remote.c\n> index 4e93753e1..c18f9de7f 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -11,6 +11,10 @@\n>  #include \"mergesort.h\"\n>  #include \"argv-array.h\"\n>  \n> +#if defined (__TANDEM)\n> +#define __attribute(a)\n> +#endif\n> +\n\nHmm, the only use of __attribute() I can find is in compat/regex/.\nIn particular, there is no use of __attribute() in regex.c.\n[__attribute__() is used in regex.c]\n\nIs this an old patch which is no longer required?\n\npuzzled.\n\nATB,\nRamsay Jones\n\n>  enum map_direction { FROM_SRC, FROM_DST };\n>  \n>  static struct refspec s_tag_refspec = {\n> \n"},{"id":"336938","messageId":"20180119212044.GA10303@sigill.intra.peff.net","threadId":"47644","inReplyTo":"86c7345f-814d-8d81-bb55-e91fe630e118@ramsayjones.plus.com","subject":"Re: [PATCH v2 1/6] Bypass GCC attributes on NonStop platform where used.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-19T21:20:44Z","receivedAt":"2018-01-19T21:20:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 19, 2018 at 08:28:48PM +0000, Ramsay Jones wrote:\n\n> > diff --git a/remote.c b/remote.c\n> > index 4e93753e1..c18f9de7f 100644\n> > --- a/remote.c\n> > +++ b/remote.c\n> > @@ -11,6 +11,10 @@\n> >  #include \"mergesort.h\"\n> >  #include \"argv-array.h\"\n> >  \n> > +#if defined (__TANDEM)\n> > +#define __attribute(a)\n> > +#endif\n> > +\n> \n> Hmm, the only use of __attribute() I can find is in compat/regex/.\n> In particular, there is no use of __attribute() in regex.c.\n> [__attribute__() is used in regex.c]\n> \n> Is this an old patch which is no longer required?\n> \n> puzzled.\n\nI'm puzzled, too. The actual gcc thing is __attribute__(), and we\nalready turn that into a noop via macro expansion if __GNUC__ is not\ndefined (in git-compat-util.h, but see below).\n\n__attribute(), without the trailing underscores, is used internally by\nthe regex compat code (but it also converts that into a noop on non-GNUC\nplatforms)>\n\nHowever the logic in git-compat-util is weird:\n\n  #if defined(__HP_cc) && (__HP_cc >= 61000)\n  #define NORETURN __attribute__((noreturn))\n  #define NORETURN_PTR\n  #elif defined(__GNUC__) && !defined(NO_NORETURN)\n  #define NORETURN __attribute__((__noreturn__))\n  #define NORETURN_PTR __attribute__((__noreturn__))\n  #elif defined(_MSC_VER)\n  #define NORETURN __declspec(noreturn)\n  #define NORETURN_PTR\n  #else\n  #define NORETURN\n  #define NORETURN_PTR\n  #ifndef __GNUC__\n  #ifndef __attribute__\n  #define __attribute__(x)\n  #endif\n  #endif\n  #endif\n\nMost of the conditional is dealing with NORETURN, but then we stick the\n__attribute()__ handling in the \"else\" block. Is it possible that this\nplatform triggers __HP_cc, but doesn't actually understand\n__attribute__?\n\n-Peff\n"},{"id":"336939","messageId":"20180119212716.GB10303@sigill.intra.peff.net","threadId":"47644","inReplyTo":"20180119173406.13324-6-randall.s.becker@rogers.com","subject":"Re: [PATCH v2 4/6] Force test suite traps to be cleared for NonStop ksh","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-19T21:27:17Z","receivedAt":"2018-01-19T21:27:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 19, 2018 at 12:34:04PM -0500, randall.s.becker@rogers.com wrote:\n\n> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> \n> * t/lib-git-daemon.sh: fix incompatibilities with ksh traps not being\n>   cleared automatically on platform. This caused tests to seem to fail\n>   while actually succeeding.\n> \n> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> ---\n>  t/lib-git-daemon.sh | 3 +++\n>  1 file changed, 3 insertions(+)\n> \n> diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\n> index 987d40680..955beecd9 100644\n> --- a/t/lib-git-daemon.sh\n> +++ b/t/lib-git-daemon.sh\n> @@ -68,6 +68,7 @@ start_git_daemon() {\n>  \t\ttest_skip_or_die $GIT_TEST_GIT_DAEMON \\\n>  \t\t\t\"git daemon failed to start\"\n>  \tfi\n> +\ttrap '' EXIT\n>  }\n\nI don't think this can be right. The way these traps are supposed to\nwork is:\n\n  - as soon as test-lib.sh is loaded, we \"trap die EXIT\" to catch an\n    accidental death/exit from one of the scripts\n\n  - when test_done is called, we clear the trap (i.e., it is OK to exit\n    now because the script has given us a positive indication that all\n    tests have been run)\n\n  - while the child git-daemon is running, we'd want to clean up after\n    ourselves. So during start_git_daemon() we add our cleanup (followed\n    by the original \"die\", because shell traps don't push onto a stack).\n    And then at stop_git_daemon(), we restore the original die trap.\n\nBut this patch means that while git-daemon is running, we have no trap\nat all! That means that a failed test which causes us to exit would\nleave a child daemon running.\n\nFurthermore, both of these functions now drop the 'die' trap entirely,\nmeaning the script would fail to notice premature exit from one of the\ntest snippets.\n\nSo while this may be papering over a problem on NonStop, I think it's\nbreaking the intent of the traps.\n\nI'm not sure what the root of the problem is, but it sounds like ksh may\nbe triggering EXIT traps when we don't expect (during function returns?\nSubshell exits? Other?)\n\n-Peff\n"},{"id":"336948","messageId":"000c01d39174$eacfff10$c06ffd30$@nexbridge.com","threadId":"47644","inReplyTo":"20180119212716.GB10303@sigill.intra.peff.net","subject":"RE: [PATCH v2 4/6] Force test suite traps to be cleared for NonStop ksh","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-19T22:29:00Z","receivedAt":"2018-01-19T22:29:20Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 19, 2018 4:27 PM, Jeff King wrote:\n> On Fri, Jan 19, 2018 at 12:34:04PM -0500, randall.s.becker@rogers.com wrote:\n> \n> > From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> >\n> > * t/lib-git-daemon.sh: fix incompatibilities with ksh traps not being\n> >   cleared automatically on platform. This caused tests to seem to fail\n> >   while actually succeeding.\n> >\n> > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> > ---\n> >  t/lib-git-daemon.sh | 3 +++\n> >  1 file changed, 3 insertions(+)\n> >\n> > diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index\n> > 987d40680..955beecd9 100644\n> > --- a/t/lib-git-daemon.sh\n> > +++ b/t/lib-git-daemon.sh\n> > @@ -68,6 +68,7 @@ start_git_daemon() {\n> >  \t\ttest_skip_or_die $GIT_TEST_GIT_DAEMON \\\n> >  \t\t\t\"git daemon failed to start\"\n> >  \tfi\n> > +\ttrap '' EXIT\n> >  }\n> \n> I don't think this can be right. The way these traps are supposed to work is:\n> \n>   - as soon as test-lib.sh is loaded, we \"trap die EXIT\" to catch an\n>     accidental death/exit from one of the scripts\n> \n>   - when test_done is called, we clear the trap (i.e., it is OK to exit\n>     now because the script has given us a positive indication that all\n>     tests have been run)\n> \n>   - while the child git-daemon is running, we'd want to clean up after\n>     ourselves. So during start_git_daemon() we add our cleanup (followed\n>     by the original \"die\", because shell traps don't push onto a stack).\n>     And then at stop_git_daemon(), we restore the original die trap.\n> \n> But this patch means that while git-daemon is running, we have no trap at all!\n> That means that a failed test which causes us to exit would leave a child\n> daemon running.\n> \n> Furthermore, both of these functions now drop the 'die' trap entirely,\n> meaning the script would fail to notice premature exit from one of the test\n> snippets.\n> \n> So while this may be papering over a problem on NonStop, I think it's\n> breaking the intent of the traps.\n> \n> I'm not sure what the root of the problem is, but it sounds like ksh may be\n> triggering EXIT traps when we don't expect (during function returns?\n> Subshell exits? Other?)\n\nThe \"unexpected\" EXIT traps are exactly the issue we found when working with the platform support team. There was some discussion about what the right expectation was, and I was not able to have a change made to ksh on the platform. The problem may have a similar (identical?) root cause to the Perl exit issues we are also experiencing that is in their hands. I'm currently retesting without this change (results in 36 hours ☹ ). Is there a preferred way you would like me to bypass this except on NonStop? I can add a check based on uname.\n\nCheers,\nRandall\n\n"},{"id":"336951","messageId":"f2fd4d75-259b-a5b0-a7f9-74fec79cc970@ramsayjones.plus.com","threadId":"47644","inReplyTo":"20180119212044.GA10303@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/6] Bypass GCC attributes on NonStop platform where used.","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-01-19T22:43:08Z","receivedAt":"2018-01-19T22:43:16Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 19/01/18 21:20, Jeff King wrote:\n> On Fri, Jan 19, 2018 at 08:28:48PM +0000, Ramsay Jones wrote:\n> \n>>> diff --git a/remote.c b/remote.c\n>>> index 4e93753e1..c18f9de7f 100644\n>>> --- a/remote.c\n>>> +++ b/remote.c\n>>> @@ -11,6 +11,10 @@\n>>>  #include \"mergesort.h\"\n>>>  #include \"argv-array.h\"\n>>>  \n>>> +#if defined (__TANDEM)\n>>> +#define __attribute(a)\n>>> +#endif\n>>> +\n>>\n>> Hmm, the only use of __attribute() I can find is in compat/regex/.\n>> In particular, there is no use of __attribute() in regex.c.\n>> [__attribute__() is used in regex.c]\n>>\n>> Is this an old patch which is no longer required?\n>>\n>> puzzled.\n\nheh, I only just noticed that I (twice) wrote regex.c when I meant\nremote.c instead. Hopefully, that didn't cause too much confusion!\n\n> I'm puzzled, too. The actual gcc thing is __attribute__(), and we\n> already turn that into a noop via macro expansion if __GNUC__ is not\n> defined (in git-compat-util.h, but see below).\n> \n> __attribute(), without the trailing underscores, is used internally by\n> the regex compat code (but it also converts that into a noop on non-GNUC\n> platforms)>\n\nIndeed.\n\n> However the logic in git-compat-util is weird:\n> \n>   #if defined(__HP_cc) && (__HP_cc >= 61000)\n>   #define NORETURN __attribute__((noreturn))\n>   #define NORETURN_PTR\n>   #elif defined(__GNUC__) && !defined(NO_NORETURN)\n>   #define NORETURN __attribute__((__noreturn__))\n>   #define NORETURN_PTR __attribute__((__noreturn__))\n>   #elif defined(_MSC_VER)\n>   #define NORETURN __declspec(noreturn)\n>   #define NORETURN_PTR\n>   #else\n>   #define NORETURN\n>   #define NORETURN_PTR\n>   #ifndef __GNUC__\n>   #ifndef __attribute__\n>   #define __attribute__(x)\n>   #endif\n>   #endif\n>   #endif\n> \n> Most of the conditional is dealing with NORETURN, but then we stick the\n> __attribute()__ handling in the \"else\" block. Is it possible that this\n> platform triggers __HP_cc, but doesn't actually understand\n> __attribute__?\n\nThat seems unlikely. However, that conditional looks a mess ... ;-)\n\nATB,\nRamsay Jones\n\n\n"},{"id":"336952","messageId":"001201d39178$472d71e0$d58855a0$@nexbridge.com","threadId":"47644","inReplyTo":"f2fd4d75-259b-a5b0-a7f9-74fec79cc970@ramsayjones.plus.com","subject":"RE: [PATCH v2 1/6] Bypass GCC attributes on NonStop platform where used.","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-19T22:53:04Z","receivedAt":"2018-01-19T22:53:20Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 19, 2018 5:43 PM, Ramsay Jones wrote:\n> On 19/01/18 21:20, Jeff King wrote:\n> > On Fri, Jan 19, 2018 at 08:28:48PM +0000, Ramsay Jones wrote:\n> >\n> >>> diff --git a/remote.c b/remote.c\n> >>> index 4e93753e1..c18f9de7f 100644\n> >>> --- a/remote.c\n> >>> +++ b/remote.c\n> >>> @@ -11,6 +11,10 @@\n> >>>  #include \"mergesort.h\"\n> >>>  #include \"argv-array.h\"\n> >>>\n> >>> +#if defined (__TANDEM)\n> >>> +#define __attribute(a)\n> >>> +#endif\n> >>> +\n> >>\n> >> Hmm, the only use of __attribute() I can find is in compat/regex/.\n> >> In particular, there is no use of __attribute() in regex.c.\n> >> [__attribute__() is used in regex.c]\n> >>\n> >> Is this an old patch which is no longer required?\n> >>\n> >> puzzled.\n> \n> heh, I only just noticed that I (twice) wrote regex.c when I meant remote.c\n> instead. Hopefully, that didn't cause too much confusion!\n> \n> > I'm puzzled, too. The actual gcc thing is __attribute__(), and we\n> > already turn that into a noop via macro expansion if __GNUC__ is not\n> > defined (in git-compat-util.h, but see below).\n> >\n> > __attribute(), without the trailing underscores, is used internally by\n> > the regex compat code (but it also converts that into a noop on\n> > non-GNUC platforms)>\n> \n> Indeed.\n> \n> > However the logic in git-compat-util is weird:\n> >\n> >   #if defined(__HP_cc) && (__HP_cc >= 61000)\n> >   #define NORETURN __attribute__((noreturn))\n> >   #define NORETURN_PTR\n> >   #elif defined(__GNUC__) && !defined(NO_NORETURN)\n> >   #define NORETURN __attribute__((__noreturn__))\n> >   #define NORETURN_PTR __attribute__((__noreturn__))\n> >   #elif defined(_MSC_VER)\n> >   #define NORETURN __declspec(noreturn)\n> >   #define NORETURN_PTR\n> >   #else\n> >   #define NORETURN\n> >   #define NORETURN_PTR\n> >   #ifndef __GNUC__\n> >   #ifndef __attribute__\n> >   #define __attribute__(x)\n> >   #endif\n> >   #endif\n> >   #endif\n> >\n> > Most of the conditional is dealing with NORETURN, but then we stick\n> > the __attribute()__ handling in the \"else\" block. Is it possible that\n> > this platform triggers __HP_cc, but doesn't actually understand\n> > __attribute__?\n> \n> That seems unlikely. However, that conditional looks a mess ... ;-)\n\nVery messy and confusing and it is working properly now, so... consider this patch gone ;-)\n\nCheers,\nRandall\n\n"},{"id":"336957","messageId":"001d01d3917d$6de77060$49b65120$@nexbridge.com","threadId":"47644","inReplyTo":"000c01d39174$eacfff10$c06ffd30$@nexbridge.com","subject":"RE: [PATCH v2 4/6] Force test suite traps to be cleared for NonStop ksh","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-19T23:29:57Z","receivedAt":"2018-01-19T23:30:23Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 19, 2018 5:29 PM, I wrote:\n> On January 19, 2018 4:27 PM, Jeff King wrote:\n> > On Fri, Jan 19, 2018 at 12:34:04PM -0500, randall.s.becker@rogers.com\n> wrote:\n> >\n> > > From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> > >\n> > > * t/lib-git-daemon.sh: fix incompatibilities with ksh traps not being\n> > >   cleared automatically on platform. This caused tests to seem to fail\n> > >   while actually succeeding.\n> > >\n> > > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> > > ---\n> > >  t/lib-git-daemon.sh | 3 +++\n> > >  1 file changed, 3 insertions(+)\n> > >\n> > > diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index\n> > > 987d40680..955beecd9 100644\n> > > --- a/t/lib-git-daemon.sh\n> > > +++ b/t/lib-git-daemon.sh\n> > > @@ -68,6 +68,7 @@ start_git_daemon() {\n> > >  \t\ttest_skip_or_die $GIT_TEST_GIT_DAEMON \\\n> > >  \t\t\t\"git daemon failed to start\"\n> > >  \tfi\n> > > +\ttrap '' EXIT\n> > >  }\n> >\n> > I don't think this can be right. The way these traps are supposed to work is:\n> >\n> >   - as soon as test-lib.sh is loaded, we \"trap die EXIT\" to catch an\n> >     accidental death/exit from one of the scripts\n> >\n> >   - when test_done is called, we clear the trap (i.e., it is OK to exit\n> >     now because the script has given us a positive indication that all\n> >     tests have been run)\n> >\n> >   - while the child git-daemon is running, we'd want to clean up after\n> >     ourselves. So during start_git_daemon() we add our cleanup (followed\n> >     by the original \"die\", because shell traps don't push onto a stack).\n> >     And then at stop_git_daemon(), we restore the original die trap.\n> >\n> > But this patch means that while git-daemon is running, we have no trap at\n> all!\n> > That means that a failed test which causes us to exit would leave a\n> > child daemon running.\n> >\n> > Furthermore, both of these functions now drop the 'die' trap entirely,\n> > meaning the script would fail to notice premature exit from one of the\n> > test snippets.\n> >\n> > So while this may be papering over a problem on NonStop, I think it's\n> > breaking the intent of the traps.\n> >\n> > I'm not sure what the root of the problem is, but it sounds like ksh\n> > may be triggering EXIT traps when we don't expect (during function\n> returns?\n> > Subshell exits? Other?)\n> \n> The \"unexpected\" EXIT traps are exactly the issue we found when working\n> with the platform support team. There was some discussion about what the\n> right expectation was, and I was not able to have a change made to ksh on\n> the platform. The problem may have a similar (identical?) root cause to the\n> Perl exit issues we are also experiencing that is in their hands. I'm currently\n> retesting without this change (results in 36 hours ☹ ). Is there a preferred\n> way you would like me to bypass this except on NonStop? I can add a check\n> based on uname.\n\nThe option that may work, if the tests that are currenting running until Sunday (sadly) fail miserably, is to use:\n\nif [ `uname` = \"NONSTOP_KERNEL\" ]; then trap '' EXIT; fi\n\nor perhaps to add a descriptive function along those lines. We have had two major operating system upgrades since the original case relating to ksh traps, so perhaps things might improve. Our baseline is that there are currently 6 breaks (t1308#23, t1405#52, t9001#31/106/107/134), most of which have been traced back to perl completion codes.\n\nCheers,\nRandall\nP.S. I am happy to explain why the tests perform the at the rate they do on the development machines I have, if anyone is interested, although dissertations might be involved 😉\n\n-- Brief whoami:\n NonStop developer since approximately 211288444200000000\n UNIX developer since approximately 421664400\n-- In my real life, I talk too much.\n\n\n\n"},{"id":"336977","messageId":"20180120111012.GA26459@tor.lan","threadId":"47644","inReplyTo":"20180119173406.13324-1-randall.s.becker@rogers.com","subject":"Re: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-01-20T11:10:12Z","receivedAt":"2018-01-20T11:11:04Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Fri, Jan 19, 2018 at 12:33:59PM -0500, randall.s.becker@rogers.com wrote:\n> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> \n> * wrapper.c: called setbuf(stream,0) to force pipe flushes not enabled by\n>   default on the NonStop platform.\n> \n> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> ---\n>  wrapper.c | 3 +++\n>  1 file changed, 3 insertions(+)\n> \n> diff --git a/wrapper.c b/wrapper.c\n> index d20356a77..671cbb4b4 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -403,6 +403,9 @@ FILE *xfdopen(int fd, const char *mode)\n>  \tFILE *stream = fdopen(fd, mode);\n>  \tif (stream == NULL)\n>  \t\tdie_errno(\"Out of memory? fdopen failed\");\n> +#ifdef __TANDEM\n> +\tsetbuf(stream,0);\n> +#endif\n\nReading the commit message, I would have expected someting similar to\n\n#ifdef FORCE_PIPE_FLUSHES\n\tsetbuf(stream,0);\n#endif\n\n(Because other systems may need the tweak as well, some day)\nOf course you need to change that in the Makefile and config.mak.uname\n\n>  \treturn stream;\n>  }\n>  \n> -- \n> 2.16.0.31.gf1a482c\n> \n"},{"id":"336979","messageId":"1153e1c0-c7d5-3e0d-ce41-ffb1230164f7@web.de","threadId":"47644","inReplyTo":"20180119173406.13324-4-randall.s.becker@rogers.com","subject":"Re: [PATCH v2 2/6] Add tar extract install options override in installation processing.","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-01-20T12:30:56Z","receivedAt":"2018-01-20T12:31:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 19.01.2018 um 18:34 schrieb randall.s.becker@rogers.com:\n> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> \n> * Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to be\n>    specified if needed. The default is xof.\n> \n> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> ---\n>   Makefile | 6 +++++-\n>   1 file changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/Makefile b/Makefile\n> index 1a9b23b67..040e9eacd 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -429,6 +429,9 @@ all::\n>   # running the test scripts (e.g., bash has better support for \"set -x\"\n>   # tracing).\n>   #\n> +# Define TAR_EXTRACT_OPTIONS if you want to change the default behaviour\n> +# from xvf to something else during installation.\n\n\"xof\" instead of \"xvf\"?\n\n> +#\n>   # When cross-compiling, define HOST_CPU as the canonical name of the CPU on\n>   # which the built Git will run (for instance \"x86_64\").\n>   \n> @@ -452,6 +455,7 @@ LDFLAGS =\n>   ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n>   ALL_LDFLAGS = $(LDFLAGS)\n>   STRIP ?= strip\n> +TAR_EXTRACT_OPTIONS = xof\n>   \n>   # Create as necessary, replace existing, make ranlib unneeded.\n>   ARFLAGS = rcs\n> @@ -2569,7 +2573,7 @@ install: all\n>   ifndef NO_GETTEXT\n>   \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'\n>   \t(cd po/build/locale && $(TAR) cf - .) | \\\n> -\t(cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -)\n> +\t(cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) $(TAR_EXTRACT_OPTIONS) -)\n\nHmm.  TAR_EXTRACT_OPTIONS always needs to have f (or -f, or --file) at\nthe end to go together with the following dash, meaning to extract from\nstdin.  And x (or -x, or --extract) is probably needed in all cases as\nwell.  So wouldn't it make more sense to only put the o (or -o, or\n--no-same-owner) into TAR_EXTRACT_OPTIONS and enforce x and f?\n\n>   endif\n>   ifndef NO_PERL\n>   \t$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install\n> \n"},{"id":"336980","messageId":"019501d391f1$e826f750$b874e5f0$@nexbridge.com","threadId":"47644","inReplyTo":"20180120111012.GA26459@tor.lan","subject":"RE: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-20T13:23:43Z","receivedAt":"2018-01-20T13:23:53Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 20, 2018 6:10 AM,  Torsten Bögershausen wrote:\n> On Fri, Jan 19, 2018 at 12:33:59PM -0500, randall.s.becker@rogers.com\n> wrote:\n> > From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> >\n> > * wrapper.c: called setbuf(stream,0) to force pipe flushes not enabled by\n> >   default on the NonStop platform.\n> >\n> > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> > ---\n> >  wrapper.c | 3 +++\n> >  1 file changed, 3 insertions(+)\n> >\n> > diff --git a/wrapper.c b/wrapper.c\n> > index d20356a77..671cbb4b4 100644\n> > --- a/wrapper.c\n> > +++ b/wrapper.c\n> > @@ -403,6 +403,9 @@ FILE *xfdopen(int fd, const char *mode)\n> >  \tFILE *stream = fdopen(fd, mode);\n> >  \tif (stream == NULL)\n> >  \t\tdie_errno(\"Out of memory? fdopen failed\");\n> > +#ifdef __TANDEM\n> > +\tsetbuf(stream,0);\n> > +#endif\n> \n> Reading the commit message, I would have expected someting similar to\n> \n> #ifdef FORCE_PIPE_FLUSHES\n> \tsetbuf(stream,0);\n> #endif\n> \n> (Because other systems may need the tweak as well, some day) Of course\n> you need to change that in the Makefile and config.mak.uname\n> \n> >  \treturn stream;\n> >  }\n\nI can definitely see the point. Would you be agreeable to expanding the scope of this as a separate patch after this one is applied? I would want to bring at least one more Not NonStop machine into the mix for testing, which is more than I can do this weekend 😊.\n\nCheers,\nRandall\n\n"},{"id":"336981","messageId":"019601d391f4$dd367de0$97a379a0$@nexbridge.com","threadId":"47644","inReplyTo":"1153e1c0-c7d5-3e0d-ce41-ffb1230164f7@web.de","subject":"RE: [PATCH v2 2/6] Add tar extract install options override in installation processing.","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-20T13:44:53Z","receivedAt":"2018-01-20T13:45:04Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 20, 2018 7:31 AM, René Scharfe wrote:\n> Am 19.01.2018 um 18:34 schrieb randall.s.becker@rogers.com:\n> > From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> >\n> > * Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to be\n> >    specified if needed. The default is xof.\n> >\n> > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> > ---\n> >   Makefile | 6 +++++-\n> >   1 file changed, 5 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/Makefile b/Makefile\n> > index 1a9b23b67..040e9eacd 100644\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -429,6 +429,9 @@ all::\n> >   # running the test scripts (e.g., bash has better support for \"set -x\"\n> >   # tracing).\n> >   #\n> > +# Define TAR_EXTRACT_OPTIONS if you want to change the default\n> > +behaviour # from xvf to something else during installation.\n> \n> \"xof\" instead of \"xvf\"?\n\nWhen I look at the parent commit, it says xof, so I wanted to preserve existing behaviour by default. Our install process wants to see the actual set of files, so we wanted to use xvof but that hardly seemed of general interest. I was hoping an option to control it would be agreeable.\n\n> > +#\n> >   # When cross-compiling, define HOST_CPU as the canonical name of the\n> CPU on\n> >   # which the built Git will run (for instance \"x86_64\").\n> >\n> > @@ -452,6 +455,7 @@ LDFLAGS =\n> >   ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n> >   ALL_LDFLAGS = $(LDFLAGS)\n> >   STRIP ?= strip\n> > +TAR_EXTRACT_OPTIONS = xof\n> >\n> >   # Create as necessary, replace existing, make ranlib unneeded.\n> >   ARFLAGS = rcs\n> > @@ -2569,7 +2573,7 @@ install: all\n> >   ifndef NO_GETTEXT\n> >   \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'\n> >   \t(cd po/build/locale && $(TAR) cf - .) | \\\n> > -\t(cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -)\n> > +\t(cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR)\n> > +$(TAR_EXTRACT_OPTIONS) -)\n> \n> Hmm.  TAR_EXTRACT_OPTIONS always needs to have f (or -f, or --file) at the\n> end to go together with the following dash, meaning to extract from stdin.\n> And x (or -x, or --extract) is probably needed in all cases as well.  So wouldn't\n> it make more sense to only put the o (or -o, or\n> --no-same-owner) into TAR_EXTRACT_OPTIONS and enforce x and f?\n\nThis is a good suggestion, and I'd love to do that, if I could guarantee a modern tar, which I can't. The platform comes with a really old-school tar from some old (seemingly BSD4.3) epoch that only takes one option set. There is a more modern tar that can be optionally installed if the sysadmin decides to that takes a slightly more modern set, which could support your request, and my team also has a gnu port that is very modern. I can't control what customers are choosing to have installed, unfortunately. Your point is well made and I am completely on board with it, but it introduces a configuration requirement.\n\nAs with the broadening setbuf (patch 2/6) change, I would like to consider this for the future, having a slightly different more complex idea. I could introduce something like this:\n\n1. HAS_ANCIENT_TAR=UnfortunatelyYes in config.mak.uname that disables this capability all together\n2. HAS_ANCIENT_TAR=AreYouKiddingMe (joke) then set up TAR_EXTRACT_ADDITIONAL_OPTIONS above and beyond the default, so --file, --no-same-owner would always be in effect for that operation.\n\nThe micro-project would also, logically, need to apply to other tar occurrences throughout the code and potentially need a test case written for it (not entirely sure what that would test, yet).\n\nIs that a reasonable approach?\n\n> \n> >   endif\n> >   ifndef NO_PERL\n> >   \t$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)'\n> > install\n> >\n\n"},{"id":"336982","messageId":"9b7bf754-90bd-c25c-f5ae-124dcd97d281@web.de","threadId":"47644","inReplyTo":"019601d391f4$dd367de0$97a379a0$@nexbridge.com","subject":"Re: [PATCH v2 2/6] Add tar extract install options override in installation processing.","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-01-20T14:24:31Z","receivedAt":"2018-01-20T14:24:52Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 20.01.2018 um 14:44 schrieb Randall S. Becker:\n> On January 20, 2018 7:31 AM, René Scharfe wrote:\n>> Am 19.01.2018 um 18:34 schrieb randall.s.becker@rogers.com:\n>>> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n>>> \n>>> * Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to\n>>> be specified if needed. The default is xof.\n>>> \n>>> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> --- \n>>> Makefile | 6 +++++- 1 file changed, 5 insertions(+), 1\n>>> deletion(-)\n>>> \n>>> diff --git a/Makefile b/Makefile index 1a9b23b67..040e9eacd\n>>> 100644 --- a/Makefile +++ b/Makefile @@ -429,6 +429,9 @@ all:: #\n>>> running the test scripts (e.g., bash has better support for \"set\n>>> -x\" # tracing). # +# Define TAR_EXTRACT_OPTIONS if you want to\n>>> change the default +behaviour # from xvf to something else during\n>>> installation.\n>> \n>> \"xof\" instead of \"xvf\"?\n> \n> When I look at the parent commit, it says xof, so I wanted to\n> preserve existing behaviour by default. Our install process wants to\n> see the actual set of files, so we wanted to use xvof but that hardly\n> seemed of general interest. I was hoping an option to control it\n> would be agreeable.\n\nPreserving the default is good. I meant that the default is \"xof\",\nbut the added line implies it was \"xvf\" instead.\n\nSeeing your actual use case confirms that my suggestion below would\nwork for you.\n\n> \n>>> +# # When cross-compiling, define HOST_CPU as the canonical name\n>>> of the\n>> CPU on\n>>> # which the built Git will run (for instance \"x86_64\").\n>>> \n>>> @@ -452,6 +455,7 @@ LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) \n>>> ALL_LDFLAGS = $(LDFLAGS) STRIP ?= strip +TAR_EXTRACT_OPTIONS =\n>>> xof\n>>> \n>>> # Create as necessary, replace existing, make ranlib unneeded. \n>>> ARFLAGS = rcs @@ -2569,7 +2573,7 @@ install: all ifndef\n>>> NO_GETTEXT $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)' \n>>> (cd po/build/locale && $(TAR) cf - .) | \\ -\t(cd\n>>> '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -) +\n>>> (cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) \n>>> +$(TAR_EXTRACT_OPTIONS) -)\n>> \n>> Hmm.  TAR_EXTRACT_OPTIONS always needs to have f (or -f, or --file)\n>> at the end to go together with the following dash, meaning to\n>> extract from stdin. And x (or -x, or --extract) is probably needed\n>> in all cases as well.  So wouldn't it make more sense to only put\n>> the o (or -o, or --no-same-owner) into TAR_EXTRACT_OPTIONS and\n>> enforce x and f?\n> \n> This is a good suggestion, and I'd love to do that, if I could\n> guarantee a modern tar, which I can't. The platform comes with a\n> really old-school tar from some old (seemingly BSD4.3) epoch that\n> only takes one option set. There is a more modern tar that can be\n> optionally installed if the sysadmin decides to that takes a slightly\n> more modern set, which could support your request, and my team also\n> has a gnu port that is very modern. I can't control what customers\n> are choosing to have installed, unfortunately. Your point is well\n> made and I am completely on board with it, but it introduces a\n> configuration requirement.\n\nLong options would be nice to nice to have, but are not my main\npoint; I included them mainly to spare readers from firing up\n\"man tar\" to look up the meaning of the short ones.\n\nI just meant to say that something like this here would make more\nsense because you always need x and f- anyway:\n\n\tTAR_EXTRACT_OPTIONS = o\n\n\t... ${TAR} x${TAR_EXTRACT_OPTIONS}f -\n\n> As with the broadening setbuf (patch 2/6) change, I would like to\n> consider this for the future, having a slightly different more\n> complex idea. I could introduce something like this:\n> \n> 1. HAS_ANCIENT_TAR=UnfortunatelyYes in config.mak.uname that disables\n> this capability all together 2. HAS_ANCIENT_TAR=AreYouKiddingMe\n> (joke) then set up TAR_EXTRACT_ADDITIONAL_OPTIONS above and beyond\n> the default, so --file, --no-same-owner would always be in effect for\n> that operation.\n> \n> The micro-project would also, logically, need to apply to other tar\n> occurrences throughout the code and potentially need a test case\n> written for it (not entirely sure what that would test, yet).\n> Is that a reasonable approach?\n\nAs long as old-school dash-less flags suffice for our purposes\n(including yours) we can just keep using that style everywhere and\navoid adding more settings.  It would be a different matter if we\nneeded features that have no short flag, or are only offered by\ncertain tar implementations.\n\nRené\n\n"},{"id":"336990","messageId":"019901d39204$4ed19980$ec74cc80$@nexbridge.com","threadId":"47644","inReplyTo":"9b7bf754-90bd-c25c-f5ae-124dcd97d281@web.de","subject":"RE: [PATCH v2 2/6] Add tar extract install options override in installation processing.","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-20T15:35:26Z","receivedAt":"2018-01-20T15:40:15Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 20, 2018 9:25 AM, René Scharfe wrote:\n> To: Randall S. Becker <rsbecker@nexbridge.com>; git@vger.kernel.org\n> Subject: Re: [PATCH v2 2/6] Add tar extract install options override in\n> installation processing.\n> \n> Am 20.01.2018 um 14:44 schrieb Randall S. Becker:\n> > On January 20, 2018 7:31 AM, René Scharfe wrote:\n> >> Am 19.01.2018 um 18:34 schrieb randall.s.becker@rogers.com:\n> >>> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> >>>\n> >>> * Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to be\n> >>> specified if needed. The default is xof.\n> >>>\n> >>> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> ---\n> >>> Makefile | 6 +++++- 1 file changed, 5 insertions(+), 1\n> >>> deletion(-)\n> >>>\n> >>> diff --git a/Makefile b/Makefile index 1a9b23b67..040e9eacd\n> >>> 100644 --- a/Makefile +++ b/Makefile @@ -429,6 +429,9 @@ all:: #\n> >>> running the test scripts (e.g., bash has better support for \"set -x\"\n> >>> # tracing). # +# Define TAR_EXTRACT_OPTIONS if you want to change\n> >>> the default +behaviour # from xvf to something else during\n> >>> installation.\n> >>\n> >> \"xof\" instead of \"xvf\"?\n> >\n> > When I look at the parent commit, it says xof, so I wanted to preserve\n> > existing behaviour by default. Our install process wants to see the\n> > actual set of files, so we wanted to use xvof but that hardly seemed\n> > of general interest. I was hoping an option to control it would be\n> > agreeable.\n> \n> Preserving the default is good. I meant that the default is \"xof\", but the\n> added line implies it was \"xvf\" instead.\n> \n> Seeing your actual use case confirms that my suggestion below would work\n> for you.\n> \n> >\n> >>> +# # When cross-compiling, define HOST_CPU as the canonical name\n> >>> of the\n> >> CPU on\n> >>> # which the built Git will run (for instance \"x86_64\").\n> >>>\n> >>> @@ -452,6 +455,7 @@ LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n> >>> ALL_LDFLAGS = $(LDFLAGS) STRIP ?= strip +TAR_EXTRACT_OPTIONS = xof\n> >>>\n> >>> # Create as necessary, replace existing, make ranlib unneeded.\n> >>> ARFLAGS = rcs @@ -2569,7 +2573,7 @@ install: all ifndef NO_GETTEXT\n> >>> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'\n> >>> (cd po/build/locale && $(TAR) cf - .) | \\ -\t(cd\n> >>> '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -) + (cd\n> >>> '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR)\n> >>> +$(TAR_EXTRACT_OPTIONS) -)\n> >>\n> >> Hmm.  TAR_EXTRACT_OPTIONS always needs to have f (or -f, or --file)\n> >> at the end to go together with the following dash, meaning to extract\n> >> from stdin. And x (or -x, or --extract) is probably needed in all\n> >> cases as well.  So wouldn't it make more sense to only put the o (or\n> >> -o, or --no-same-owner) into TAR_EXTRACT_OPTIONS and enforce x and\n> f?\n> >\n> > This is a good suggestion, and I'd love to do that, if I could\n> > guarantee a modern tar, which I can't. The platform comes with a\n> > really old-school tar from some old (seemingly BSD4.3) epoch that only\n> > takes one option set. There is a more modern tar that can be\n> > optionally installed if the sysadmin decides to that takes a slightly\n> > more modern set, which could support your request, and my team also\n> > has a gnu port that is very modern. I can't control what customers are\n> > choosing to have installed, unfortunately. Your point is well made and\n> > I am completely on board with it, but it introduces a configuration\n> > requirement.\n> \n> Long options would be nice to nice to have, but are not my main point; I\n> included them mainly to spare readers from firing up \"man tar\" to look up\n> the meaning of the short ones.\n> \n> I just meant to say that something like this here would make more sense\n> because you always need x and f- anyway:\n> \n> \tTAR_EXTRACT_OPTIONS = o\n> \n> \t... ${TAR} x${TAR_EXTRACT_OPTIONS}f -\n> \n> > As with the broadening setbuf (patch 2/6) change, I would like to\n> > consider this for the future, having a slightly different more complex\n> > idea. I could introduce something like this:\n> >\n> > 1. HAS_ANCIENT_TAR=UnfortunatelyYes in config.mak.uname that disables\n> > this capability all together 2. HAS_ANCIENT_TAR=AreYouKiddingMe\n> > (joke) then set up TAR_EXTRACT_ADDITIONAL_OPTIONS above and beyond\n> the\n> > default, so --file, --no-same-owner would always be in effect for that\n> > operation.\n> >\n> > The micro-project would also, logically, need to apply to other tar\n> > occurrences throughout the code and potentially need a test case\n> > written for it (not entirely sure what that would test, yet).\n> > Is that a reasonable approach?\n> \n> As long as old-school dash-less flags suffice for our purposes (including\n> yours) we can just keep using that style everywhere and avoid adding more\n> settings.  It would be a different matter if we needed features that have no\n> short flag, or are only offered by certain tar implementations.\n\nPoints taken. I will reissue this particular patch shortly.\n\n"},{"id":"336992","messageId":"877esci81l.fsf@evledraar.gmail.com","threadId":"47644","inReplyTo":"019901d39204$4ed19980$ec74cc80$@nexbridge.com","subject":"Re: [PATCH v2 2/6] Add tar extract install options override in installation processing.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-01-20T20:34:14Z","receivedAt":"2018-01-20T20:34:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Jan 20 2018, Randall S. Becker jotted:\n\n> I will reissue this particular patch shortly.\n\nIt makes sense to base that submission on the next branch instead of\nmaster. I have a patch queued up there which adds two new tar\ninvocations.\n\nAlso re your commit message see the formatting guide in\nDocumentation/SubmittingPatches, in particular: instead of:\n\n - Add a brief subject line\n\n - Just make the body be a normal paragraph instead of a bullet-point\n   list with one item.\n"},{"id":"336994","messageId":"01ad01d39230$a41c9b20$ec55d160$@nexbridge.com","threadId":"47644","inReplyTo":"877esci81l.fsf@evledraar.gmail.com","subject":"RE: [PATCH v2 2/6] Add tar extract install options override in installation processing.","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-20T20:52:47Z","receivedAt":"2018-01-20T20:53:01Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 20, 2018 3:34 PM, Ævar Arnfjörð Bjarmason\n> On Sat, Jan 20 2018, Randall S. Becker jotted:\n> \n> > I will reissue this particular patch shortly.\n> \n> It makes sense to base that submission on the next branch instead of master.\n> I have a patch queued up there which adds two new tar invocations.\n> \n> Also re your commit message see the formatting guide in\n> Documentation/SubmittingPatches, in particular: instead of:\n> \n>  - Add a brief subject line\n> \n>  - Just make the body be a normal paragraph instead of a bullet-point\n>    list with one item.\n\nGot it. I've been swapping between contribution styles so got a little confused. I'm going to reissue all of the patches required for the NonStop port later tonight or tomorrow, once the test suite run is finished. I'll take everyone's comments into account for that. Thanks for your (and everyone else's) guidance.\n\nCheers,\nRandall\n\n-- Brief whoami:\n  NonStop developer since approximately NonStop(211288444200000000)\n  UNIX developer since approximately 421664400\n-- In my real life, I talk too much.\n\n\n\n"},{"id":"337016","messageId":"001001d3926d$6c4f6730$44ee3590$@nexbridge.com","threadId":"47644","inReplyTo":"000c01d39174$eacfff10$c06ffd30$@nexbridge.com","subject":"RE: [PATCH v2 4/6] Force test suite traps to be cleared for NonStop ksh","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-21T04:07:52Z","receivedAt":"2018-01-21T04:08:13Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 19, 2018 5:29 PM, I wrote:\n> On January 19, 2018 4:27 PM, Jeff King wrote:\n> > On Fri, Jan 19, 2018 at 12:34:04PM -0500, randall.s.becker@rogers.com\n> wrote:\n> >\n> > > From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> > >\n> > > * t/lib-git-daemon.sh: fix incompatibilities with ksh traps not being\n> > >   cleared automatically on platform. This caused tests to seem to fail\n> > >   while actually succeeding.\n> > >\n> > > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> > > ---\n> > >  t/lib-git-daemon.sh | 3 +++\n> > >  1 file changed, 3 insertions(+)\n> > >\n> > > diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index\n> > > 987d40680..955beecd9 100644\n> > > --- a/t/lib-git-daemon.sh\n> > > +++ b/t/lib-git-daemon.sh\n> > > @@ -68,6 +68,7 @@ start_git_daemon() {\n> > >  \t\ttest_skip_or_die $GIT_TEST_GIT_DAEMON \\\n> > >  \t\t\t\"git daemon failed to start\"\n> > >  \tfi\n> > > +\ttrap '' EXIT\n> > >  }\n> >\n> > I don't think this can be right. The way these traps are supposed to work is:\n> >\n> >   - as soon as test-lib.sh is loaded, we \"trap die EXIT\" to catch an\n> >     accidental death/exit from one of the scripts\n> >\n> >   - when test_done is called, we clear the trap (i.e., it is OK to exit\n> >     now because the script has given us a positive indication that all\n> >     tests have been run)\n> >\n> >   - while the child git-daemon is running, we'd want to clean up after\n> >     ourselves. So during start_git_daemon() we add our cleanup (followed\n> >     by the original \"die\", because shell traps don't push onto a stack).\n> >     And then at stop_git_daemon(), we restore the original die trap.\n> >\n> > But this patch means that while git-daemon is running, we have no trap at\n> all!\n> > That means that a failed test which causes us to exit would leave a\n> > child daemon running.\n> >\n> > Furthermore, both of these functions now drop the 'die' trap entirely,\n> > meaning the script would fail to notice premature exit from one of the\n> > test snippets.\n> >\n> > So while this may be papering over a problem on NonStop, I think it's\n> > breaking the intent of the traps.\n> >\n> > I'm not sure what the root of the problem is, but it sounds like ksh\n> > may be triggering EXIT traps when we don't expect (during function\n> returns?\n> > Subshell exits? Other?)\n> \n> The \"unexpected\" EXIT traps are exactly the issue we found when working\n> with the platform support team. There was some discussion about what the\n> right expectation was, and I was not able to have a change made to ksh on\n> the platform. The problem may have a similar (identical?) root cause to the\n> Perl exit issues we are also experiencing that is in their hands. I'm currently\n> retesting without this change (results in 36 hours ☹ ). Is there a preferred\n> way you would like me to bypass this except on NonStop? I can add a check\n> based on uname.\n\nAfter running through the git test suite, it turns out that this particular need has gone away as of the latest OS releases. The test results without the trap '' EXIT are identical to that with the trap (6 breaks that look completion code handling-related on the platform). I'm going to drop the need for this and repackage the entire set of patches applying everyone's comments and removing this (4/6) and the GCC attribute (1/6) patch. This will be followed-up with generalizing the setbuf and tar patches for a broader audience, but I need a bit more time for that generalization.\n\nPlease accept my thanks and expect the updated set tomorrow, which will be sufficient to bring the NonStop NSE, NSX, and NSV platforms into the common code-base for git at long last.\n\nCheers,\nRandall\n\n"},{"id":"337109","messageId":"xmqqshaxjzcc.fsf@gitster.mtv.corp.google.com","threadId":"47644","inReplyTo":"20180120111012.GA26459@tor.lan","subject":"Re: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-22T22:36:03Z","receivedAt":"2018-01-22T22:36:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> On Fri, Jan 19, 2018 at 12:33:59PM -0500, randall.s.becker@rogers.com wrote:\n>> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n>> \n>> * wrapper.c: called setbuf(stream,0) to force pipe flushes not enabled by\n>>   default on the NonStop platform.\n>> \n>> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n>> ---\n>>  wrapper.c | 3 +++\n>>  1 file changed, 3 insertions(+)\n>> \n>> diff --git a/wrapper.c b/wrapper.c\n>> index d20356a77..671cbb4b4 100644\n>> --- a/wrapper.c\n>> +++ b/wrapper.c\n>> @@ -403,6 +403,9 @@ FILE *xfdopen(int fd, const char *mode)\n>>  \tFILE *stream = fdopen(fd, mode);\n>>  \tif (stream == NULL)\n>>  \t\tdie_errno(\"Out of memory? fdopen failed\");\n>> +#ifdef __TANDEM\n>> +\tsetbuf(stream,0);\n>> +#endif\n>\n> Reading the commit message, I would have expected someting similar to\n>\n> #ifdef FORCE_PIPE_FLUSHES\n> \tsetbuf(stream,0);\n> #endif\n>\n> (Because other systems may need the tweak as well, some day)\n> Of course you need to change that in the Makefile and config.mak.uname\n\nI actually wouldn't have expected anything like that after reading\nthe commit message.  \n\nFirst I thought it was describing only what it does (i.e. \"let's use\nsetbuf() to set the stream unbuffered on TANDEM\"), which is a\nuseless description that only says what it does which we can read\nfrom the diff, but \"NonStop by default creates pipe that does not\nflush\" is a potentially useful information the log message adds.\nBut it is just \"potentially\"---we cannot read what exact problem the\nchange is trying to address.  Making a pipe totally unbuffered is a\nheavy hammer that may not be an appropriate solution---it could be\nthat we are missing calls to fflush() where we need and have been\nlucky because most of the systems we deal with do line-buffered by\ndefault, or something silly/implausible like that, and if that is\nthe case, a more proper fix may be to add these missing fflush() to\nright places.\n\nIOW, I do not see it explained clearly why this change is needed on\nany single platform---so \"that issue may be shared by others, too\"\nis a bit premature thing for me to listen to and understand, as\n\"that issue\" is quite unclear to me.\n"},{"id":"337111","messageId":"001401d393d2$73458ef0$59d0acd0$@nexbridge.com","threadId":"47644","inReplyTo":"xmqqshaxjzcc.fsf@gitster.mtv.corp.google.com","subject":"RE: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-22T22:43:34Z","receivedAt":"2018-01-22T22:43:49Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 22, 2018 5:36 PM, Junio C Hamano wrote:\n> Torsten Bögershausen <tboegi@web.de> writes:\n> \n> > On Fri, Jan 19, 2018 at 12:33:59PM -0500, randall.s.becker@rogers.com\n> wrote:\n> >> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> >>\n> >> * wrapper.c: called setbuf(stream,0) to force pipe flushes not enabled by\n> >>   default on the NonStop platform.\n> >>\n> >> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> >> ---\n> >>  wrapper.c | 3 +++\n> >>  1 file changed, 3 insertions(+)\n> >>\n> >> diff --git a/wrapper.c b/wrapper.c\n> >> index d20356a77..671cbb4b4 100644\n> >> --- a/wrapper.c\n> >> +++ b/wrapper.c\n> >> @@ -403,6 +403,9 @@ FILE *xfdopen(int fd, const char *mode)\n> >>  \tFILE *stream = fdopen(fd, mode);\n> >>  \tif (stream == NULL)\n> >>  \t\tdie_errno(\"Out of memory? fdopen failed\");\n> >> +#ifdef __TANDEM\n> >> +\tsetbuf(stream,0);\n> >> +#endif\n> >\n> > Reading the commit message, I would have expected someting similar to\n> >\n> > #ifdef FORCE_PIPE_FLUSHES\n> > \tsetbuf(stream,0);\n> > #endif\n> >\n> > (Because other systems may need the tweak as well, some day) Of course\n> > you need to change that in the Makefile and config.mak.uname\n> \n> I actually wouldn't have expected anything like that after reading the commit\n> message.\n> \n> First I thought it was describing only what it does (i.e. \"let's use\n> setbuf() to set the stream unbuffered on TANDEM\"), which is a useless\n> description that only says what it does which we can read from the diff, but\n> \"NonStop by default creates pipe that does not flush\" is a potentially useful\n> information the log message adds.\n> But it is just \"potentially\"---we cannot read what exact problem the change is\n> trying to address.  Making a pipe totally unbuffered is a heavy hammer that\n> may not be an appropriate solution---it could be that we are missing calls to\n> fflush() where we need and have been lucky because most of the systems\n> we deal with do line-buffered by default, or something silly/implausible like\n> that, and if that is the case, a more proper fix may be to add these missing\n> fflush() to right places.\n> \n> IOW, I do not see it explained clearly why this change is needed on any single\n> platform---so \"that issue may be shared by others, too\"\n> is a bit premature thing for me to listen to and understand, as \"that issue\" is\n> quite unclear to me.\n\nv4 might be a little better. The issue seems to be specific to NonStop that it's PIPE mechanism needs to have setbuf(pipe,NULL) called for git to be happy. The default behaviour appears to be different on NonStop from other platforms from our testing. We get hung up waiting on pipes unless this is done. At the moment, this is platform-specific. Other parts of the discussion led to the conclusion that we should make this available to any platform using a new configuration option, but my objective is to get the NonStop port integrated with the main git code base and when my $DAYJOB permits it, spend the time adding the option. Note: __TANDEM is #define automatically emitted by the NonStop compilers. \n\nDoes that help?\n\nSincerely,\nRandall\n\n"},{"id":"337165","messageId":"xmqqtvvcigui.fsf@gitster.mtv.corp.google.com","threadId":"47644","inReplyTo":"001401d393d2$73458ef0$59d0acd0$@nexbridge.com","subject":"Re: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-23T18:13:09Z","receivedAt":"2018-01-23T18:13:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Randall S. Becker\" <rsbecker@nexbridge.com> writes:\n\n>> IOW, I do not see it explained clearly why this change is needed on any single\n>> platform---so \"that issue may be shared by others, too\"\n>> is a bit premature thing for me to listen to and understand, as \"that issue\" is\n>> quite unclear to me.\n>\n> v4 might be a little better. The issue seems to be specific to\n> NonStop that it's PIPE mechanism needs to have setbuf(pipe,NULL)\n> called for git to be happy.  The default behaviour appears to be\n> different on NonStop from other platforms from our testing. We get\n> hung up waiting on pipes unless this is done.\n\nI am afraid that that is not a \"diagnosis\" enough to allow us moving\nforward.  We get hung up because...?  When the process that has the\nother end of pipe open exits, NonStop does not close the pipe\nproperly?  Or NonStop does not flush the data buffered in the pipe?\nWould it help if a compat wrapper that does setbuf(fd, NULL)\nimmediately before closing the fd, or some other more targetted\nmechanism, is used only on NonStop, for example?  Potentially\nmegabytes of data can pass thru a pipe, and if the platform bug\naffects only at the tail end of the transfer, marking the pipe not\nto buffer at all at the beginning is too big a hammer to work it\naround.  With the explanation given so far, this still smells more\nlike \"we have futzed around without understanding why, and this\nhappens to work.\"  It may be good enough for your purpose of making\nprogress (after all, I'd imagine that you'd need to work this around\none way or another to hunt for and fix more issues on the platform),\nbut it does not sound like \"we know what the problem is, and this is\nthe best workaround for that\", which is what we want if it wants to\nbecome part of the official codebase.\n"},{"id":"337182","messageId":"007501d3948b$3b81e590$b285b0b0$@nexbridge.com","threadId":"47644","inReplyTo":"xmqqtvvcigui.fsf@gitster.mtv.corp.google.com","subject":"RE: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-23T20:46:18Z","receivedAt":"2018-01-23T20:46:34Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 23, 2018 1:13 PM, Junio C Hamano wrote:\n> \"Randall S. Becker\" <rsbecker@nexbridge.com> writes:\n> \n> >> IOW, I do not see it explained clearly why this change is needed on\n> >> any single platform---so \"that issue may be shared by others, too\"\n> >> is a bit premature thing for me to listen to and understand, as \"that\n> >> issue\" is quite unclear to me.\n> >\n> > v4 might be a little better. The issue seems to be specific to NonStop\n> > that it's PIPE mechanism needs to have setbuf(pipe,NULL) called for\n> > git to be happy.  The default behaviour appears to be different on\n> > NonStop from other platforms from our testing. We get hung up waiting\n> > on pipes unless this is done.\n> \n> I am afraid that that is not a \"diagnosis\" enough to allow us moving forward.\n> We get hung up because...?  When the process that has the other end of\n> pipe open exits, NonStop does not close the pipe properly?  Or NonStop\n> does not flush the data buffered in the pipe?\n> Would it help if a compat wrapper that does setbuf(fd, NULL) immediately\n> before closing the fd, or some other more targetted mechanism, is used only\n> on NonStop, for example?  Potentially megabytes of data can pass thru a\n> pipe, and if the platform bug affects only at the tail end of the transfer,\n> marking the pipe not to buffer at all at the beginning is too big a hammer to\n> work it around.  With the explanation given so far, this still smells more like\n> \"we have futzed around without understanding why, and this happens to\n> work.\"  It may be good enough for your purpose of making progress (after\n> all, I'd imagine that you'd need to work this around one way or another to\n> hunt for and fix more issues on the platform), but it does not sound like \"we\n> know what the problem is, and this is the best workaround for that\", which is\n> what we want if it wants to become part of the official codebase.\n\nI am retesting without setbuf(NULL) but this is unlikely to be enlightening. The test cases do not adequately simulate the configuration in which my team originally encountered the problem. This requires a guarantee of the source and destination coming through different logical CPUs. We never encountered the issue in the test suite, only when end users got hold of it. We had two distinct problems, one which was the revent=0 related hang (already solved) and other was a stream flush problem. The two are related but distinct. The platform support group insisted that we should have the setbuf(NULL) in place for interprocess communications in the form used here - I'm worried about losing this, but completely aware that this is far too heavy for other platforms (hence the __TANDEM guard in wrapper.c). If the form of the wrapper should be different, I would be happy to comply.\n\nI have a much longer explanation about the platform message stack structure, but that doesn't belong here. Happy to respond privately if requested.\n\nCheers,\nRandall\n\n-- Brief whoami:\n NonStop developer since approximately 211288444200000000\n UNIX developer since approximately 421664400\n-- In my real life, I talk too much.\n\n\n\n"},{"id":"337355","messageId":"006901d3958e$98ac26f0$ca0474d0$@nexbridge.com","threadId":"47644","inReplyTo":"xmqqtvvcigui.fsf@gitster.mtv.corp.google.com","subject":"RE: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-25T03:42:54Z","receivedAt":"2018-01-25T03:43:12Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 23, 2018 1:13 PM, Junio C Hamano wrote:\n> \n> \"Randall S. Becker\" <rsbecker@nexbridge.com> writes:\n> \n> >> IOW, I do not see it explained clearly why this change is needed on\n> >> any single platform---so \"that issue may be shared by others, too\"\n> >> is a bit premature thing for me to listen to and understand, as \"that\n> >> issue\" is quite unclear to me.\n> >\n> > v4 might be a little better. The issue seems to be specific to NonStop\n> > that it's PIPE mechanism needs to have setbuf(pipe,NULL) called for\n> > git to be happy.  The default behaviour appears to be different on\n> > NonStop from other platforms from our testing. We get hung up waiting\n> > on pipes unless this is done.\n> \n> I am afraid that that is not a \"diagnosis\" enough to allow us moving forward.\n> We get hung up because...?  When the process that has the other end of\n> pipe open exits, NonStop does not close the pipe properly?  Or NonStop\n> does not flush the data buffered in the pipe?\n> Would it help if a compat wrapper that does setbuf(fd, NULL) immediately\n> before closing the fd, or some other more targetted mechanism, is used only\n> on NonStop, for example?  Potentially megabytes of data can pass thru a\n> pipe, and if the platform bug affects only at the tail end of the transfer,\n> marking the pipe not to buffer at all at the beginning is too big a hammer to\n> work it around.  With the explanation given so far, this still smells more like\n> \"we have futzed around without understanding why, and this happens to\n> work.\"  It may be good enough for your purpose of making progress (after\n> all, I'd imagine that you'd need to work this around one way or another to\n> hunt for and fix more issues on the platform), but it does not sound like \"we\n> know what the problem is, and this is the best workaround for that\", which is\n> what we want if it wants to become part of the official codebase.\n\nAs I feared, the test suite was unable to reproduce the issue without setbuf(NULL) - primary because the test structure ends up with both ends of the git dialogs on clone and fetch in the same CPU (even if different IPUs), which does not experience the issue and we can't loop-back through the platform's proprietary SSH. I am not comfortable releasing without it at this stage, but if you don't want to go forward with this fix, my team can run it for a few months internally in the hope that this works out for the better. The situation is timing related and is fine 99.98-ish% of the time. I really do want the setbuf present in any compiled versions that our community might get, primarily because I don't like sleepless nights chasing this down (again).\n\nCheers,\nRandall\n\n"}]}