Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
> On 2023-02-25, at 12:47 AM, Pádraig Brady 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
Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
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
Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
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 ttys0200: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
Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
> On 2023-02-24, at 5:43 PM, Pádraig Brady wrote: > > On 24/02/2023 14:33, George Valkov wrote: >>> On 2023-02-24, at 12:23 AM, 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. >> 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 ttys0100: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 com
Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
On 24/02/2023 14:33, George Valkov wrote:
On 2023-02-24, at 12:23 AM, 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.
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 ttys0100: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ádraigFrom 952c8bad4c297ed48cddbb81f1030a35812ca980 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?=
Date: Fri, 24 Feb 2023 15:40:37 +
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
Re: bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS
> On 2023-02-24, at 12:23 AM, 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. 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 ttys0100: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 ./bootstr
