[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1058522: (no subject)



On 2023-12-24, at 10:00:26 +0500, Lev Lamberov wrote:
> Чт 21 дек 2023 @ 15:06 Jeremy Sowden:
> > On 2023-12-21, at 11:21:48 +0000, Jeremy Sowden wrote:
> >> On 2023-12-21, at 11:10:28 +0500, Lev Lamberov wrote:
> >> > Since I cannot reproduce the bug, I'm downgrading the severity of
> >> > this bug report.
> >> 
> >> I cloned the git repo, ran `gbp buildpackage --git-pbuilder` and
> >> reproduced it (log attached).  I'll see if I can work out what's going
> >> on.
> 
> Thanks for your input. I double checked this bug report both under
> git-pbuilder and sbuild. E. g., here is the sbuild environment:
> 
> $ sudo sbuild-shell unstable
> I: /bin/sh
> # bash
> (unstable-amd64-sbuild)root@shakva:/# du --version
> du (GNU coreutils) 9.4
> Copyright (C) 2023 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>.
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> 
> Written by Torbjörn Granlund, David MacKenzie, Paul Eggert,
> and Jim Meyering.
> (unstable-amd64-sbuild)root@shakva:/# exit
> 
> But I still cannot reproduce the bug. Don't know...

First things first, do you see the following in your chroot?

  (unstable-amd64-sbuild)root@ulthar:~# d=$(mktemp -d)
  (unstable-amd64-sbuild)root@ulthar:~# du -sb $d
  0       /tmp/tmp.qay80HZ09w

This is the cause of the problem.  If your du is not reporting zero size
for an empty directory, that would explain the discrepancy and the rest
of this e-mail is irrelevant. :)

Now I'm going to walk through the test.  The dired directory listings I
quote below were got by running:

  # apt-get source dired-du
  # cd dired-du-0.5.2
  # export TERM=rxvt
  # emacs -nw -l dired-du.el

in an sbuild unstable chroot, entering extracts from the test source
adapted to run in the *scratch* buffer and executing them, and observing
the results in a dired buffer in the same frame.

The test source begins:

  ;; Sort by size only supported in `ls-lisp'.
  (ert-deftest dired-du-sort-by-size ()
    "Test ls-lisp sort by size with recursive size dir feature."
    (let* ((dir (make-temp-file "dired-du" 'dir))
           (filled-subdir (expand-file-name "filled-subdir" dir))
           (empty-subdir (expand-file-name "empty-subdir" dir))
           (external-file (expand-file-name "external-file" dir))
           (inner-file (expand-file-name "inner-file" filled-subdir))

We're going to create the following directory structure:

  dired-duXXXXXX/
   |
   +-> empty-subdir/
   |
   +-> external-file
   |
   +-> filled-subdir/
        |
        +-> inner-file

The test source continues:

           (ls-lisp-use-insert-directory-program nil)

We force the use of ls-lisp.

           (orig-def-dir default-directory)
           (dired-listing-switches "-lS")

We tell dired to sort by size.

           (buffers '()) mode-on)
      (unwind-protect
          (let (filled-subdir-size empty-subdir-size file-size)
            (make-directory filled-subdir)
            (make-directory empty-subdir)
            (setq default-directory dir)
            (add-to-list 'buffers (dired dir))
            (dired dir)

At this point we have created the two subdirectories and the dired
buffer contains:

  /tmp/dired-duSKABAl: (557 GiB available)
  drwxr-xr-x 2 root root 4096 Dec 29 16:40 empty-subdir
  drwxr-xr-x 2 root root 4096 Dec 29 16:40 filled-subdir

Note that the directory sizes come from opendir -> readdir -> fstat.  In
what follows, however, the recursive directory sizes are got by calling
du.

            (setq empty-subdir-size (dired-du--get-recursive-dir-size "empty-subdir"))

The test expects that the recursive size of "empty-subdir" will be some
non-zero value, $x, like the 4kB reported by fstat above.

	    (setq file-size (* empty-subdir-size 2))
            (setq filled-subdir-size (+ empty-subdir-size file-size))
            (dolist (file (list external-file inner-file))
              (write-region (make-string file-size ?.) nil file))

2 * $x bytes are written to "external-file" and "inner-file".  At this
point, the expectation is that "empty-subdir" has a recursive size $x,
the two files have size 2 * $x, and "filled-subdir" has a recursive size
$x + 2 * $x.  However, because du now reports directories as having zero
size, $x equals 0, the two files contain 0B, and the two directories
have recursive sizes of 0B.

            (dired-revert) ; Revert to show external-file

On running `M-x revert-buffer` in the dired buffer, it includes the
empty "external-file":

  /tmp/dired-duSKABAl: (557 GiB available)
  drwxr-xr-x 2 root root 4096 Dec 29 16:40 empty-subdir
  drwxr-xr-x 2 root root 4096 Dec 29 16:50 filled-subdir
  -rw-r--r-- 1 root root    0 Dec 29 16:50 external-file

The test source continues:

            (setq filled-subdir-size (dired-du--get-recursive-dir-size "filled-subdir"))
            (setq mode-on dired-du-mode)
            (dired-du-mode 1)

When I run `M-x dired-du-mode` in the dired buffer, the sizes of the
directories are replaced.  Those sizes are now also zero:

  /tmp/dired-duSKABAl: (557 GiB available)
  drwxr-xr-x 2 root root 0 Dec 29 16:40 empty-subdir
  drwxr-xr-x 2 root root 0 Dec 29 16:50 filled-subdir
  -rw-r--r-- 1 root root 0 Dec 29 16:50 external-file

The test source continues:

            ;; Enable the mode just replace the recursive dir sizes; it won't reorder the Dired buffer.
            ;; Revert the buffer to force a reorder.
            (dired-revert)

This forces the directory to be re-read and re-sorted.  However, as
mentioned previously, the size of the "external-file" is zero, and the
recursive sizes of the two subdirectories reported by du are also zero.
Since `(sort)` is stable, sorting by size will leave the directory
entries in the order in which they were returned by `readdir` and that
is indeterminate.  In my case, that order is:

  (unstable-amd64-sbuild)root@ulthar:/# ls -Ul /tmp/dired-duSKABAl/
  total 8
  drwxr-xr-x 2 root root 4096 Dec 29 16:40 empty-subdir
  drwxr-xr-x 2 root root 4096 Dec 29 16:50 filled-subdir
  -rw-r--r-- 1 root root    0 Dec 29 16:50 external-file

and so, after running `M-x revert-buffer` in the dired buffer, the order
remains:

  /tmp/dired-duSKABAl: (557 GiB available)
  drwxr-xr-x 2 root root 0 Dec 29 16:40 empty-subdir
  drwxr-xr-x 2 root root 0 Dec 29 16:50 filled-subdir
  -rw-r--r-- 1 root root 0 Dec 29 16:50 external-file

The test source continues:

            ;; At this point, the Dired buffer must be ordered by size as follows:
            ;; file/dir name        size
            ;; filled-subdir        3x
            ;; external-file        2x
            ;; empty subdir         x
            (dired-toggle-marks)
            (should (equal '("filled-subdir" "external-file" "empty-subdir")
                           (dired-get-marked-files 'local))))

Since the entries are not ordered as expected, the test fails.  In your
case, however, it appears that `readdir` happens to return the entries
in the right order, and the test passes.

In the process of putting this more detailed explanation together, it
occurred to me that while the patch I proposed in my previous message
(using `du -sB1` instead of `du -sb`) would restore something close to
the original behaviour, the upshot of the change in du's behaviour is
not that dired-du is broken, but that the specific expectations of this
test have been violated.  Thus, perhaps it makes more sense to fix the
test and add a note to NEWS.debian or README.debian.  I've attached a
patch that fixes just the test.

J.
From b4edae465ccc0f49e60c16376e3cef49ec68c2be Mon Sep 17 00:00:00 2001
From: Jeremy Sowden <jeremy@azazel.net>
Date: Fri, 29 Dec 2023 19:53:01 +0000
Subject: [PATCH] Don't derive file-size from directory-size when it is zero

It is assumed that the size of any empty directory reported by du(1) will be
non-zero, but with recent versions of GNU du(1), this may not be the case.
Therefore, if the directory-size is zero, explicitly set the file-sizes to
constant values.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 dired-du-tests.el | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/dired-du-tests.el b/dired-du-tests.el
index 4c77932c5147..6436373e4088 100644
--- a/dired-du-tests.el
+++ b/dired-du-tests.el
@@ -246,17 +246,25 @@
          (dired-listing-switches "-lS")
          (buffers '()) mode-on)
     (unwind-protect
-        (let (filled-subdir-size empty-subdir-size file-size)
+        (let (filled-subdir-size empty-subdir-size file-size external-file-size inner-file-size)
           (make-directory filled-subdir)
           (make-directory empty-subdir)
           (setq default-directory dir)
           (add-to-list 'buffers (dired dir))
           (dired dir)
           (setq empty-subdir-size (dired-du--get-recursive-dir-size "empty-subdir"))
-          (setq file-size (* empty-subdir-size 2))
-          (setq filled-subdir-size (+ empty-subdir-size file-size))
-          (dolist (file (list external-file inner-file))
-            (write-region (make-string file-size ?.) nil file))
+          (if (> empty-subdir-size 0)
+              (progn
+                (setq file-size (* empty-subdir-size 2))
+                (setq external-file-size file-size)
+                (setq inner-file-size file-size)
+                (setq filled-subdir-size (+ empty-subdir-size inner-file-size)))
+            (setq external-file-size 8192)
+            (setq inner-file-size 12288)
+            (setq filled-subdir-size inner-file-size))
+          (dolist (fs (list (cons external-file external-file-size)
+                            (cons inner-file inner-file-size)))
+            (write-region (make-string (cdr fs) ?.) nil (car fs)))
           (dired-revert) ; Revert to show external-file
           (setq filled-subdir-size (dired-du--get-recursive-dir-size "filled-subdir"))
           (setq mode-on dired-du-mode)
-- 
2.43.0

Attachment: signature.asc
Description: PGP signature


Reply to: