unofficial mirror of bug-guile@gnu.org 
 help / color / mirror / Atom feed
* bug#75590: ports.test: "SEEK_DATA while in hole" fails on some hosts
@ 2025-01-15 17:30 Rob Browning
  2025-01-17 14:09 ` Mikael Djurfeldt
  0 siblings, 1 reply; 3+ messages in thread
From: Rob Browning @ 2025-01-15 17:30 UTC (permalink / raw)
  To: 75590


This test failed on one of the debian buildds[1] like this:

  Running popen.test
  Backtrace:
             4 (primitive-load "/build/reproducible-path/guile-3.0-3.0.10+really3.0.10/test-suite/tests/ports.test")
  In ice-9/eval.scm:
      619:8  3 (_ #(#(#<directory (test-suite test-ports) 7fff9a860460> "/build/reproducible-path/guile-3.0-3.0.10+really3.0.10/ports-test.tmp") #<closed: file 7fff959be8c0>))
      619:8  2 (_ #(#(#(#<directory (test-suite lib) 7fff9a8d80a0> #<variable 7fff9a90a5d0 value: #t>) "SEEK_DATA while in hole" #t #<procedure 7fff95a17c80 at ice-9/eval.scm:330:13 ()>) ("ports.test" "SEEK_DATA while in hole")))
  In ice-9/boot-9.scm:
     260:13  1 (for-each #<procedure 7fff95a17240 at ice-9/eval.scm:333:13 (a)> _)
  In ice-9/eval.scm:
      619:8  0 (_ #(#(#<directory (test-suite lib automake) 7fff9a860820> (fail ("ports.test" "SEEK_DATA while in hole") expected-value 4096 actual-value 10))))

  ice-9/eval.scm:619:8: Throw to key `match-error' with args `("match" "no matching pattern" (fail ("ports.test" "SEEK_DATA while in hole") expected-value 4096 actual-value 10))'.
  Running ports.test
  FAIL: ports.test: SEEK_DATA while in hole - arguments: (expected-value 4096 actual-value 10)

I suspect it's because that buildd is running in an unshare/chroot with
a filesystem that just doesn't support the seek data/hole semantics we
expect, and it sounds like returning the seek offset (here 10) is
allowed (at least by Debian's lseek(2)):

  In the simplest implementation, a filesystem can support the
  operations by making SEEK_HOLE always return the offset of the end of
  the file, and making SEEK_DATA always return offset (i.e., even if the
  location referred to by offset is a hole, it can be considered to
  consist of data that is a sequence of zeros).

How we might want to fix this depends of course on exactly what the test
is trying to verify, but if it's trying to verify that Guile's functions
work properly when the filesystem does support the semantics we expect,
then we could consider limiting the tests to relevant filesystems
(e.g. ext4, ...).

In that case, while I don't know of a simple way to detect the type of
the test filesystem, if it's useful, bup currently uses this to handle
some platforms
(https://codeberg.org/bup/bup/src/branch/main/dev/path-fs):

  #!/usr/bin/env bash

  set -ueo pipefail

  kernel="$(uname -s)"
  case "$kernel" in
      NetBSD)
          fs() { df -G "$1" | sed -En 's/.* ([^ ]*) fstype.*/\1/p'; }
          ;;
      SunOS)
          fs() { df -g "$1" | sed -En 's/.* ([^ ]*) fstype.*/\1/p'; }
          ;;
      *)
          fs() { df -T "$1" | awk 'END{print $2}'; }
  esac

  while test $# -ne 0; do
      fs "$1"
      shift
  done

[1] From https://buildd.debian.org/status/fetch.php?pkg=guile-3.0&arch=ppc64el&ver=3.0.10%2Breally3.0.10-1&stamp=1736918931&raw=0

Thanks
-- 
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4





^ permalink raw reply	[flat|nested] 3+ messages in thread

* bug#75590: ports.test: "SEEK_DATA while in hole" fails on some hosts
  2025-01-15 17:30 bug#75590: ports.test: "SEEK_DATA while in hole" fails on some hosts Rob Browning
@ 2025-01-17 14:09 ` Mikael Djurfeldt
  2025-01-19 20:14   ` Rob Browning
  0 siblings, 1 reply; 3+ messages in thread
From: Mikael Djurfeldt @ 2025-01-17 14:09 UTC (permalink / raw)
  To: Rob Browning; +Cc: Mikael Djurfeldt, 75590

[-- Attachment #1: Type: text/plain, Size: 3607 bytes --]

We could, in addition to "stat", provide "statfs" (on systems that provide
it).

The test could then first test for the presence of "statfs" and then check
if the filesystem is ext4.

Best regards,
Mikael

On Wed, Jan 15, 2025 at 6:32 PM Rob Browning <rlb@defaultvalue.org> wrote:

>
> This test failed on one of the debian buildds[1] like this:
>
>   Running popen.test
>   Backtrace:
>              4 (primitive-load
> "/build/reproducible-path/guile-3.0-3.0.10+really3.0.10/test-suite/tests/ports.test")
>   In ice-9/eval.scm:
>       619:8  3 (_ #(#(#<directory (test-suite test-ports) 7fff9a860460>
> "/build/reproducible-path/guile-3.0-3.0.10+really3.0.10/ports-test.tmp")
> #<closed: file 7fff959be8c0>))
>       619:8  2 (_ #(#(#(#<directory (test-suite lib) 7fff9a8d80a0>
> #<variable 7fff9a90a5d0 value: #t>) "SEEK_DATA while in hole" #t
> #<procedure 7fff95a17c80 at ice-9/eval.scm:330:13 ()>) ("ports.test"
> "SEEK_DATA while in hole")))
>   In ice-9/boot-9.scm:
>      260:13  1 (for-each #<procedure 7fff95a17240 at ice-9/eval.scm:333:13
> (a)> _)
>   In ice-9/eval.scm:
>       619:8  0 (_ #(#(#<directory (test-suite lib automake) 7fff9a860820>
> (fail ("ports.test" "SEEK_DATA while in hole") expected-value 4096
> actual-value 10))))
>
>   ice-9/eval.scm:619:8: Throw to key `match-error' with args `("match" "no
> matching pattern" (fail ("ports.test" "SEEK_DATA while in hole")
> expected-value 4096 actual-value 10))'.
>   Running ports.test
>   FAIL: ports.test: SEEK_DATA while in hole - arguments: (expected-value
> 4096 actual-value 10)
>
> I suspect it's because that buildd is running in an unshare/chroot with
> a filesystem that just doesn't support the seek data/hole semantics we
> expect, and it sounds like returning the seek offset (here 10) is
> allowed (at least by Debian's lseek(2)):
>
>   In the simplest implementation, a filesystem can support the
>   operations by making SEEK_HOLE always return the offset of the end of
>   the file, and making SEEK_DATA always return offset (i.e., even if the
>   location referred to by offset is a hole, it can be considered to
>   consist of data that is a sequence of zeros).
>
> How we might want to fix this depends of course on exactly what the test
> is trying to verify, but if it's trying to verify that Guile's functions
> work properly when the filesystem does support the semantics we expect,
> then we could consider limiting the tests to relevant filesystems
> (e.g. ext4, ...).
>
> In that case, while I don't know of a simple way to detect the type of
> the test filesystem, if it's useful, bup currently uses this to handle
> some platforms
> (https://codeberg.org/bup/bup/src/branch/main/dev/path-fs):
>
>   #!/usr/bin/env bash
>
>   set -ueo pipefail
>
>   kernel="$(uname -s)"
>   case "$kernel" in
>       NetBSD)
>           fs() { df -G "$1" | sed -En 's/.* ([^ ]*) fstype.*/\1/p'; }
>           ;;
>       SunOS)
>           fs() { df -g "$1" | sed -En 's/.* ([^ ]*) fstype.*/\1/p'; }
>           ;;
>       *)
>           fs() { df -T "$1" | awk 'END{print $2}'; }
>   esac
>
>   while test $# -ne 0; do
>       fs "$1"
>       shift
>   done
>
> [1] From
> https://buildd.debian.org/status/fetch.php?pkg=guile-3.0&arch=ppc64el&ver=3.0.10%2Breally3.0.10-1&stamp=1736918931&raw=0
>
> Thanks
> --
> Rob Browning
> rlb @defaultvalue.org and @debian.org
> GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
> GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 4915 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* bug#75590: ports.test: "SEEK_DATA while in hole" fails on some hosts
  2025-01-17 14:09 ` Mikael Djurfeldt
@ 2025-01-19 20:14   ` Rob Browning
  0 siblings, 0 replies; 3+ messages in thread
From: Rob Browning @ 2025-01-19 20:14 UTC (permalink / raw)
  To: Mikael Djurfeldt; +Cc: Mikael Djurfeldt, 75590

[-- Attachment #1: Type: text/plain, Size: 336 bytes --]

Mikael Djurfeldt <mikael@djurfeldt.com> writes:

> We could, in addition to "stat", provide "statfs" (on systems that provide
> it).
>
> The test could then first test for the presence of "statfs" and then check
> if the filesystem is ext4.

Here's what I've done in Debian for now after double-checking the
test on a few filesystems:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: only-test-sparse-data-hole-where-expected-to-work.patch --]
[-- Type: text/x-diff, Size: 2927 bytes --]

From ea1750ad7f27593ee42ca48c74cc5b3e3cdd2d1d Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@defaultvalue.org>
Date: Wed, 15 Jan 2025 12:54:02 -0600
Subject: [PATCH 1/1] Skip ports.test seek tests unless filesystem looks
 suitable

For now, only test on filesystems that we know have the expected
semantics, i.e. btrfs, ext4, and xfs.
---
 test-suite/tests/ports.test | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/test-suite/tests/ports.test b/test-suite/tests/ports.test
index bec5e356c5..baa9edb700 100644
--- a/test-suite/tests/ports.test
+++ b/test-suite/tests/ports.test
@@ -39,9 +39,23 @@
 (define (test-file)
   (data-file-name "ports-test.tmp"))
 
-(define (skip-on-darwin)
-  (when (string-ci=? "darwin" (utsname:sysname (uname)))
-    (throw 'untested)))
+(define (skip-unless-fs-handles-holes-as-expected)
+  ;; For now only allow filesystems that should have the seek hole/data
+  ;; semantics the tests expect.  Filesystems vary both in how they
+  ;; handle sparseness in general (e.g. granularity), how they handle
+  ;; SEEK_DATA and SEEK_HOLE (see lseek(2) for some related info), and
+  ;; even how quickly they reflect changes.  du's output, for example,
+  ;; may not immediately reflect sparseness changes (previously observed
+  ;; on btrfs and zfs).
+  (let* ((p (open-input-pipe "debian/bin/path-fs ."))
+         (fs (read-all p)))
+    (close p)
+    (when (string=? "\n" fs)
+      (error "unexpected output from debian/bin/path-fs" fs))
+    (or (string=? "btrfs\n" fs)
+        (string=? "ext4\n" fs)
+        (string=? "xfs\n" fs)
+        (throw 'untested))))
 
 \f
 ;;;; Some general utilities for testing ports.
@@ -189,7 +203,6 @@
     (close-port iport))
   (delete-file filename))
 
-;;; Note: Holes are weird on Darwin.
 (let* ((file (test-file))
        (port (open-output-file file)))
   (seek port 4096 SEEK_SET)
@@ -198,15 +211,12 @@
 
   (pass-if-equal "size of sparse file"
       4100
-    ;; XXX: On macOS, APFS does support sparse files, they do not behave
-    ;; like on Linux.  Skip these tests on macOS.
-    (skip-on-darwin)
-
+    (skip-unless-fs-handles-holes-as-expected)
     (stat:size (stat file)))
 
   (pass-if-equal "SEEK_DATA while on data"
       4096
-    (skip-on-darwin)
+    (skip-unless-fs-handles-holes-as-expected)
     (if (defined? 'SEEK_DATA)
         (call-with-input-file file
           (lambda (port)
@@ -219,7 +229,7 @@
 
   (pass-if-equal "SEEK_DATA while in hole"
       4096
-    (skip-on-darwin)
+    (skip-unless-fs-handles-holes-as-expected)
     (if (defined? 'SEEK_DATA)
         (call-with-input-file file
           (lambda (port)
@@ -232,7 +242,7 @@
 
   (pass-if-equal "SEEK_HOLE while in hole"
       10
-    (skip-on-darwin)
+    (skip-unless-fs-handles-holes-as-expected)
     (if (defined? 'SEEK_HOLE)
         (call-with-input-file file
           (lambda (port)
-- 
2.45.2


[-- Attachment #3: Type: text/plain, Size: 267 bytes --]


But happy to help with whatever solution we prefer in guile itself.

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-01-19 20:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-15 17:30 bug#75590: ports.test: "SEEK_DATA while in hole" fails on some hosts Rob Browning
2025-01-17 14:09 ` Mikael Djurfeldt
2025-01-19 20:14   ` Rob Browning

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).