Re: Git 2.54.0-rc1, subtests of t5310, t5326, t5327

From

Jeff King <peff@peff.net>

Date

Apr 9, 2026, 20:51 UTC

Message-ID

<20260409205145.GC3076846@coredump.intra.peff.net>

In-Reply-To

<addgkjiB80pgKw69@pks.im>

On Thu, Apr 09, 2026 at 10:17:22AM +0200, Patrick Steinhardt wrote:

> > that people should be able to send.  I'd personally expect to be able to
> > send values much larger, at least 512 KiB, and I have code that expects
> > even larger (16 MiB).
> > 
> > So I'd simply say that for systems that have a constraint on the size
> > that is "too small", they should just use NO_WRITEV.
> 
> I would be happy with this as an intermediate step, as Randall has
> confirmed it would fix the issue. It is the least intrusive step and has
> the lowest risk.

diff --git a/Makefile b/Makefile
index 5d22394c2e..f9b4dd8ae3 100644
--- a/Makefile
+++ b/Makefile
@@ -1125,6 +1125,7 @@ LIB_OBJS += compat/nonblock.o
 LIB_OBJS += compat/obstack.o
 LIB_OBJS += compat/open.o
 LIB_OBJS += compat/terminal.o
+LIB_OBJS += compat/writev.o
 LIB_OBJS += compiler-tricks/not-constant.o
 LIB_OBJS += config.o
 LIB_OBJS += connect.o
@@ -2031,7 +2032,6 @@ ifdef NO_PREAD
 endif
 ifdef NO_WRITEV
 	COMPAT_CFLAGS += -DNO_WRITEV
-	COMPAT_OBJS += compat/writev.o
 endif
 ifdef NO_FAST_WORKING_DIRECTORY
 	BASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY
diff --git a/compat/posix.h b/compat/posix.h
index 94699a03fa..57c11938c7 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -326,6 +326,39 @@ int git_lstat(const char *, struct stat *);
 ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);
 #endif
 
+/*
+ * Limit size of IO chunks, because huge chunks only cause pain.  OS X
+ * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
+ * the absence of bugs, large chunks can result in bad latencies when
+ * you decide to kill the process.
+ *
+ * We pick 8 MiB as our default, but if the platform defines SSIZE_MAX
+ * that is smaller than that, clip it to SSIZE_MAX, as a call to
+ * read(2) or write(2) larger than that is allowed to fail.  As the last
+ * resort, we allow a port to pass via CFLAGS e.g. "-DMAX_IO_SIZE=value"
+ * to override this, if the definition of SSIZE_MAX given by the platform
+ * is broken.
+ */
+#ifndef MAX_IO_SIZE
+# define MAX_IO_SIZE_DEFAULT (8*1024*1024)
+# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
+#  define MAX_IO_SIZE SSIZE_MAX
+# else
+#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
+# endif
+#endif
+
+/*
+ * Temporary hack: our use of writev() does not respect MAX_IO_SIZE, but we
+ * never feed more than a single pkt-line packet to it. Disable writev()
+ * and use our fallback wrapper if the system can't support that size.
+ *
+ * This should be removed once writev() supports MAX_IO_SIZE.
+ */
+#if !defined(NO_WRITEV) && (MAX_IO_SIZE < 65536)
+#define NO_WRITEV
+#endif
+
 #ifdef NO_WRITEV
 #define writev git_writev
 #define iovec git_iovec
diff --git a/git-compat-util.h b/git-compat-util.h
index 4b4ea2498f..60108459f3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -677,28 +677,6 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
 	return a + b;
 }
 
-/*
- * Limit size of IO chunks, because huge chunks only cause pain.  OS X
- * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
- * the absence of bugs, large chunks can result in bad latencies when
- * you decide to kill the process.
- *
- * We pick 8 MiB as our default, but if the platform defines SSIZE_MAX
- * that is smaller than that, clip it to SSIZE_MAX, as a call to
- * read(2) or write(2) larger than that is allowed to fail.  As the last
- * resort, we allow a port to pass via CFLAGS e.g. "-DMAX_IO_SIZE=value"
- * to override this, if the definition of SSIZE_MAX given by the platform
- * is broken.
- */
-#ifndef MAX_IO_SIZE
-# define MAX_IO_SIZE_DEFAULT (8*1024*1024)
-# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
-#  define MAX_IO_SIZE SSIZE_MAX
-# else
-#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
-# endif
-#endif
-
 #ifdef HAVE_ALLOCA_H
 # include <alloca.h>
 # define xalloca(size)      (alloca(size))


It's not _too_ complicated, but it's a little more than I'd like for an
-rc2. Yet another option is to swap out NO_WRITEV for USE_WRITEV,
effectively reverting the feature temporarily until it supports
MAX_IO_SIZE. But that's also slightly non-trivial.

So I dunno. Do we want just:

diff --git a/config.mak.uname b/config.mak.uname
index ccb3f71881..b672b1d077 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -654,6 +654,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	CSPRNG_METHOD = openssl
 	SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
 	SHELL_PATH = /usr/coreutils/bin/bash
+	NO_WRITEV = NotUntilItSupportsMaxIOSize
 endif
 ifeq ($(uname_S),OS/390)
 	NO_SYS_POLL_H = YesPlease

and hope it's the only one?

-Peff