Re: [ANNOUNCE] Git v2.32.0-rc3 - t5300 Still Broken on NonStop ia64/x86

On Wed, Jun 02, 2021 at 03:49:31PM -0400, Jeff King wrote:

And so when he gets this error:

  fatal: fsync error on '.git/objects/pack/tmp_pack_NkPgqN': Interrupted system call

presumably we were in fsync() when the signal arrived, and unlike most
other platforms, the call needs to be restarted manually (even though we
set up the signal with SA_RESTART).

Aha, thanks for explaining. That makes sense to me.

I'm not sure if this violates POSIX or not (I couldn't find a
definitive answer to the set of interruptible functions in the
standard).

I couldn't find anything either, but I believe you that what you
outlined is the right solution.

But either way, the workaround is probably something like:

Seems very reasonable. Here's a patch:

--- >8 ---

Subject: [PATCH] compat: introduce git_fsync_with_restart()

Some platforms, like NonStop do not automatically restart fsync() when
interrupted by a signal, even when that signal is setup with SA_RESTART.

This can lead to test breakage, e.g., where "--progress" is used, thus
SIGALRM is sent often, and can interrupt an fsync() syscall.

Add a Makefile knob FSYNC_NEEDS_RESTART to replace fsync() with one that
gracefully handles getting EINTR.

Reported-by: Randall S. Becker <redacted>
Signed-off-by: Jeff King <redacted>
Signed-off-by: Taylor Blau <redacted>
---
 Makefile          |  8 ++++++++
 compat/fsync.c    | 10 ++++++++++
 git-compat-util.h |  6 ++++++
 3 files changed, 24 insertions(+)
 create mode 100644 compat/fsync.c

diff --git a/Makefile b/Makefile
index c3565fc0f8..ebea693a2c 100644
--- a/Makefile
+++ b/Makefile

@@ -423,6 +423,9 @@ all::
 # Define NEED_ACCESS_ROOT_HANDLER if access() under root may success for X_OK
 # even if execution permission isn't granted for any user.
 #
+# Define FSYNC_NEEDS_RESTART if your platform's fsync() needs to be restarted
+# manually when interrupted by a signal.
+#
 # Define PAGER_ENV to a SP separated VAR=VAL pairs to define
 # default environment variables to be passed when a pager is spawned, e.g.
 #

@@ -1926,6 +1929,11 @@ ifdef NEED_ACCESS_ROOT_HANDLER
 	COMPAT_OBJS += compat/access.o
 endif

+ifdef FSYNC_NEEDS_RESTART
+	COMPAT_CFLAGS += -DFSYNC_NEEDS_RESTART
+	COMPAT_OBJS += compat/fsync.o
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif

diff --git a/compat/fsync.c b/compat/fsync.c
new file mode 100644
index 0000000000..f340fc628b
--- /dev/null
+++ b/compat/fsync.c

@@ -0,0 +1,10 @@
+#include "git-compat-util.h"
+
+#undef fsync
+int git_fsync_with_restart(int fd)
+{
+	int ret;
+	while ((ret = fsync(fd)) < 0 && errno == EINTR)
+		; /* try again */
+	return ret;
+}

diff --git a/git-compat-util.h b/git-compat-util.h
index a508dbe5a3..972fdb609f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -833,6 +833,12 @@ const char *inet_ntop(int af, const void *src, char *dst, size_t size);
 int git_atexit(void (*handler)(void));
 #endif

+#ifdef FSYNC_NEEDS_RESTART
+#undef fsync
+#define fsync git_fsync_with_restart
+int git_fsync_with_restart(int fd);
+#endif
+
 static inline size_t st_add(size_t a, size_t b)
 {
 	if (unsigned_add_overflows(a, b))
--

2.31.1.163.ga65ce7f831