* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
[not found] ` <9139A642-34D8-42A8-887F-CE027AAAB2F7@gmail.com>
@ 2023-02-19 6:08 ` Paul Eggert
2023-02-19 16:31 ` Pádraig Brady
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Paul Eggert @ 2023-02-19 6:08 UTC (permalink / raw)
To: George Valkov; +Cc: 61386, Gnulib bugs, Pádraig Brady
[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]
George, given what you've written I suppose we should give up the idea
of copying sparse files efficiently on macOS (and on FreeBSD
13.0-RELEASE, as it has a similar bug with SEEK_HOLE and SEEK_DATA), in
cases where fclonefileat does not work.
Please try the attached Gnulib patch; it uses your idea of disabling
SEEK_HOLE and SEEK_DATA on macOS, except on steroids because it disables
these two macros everywhere, for all Gnulib-using applications, and it
also disables them on FreeBSD < 14. For coreutils you need only this
patch's change to lib/unistd.in.h. If this works for you I suppose we
can install it into Gnulib and propagate that into coreutils etc.
Also, you reported the bug against macOS 12.6. Can you check whether the
same bug occurs on macOS 13? If it's still there I suppose the attached
patch will need to be updated since it guesses the bug is fixed in macOS 13.
I'll cc this email to bug-gnulib to give them a heads-up. (Gnulib
readers can see <https://bugs.gnu.org/61386> for context.)
Really, Apple should fix this serious data corruption bug in APFS and macOS.
[-- Attachment #2: 0001-lseek-avoid-SEEK_HOLE-bugs-in-FreeBSD-macOS.patch --]
[-- Type: text/x-patch, Size: 3095 bytes --]
From 0024b8460dfe5e133f9e0d63ed4f75314dea2f82 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
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 (or indeed whether) Apple fixed
macOS but this patch guesses macOS 13.
---
ChangeLog | 12 ++++++++++++
doc/posix-functions/lseek.texi | 5 +++++
lib/unistd.in.h | 12 ++++++++++++
3 files changed, 29 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index c1ca610548..c8e9726916 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2023-02-18 Paul Eggert <eggert@cs.ucla.edu>
+
+ 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 (or indeed whether) Apple fixed
+ macOS but this patch guesses macOS 13.
+
2023-02-18 Bruno Haible <bruno@clisp.org>
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..47b32eb445 100644
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -40,6 +40,18 @@
# undef _GL_INCLUDING_UNISTD_H
#endif
+/* Avoid lseek bugs in FreeBSD, macOS <https://bugs.gnu.org/61386>. */
+#if defined __FreeBSD__ && __FreeBSD__ < 14
+# undef SEEK_DATA
+# undef SEEK_HOLE
+#elif (defined __APPLE__ && defined __MACH__ \
+ && (!defined MAC_OS_X_VERSION_MIN_REQUIRED \
+ || MAC_OS_X_VERSION_MIN_REQUIRED < 130000))
+# include <sys/fcntl.h> /* It also defines the two macros. */
+# undef SEEK_DATA
+# undef SEEK_HOLE
+#endif
+
/* Get all possible declarations of gethostname(). */
#if @GNULIB_GETHOSTNAME@ && @UNISTD_H_HAVE_WINSOCK2_H@ \
&& !defined _GL_INCLUDING_WINSOCK2_H
--
2.39.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-19 6:08 ` bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS Paul Eggert
@ 2023-02-19 16:31 ` Pádraig Brady
2023-02-20 10:21 ` George Valkov
2023-02-20 15:02 ` George Valkov
2 siblings, 0 replies; 21+ messages in thread
From: Pádraig Brady @ 2023-02-19 16:31 UTC (permalink / raw)
To: Paul Eggert, George Valkov; +Cc: 61386, Gnulib bugs
On 19/02/2023 06:08, Paul Eggert wrote:
> George, given what you've written I suppose we should give up the idea
> of copying sparse files efficiently on macOS (and on FreeBSD
> 13.0-RELEASE, as it has a similar bug with SEEK_HOLE and SEEK_DATA), in
> cases where fclonefileat does not work.
>
> Please try the attached Gnulib patch; it uses your idea of disabling
> SEEK_HOLE and SEEK_DATA on macOS, except on steroids because it disables
> these two macros everywhere, for all Gnulib-using applications, and it
> also disables them on FreeBSD < 14. For coreutils you need only this
> patch's change to lib/unistd.in.h. If this works for you I suppose we
> can install it into Gnulib and propagate that into coreutils etc.
>
> Also, you reported the bug against macOS 12.6. Can you check whether the
> same bug occurs on macOS 13? If it's still there I suppose the attached
> patch will need to be updated since it guesses the bug is fixed in macOS 13.
>
> I'll cc this email to bug-gnulib to give them a heads-up. (Gnulib
> readers can see <https://bugs.gnu.org/61386> for context.)
>
> Really, Apple should fix this serious data corruption bug in APFS and macOS.
Yes I concur.
I'll adjust the coreutils tests to avoid
using SEEK_DATA if not available in a more dynamic manner.
thank you,
Pádraig
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-19 6:08 ` bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS Paul Eggert
2023-02-19 16:31 ` Pádraig Brady
@ 2023-02-20 10:21 ` George Valkov
2023-02-20 15:02 ` George Valkov
2 siblings, 0 replies; 21+ messages in thread
From: George Valkov @ 2023-02-20 10:21 UTC (permalink / raw)
To: Paul Eggert; +Cc: 61386, Gnulib bugs, Pádraig Brady
[-- Attachment #1: Type: text/plain, Size: 2822 bytes --]
> On 2023-02-19, at 8:08 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
>
> George, given what you've written I suppose we should give up the idea of copying sparse files efficiently on macOS (and on FreeBSD 13.0-RELEASE, as it has a similar bug with SEEK_HOLE and SEEK_DATA), in cases where fclonefileat does not work.
I agree, Paul. I just ran some benchmarks: the time it takes to call msync unconditionally hurts the performance: 502 us if the memory view is already synced seems fine, however 28177-36585 us whenever a sync is required is more than the time it takes to performing a full-copy 7042-21528 us.
> Please try the attached Gnulib patch; it uses your idea of disabling SEEK_HOLE and SEEK_DATA on macOS, except on steroids because it disables these two macros everywhere, for all Gnulib-using applications, and it also disables them on FreeBSD < 14. For coreutils you need only this patch's change to lib/unistd.in.h. If this works for you I suppose we can install it into Gnulib and propagate that into coreutils etc.
What is the correct way to apply your patch? I tried
git apply 0001-lseek-avoid-SEEK_HOLE-bugs-in-FreeBSD-macOS.patch
error: patch failed: ChangeLog:1
error: ChangeLog: patch does not apply
error: doc/posix-functions/lseek.texi: No such file or directory
error: lib/unistd.in.h: wrong type
Then I made the changes to lib/unistd.in.h manually, and ran the tests:
* before: corrupted
* after: fixed
Regarding FreeBSD, I have a FreeBSD 13.1-RELEASE VM running on Hyper-V, so I compiled and ran my samples: m.c, d.c: the copy was correct, however d.c reports that there are no segments to skip, which means that the file created by m.c using mmap is not sparse even though it should contain one blank segment and be sparse. Next I installed a fresh copy of FreeBSD 13.0-RELEASE and observed the same behaviour. Both installations use ZFS and the tests were ran as root.
If you know what conditions are required to reproduce the issue on FreeBSD, let me know and I will test it for you. Otherwise I am not sure if we need to make any changes to FreeBSD, since I did not observe anything wrong there.
Is it ok if I continue testing on FreeBSD 13.1, because the 13.0 I just installed lacks any configuration and is quite uncomfortable to use. It also rejects my password over SSH, so I’m forced to use the console. I can test pretty much anything that runs on Hyper-V.
> Also, you reported the bug against macOS 12.6. Can you check whether the same bug occurs on macOS 13? If it's still there I suppose the attached patch will need to be updated since it guesses the bug is fixed in macOS 13.
Yes it does. I just ran my samples under macOS Internet recovery, which reports as Ventura with kernel compiled on the 30th of January 2023. See the attached log:
[-- Attachment #2: ventura-internet-recovery.txt --]
[-- Type: text/plain, Size: 1392 bytes --]
uname -a
Darwin gMac.lan 22.3.0 Darwin Kernel Version 22.3.0: Mon Jan 30 20:42:11 PST 2023; root:xnu-8792.81.3~2/RELEASE_X86_64 x86_64
./m
src 3 dst 4
00001000 skip
total bytes copied 1a45640 / 1a46640 1a46640 100535 us
/gfc cc1-mmap
16d835378ab973a114082a585cc76958bdbccec0 cc1-mmap
./d
src 3 dst 4
c a46640 p 1a46640 h 1a46640 d 1000000
total bytes copied a46640 / 1a46640 7117 us
./gfc cc1-sparse
75e6f6cb303cb5d3909c6d7830c417fc6ed658c3 cc1-sparse
./n
dst 3 MS_ASYNC 1 MS_INVALIDATE 2 MS_SYNC 16
size bytes 1a46640 msync ok 43770 us
./d
src 3 dst 4
c 1000 p 1000 h 1000 d 0
c 1a44640 p 1a46640 h 1a46640 d 2000
total bytes copied 1a45640 / 1a46640 41322 us
./gfc cc1-sparse
16d835378ab973a114082a585cc76958bdbccec0 cc1-sparse
./m
src 3 dst 4
00001000 skip
total bytes copied 1a45640 / 1a46640 1a46640 59198 us
./d
src 3 dst 4
c a46640 p 1a46640 h 1a46640 d 1000000
total bytes copied a46640 / 1a46640 12144 us
./gfc cc1-sparse
75e6f6cb303cb5d3909c6d7830c417fc6ed658c3 cc1-sparse
./n
dst 3 MS_ASYNC 1 MS_INVALIDATE 2 MS_SYNC 16
size bytes 1a46640 msync ok 27462 us
./d
src 3 dst 4
c 1000 p 1000 h 1000 d 0
c 1a44640 p 1a46640 h 1a46640 d 2000
total bytes copied 1a45640 / 1a46640 32125 us
./gfc cc1-sparse
16d835378ab973a114082a585cc76958bdbccec0 cc1-sparse
[-- Attachment #3: Type: text/plain, Size: 1075 bytes --]
> I'll cc this email to bug-gnulib to give them a heads-up. (Gnulib readers can see <https://bugs.gnu.org/61386> for context.)
Thanks, I’ll check it.
> Really, Apple should fix this serious data corruption bug in APFS and macOS.
> <0001-lseek-avoid-SEEK_HOLE-bugs-in-FreeBSD-macOS.patch>
I agree, I opened a new ticket with Apple on 2023-02-16. Both via e-mail, and https://security.apple.com/reports/OE1928608366012
This was at the same time I reported about mmap and msync here. I got this replay on the following day:
> Thanks for the additional information. We are reviewing it and will follow up if we have any questions or need additional details.
Feel free to write them at product-security@apple.com and include this line
OE0928602070811 - please include this ID in replies to this thread.
Introduce your self as part of GNU/coreutils and let them know it has been 18 months since the issue was reported privately to them, so we’ve gone public now. CC me <same as gmail> at abv dot bg.
Georgi Valkov
httpstorm.com
nano RTOS
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-19 6:08 ` bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS Paul Eggert
2023-02-19 16:31 ` Pádraig Brady
2023-02-20 10:21 ` George Valkov
@ 2023-02-20 15:02 ` George Valkov
2023-02-20 17:49 ` Pádraig Brady
2 siblings, 1 reply; 21+ messages in thread
From: George Valkov @ 2023-02-20 15:02 UTC (permalink / raw)
To: Paul Eggert; +Cc: 61386, Gnulib bugs, Pádraig Brady
Hi Paul, the following tests fail after your patch:
FAIL: tests/split/filter.sh
FAIL: tests/split/b-chunk.sh
FAIL: tests/split/l-chunk.sh
FAIL: tests/cp/sparse-perf.sh
Before patch
https://httpstorm.com/share/.openwrt/test/2023-02-06_coreutils-9.1/test-05-95f4ee0577dd836de523f46999777fbbbe9d2772-ori.txt
After patch
https://httpstorm.com/share/.openwrt/test/2023-02-06_coreutils-9.1/test-05-95f4ee0577dd836de523f46999777fbbbe9d2772-patch.txt
Georgi Valkov
httpstorm.com
nano RTOS
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-20 15:02 ` George Valkov
@ 2023-02-20 17:49 ` Pádraig Brady
2023-02-20 19:35 ` George Valkov
0 siblings, 1 reply; 21+ messages in thread
From: Pádraig Brady @ 2023-02-20 17:49 UTC (permalink / raw)
To: George Valkov, Paul Eggert; +Cc: 61386, Gnulib bugs
On 20/02/2023 15:02, George Valkov wrote:
> Hi Paul, the following tests fail after your patch:
> FAIL: tests/split/filter.sh
> FAIL: tests/split/b-chunk.sh
> FAIL: tests/split/l-chunk.sh
These look unrelated and may be due to some other change in your environment.
Specifically lseek() is failing on /dev/null and returning:
split: /dev/null: cannot determine file size
> FAIL: tests/cp/sparse-perf.sh
As expected.
I've a fix for this locally
which leverages another patch to determine
dynamically if we support SEEK_HOLE.
cheers,
Pádraig
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-20 17:49 ` Pádraig Brady
@ 2023-02-20 19:35 ` George Valkov
2023-02-20 21:14 ` Pádraig Brady
0 siblings, 1 reply; 21+ messages in thread
From: George Valkov @ 2023-02-20 19:35 UTC (permalink / raw)
To: Pádraig Brady; +Cc: Paul Eggert, 61386, Gnulib bugs
> On 2023-02-20, at 7:49 PM, Pádraig Brady <P@draigBrady.com> wrote:
>
> On 20/02/2023 15:02, George Valkov wrote:
>> Hi Paul, the following tests fail after your patch:
>> FAIL: tests/split/filter.sh
>> FAIL: tests/split/b-chunk.sh
>> FAIL: tests/split/l-chunk.sh
>
> These look unrelated and may be due to some other change in your environment.
> Specifically lseek() is failing on /dev/null and returning:
> split: /dev/null: cannot determine file size
I deleted the lines that were introduced by the patch in unistd.in.h, then make clean; make -j 16 and ran all tests: back to 5 failed. Then I restored the deleted lines, rebuild and I got 9 failed tests again.
>> FAIL: tests/cp/sparse-perf.sh
>
> As expected.
> I've a fix for this locally
> which leverages another patch to determine
> dynamically if we support SEEK_HOLE.
All right! Cheers mate!
Georgi Valkov
httpstorm.com
nano RTOS
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-20 19:35 ` George Valkov
@ 2023-02-20 21:14 ` Pádraig Brady
2023-02-20 21:25 ` George Valkov
2023-02-23 22:23 ` Paul Eggert
0 siblings, 2 replies; 21+ messages in thread
From: Pádraig Brady @ 2023-02-20 21:14 UTC (permalink / raw)
To: George Valkov, Paul Eggert; +Cc: 61386, Gnulib bugs
On 20/02/2023 19:35, George Valkov wrote:
>
>> On 2023-02-20, at 7:49 PM, Pádraig Brady <P@draigBrady.com> wrote:
>>
>> On 20/02/2023 15:02, George Valkov wrote:
>>> Hi Paul, the following tests fail after your patch:
>>> FAIL: tests/split/filter.sh
>>> FAIL: tests/split/b-chunk.sh
>>> FAIL: tests/split/l-chunk.sh
>>
>> These look unrelated and may be due to some other change in your environment.
>> Specifically lseek() is failing on /dev/null and returning:
>> split: /dev/null: cannot determine file size
>
> I deleted the lines that were introduced by the patch in unistd.in.h, then make clean; make -j 16 and ran all tests: back to 5 failed. Then I restored the deleted lines, rebuild and I got 9 failed tests again.
Oh right sorry.
I think I see what's happening.
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.
cheers,
Pádraig
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-20 21:14 ` Pádraig Brady
@ 2023-02-20 21:25 ` George Valkov
2023-02-22 13:59 ` George Valkov
2023-02-23 22:23 ` Paul Eggert
1 sibling, 1 reply; 21+ messages in thread
From: George Valkov @ 2023-02-20 21:25 UTC (permalink / raw)
To: Pádraig Brady; +Cc: Paul Eggert, 61386, Gnulib bugs
> On 2023-02-20, at 11:14 PM, Pádraig Brady <P@draigBrady.com> wrote:
>
> On 20/02/2023 19:35, George Valkov wrote:
>>> On 2023-02-20, at 7:49 PM, Pádraig Brady <P@draigBrady.com> wrote:
>>>
>>> On 20/02/2023 15:02, George Valkov wrote:
>>>> Hi Paul, the following tests fail after your patch:
>>>> FAIL: tests/split/filter.sh
>>>> FAIL: tests/split/b-chunk.sh
>>>> FAIL: tests/split/l-chunk.sh
>>>
>>> These look unrelated and may be due to some other change in your environment.
>>> Specifically lseek() is failing on /dev/null and returning:
>>> split: /dev/null: cannot determine file size
>> I deleted the lines that were introduced by the patch in unistd.in.h, then make clean; make -j 16 and ran all tests: back to 5 failed. Then I restored the deleted lines, rebuild and I got 9 failed tests again.
>
> Oh right sorry.
> I think I see what's happening.
> 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.
As I reported in one of my previous comments, there is no need for that hack in lseek.c. Either use the original lseek from macOS and make sure it doesn't return cached data, or disable sparse support until it gets fixed by Apple. The more people report it to them the higher the chance for them to take any action.
Let me know when you fix it.
Good luck!
Georgi Valkov
httpstorm.com
nano RTOS
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-20 21:25 ` George Valkov
@ 2023-02-22 13:59 ` George Valkov
0 siblings, 0 replies; 21+ messages in thread
From: George Valkov @ 2023-02-22 13:59 UTC (permalink / raw)
To: Pádraig Brady, Paul Eggert; +Cc: 61386, Gnulib bugs
I received an update from Apple
> We reproduced the issue and are investigating.
> Our engineers are investigating the root cause of the issue you reported. If we need more information from you, we’ll add a comment and send you an email.
Georgi Valkov
httpstorm.com
nano RTOS
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-20 21:14 ` Pádraig Brady
2023-02-20 21:25 ` George Valkov
@ 2023-02-23 22:23 ` Paul Eggert
2023-02-24 0:51 ` Pádraig Brady
2023-02-24 14:33 ` George Valkov
1 sibling, 2 replies; 21+ messages in thread
From: Paul Eggert @ 2023-02-23 22:23 UTC (permalink / raw)
To: Pádraig Brady, George Valkov; +Cc: 61386, Gnulib bugs
[-- Attachment #1: Type: text/plain, Size: 2197 bytes --]
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.
[-- Attachment #2: 0001-lseek-avoid-SEEK_HOLE-bugs-in-FreeBSD-macOS.patch --]
[-- Type: text/x-patch, Size: 5352 bytes --]
From 7352d9fec59398c76c3bb8a2ef86ba58818f0342 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
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 <eggert@cs.ucla.edu>
+
+ 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 <bruno@clisp.org>
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-23 22:23 ` Paul Eggert
@ 2023-02-24 0:51 ` Pádraig Brady
2023-02-24 14:33 ` George Valkov
1 sibling, 0 replies; 21+ messages in thread
From: Pádraig Brady @ 2023-02-24 0:51 UTC (permalink / raw)
To: Paul Eggert, George Valkov; +Cc: 61386, Gnulib bugs
On 23/02/2023 22:23, Paul Eggert wrote:
> 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.
Cool.
> 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.
The latest coreutils that should include all fixes for this issue is at
https://www.pixelbeat.org/cu/coreutils-9.1.160-5c8c2.tar.gz
That should be buildable with the standard ./configure && make combo
thanks!,
Pádraig
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-23 22:23 ` Paul Eggert
2023-02-24 0:51 ` Pádraig Brady
@ 2023-02-24 14:33 ` George Valkov
2023-02-24 15:43 ` Pádraig Brady
1 sibling, 1 reply; 21+ messages in thread
From: George Valkov @ 2023-02-24 14:33 UTC (permalink / raw)
To: Paul Eggert; +Cc: Pádraig Brady, 61386, Gnulib bugs
> On 2023-02-24, at 12:23 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
>
> 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.
Nice: I just downloaded a fresh copy of Savannah, and it already includes the attached patch, so no action needed on my side.
The copy created by cp is good. I noticed that you have added a —debug switch, so I gave it a test:
1. If the target exists I get this output. I would assume zeroes means that all data is read from the source, but pages containing only zeroes are skipped, while pages containing data are written to the target, which is fine. I would guess 4 KB page granularity or some form of detection takes place.
./coreutils-2023-02-24/src/cp --debug cc1-mmap cc1-ori
'cc1-mmap' -> 'cc1-ori'
copy offload: avoided, reflink: unsupported, sparse detection: zeros
2. If the target does not exist, the file is cloned. You might want to report something about that in the debug output. Otherwise the clone is good.
./coreutils-2023-02-24/src/cp --debug cc1-mmap cc1-ori
'cc1-mmap' -> 'cc1-ori’
My first attempt to run the tests stopped here, so I had to interrupt it 10 minutes later with Control+C.
make check-TESTS
make[1]: Entering directory '/Volumes/coreutils/coreutils-2023-02-24'
PASS: tests/misc/help-version.sh
PASS: tests/misc/help-version-getopt.sh
Then I ran it again, and I can see the tests run very slowly now. This time it hanged after:
PASS: tests/rm/isatty.sh
^Cmake[1]: *** Deleting file 'tests/rm/empty-inacc.log'
make[1]: *** [Makefile:21399: tests/rm/empty-inacc.log] Error 130
make: *** [Makefile:21381: check-TESTS] Interrupt: 2
make check-TESTS
make[1]: Entering directory '/Volumes/coreutils/coreutils-2023-02-24'
PASS: tests/misc/help-version.sh
PASS: tests/misc/help-version-getopt.sh
^Cmake[1]: *** Deleting file 'tests/tail-2/inotify-race.log'
^C^C^C^C
Now it hangs here and I cannot interrupt it. I don’t see any CPU usage related to the tests. My laptop is idle. Attempting to close the Terminal window gave me this warning:
Do you want to terminate running processes in this tab?
Closing this tab will terminate the running processes: gdb, bash, make (2), sh (4).
That’s a good culprit. Two days ago brew updated gdb to version 13.1 and there was a message asking me to sign it with some entitlements to make it more functional. So, I signed gdb. At first though make check-TESTS does not play nice when gdb is signed, so I removed the signature, but that did not help. Next I restored version 12.1_2 and ran make check-TESTS again, which still hangs at the same points. So gdb signature and version have nothing to do with this new issue.
Next I ran the tests on top of my previous unpatched build directory coreutils cf80f988eeb97cc3f8c65ae58e735d36f865277b, gnulib 32c16c45d7378b014d9aac6130104c4d02a9acdb and it works fine, so I would assume something related to the tests has been broken recently in coreutils or gnulib. I restored gdb 13.1 signed and the tests completed again.
Back to latest unmodified coreutils 5c8c2a5161c0b6f84212778f694c526105f10dab, gnulib 7352d9fec59398c76c3bb8a2ef86ba58818f0342, the tests hang.
make check-TESTS
make[1]: Entering directory '/Volumes/coreutils/coreutils-2023-02-24'
PASS: tests/misc/help-version.sh
PASS: tests/misc/help-version-getopt.sh
^Cmake[1]: *** Deleting file 'tests/tail-2/inotify-race.log'
^C^C^C^C^C^C
killall gdb
ps -A |grep gdb
29584 ?? 0:00.09 gdb -nx --batch-silent --eval-command=break 1475 --eval-command=run --pid=29583 -f file --eval-command=quit tail
23269 ttys010 0:00.09 gdb -nx --batch-silent --eval-command=break 1475 --eval-command=run --pid=23268 -f file --eval-command=quit tail
killall -9 gdb
ps -A |grep gdb
Killing gdb allowed the tests to continue, I had to do it twice:
https://httpstorm.com/share/.openwrt/test/2023-02-06_coreutils-9.1/test-06-5c8c2a5161c0b6f84212778f694c526105f10dab-ori.txt
> 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.
I’m starting to think I made a mistake by trying to apply your patch inside coreutils, since it already had links to the affected files. I guess I should have applied the patch inside coreutils/gnulib?
So my question was: what command do you use to apply your patch and in which directory do you run it?
> 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.
All the development tools should already be installed on the Mac using brew. I can confirm autoconf is installed:
brew list | grep autoconf
autoconf
I always start with these commands:
git clone https://github.com/coreutils/coreutils.git
cd coreutils
git submodule foreach git pull origin master
./bootstrap
./configure
make -j 16
git checkout -b 2023-02-24
Next I apply the changes you want, make clean ; make -j 16
Finally I proceed with testing.
> > 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.
I see they reproduce the bug on both arm64 and x64. Based on the conversation their issue is very similar: they use a linker to craft a file, that linker is likely to use mmap. But their issue is fixed with fsync. And they also talk about nulls. macOS is indeed based on BSD.
Also if I read correctly, they do not experience the issue when working with the root file-system on FreeBSD, its the same on macOS: I need to mount an APFS sparse disk image, they use nullfs.
Comments say they seem to have fixed it partiality in 42881526d401e7a9c09241e392b7ffa18cfe11d6, and then completely later. I am too busy to play with FreeBSD at the moment, as it takes more advanced preparation.
> On 2023-02-24, at 2:51 AM, Pádraig Brady <P@draigBrady.com> wrote:
>
>> 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.
>
> The latest coreutils that should include all fixes for this issue is at
> https://www.pixelbeat.org/cu/coreutils-9.1.160-5c8c2.tar.gz
> That should be buildable with the standard ./configure && make combo
Pádraig, I compiled from your archive, cp produces a good copy, however make check-TESTS hangs the same way as with the clone I compiled on the latest Savannah master.
Good luck!
Georgi Valkov
httpstorm.com
nano RTOS
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-24 14:33 ` George Valkov
@ 2023-02-24 15:43 ` Pádraig Brady
2023-02-24 19:34 ` George Valkov
2023-02-24 22:06 ` George Valkov
0 siblings, 2 replies; 21+ messages in thread
From: Pádraig Brady @ 2023-02-24 15:43 UTC (permalink / raw)
To: George Valkov, Paul Eggert; +Cc: Gnulib bugs, 61386
[-- Attachment #1: Type: text/plain, Size: 2516 bytes --]
On 24/02/2023 14:33, George Valkov wrote:
>
>> On 2023-02-24, at 12:23 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
>>
>> 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.
>
> Nice: I just downloaded a fresh copy of Savannah, and it already includes the attached patch, so no action needed on my side.
>
> The copy created by cp is good. I noticed that you have added a —debug switch, so I gave it a test:
> 1. If the target exists I get this output. I would assume zeroes means that all data is read from the source, but pages containing only zeroes are skipped, while pages containing data are written to the target, which is fine. I would guess 4 KB page granularity or some form of detection takes place.
Exactly.
> ./coreutils-2023-02-24/src/cp --debug cc1-mmap cc1-ori
> 'cc1-mmap' -> 'cc1-ori'
> copy offload: avoided, reflink: unsupported, sparse detection: zeros
>
> 2. If the target does not exist, the file is cloned. You might want to report something about that in the debug output. Otherwise the clone is good.
>
> ./coreutils-2023-02-24/src/cp --debug cc1-mmap cc1-ori
> 'cc1-mmap' -> 'cc1-ori’
Yes that was a mistake.
Should be fixed with
https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=65bb27656
> My first attempt to run the tests stopped here, so I had to interrupt it 10 minutes later with Control+C.
> ^Cmake[1]: *** Deleting file 'tests/tail-2/inotify-race.log'
> ^C^C^C^C^C^C
>
> killall gdb
> ps -A |grep gdb
> 29584 ?? 0:00.09 gdb -nx --batch-silent --eval-command=break 1475 --eval-command=run --pid=29583 -f file --eval-command=quit tail
> 23269 ttys010 0:00.09 gdb -nx --batch-silent --eval-command=break 1475 --eval-command=run --pid=23268 -f file --eval-command=quit tail
>
> killall -9 gdb
> ps -A |grep gdb
>
> Killing gdb allowed the tests to continue, I had to do it twice:
That's awkward.
That test documents the issue with protecting the gdb invocation with timeout(1).
I suppose we could restrict this test to inotify capable systems,
which would have the side effect of avoiding its use on non linux systems.
The attached patch does that.
thanks for all the testing.
Pádraig
[-- Attachment #2: macos-gdb-hang.patch --]
[-- Type: text/x-patch, Size: 1484 bytes --]
From 952c8bad4c297ed48cddbb81f1030a35812ca980 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P@draigBrady.com>
Date: Fri, 24 Feb 2023 15:40:37 +0000
Subject: [PATCH] tests: avoid gdb hang on macOS
* tests/tail-2/inotify-race.sh: Restrict the test to
inotify capable systems to avoid the hang with some gdbs.
* tests/tail-2/inotify-race.sh: Likewise.
---
tests/tail-2/inotify-race.sh | 3 +++
tests/tail-2/inotify-race2.sh | 3 +++
2 files changed, 6 insertions(+)
diff --git a/tests/tail-2/inotify-race.sh b/tests/tail-2/inotify-race.sh
index c722fb9fa..63f906536 100755
--- a/tests/tail-2/inotify-race.sh
+++ b/tests/tail-2/inotify-race.sh
@@ -23,6 +23,9 @@
. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
print_ver_ tail sleep
+grep '^#define HAVE_INOTIFY 1' "$CONFIG_HEADER" >/dev/null && is_local_dir_ . \
+ || skip_ 'inotify is not supported'
+
# Terminate any background gdb/tail process
cleanup_() {
kill $pid 2>/dev/null && wait $pid
diff --git a/tests/tail-2/inotify-race2.sh b/tests/tail-2/inotify-race2.sh
index 89b02c6cf..19219b72e 100755
--- a/tests/tail-2/inotify-race2.sh
+++ b/tests/tail-2/inotify-race2.sh
@@ -22,6 +22,9 @@
. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
print_ver_ tail sleep
+grep '^#define HAVE_INOTIFY 1' "$CONFIG_HEADER" >/dev/null && is_local_dir_ . \
+ || skip_ 'inotify is not supported'
+
# Terminate any background gdb/tail process
cleanup_() {
kill $pid 2>/dev/null && wait $pid
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-24 15:43 ` Pádraig Brady
@ 2023-02-24 19:34 ` George Valkov
2023-02-24 22:06 ` George Valkov
1 sibling, 0 replies; 21+ messages in thread
From: George Valkov @ 2023-02-24 19:34 UTC (permalink / raw)
To: Pádraig Brady; +Cc: Paul Eggert, Gnulib bugs, 61386
> On 2023-02-24, at 5:43 PM, Pádraig Brady <P@draigBrady.com> wrote:
>
> On 24/02/2023 14:33, George Valkov wrote:
>>> On 2023-02-24, at 12:23 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
>>>
>>> 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.
>> Nice: I just downloaded a fresh copy of Savannah, and it already includes the attached patch, so no action needed on my side.
>> The copy created by cp is good. I noticed that you have added a —debug switch, so I gave it a test:
>> 1. If the target exists I get this output. I would assume zeroes means that all data is read from the source, but pages containing only zeroes are skipped, while pages containing data are written to the target, which is fine. I would guess 4 KB page granularity or some form of detection takes place.
>
> Exactly.
>
>> ./coreutils-2023-02-24/src/cp --debug cc1-mmap cc1-ori
>> 'cc1-mmap' -> 'cc1-ori'
>> copy offload: avoided, reflink: unsupported, sparse detection: zeros
>> 2. If the target does not exist, the file is cloned. You might want to report something about that in the debug output. Otherwise the clone is good.
>> ./coreutils-2023-02-24/src/cp --debug cc1-mmap cc1-ori
>> 'cc1-mmap' -> 'cc1-ori’
>
> Yes that was a mistake.
> Should be fixed with
> https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=65bb27656
Looks good
commit 65bb2765646df18488b266e6c1851593d3f9c966
./coreutils-2023-02-24.b/src/cp --debug cc1-mmap cc1-ori
'cc1-mmap' -> 'cc1-ori'
copy offload: unknown, reflink: yes, sparse detection: unknown
>> My first attempt to run the tests stopped here, so I had to interrupt it 10 minutes later with Control+C.
>
>> ^Cmake[1]: *** Deleting file 'tests/tail-2/inotify-race.log'
>> ^C^C^C^C^C^C
>> killall gdb
>> ps -A |grep gdb
>> 29584 ?? 0:00.09 gdb -nx --batch-silent --eval-command=break 1475 --eval-command=run --pid=29583 -f file --eval-command=quit tail
>> 23269 ttys010 0:00.09 gdb -nx --batch-silent --eval-command=break 1475 --eval-command=run --pid=23268 -f file --eval-command=quit tail
>> killall -9 gdb
>> ps -A |grep gdb
>> Killing gdb allowed the tests to continue, I had to do it twice:
>
> That's awkward.
> That test documents the issue with protecting the gdb invocation with timeout(1).
> I suppose we could restrict this test to inotify capable systems,
> which would have the side effect of avoiding its use on non linux systems.
> The attached patch does that.
Still hanging out there after
PASS: tests/rm/isatty.sh
There were no gdb instances to kill, but if I press enter, it continues
empty-inacc.sh: set-up failure:
ERROR: tests/rm/empty-inacc.sh
PASS: tests/rm/empty-name.pl
…
https://httpstorm.com/share/.openwrt/test/2023-02-06_coreutils-9.1/test-07-65bb2765646df18488b266e6c1851593d3f9c966-patch.txt
coreutils: git checkout cf80f988eeb97cc3f8c65ae58e735d36f865277b
gnulib: git checkout 32c16c45d7378b014d9aac6130104c4d02a9acdb
./bootstrap && ./configure && make clean && make -j 16
make check-TESTS # still hangs: gdb
git checkout -b cf80-macos-gdb-hang
git am < macos-gdb-hang.patch
make clean && make -j 16
make check-TESTS # completes successfully
Clone and configure another fresh copy; gnulib at master; test various coreutils commits, applying macos-gdb-hang.patch on top of them; make clean && make -j 16
cf80f988eeb97cc3f8c65ae58e735d36f865277b hangs: gdb
I would suspect either some change in gnulib or some of the other repositories is causing these. I cannot checkout random gnulib commits, since the build breaks completely.
Check various commits of gnulib
gnulib 7352d9fec59398c76c3bb8a2ef86ba58818f0342 master hangs: ENTER
gnulib bb3fd10e6309f017618a12b2c10d3bfb813bfc08 hangs: ENTER
gnulib f77a31de60963c994cd9b42c8088be0e734962d7 aclocal-1.16: error: aclocal: file 'm4/build-to-host.m4' does not exist
Trying to revert some commits and go back in time:
git revert f77a31de60963c994cd9b42c8088be0e734962d7 fails
git revert 1e29238e40d118d4f769f7516700dd4fc494bfcd fails
> thanks for all the testing.
Look Pádraig, I’m glad to help, but this is really taking a lot of energy, these tests took another full day, and I’d be thankful if we can make everything work sooner. I’ve been spending many hours each day for a few weeks now. I need to finish my own tasks and find a job.
In the old build directory if I ./bootstrap && ./configure and make -j 16, the tests complete. It is using these checkpoints:
coreutils: git checkout cf80f988eeb97cc3f8c65ae58e735d36f865277b
gnulib: git checkout 32c16c45d7378b014d9aac6130104c4d02a9acdb
However if I clone a fresh copy ./bootstrap && ./configure, then check the same commits, ./configure again and make -j 16, I need to also apply your gdb patch, otherwise it hangs. And on master I need your patch and I need to press enter after PASS: tests/rm/isatty.sh.
Georgi Valkov
httpstorm.com
nano RTOS
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-24 15:43 ` Pádraig Brady
2023-02-24 19:34 ` George Valkov
@ 2023-02-24 22:06 ` George Valkov
2023-02-24 22:47 ` Pádraig Brady
1 sibling, 1 reply; 21+ messages in thread
From: George Valkov @ 2023-02-24 22:06 UTC (permalink / raw)
To: Pádraig Brady; +Cc: Paul Eggert, Gnulib bugs, 61386
If I revert a0803c4bad6f8e11bb05effcc42ef433f4fc3f9b, the requirement to press enter after PASS: tests/rm/isatty.sh is fixed.
Sometimes it might randomly hang: gdb after
PASS: tests/rm/r-4.sh
^Cmake[1]: *** Deleting file 'tests/rm/r-root.log'
^C^C^C
ps -A |grep gdb
55051 ttys020 0:00.09 gdb -nx --batch-silent -return-child-result --eval-command=set exec-wrapper\011\011\011\011\011 env 'LD_PRELOAD=:./k.so' --eval-command=break '/Volumes/coreutils/coreutils-2023-02-24.d/src/remove.c:377' --eval-command=source bp.py --eval-command=run -rv --one-file-system dir --eval-command=quit rm
even though I have applied macos-gdb-hang.patch. Other times it goes past that:
r-root.sh: skipped test: internal test failure: maybe LD_PRELOAD or gdb doesn't work?
and eventually the tests complete.
If it hangs and I killall -9 gdb, it continues
r-root.sh: skipped test: internal test failure: maybe LD_PRELOAD or gdb doesn't work?
SKIP: tests/rm/r-root.sh
PASS: tests/rm/readdir-bug.sh
…
Georgi Valkov
httpstorm.com
nano RTOS
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-24 22:06 ` George Valkov
@ 2023-02-24 22:47 ` Pádraig Brady
2023-02-24 23:23 ` George Valkov
0 siblings, 1 reply; 21+ messages in thread
From: Pádraig Brady @ 2023-02-24 22:47 UTC (permalink / raw)
To: George Valkov; +Cc: Paul Eggert, Gnulib bugs, 61386-done
On 24/02/2023 22:06, George Valkov wrote:
> If I revert a0803c4bad6f8e11bb05effcc42ef433f4fc3f9b, the requirement to press enter after PASS: tests/rm/isatty.sh is fixed.
>
Ah very useful info.
I'll test this on my macOS 13.2 system.
> Sometimes it might randomly hang: gdb after
> PASS: tests/rm/r-4.sh
> ^Cmake[1]: *** Deleting file 'tests/rm/r-root.log'
> ^C^C^C
This is the same gdb issue, which I'll skip similarly
to the tail-2/inotify tests.
Since all issues related to sparse copy on macOS are now addressed,
I'm marking this bug as done.
The above bugs are unrelated, and I'll take them from here.
thanks again for all the testing.
Pádraig
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-24 22:47 ` Pádraig Brady
@ 2023-02-24 23:23 ` George Valkov
2023-02-25 12:27 ` Pádraig Brady
0 siblings, 1 reply; 21+ messages in thread
From: George Valkov @ 2023-02-24 23:23 UTC (permalink / raw)
To: Pádraig Brady; +Cc: Paul Eggert, Gnulib bugs, 61386-done
> On 2023-02-25, at 12:47 AM, Pádraig Brady <P@draigBrady.com> wrote:
>
> On 24/02/2023 22:06, George Valkov wrote:
>> If I revert a0803c4bad6f8e11bb05effcc42ef433f4fc3f9b, the requirement to press enter after PASS: tests/rm/isatty.sh is fixed.
>
> Ah very useful info.
> I'll test this on my macOS 13.2 system.
>
>> Sometimes it might randomly hang: gdb after
>> PASS: tests/rm/r-4.sh
>> ^Cmake[1]: *** Deleting file 'tests/rm/r-root.log'
>> ^C^C^C
>
> This is the same gdb issue, which I'll skip similarly
> to the tail-2/inotify tests.
>
>
> Since all issues related to sparse copy on macOS are now addressed,
> I'm marking this bug as done.
>
> The above bugs are unrelated, and I'll take them from here.
>
> thanks again for all the testing.
Thank you Pádraig! You and Paul did a lot of work, and I’m happy the issue is resolved. I’ll let you know if I get any update from Apple.
Are we in a state where I can update OpenWRT to the latest coreutils master or perhaps it would be better to wait until you fix the tests and coreutils-9.2 is released? You mentioned the release is coming soon?
Georgi Valkov
httpstorm.com
nano RTOS
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-24 23:23 ` George Valkov
@ 2023-02-25 12:27 ` Pádraig Brady
2023-03-06 7:37 ` Paul Eggert
0 siblings, 1 reply; 21+ messages in thread
From: Pádraig Brady @ 2023-02-25 12:27 UTC (permalink / raw)
To: George Valkov; +Cc: Paul Eggert, Gnulib bugs, 61386-done
On 24/02/2023 23:23, George Valkov wrote:
> Are we in a state where I can update OpenWRT to the latest coreutils master or perhaps it would be better to wait until you fix the tests and coreutils-9.2 is released? You mentioned the release is coming soon?
Probably best to wait for the 9.2 release at this stage.
Best guess is 1.5 weeks away.
cheers,
Pádraig.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-02-25 12:27 ` Pádraig Brady
@ 2023-03-06 7:37 ` Paul Eggert
2023-03-06 11:13 ` Pádraig Brady
2023-03-06 21:49 ` George Valkov
0 siblings, 2 replies; 21+ messages in thread
From: Paul Eggert @ 2023-03-06 7:37 UTC (permalink / raw)
To: Pádraig Brady, George Valkov; +Cc: Gnulib bugs, 61386-done
I recall reading somewhere in this thread that a 'split' test was
failing on macOS because it doesn't let you lseek on /dev/null. I fixed
that problem here:
https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=aa266f1b3dc4e12acdc46cc0f562adc03c2c0b8f
and fixed some other 'split' issues in adjacent patches, while I was in
the neighborhood.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-03-06 7:37 ` Paul Eggert
@ 2023-03-06 11:13 ` Pádraig Brady
2023-03-06 21:49 ` George Valkov
1 sibling, 0 replies; 21+ messages in thread
From: Pádraig Brady @ 2023-03-06 11:13 UTC (permalink / raw)
To: Paul Eggert, George Valkov; +Cc: Gnulib bugs, 61386-done
On 06/03/2023 07:37, Paul Eggert wrote:
> I recall reading somewhere in this thread that a 'split' test was
> failing on macOS because it doesn't let you lseek on /dev/null. I fixed
> that problem here:
>
> https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=aa266f1b3dc4e12acdc46cc0f562adc03c2c0b8f
>
> and fixed some other 'split' issues in adjacent patches, while I was in
> the neighborhood.
The lseek /dev/null issue was only in your undef SEEK_DATA test patch,
and already addressed in your final gnulib patch in the area, as discussed at:
https://lists.gnu.org/archive/html/bug-coreutils/2023-02/msg00081.html
Also immediately rejecting input where we can't determine the size is a feature.
I.e. the following is the expected behavior:
$ : | split -n l/1
split: -: cannot determine file size
With the changes we now have:
$ : | split -n l/1 # Creates an empty file
$ yes | split -n l/1
split: -: cannot determine file size
This is inconsistent, and an insidious issue that users may introduce to scripts,
that will only fail once input data hits a certain size.
Also there are a few `make syntax-check` issues in the new split code.
Would it be possible to revert this change in isolation?
thanks,
Pádraig
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
2023-03-06 7:37 ` Paul Eggert
2023-03-06 11:13 ` Pádraig Brady
@ 2023-03-06 21:49 ` George Valkov
1 sibling, 0 replies; 21+ messages in thread
From: George Valkov @ 2023-03-06 21:49 UTC (permalink / raw)
To: Paul Eggert; +Cc: Pádraig Brady, Gnulib bugs, 61386-done
> On 2023-03-06, at 9:37 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
>
> I recall reading somewhere in this thread that a 'split' test was failing on macOS because it doesn't let you lseek on /dev/null. I fixed that problem here:
>
> https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=aa266f1b3dc4e12acdc46cc0f562adc03c2c0b8f
>
> and fixed some other 'split' issues in adjacent patches, while I was in the neighborhood.
I would assume the test are irrelevant, but since I already ran then, here you go:
https://httpstorm.com/share/.openwrt/test/2023-02-06_coreutils-9.1/test-09-e30af1e5840fdcabe3856f9ffcd6354b94d0a7ee-ori.txt
Georgi Valkov
httpstorm.com
nano RTOS
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-03-06 21:50 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <9A894CE0-00EC-44A2-A387-14BEF8BC8FCE@gmail.com>
[not found] ` <7b7f50d7-e50a-6272-0fec-038ff17ec61f@draigBrady.com>
[not found] ` <6878AF58-2F0E-4EE5-A72E-55263D45A8CF@gmail.com>
[not found] ` <2907ea4c-874b-c012-c02a-6fc2f97556f0@draigBrady.com>
[not found] ` <acbadedf-49de-3970-cb7b-84f6a4814bf8@cs.ucla.edu>
[not found] ` <1D804E0D-EC28-4C3B-B765-C9CA8FF3E974@gmail.com>
[not found] ` <335a749b-4a89-d04f-8db6-41a60becf98c@cs.ucla.edu>
[not found] ` <DD5D647B-0539-483A-943B-AFDBBCDBE9D7@gmail.com>
[not found] ` <82620d01-3dc1-b038-43a2-4a97bca5c482@cs.ucla.edu>
[not found] ` <375fb1b4-3b71-d6e4-4c44-aa2d7e588171@draigBrady.com>
[not found] ` <895887d1-1196-c45d-b522-0f3545a4e9f4@draigBrady.com>
[not found] ` <f7f2a9d8-9fac-13a9-1b06-57eca40f83c3@draigBrady.com>
[not found] ` <63F8AEAE-94AE-4E7B-B148-57E71397D840@gmail.com>
[not found] ` <f2aa59ba-0ef9-df0b-6cdc-ae3413f5502f@cs.ucla.edu>
[not found] ` <1f4bb896-45cc-9372-5cee-3882bb03529e@draigBrady.com>
[not found] ` <E138F1AF-24F6-43B3-92F6-8E7AE3657110@gmail.com>
[not found] ` <3caf257f-e085-929f-eaad-f05bc65d3a98@cs.ucla.edu>
[not found] ` <4E376F75-7440-4FEB-8934-3748F6AE06F4@gmail.com>
[not found] ` <9139A642-34D8-42A8-887F-CE027AAAB2F7@gmail.com>
2023-02-19 6:08 ` bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS Paul Eggert
2023-02-19 16:31 ` Pádraig Brady
2023-02-20 10:21 ` George Valkov
2023-02-20 15:02 ` George Valkov
2023-02-20 17:49 ` Pádraig Brady
2023-02-20 19:35 ` George Valkov
2023-02-20 21:14 ` Pádraig Brady
2023-02-20 21:25 ` George Valkov
2023-02-22 13:59 ` George Valkov
2023-02-23 22:23 ` Paul Eggert
2023-02-24 0:51 ` Pádraig Brady
2023-02-24 14:33 ` George Valkov
2023-02-24 15:43 ` Pádraig Brady
2023-02-24 19:34 ` George Valkov
2023-02-24 22:06 ` George Valkov
2023-02-24 22:47 ` Pádraig Brady
2023-02-24 23:23 ` George Valkov
2023-02-25 12:27 ` Pádraig Brady
2023-03-06 7:37 ` Paul Eggert
2023-03-06 11:13 ` Pádraig Brady
2023-03-06 21:49 ` George Valkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).