---------- Forwarded message ---------
Van: Harm Van der Vegt <harmvegt@gmail.com>
Date: zo 1 sep 2024 om 20:11
Subject: Re: bug#72919: 29.1; chart-space-usage in chart.el does not work correctly on windows
To: Eli Zaretskii <eliz@gnu.org>


>Thanks.  However, I think your changes are not entirely correct: they
>fail to account for space usage of files inside subdirectories of the
>directory which the user types at the prompt, whereas the
>implementation with "du" does account for that.

That is indeed a pretty major difference, my bad!

>In addition, I think if someone has 'du' on Windows, it should be used.

Is there a preference for using system binaries above elisp for cases like these?

>So I came up with the following changes instead.  Could you please try
>them, both with and without du.exe on PATH?  If these changes give
>good results, I will install them.

Thanks.

I've tried your changes with and without du. This uncovered something in the
original implementation, namely that the original implementation did not count
hidden files and directories. du * skips dotfiles for me.

With du present chart-space-usage shows the lisp directory as the largest in the 
emacs repository root. Without du it shows .git as the largest.

I'm not sure which output is the wanted output.

I've attempted to find a shell independent way to have du show dotfiles, but it 
appears to be rather tricky.

I've made a new patch that makes the elisp implementation recursive, uses the
rounding function provided by you and shows dotfiles.

Op zo 1 sep 2024 om 10:13 schreef Eli Zaretskii <eliz@gnu.org>:
> From: Harm Van der Vegt <harmvegt@gmail.com>
> Date: Sat, 31 Aug 2024 20:52:41 +0200
>
> Load the builtin chart package. When chart-space-usage is run, targeting
> any directory, chart fails to find any files and therefore can't create
> the chart. The expected behavior is that a bar chart is shown, ranking
> the files of the directory by size.
>
> This only fails on my Windows machine, it works on my Ubuntu
> installation.
>
> The output as shown in *Messages* is as follows:
>
> Collecting statistics...
> Running ‘cd g:/Shakespeare/;du -sk *’...
> Scanning output ...
> chart-space-usage: No files found!

Yes, the current implementation is not portable.

> chart-space-usage makes use of the du (disk usage) utility, which might
> not be present on all machines. In my case I have du installed, but
> chart-space-usage still failed to find any files.
>
> I have patched chart-space-usage to remove the dependency on du and make
> it OS agnostic and attached the patch to this email.

Thanks.  However, I think your changes are not entirely correct: they
fail to account for space usage of files inside subdirectories of the
directory which the user types at the prompt, whereas the
implementation with "du" does account for that.  In addition, I think
if someone has 'du' on Windows, it should be used.

So I came up with the following changes instead.  Could you please try
them, both with and without du.exe on PATH?  If these changes give
good results, I will install them.

diff --git a/lisp/emacs-lisp/chart.el b/lisp/emacs-lisp/chart.el
index da61e45..03dbe33 100644
--- a/lisp/emacs-lisp/chart.el
+++ b/lisp/emacs-lisp/chart.el
@@ -641,27 +641,63 @@ chart-file-count
                       (lambda (a b) (> (cdr a) (cdr b))))
     ))

+;; This assumes 4KB blocks
+(defun chart--file-size (size)
+  (* (/ (+ size 4095) 4096) 4096))
+
+(defun chart--directory-size (dir)
+  "Compute total size of files in directory DIR and its subdirectories.
+DIR is assumed to be a directory, verified by the caller."
+  (let ((size 0))
+    (dolist (file (directory-files-recursively dir "." t))
+      (let ((fsize (nth 7 (file-attributes file))))
+        (if (> fsize 0)
+            (setq size
+                  (+ size (chart--file-size fsize))))))
+    size))
+
 (defun chart-space-usage (d)
   "Display a top usage chart for directory D."
   (interactive "DDirectory: ")
   (message "Collecting statistics...")
   (let ((nmlst nil)
        (cntlst nil)
-       (b (get-buffer-create " *du-tmp*")))
-    (set-buffer b)
-    (erase-buffer)
-    (insert "cd " d ";du -sk * \n")
-    (message "Running `cd %s;du -sk *'..." d)
-    (call-process-region (point-min) (point-max) shell-file-name t
-                        (current-buffer) nil)
-    (goto-char (point-min))
-    (message "Scanning output ...")
-    (while (re-search-forward "^\\([0-9]+\\)[ \t]+\\([^ \n]+\\)$" nil t)
-      (let* ((nam (buffer-substring (match-beginning 2) (match-end 2)))
-            (num (buffer-substring (match-beginning 1) (match-end 1))))
-       (setq nmlst (cons nam nmlst)
-             ;; * 1000 to put it into bytes
-             cntlst (cons (* (string-to-number num) 1000) cntlst))))
+        b)
+    (if (executable-find "du")
+        (progn
+         (setq b (get-buffer-create " *du-tmp*"))
+          (set-buffer b)
+          (erase-buffer)
+          (if (and (memq system-type '(windows-nt ms-dos))
+                   (fboundp 'w32-shell-dos-semantics)
+                   (w32-shell-dos-semantics))
+              (progn
+                ;; With Windows shells, 'cd' does not change the drive,
+                ;; and ';' is not reliable for running multiple
+                ;; commands, so use alternatives.  We quote the
+                ;; directory because otherwise pushd will barf on a
+                ;; directory with forward slashes.
+                (insert "pushd \"" d "\" && du -sk * \n")
+                (message "Running `pushd \"%s\" && du -sk *'..." d))
+            (insert "cd " d ";du -sk * \n")
+            (message "Running `cd %s;du -sk *'..." d))
+          (call-process-region (point-min) (point-max) shell-file-name t
+                              (current-buffer) nil)
+          (goto-char (point-min))
+          (message "Scanning output ...")
+          (while (re-search-forward "^\\([0-9]+\\)[ \t]+\\([^ \n]+\\)$" nil t)
+            (let* ((nam (buffer-substring (match-beginning 2) (match-end 2)))
+                  (num (buffer-substring (match-beginning 1) (match-end 1))))
+             (setq nmlst (cons nam nmlst)
+                   ;; * 1000 to put it into bytes
+                   cntlst (cons (* (string-to-number num) 1000) cntlst)))))
+      (dolist (file (directory-files d t directory-files-no-dot-files-regexp))
+        (setq nmlst (cons (file-name-nondirectory file) nmlst))
+        (if (file-regular-p file)
+            (setq cntlst (cons (chart--file-size
+                                (nth 7 (file-attributes file)))
+                               cntlst))
+          (setq cntlst (cons (chart--directory-size file) cntlst)))))
     (if (not nmlst)
        (error "No files found!"))
     (chart-bar-quickie 'vertical (format "Largest files in %s" d)