Subject: Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS

From: Paul Eggert <[email-masked]>
Date: Thu, 23 Feb 2023 14:23:29 -0800

On 2/20/23 13:14, Pádraig Brady wrote:
> Since https://github.com/coreutils/gnulib/commit/4db8db34
> gnulib has been unconditionally replacing lseek() on macos.
> Now with SEEK_DATA undefined that replaced lseek()
> will run the code intended for BeOS.
> So either the lseek.m4 or lseek.c needs adjusting accordingly.

Good catch, thanks. I updated the Gnulib patch accordingly; see attached.

On 2/20/23 02:21, George Valkov wrote:
 > What is the correct way to apply your patch?

Sounds like patching is a bit of a hassle, so to make things easier I 
installed the attached patch into Gnulib, and propagated this into 
Coreutils.

You should be able to get the latest version of Coreutils from 
savannah.gnu.org, and then run './bootstrap; ./configure; make' if you 
have suitable development tools like Autoconf. You can run ./bootstrap 
on a GNU/Linux platform and then copy the directory to macOS and run 
'./configure; make' on macOS. Please give it a try.

 > If you know what conditions are required to reproduce the issue on 
FreeBSD,

I don't. I'm relying on FreeBSD bug report 256205. I cited that in the 
attached patch.

 > Is it ok if I continue testing on FreeBSD 13.1

Sure, that's fine. Possibly the bug is fixed in FreeBSD 13.1; if so, 
perhaps we can improve performance on 13.1 by changing "__FreeBSD__ < 
14" to something else.

On 2/20/23 13:25, George Valkov wrote:
 > there is no need for that hack in lseek.c.

Yes, we don't need any new hack in lseek.c. And strictly speaking, even 
the existing macOS-specific hack in lseek.c (look for __APPLE__) is no 
longer needed, since (with the changes I just installed) lseek.c is no 
longer compiled for macOS.

However, assuming Apple fixes the macOS bug in (say) macOS 14, so that 
we change the "99990000" to "140000" in Gnulib's lib/unistd.in.h and 
m4/lseek.m4, we'll likely need that hack back because it works around an 
incompatibility between GNU-or-FreeBSD-or-Solaris SEEK_DATA and macOS 
SEEK_DATA; see 
<https://www.gnu.org/software/gnulib/manual/html_node/lseek.html>. So I 
thought it nicer to leave it in for now; it has zero runtime cost on all 
platforms.

From 7352d9fec59398c76c3bb8a2ef86ba58818f0342 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sun, 19 Feb 2023 00:05:24 -0600
Subject: [PATCH] lseek: avoid SEEK_HOLE bugs in FreeBSD, macOS
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This attempts to fix <https://bugs.gnu.org/61386>, a bug in GNU cp
caused by a serious data corruption bug in FreeBSD and macOS.
* doc/posix-functions/lseek.texi: Mention the bug.
* lib/unistd.in.h (SEEK_DATA, SEEK_HOLE): Undef in macOS < 13 and
FreeBSD < 14.  FreeBSD fixed the bug sometime during FreeBSD 13
<https://bugs.freebsd.org/256205>, so the "FreeBSD < 14" is
conservative.  It’s unknown when Apple will fix macOS so use
macOS "9999" as a placeholder.
* m4/lseek.m4 (gl_FUNC_LSEEK): Replace lseek if on one of the
above platforms.
---
 ChangeLog                      | 14 ++++++++++++++
 doc/posix-functions/lseek.texi |  5 +++++
 lib/unistd.in.h                | 18 ++++++++++++++++++
 m4/lseek.m4                    | 32 ++++++++++++++++++++++++++------
 4 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c1ca610548..5e436f5b3a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2023-02-23  Paul Eggert  <[email-masked]>
+
+	lseek: avoid SEEK_HOLE bugs in FreeBSD, macOS
+	This attempts to fix <https://bugs.gnu.org/61386>, a bug in GNU cp
+	caused by a serious data corruption bug in FreeBSD and macOS.
+	* doc/posix-functions/lseek.texi: Mention the bug.
+	* lib/unistd.in.h (SEEK_DATA, SEEK_HOLE): Undef in macOS < 13 and
+	FreeBSD < 14.  FreeBSD fixed the bug sometime during FreeBSD 13
+	<https://bugs.freebsd.org/256205>, so the "FreeBSD < 14" is
+	conservative.  It’s unknown when Apple will fix macOS so use
+	macOS "9999" as a placeholder.
+	* m4/lseek.m4 (gl_FUNC_LSEEK): Replace lseek if on one of the
+	above platforms.
+
 2023-02-18  Bruno Haible  <[email-masked]>
 
 	configmake: Add support for $build_os != $host_os.
diff --git a/doc/posix-functions/lseek.texi b/doc/posix-functions/lseek.texi
index 2f8e2b5877..3470524b12 100644
--- a/doc/posix-functions/lseek.texi
+++ b/doc/posix-functions/lseek.texi
@@ -37,4 +37,9 @@ IRIX 6.5.
 @item
 Some systems do not support @code{SEEK_DATA} and @code{SEEK_HOLE}:
 AIX, HP-UX, Microsoft Windows, NetBSD, OpenBSD.
+@item
+Some systems have a buggy @code{SEEK_DATA} and @code{SEEK_HOLE},
+and Gnulib works around the problem via @code{#undef SEEK_DATA}
+and @code{#undef SEEK_HOLE}:
+FreeBSD 13, macOS 12.
 @end itemize
diff --git a/lib/unistd.in.h b/lib/unistd.in.h
index bfc501e5a7..8ba9867894 100644
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -40,6 +40,24 @@
 # undef _GL_INCLUDING_UNISTD_H
 #endif
 
+/* Avoid lseek bugs in FreeBSD, macOS <https://bugs.gnu.org/61386>.
+   This bug is fixed after FreeBSD 13; see <https://bugs.freebsd.org/256205>.
+   Use macOS "9999" to stand for a future fixed macOS version.  */
+#if defined __FreeBSD__ && __FreeBSD__ < 14
+# undef SEEK_DATA
+# undef SEEK_HOLE
+#elif defined __APPLE__ && defined __MACH__ && defined SEEK_DATA
+# ifdef __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__
+#  include <AvailabilityMacros.h>
+# endif
+# if (!defined MAC_OS_X_VERSION_MIN_REQUIRED \
+      || MAC_OS_X_VERSION_MIN_REQUIRED < 99990000)
+#  include <sys/fcntl.h> /* It also defines the two macros.  */
+#  undef SEEK_DATA
+#  undef SEEK_HOLE
+# endif
+#endif
+
 /* Get all possible declarations of gethostname().  */
 #if @GNULIB_GETHOSTNAME@ && @UNISTD_H_HAVE_WINSOCK2_H@ \
   && !defined _GL_INCLUDING_WINSOCK2_H
diff --git a/m4/lseek.m4 b/m4/lseek.m4
index fd4f1f27d3..6e1ab6ffaa 100644
--- a/m4/lseek.m4
+++ b/m4/lseek.m4
@@ -1,4 +1,4 @@
-# lseek.m4 serial 12
+# lseek.m4 serial 13
 dnl Copyright (C) 2007, 2009-2023 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -70,9 +70,29 @@ AC_DEFUN([gl_FUNC_LSEEK],
     REPLACE_LSEEK=1
   fi
 
-  dnl macOS SEEK_DATA is incompatible with other platforms.
-  case $host_os in
-    darwin*)
-      REPLACE_LSEEK=1;;
-  esac
+  AS_IF([test $REPLACE_LSEEK = 0],
+    [AC_CACHE_CHECK([whether SEEK_DATA works but is incompatible with GNU],
+       [gl_cv_func_lseek_works_but_incompatible],
+       [AC_PREPROC_IFELSE(
+          [AC_LANG_SOURCE(
+             dnl Use macOS "9999" to stand for a future fixed macOS version.
+             dnl See ../lib/unistd.in.h and <https://bugs.gnu.org/61386>.
+             [[#include <unistd.h>
+               #if defined __APPLE__ && defined __MACH__ && defined SEEK_DATA
+               # ifdef __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__
+               #  include <AvailabilityMacros.h>
+               # endif
+               # if 99990000 <= MAC_OS_X_VERSION_MIN_REQUIRED
+               #  define LSEEK_WORKS_BUT_IS_INCOMPATIBLE_WITH_GNU
+               # endif
+               #endif
+               #ifndef LSEEK_WORKS_BUT_IS_INCOMPATIBLE_WITH_GNU
+                #error "No need to work around the bug"
+               #endif
+             ]])],
+          [gl_cv_func_lseek_works_but_incompatible=yes],
+          [gl_cv_func_lseek_works_but_incompatible=no])])
+     if test "$gl_cv_func_lseek_works_but_incompatible" = yes; then
+       REPLACE_LSEEK=1
+     fi])
 ])
-- 
2.39.2