GNU bug report logs - #50630
28.0.50; list-directory shows free space for current directory, not the specified one

Previous Next

Package: emacs;

Reported by: John Cummings <john <at> rootabega.net>

Date: Thu, 16 Sep 2021 23:39:02 UTC

Severity: normal

Found in version 28.0.50

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 50630 in the body.
You can then email your comments to 50630 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Thu, 16 Sep 2021 23:39:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to John Cummings <john <at> rootabega.net>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 16 Sep 2021 23:39:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: 28.0.50;
 list-directory shows free space for current directory, not the
 specified one
Date: Thu, 16 Sep 2021 23:00:50 +0000
[Message part 1 (text/plain, inline)]
Hello, long time emacs user; first time reporter. Apologies if I do anything wrong here.

The verbose output for list-directory always shows the free space for the current directory of the buffer where it was called, not the target directory that is supplied as an argument.

This may have been first introduced in 849051903e7, while fixing this report:
https://lists.gnu.org/archive/html/bug-gnu-emacs/2002-03/msg00251.html
The result was that, while the *Directory* output buffer's current directory was set correctly, setting it last meant that the free space calculation always took place in "." rather than the argument to list-directory.

Here are commands/output to demonstrate. Note that the free space changes for the same directory argument when I change the current directory, since the two different directories are on different disks.


  (eval-expression (pwd) t)
  "Directory ~/emacs/"

  (eval-expression (list-directory "~" t) t)
  "/home/emacsdev/"

  (insert-buffer-substring "*Directory*")
  Directory /home/emacsdev
  total used in directory 0 available 15.5 GiB
  lrwxrwxrwx 1 /emacsdev /emacsdev 26 Sep 16 17:45 emacs -> /media/ubuntu/turkey/emacs
  lrwxrwxrwx 1 /emacsdev /emacsdev 34 Sep 16 17:44 emacs-project -> /media/ubuntu/turkey/emacs-project


  (cd "/")

  (eval-expression (pwd) t)
  "Directory /"

  (eval-expression (list-directory "~" t) t)
  "/home/emacsdev/"

  (insert-buffer-substring "*Directory*")
  Directory /home/emacsdev
  total used in directory 0 available 7.7 GiB
  lrwxrwxrwx 1 /emacsdev /emacsdev 26 Sep 16 17:45 emacs -> /media/ubuntu/turkey/emacs
  lrwxrwxrwx 1 /emacsdev /emacsdev 34 Sep 16 17:44 emacs-project -> /media/ubuntu/turkey/emacs-project

Changing the call to get-free-disk-space seemed to work in the attached patch, but I haven't attempted to dig deeper and check for any regressions. I would be glad to do that, but would need more time to research and would probably ask for more help to do it.

Thanks for everything!



In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
 of 2021-09-10 built on ubuntu
Repository revision: ceb60225bacc7650b5e52032c0c33b9d67f9a6d7
Repository branch: master
System Description: Ubuntu 20.04.1 LTS

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG SECCOMP SOUND
THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg rfc6068 epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils subr-x pcase
misearch multi-isearch vc-git diff-mode easy-mmode vc-dispatcher
bug-reference find-func cl-extra shortdoc text-property-search seq
mule-diag thingatpt help-fns radix-tree help-mode cl-loaddefs cl-lib
term/xterm xterm byte-opt gv bytecomp byte-compile cconv iso-transl
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 99674 10407)
 (symbols 48 8011 1)
 (strings 32 25795 958)
 (string-bytes 1 846999)
 (vectors 16 13457)
 (vector-slots 8 152638 6184)
 (floats 8 90 749)
 (intervals 56 1088 62)
 (buffers 992 16))


[0001-text-verbose-dir-fix.patch (text/x-patch, attachment)]

Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Fri, 17 Sep 2021 07:09:01 GMT) Full text and rfc822 format available.

Notification sent to John Cummings <john <at> rootabega.net>:
bug acknowledged by developer. (Fri, 17 Sep 2021 07:09:01 GMT) Full text and rfc822 format available.

Message #10 received at 50630-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: John Cummings <john <at> rootabega.net>
Cc: 50630-done <at> debbugs.gnu.org
Subject: Re: bug#50630: 28.0.50;
 list-directory shows free space for current directory, not the
 specified one
Date: Fri, 17 Sep 2021 10:07:59 +0300
> Date: Thu, 16 Sep 2021 23:00:50 +0000
> From: John Cummings <john <at> rootabega.net>
> 
> Hello, long time emacs user; first time reporter. Apologies if I do anything wrong here.

Thanks.  In the future, please make sure Git uses your actual email
address, as without that we need to apply the patch by hand.  Also, we
request a commit log message formatted according to ChangeLog rules;
see CONTRIBUTE for the details.  (I made the necessary changes for you
this time.)

Also, if you intend to contribute in the future, I recommend starting
your legal paperwork rolling for assigning copyright to the FSF.  If
you agree, I will send you the form to fill and the instructions to go
with it.

> Changing the call to get-free-disk-space seemed to work in the attached patch, but I haven't attempted to dig deeper and check for any regressions. I would be glad to do that, but would need more time to research and would probably ask for more help to do it.

The patch makes sense to me, so I installed it, and I'm closing this
bug.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Fri, 17 Sep 2021 10:37:01 GMT) Full text and rfc822 format available.

Message #13 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: John Cummings <john <at> rootabega.net>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: 28.0.50;
 list-directory shows free space for current directory, not the
 specified one
Date: Fri, 17 Sep 2021 13:36:19 +0300
> Date: Fri, 17 Sep 2021 10:24:30 +0000
> From: John Cummings <john <at> rootabega.net>
> Cc: 50630-done <at> debbugs.gnu.org
> 
> On Friday, September 17th, 2021 at 3:07 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
> Sorry, I expected to have to do some more research and change a few other things; just intended that patch to be a proof of concept. Thank you for correcting it for me. I will do some deeper research as well as follow up with a test.

It looked like TRT to me, but of course more testing will be welcome.

Thanks!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Fri, 17 Sep 2021 15:57:01 GMT) Full text and rfc822 format available.

Message #16 received at 50630-done <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630-done <at> debbugs.gnu.org
Subject: Re: bug#50630: 28.0.50;
 list-directory shows free space for current directory, not the
 specified one
Date: Fri, 17 Sep 2021 10:24:30 +0000
On Friday, September 17th, 2021 at 3:07 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:

> Thanks. In the future,
...
>
> Also, if you intend to contribute in the future, I recommend starting
>
> your legal paperwork rolling for assigning copyright to the FSF. If
>
> you agree, I will send you the form to fill and the instructions to go
>
> with it.

That sounds great, please do send me the form.

Sorry, I expected to have to do some more research and change a few other things; just intended that patch to be a proof of concept. Thank you for correcting it for me. I will do some deeper research as well as follow up with a test.

Thanks!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Fri, 17 Sep 2021 17:39:02 GMT) Full text and rfc822 format available.

Message #19 received at 50630-done <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Eli Zaretskii <eliz <at> gnu.org>, John Cummings <john <at> rootabega.net>
Cc: "50630-done <at> debbugs.gnu.org" <50630-done <at> debbugs.gnu.org>
Subject: RE: [External] : bug#50630: 28.0.50; list-directory shows free space
 for current directory, not the specified one
Date: Fri, 17 Sep 2021 17:38:42 +0000
> The patch makes sense to me, so I installed it, and I'm closing this
> bug.

I believe the same change is needed in ls-lisp.el, for function `ls-lisp--insert-directory'.

  (let ((available (get-free-disk-space ".")))

should be:

  (let ((available (get-free-disk-space file)))

(But you might want to check whether FILE is what's needed, and not, say, ORIG-FILE.)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Fri, 17 Sep 2021 18:05:02 GMT) Full text and rfc822 format available.

Message #22 received at 50630-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: john <at> rootabega.net, 50630-done <at> debbugs.gnu.org
Subject: Re: [External] : bug#50630: 28.0.50; list-directory shows free space
 for current directory, not the specified one
Date: Fri, 17 Sep 2021 21:04:28 +0300
> From: Drew Adams <drew.adams <at> oracle.com>
> CC: "50630-done <at> debbugs.gnu.org" <50630-done <at> debbugs.gnu.org>
> Date: Fri, 17 Sep 2021 17:38:42 +0000
> Accept-Language: en-US
> 
> > The patch makes sense to me, so I installed it, and I'm closing this
> > bug.
> 
> I believe the same change is needed in ls-lisp.el, for function `ls-lisp--insert-directory'.
> 
>   (let ((available (get-free-disk-space ".")))
> 
> should be:
> 
>   (let ((available (get-free-disk-space file)))

Why, do you see the wrong values being reported?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Fri, 17 Sep 2021 20:34:02 GMT) Full text and rfc822 format available.

Message #25 received at 50630-done <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630-done <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#50630: [External] : bug#50630: 28.0.50;
 list-directory shows free space for current directory, not the
 specified one
Date: Fri, 17 Sep 2021 20:32:53 +0000
On Friday, September 17th, 2021 at 2:04 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Drew Adams drew.adams <at> oracle.com
> > I believe the same change is needed in ls-lisp.el, for function `ls-lisp--insert-directory'.
> >
> > (let ((available (get-free-disk-space ".")))
> >
> > should be:
> >
> > (let ((available (get-free-disk-space file)))
>
> Why, do you see the wrong values being reported?

The values look wrong to me here. Thanks for the heads up. I've got a working test for files.el and ls-lisp.el, and after I polish it, I'll also look into how to handle FILE vs ORIG-FILE




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 18 Sep 2021 05:58:02 GMT) Full text and rfc822 format available.

Message #28 received at 50630-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: John Cummings <john <at> rootabega.net>
Cc: 50630-done <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#50630: [External] : bug#50630: 28.0.50;
 list-directory shows free space for current directory, not the
 specified one
Date: Sat, 18 Sep 2021 08:56:45 +0300
> Date: Fri, 17 Sep 2021 20:32:53 +0000
> From: John Cummings <john <at> rootabega.net>
> Cc: Drew Adams <drew.adams <at> oracle.com>, 50630-done <at> debbugs.gnu.org
> 
> On Friday, September 17th, 2021 at 2:04 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
> > > From: Drew Adams drew.adams <at> oracle.com
> > > I believe the same change is needed in ls-lisp.el, for function `ls-lisp--insert-directory'.
> > >
> > > (let ((available (get-free-disk-space ".")))
> > >
> > > should be:
> > >
> > > (let ((available (get-free-disk-space file)))
> >
> > Why, do you see the wrong values being reported?
> 
> The values look wrong to me here.

In my testing, they reflect the correct volume (because the call is in
the buffer whose default-directory is set to the directory we are
going to display), which is why I installed the changes.  Please tell
how you test, starting from "emacs -Q", because I'd like to look at
this.

> Thanks for the heads up. I've got a working test for files.el and ls-lisp.el, and after I polish it, I'll also look into how to handle FILE vs ORIG-FILE

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 18 Sep 2021 10:06:02 GMT) Full text and rfc822 format available.

Message #31 received at 50630-done <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630-done <at> debbugs.gnu.org
Subject: Re: bug#50630: [External] : bug#50630: 28.0.50;
 list-directory shows free space for current directory, not the
 specified one
Date: Sat, 18 Sep 2021 10:04:47 +0000
On Saturday, September 18th, 2021 at 1:56 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:

> > The values look wrong to me here.
>
> In my testing, they reflect the correct volume (because the call is in
> the buffer whose default-directory is set to the directory we are
> going to display), which is why I installed the changes. Please tell
> how you test, starting from "emacs -Q", because I'd like to look at
> this.

Here's a dirty manual way to confirm it. I just chose directories that
I know are on different volumes, but I also confirmed it works with a loopback
filesystem created with something like losetup. This worked, i.e. showed me different
free space, with emacs -Q on a clean build of c1356d3a05a789555c327dd4cb0f444fdf7be205


;;doesn't matter; just use an existing dir
(setq ld-target "~")

;;two dirs on different volumes
(setq ld-working-dirs '("/" "~/emacs/"))


(require 'ls-lisp)
;;make sure to use the ls-lisp functionality
(setq ls-lisp-use-insert-directory-program nil)


;;shows the list-directory output for all working dirs
(defun insert-ld-bug ()
  (dolist (ld-working-dir ld-working-dirs)
    (let ((default-directory ld-working-dir))
      (list-directory ld-target t)
      (insert-buffer-substring "*Directory*")
      (insert "=====\n\n\n"))))

(with-current-buffer-window "ld-bug" nil nil
  (insert-ld-bug))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Tue, 21 Sep 2021 10:59:01 GMT) Full text and rfc822 format available.

Message #34 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: "50630 <at> debbugs.gnu.org" <50630 <at> debbugs.gnu.org>
Subject: Confirmation of instances in ls-lisp.el and tramp-sh.el
Date: Tue, 21 Sep 2021 10:57:53 +0000
[Message part 1 (text/plain, inline)]
I think I've found all the instances of this bug: files.el (already
fixed) and ls-lisp.el (already identified) and tramp-sh.el. I attached
recipes to reproduce against master @
0646c6817139aa905a2f6079fdc82eb4be944de0.  The preceding libraries are
the 3 implementations of insert-directory I found. From what I can
tell, these are usually expected to be called from dired and
list-directory, the former of which ensures default-directory is set
as expected before calling insert-directory, and the latter of which
does not, which is where this was initially reported.

Testing these in a real scenario doesn't show any other interesting
behaviors.  Invoking via dired always does the correct thing, and
invoking via list-directory, or calling insert-directory directly,
always does the wrong thing (i.e. shows the free space of
default-directory instead of the insert-directory file arg) when the
default-directory does not match the file argument. This is true when
either/both of the insert-directory arg or the default-directory are
standard paths or tramp paths.

I think the fixes for the remaining two libraries are going to be as
simple as the previous fix: pass file-system-info the actual file arg
instead of ".". While ensuring default-directory is set as we want
seems to allow "." to work correctly, I haven't found a concrete
advantage to that over just passing the file arg through. (e.g. some
path expansion that only file-system-info can do.) I'm not very
confident that I understand that space fully, though.

Though the fixes may be simple, the tests for even one of the
libraries would be large enough to require my completed copyright
assignment, which is in progress now. The approach I have working now
is just to stub out file-system-info to simulate returning different
free space for different paths and verifying
list-directory/insert-directory shows the matching free space. Anybody
see any big problems with that or have other suggestions?  I also
don't want to duplicate that test for each of the 3 libraries, but I
haven't found a good way to share test utilities like that yet.
[50630-recipe.txt (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Fri, 24 Sep 2021 19:59:02 GMT) Full text and rfc822 format available.

Message #37 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: "50630 <at> debbugs.gnu.org" <50630 <at> debbugs.gnu.org>
Subject: [PATCH] Add tests for insert-directory
Date: Fri, 24 Sep 2021 19:58:25 +0000
[Message part 1 (text/plain, inline)]
The attached patch adds a regression test for this bug, as well as a
couple other basic functionality tests for insert-directory. This does
not test the other unfixed cases still in ls-lisp.el and tramp-sh.el,
but I thought it prudent to get these tests in before trying to apply
them in other libraries.

Along those lines, I also attempted to skip the test when ls-lisp
would be used during files-tests.el, which I predict might happen when
building on Windows?

I sent in my copyright assignment, but I think it's still being
finalized.

Thank you!
[0001-Add-tests-for-insert-directory.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 01:49:01 GMT) Full text and rfc822 format available.

Message #40 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: John Cummings <john <at> rootabega.net>
Cc: "50630 <at> debbugs.gnu.org" <50630 <at> debbugs.gnu.org>
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 03:47:45 +0200
John Cummings <john <at> rootabega.net> writes:

> I sent in my copyright assignment, but I think it's still being
> finalized.

Skimming the patch, it looks good to me, but as you say, we have to wait
until the copyright process is finished.

But this bug report has been closed as fixed, so to avoid your patch
being lost, can you resubmit it with `M-x submit-emacs-patch' so that it
gets its own bug number?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 06:11:01 GMT) Full text and rfc822 format available.

Message #43 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: John Cummings <john <at> rootabega.net>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 09:10:31 +0300
> Date: Fri, 24 Sep 2021 19:58:25 +0000
> From: John Cummings <john <at> rootabega.net>
> 
> Along those lines, I also attempted to skip the test when ls-lisp
> would be used during files-tests.el, which I predict might happen when
> building on Windows?

Why did you need to skip those tests?  Can you elaborate?

> +       ;;Try to verify that insert-directory will come from files.el,
> +       ;;not ls-lisp.el. Windows builds will probably use ls-lisp.el
> +       ;;by default, which will invalidate some tests.
> +       (insert-directory-program-used (or (not (featurep 'ls-lisp))
> +                                          ls-lisp-use-insert-directory-program)))

I guess this is part of the same question I asked above?  Because I
don't think I understand what you are trying to do here and why.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 10:46:01 GMT) Full text and rfc822 format available.

Message #46 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: "50630 <at> debbugs.gnu.org" <50630 <at> debbugs.gnu.org>
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 10:45:08 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> wrote:

> But this bug report has been closed as fixed, so to avoid your patch
> being lost, can you resubmit it with `M-x submit-emacs-patch' so that it
> gets its own bug number?

I'd be glad to do that, but before I do, let me make sure we're all on
the same page. Earlier in the discussion, it was revealed that this
same bug is present in ls-lisp.el and tramp-sh.el, and I plan to
submit the trivial fixes for those next. (Let me know if the recipes
to confirm those bugs I attached earlier don't work for you.) The
cause of, and fix for, all of them are essentially the same, just
located in 3 different places. Would you prefer to have a bug number
for each of the places where it occurs, or to reopen this one (if
that's something that's done) to hold all the fixes for essentially
the same bug, as well as for the regression tests linked to those
fixes?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 11:39:02 GMT) Full text and rfc822 format available.

Message #49 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 11:38:08 +0000
Eli Zaretskii <eliz <at> gnu.org> wrote:

>> Date: Fri, 24 Sep 2021 19:58:25 +0000
>> From: John Cummings <john <at> rootabega.net>
>>
>> Along those lines, I also attempted to skip the test when ls-lisp
>> would be used during files-tests.el, which I predict might happen when
>> building on Windows?
>
> Why did you need to skip those tests?  Can you elaborate?

ls-lisp has its own implementation of insert-directory, which
duplicates the bug, and which will also duplicate the fix when I
submit it soon.  (Let me know if the recipe to confirm the bug is
present in ls-lisp.el attached earlier doesn't work for you.)  So if
ls-lisp was "active" during this test, it could be a false
positive/negative for this test, depending on whether ls-lisp had been
fixed yet. (See my reply to your later question in this message for
what I mean when I say ls-lisp is "active", because just requiring the
library isn't enough.)

On my GNU/Linux system, ls-lisp is not active when running this test
from the Makefile, but I don't want to assume that that is true of
every system. Since Windows GNU Emacs uses ls-lisp.el by default when
running, I thought it might be one of those exceptions at build time
as well. I confirmed that it's also true if someone runs this test
interactively after having activated ls-lisp, regardless of the
system. That is, I activated ls-lisp, eval'd files-tests.el, and ran
ert for that file, and the test failed because it used the buggy
insert-directory from ls-lisp. So even if building a Windows Emacs (or
any system) does not use ls-lisp by default, is there any harm in
having this skip condition (other than potentially confusing the
reader)?

I admit I may not correctly or completely understand the way that
ls-lisp could relate to a batch ert run at build time, or how that
varies between architectures. If it's NOT something to worry about
during a 'make', would I still need to worry about someone who ran
this test after enabling ls-lisp manually? Perhaps I could fail the
test so the user knows what they did didn't make sense?

Also, note that this is only true for the 50630 regression test. That
may have been the wrong choice. Perhaps it would make sense to run any
test of insert-directory's implementation if and only if ls-lisp.el is
not active?

>> +       ;;Try to verify that insert-directory will come from files.el,
>> +       ;;not ls-lisp.el. Windows builds will probably use ls-lisp.el
>> +       ;;by default, which will invalidate some tests.
>> +       (insert-directory-program-used (or (not (featurep 'ls-lisp))
>> +                                          ls-lisp-use-insert-directory-program)))

> I guess this is part of the same question I asked above?  Because I
> don't think I understand what you are trying to do here and why.

Yes, my logic was that this being non-nil means the test will be using
the Emacs-default insert-directory implementation from files.el, and
not the ls-lisp.el one.  In order to use the ls-lisp one (what I've
referred to as ls-lisp being "active" previously), the ls-lisp library
needs to be loaded, and the variable on the second line needs to be
nil, otherwise ls-lisp will just delegate back to files.el for
insert-directory. I personally find the variable a little confusing,
and think that negating its name and meaning would be more natural,
but the documentation does make it clear, even if I have to think
about it for a bit every time I set it.






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 11:39:02 GMT) Full text and rfc822 format available.

Message #52 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: John Cummings <john <at> rootabega.net>
Cc: "50630 <at> debbugs.gnu.org" <50630 <at> debbugs.gnu.org>,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 13:38:18 +0200
John Cummings <john <at> rootabega.net> writes:

Hi John,

>> But this bug report has been closed as fixed, so to avoid your patch
>> being lost, can you resubmit it with `M-x submit-emacs-patch' so that it
>> gets its own bug number?
>
> I'd be glad to do that, but before I do, let me make sure we're all on
> the same page. Earlier in the discussion, it was revealed that this
> same bug is present in ls-lisp.el and tramp-sh.el, and I plan to
> submit the trivial fixes for those next. (Let me know if the recipes
> to confirm those bugs I attached earlier don't work for you.) The
> cause of, and fix for, all of them are essentially the same, just
> located in 3 different places. Would you prefer to have a bug number
> for each of the places where it occurs, or to reopen this one (if
> that's something that's done) to hold all the fixes for essentially
> the same bug, as well as for the regression tests linked to those
> fixes?

For Tramp (I'm the maintainer) I don't care whether it is a separate bug
number, or not. If you want to add/extend a regression test for the
Tramp case, you might check whether tramp-test17-insert-directory in
test/lisp/net/tramp-tests.el is a proper place.

Best regards, Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 12:14:01 GMT) Full text and rfc822 format available.

Message #55 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: "50630 <at> debbugs.gnu.org" <50630 <at> debbugs.gnu.org>,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 12:13:26 +0000
Michael Albinus <michael.albinus <at> gmx.de> wrote:

> For Tramp (I'm the maintainer) I don't care whether it is a separate bug
> number, or not.

Thanks Michael, I appreciate the guidance and flexibility. I see how
what looks like a single simple bug could end up lumping together a
bunch of otherwise unrelated maintainers or contributors, so if
separate bugs turns out to be the decision, so be it. I will certainly
keep this in mind if I run into a bug which is larger or more complex,
or can't be fixed quickly enough to make it into the same release.

> If you want to add/extend a regression test for the
> Tramp case, you might check whether tramp-test17-insert-directory in
> test/lisp/net/tramp-tests.el is a proper place.

The test17 test(s) came up on my radar while I was trying to measure
the extent of the bug. I will make sure that I give good consideration
to possibly extending those tests instead of only adding new
ones. From what I remember, even though several existing tramp tests
called insert-directory, they were orthogonal to the free space aspect.






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 12:31:01 GMT) Full text and rfc822 format available.

Message #58 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 12:30:51 +0000
John Cummings <john <at> rootabega.net> wrote:

> Eli Zaretskii eliz <at> gnu.org wrote:
> > > Date: Fri, 24 Sep 2021 19:58:25 +0000
> > > From: John Cummings john <at> rootabega.net
> > > Along those lines, I also attempted to skip the test when ls-lisp
> > > would be used during files-tests.el, which I predict might happen when
> > > building on Windows?
> > Why did you need to skip those tests? Can you elaborate?
> ls-lisp has its own implementation of insert-directory, which
[snipped long section about WHY I thought the test should be skipped]

Also, I now see how making files-tests.el responsible for
accommodating ls-lisp.el might be backwards. If skipping the tests is
still a good idea, I'll try to learn if there's a more conventional
way to do it, e.g. having files-tests.el look for more general signs
that the insert-directory implementation has been overridden, or
maybe having ls-lisp.el be responsible for asking that that test be
skipped.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 13:07:01 GMT) Full text and rfc822 format available.

Message #61 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: John Cummings <john <at> rootabega.net>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 16:06:28 +0300
> Date: Sat, 25 Sep 2021 11:38:08 +0000
> From: John Cummings <john <at> rootabega.net>
> Cc: 50630 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
> >> Date: Fri, 24 Sep 2021 19:58:25 +0000
> >> From: John Cummings <john <at> rootabega.net>
> >>
> >> Along those lines, I also attempted to skip the test when ls-lisp
> >> would be used during files-tests.el, which I predict might happen when
> >> building on Windows?
> >
> > Why did you need to skip those tests?  Can you elaborate?
> 
> ls-lisp has its own implementation of insert-directory, which
> duplicates the bug, and which will also duplicate the fix when I
> submit it soon.

Some context is probably lost here: which bug are you talking about?

And if that bug will be fixed in ls-lisp, then why do you need to
single ls-lisp out?

> I admit I may not correctly or completely understand the way that
> ls-lisp could relate to a batch ert run at build time, or how that
> varies between architectures.

Whenever you invoke insert-directory, on MS-Windows it will invoke
ls-lisp.

> I personally find the variable a little confusing,
> and think that negating its name and meaning would be more natural,
> but the documentation does make it clear, even if I have to think
> about it for a bit every time I set it.

You are talking about ls-lisp-use-insert-directory-program?  Why it is
confusing?  ls-lisp by default doesn't use any programs, it accesses
the files' attributes using Emacs Lisp APIs; so the "program" part is
a reference to the fact that on Posix platforms insert-directory uses
the 'ls' program instead.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 14:30:02 GMT) Full text and rfc822 format available.

Message #64 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 14:29:17 +0000
Eli Zaretskii wrote:

>> Date: Sat, 25 Sep 2021 11:38:08 +0000
>> From: John Cummings <john <at> rootabega.net>
>> Cc: 50630 <at> debbugs.gnu.org
>>
>> Eli Zaretskii <eliz <at> gnu.org> wrote:
>>
>> >> Date: Fri, 24 Sep 2021 19:58:25 +0000
>> >> From: John Cummings <john <at> rootabega.net>
>> >>
>> >> Along those lines, I also attempted to skip the test when ls-lisp
>> >> would be used during files-tests.el, which I predict might happen when
>> >> building on Windows?
>> >
>> > Why did you need to skip those tests?  Can you elaborate?
>>
>> ls-lisp has its own implementation of insert-directory, which
>> duplicates the bug, and which will also duplicate the fix when I
>> submit it soon.
>
> Some context is probably lost here: which bug are you talking about?

50630. A breakdown of the context from other messages in this thread
that've driven my responses so far:

 I found that list-directory in files.el reported the free space of
 default-directory instead of the function's DIRNAME argument. The fix I
 submitted addressed that in insert-directory in files.el, so it
 seems like people are OK with treating this as a problem in
 insert-directory. i.e. the same incorrect free space was reported
 whether you called list-directory or insert-directory.

 I and others found that the same problem was duplicated in ls-lisp.el
 and tramp-sh.el, and I provided instructions to reproduce attached
 earlier in the thread:
 https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-09/msg01813.html
 You can confirm the bug in tramp-sh.el and ls-lisp.el against master
 (currently c17eded545) , and you can confirm the bug in files.el
 against revision a462ccd536 and earlier.

 I've now submitted a regression test for 50630, which I thought would
 be appropriate to include in the 50630 context. I also asked
 elsewhere in the thread whether we shouldn't also handle the instances
 of the bug in ls-lisp.el and tramp-sh.el in that same bug instead of
 creating additional bugs for what is essentially the same small bug
 duplicated a few times. But it's just a question, and if the leaders
 want to have one bug for every library, I'll understand.

> And if that bug will be fixed in ls-lisp, then why do you need to
> single ls-lisp out?

I want to avoid situations where someone runs files-tests.el and it
actually tests the insert-directory implementation from ls-lisp.el. My
plan is to try to reuse, or at worst, copy the testing helpers that I
already submitted for files.el to perform the same test scenario in
ls-lisp-tests.el and in an appropriate place in the tramp tests.
But as I mentioned in another reply, I also understand that making
files-tests.el have to worry about this might not be appropriate, and
if there's another way to handle that, I'd be glad to hear about it
and will be researching that myself, too.


>> I admit I may not correctly or completely understand the way that
>> ls-lisp could relate to a batch ert run at build time, or how that
>> varies between architectures.
>
> Whenever you invoke insert-directory, on MS-Windows it will invoke
> ls-lisp.

That aligns with my reasoning, and so someone running the 50630
regression test for files.el on MS-Windows would actually be testing
ls-lisp.el, and would see inaccurate regression status for files.el.
As of today, the regression test (files-tests-bug-50630) would fail,
and I wouldn't want to have me adding this test be a reason for
any failing MS-Windows builds or other control gates. In the future,
anyone running that test on MS-Windows wouldn't see a failure, but
they still wouldn't have tested the code that files-tests.el is
intended to test.


>> I personally find the variable a little confusing,
>> and think that negating its name and meaning would be more natural,
>> but the documentation does make it clear, even if I have to think
>> about it for a bit every time I set it.
>
> You are talking about ls-lisp-use-insert-directory-program?  Why it is
> confusing?  ls-lisp by default doesn't use any programs, it accesses
> the files' attributes using Emacs Lisp APIs; so the "program" part is
> a reference to the fact that on Posix platforms insert-directory uses
> the 'ls' program instead.

I only mentioned that because I thought it might have been a reason
why it was hard to understand what I was doing with that variable in
files-tests.el. The reason I find it confusing, which could be
personal to me, is that I think it's more natural that a library's
config variables would need to be non-nil when wanting to enable a
feature or function of that library. So a name like
'ls-lisp-use-ls-lisp-insert-directory', and inverting the values,
would have been easier for me to remember, and simpler to express:

  (and (featurep 'ls-lisp)
       ls-lisp-use-ls-lisp-insert-directory)

would mean that we're using the ls-lisp implementation.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 14:56:01 GMT) Full text and rfc822 format available.

Message #67 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: John Cummings <john <at> rootabega.net>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 17:55:16 +0300
> Date: Sat, 25 Sep 2021 14:29:17 +0000
> From: John Cummings <john <at> rootabega.net>
> Cc: 50630 <at> debbugs.gnu.org
> 
> > Some context is probably lost here: which bug are you talking about?
> 
> 50630. A breakdown of the context from other messages in this thread
> that've driven my responses so far:
> 
>  I found that list-directory in files.el reported the free space of
>  default-directory instead of the function's DIRNAME argument. The fix I
>  submitted addressed that in insert-directory in files.el, so it
>  seems like people are OK with treating this as a problem in
>  insert-directory. i.e. the same incorrect free space was reported
>  whether you called list-directory or insert-directory.

At the time, I looked at what happened on Windows, and didn't see the
problem, because ls-lisp does

    (get-free-disk-space ".")

and the default-directory of the buffer at that point is already set
to the directory whose listing we will present.

>  I and others found that the same problem was duplicated in ls-lisp.el
>  and tramp-sh.el, and I provided instructions to reproduce attached
>  earlier in the thread:
>  https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-09/msg01813.html

That recipe invokes insert-directory in some arbitrary way, which is
probably different from what happens when one invokes Dired.  So I'm
not sure we should worry about that.  But anyway, I tried that recipe
on MS-Windows, and I don't see the problem: the values shown on the
first line reflect my home directory, not pwd-a or pwd-b.

>  I've now submitted a regression test for 50630, which I thought would
>  be appropriate to include in the 50630 context. I also asked
>  elsewhere in the thread whether we shouldn't also handle the instances
>  of the bug in ls-lisp.el and tramp-sh.el in that same bug instead of
>  creating additional bugs for what is essentially the same small bug
>  duplicated a few times. But it's just a question, and if the leaders
>  want to have one bug for every library, I'll understand.

I suggest that you remove the special treatment of ls-lisp from that
test.  As I say above, I don't expect it to need any special
treatment, I expect it to pass the test without any changes.  And if
it fails, let's take it from there.

In any case, I see no reason to move the ls-lisp specific test of
insert-directory to ls-lisp-tests.el: since we are testing
insert-directory, the natural place for that test is in
files-tests.el, regardless of what platform-specific tricks are at
work behind the scenes.

> I want to avoid situations where someone runs files-tests.el and it
> actually tests the insert-directory implementation from ls-lisp.el.

I see no reason to avoid that.  The entry point is insert-directory,
and that is what's being tested here.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 17:16:02 GMT) Full text and rfc822 format available.

Message #70 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 17:15:21 +0000
>>  I and others found that the same problem was duplicated in ls-lisp.el
>>  and tramp-sh.el, and I provided instructions to reproduce attached
>>  earlier in the thread:
>>  https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-09/msg01813.html

> That recipe invokes insert-directory in some arbitrary way, which is
> probably different from what happens when one invokes Dired.  So I'm
> not sure we should worry about that.  But anyway, I tried that recipe
> on MS-Windows, and I don't see the problem: the values shown on the
> first line reflect my home directory, not pwd-a or pwd-b.

I'm replying only to this part for now, because we need to agree on
the facts in order to do anything else. If what you say is accurate,
then this bug may not really exist as I've described it, but I think
I've done everything reasonable to eliminate that possibility. Are you
sure that you picked values for pwd-a and pwd-b that would have
different amounts of available space on MS-Windows, like two drive
letters on physically separate disks?  But if Windows uses ls-lisp for
this, we should see the same results on any system using ls-lisp, so I
think it's OK to focus on ls-lisp and forget about MS-Windows for the
purposes of this reply.

I tried to supply values for pwd-a and pwd-b that would be under
different mount points on a Unix-like system, / and /proc, but if your
system is different, you may need to pick different Unix-like values
for those, too.

I just built 285f59cbe230701f15d28dfe8036cf2feb9d1d31 (terminal only)
from a brand new git clone on Ubuntu 20,and here are the results of
the recipes. I'll use a "[" character to quote the text I copied off
my terminal:

files.el (this case is already fixed):
 [emacsdev <at> ubuntu:~/freshemacs/emacs$ src/emacs -Q --eval "\
 [> (progn \
 [>    (setq pwd-a \"/\") \
 [>    (setq pwd-b \"/proc\")  \
 [>    (setq list-dir \"~\") \
 [>    (with-current-buffer-window \"a\" nil nil \
 [>       (cd pwd-a) \
 [>       (insert-directory list-dir \"-l\" nil t) \
 [>       (insert (format \"listing of %s in cwd %s\" list-dir (pwd)))) \
 [>    (with-current-buffer-window \"b\" nil nil \
 [>       (cd pwd-b) \
 [>       (insert-directory list-dir \"-l\" nil t) \
 [>       (insert (format \"listing of %s in cwd %s\" list-dir (pwd)))) \
 [>    (switch-to-buffer \"a\") \
 [>    (switch-to-buffer-other-window \"b\"))"

Resulting Emacs frame:
 [File Edit Options Buffers Tools Help
 [total used in directory 0 available 7.6 GiB
 [lrwxrwxrwx 1 emacsdev emacsdev 26 Sep 24 12:18 emacs -> /media/ubuntu/turkey/emacs
 [lrwxrwxrwx 1 emacsdev emacsdev 31 Sep 25 12:26 freshemacs -> /media/ubuntu/turkey/freshemacs
 [listing of ~ in cwd Directory /
 [
 [
 [
 [
 [
 [
 [
 [-UUU:%%--F1  a              All L1     (Fundamental) -----------------------------------------
 [total used in directory 0 available 7.6 GiB
 [lrwxrwxrwx 1 emacsdev emacsdev 26 Sep 24 12:18 emacs -> /media/ubuntu/turkey/emacs
 [lrwxrwxrwx 1 emacsdev emacsdev 31 Sep 25 12:26 freshemacs -> /media/ubuntu/turkey/freshemacs
 [listing of ~ in cwd Directory /proc/
 [
 [
 [
 [
 [
 [
 [-UUU:%%--F1  b              All L1     (Fundamental) -----------------------------------------
 [



ls-lisp.el (not fixed yet):
 [emacsdev <at> ubuntu:~/freshemacs/emacs$ src/emacs -Q --eval "\
 [> (progn \
 [>    (setq pwd-a \"/\") \
 [>    (setq pwd-b \"/proc\")  \
 [>    (setq list-dir \"~\") \
 [>    (require 'ls-lisp) \
 [>    (setq ls-lisp-use-insert-directory-program nil) \
 [>    (with-current-buffer-window \"a\" nil nil \
 [>       (cd pwd-a) \
 [>       (insert-directory list-dir \"-l\" nil t) \
 [>       (insert (format \"listing of %s in cwd %s\" list-dir (pwd)))) \
 [>    (with-current-buffer-window \"b\" nil nil \
 [>       (cd pwd-b) \
 [>       (insert-directory list-dir \"-l\" nil t) \
 [>       (insert (format \"listing of %s in cwd %s\" list-dir (pwd)))) \
 [>    (switch-to-buffer \"a\") \
 [>    (switch-to-buffer-other-window \"b\"))"

 Resulting Emacs frame with different available space:
 [File Edit Options Buffers Tools Help
 [total used in directory 1 available 7.6 GiB
 [lrwxrwxrwx  1 emacsdev emacsdev 26 09-24 12:18 emacs -> /media/ubuntu/turkey/emacs
 [lrwxrwxrwx  1 emacsdev emacsdev 31 09-25 12:26 freshemacs -> /media/ubuntu/turkey/freshemacs
 [listing of ~ in cwd Directory /
 [
 [
 [
 [
 [
 [
 [
 [-UUU:%%--F1  a              All L1     (Fundamental) -----------------------------------------
 [total used in directory 1 available 0 B
 [lrwxrwxrwx  1 emacsdev emacsdev 26 09-24 12:18 emacs -> /media/ubuntu/turkey/emacs
 [lrwxrwxrwx  1 emacsdev emacsdev 31 09-25 12:26 freshemacs -> /media/ubuntu/turkey/freshemacs
 [listing of ~ in cwd Directory /proc/
 [
 [
 [
 [
 [
 [
 [-UUU:%%--F1  b              All L1     (Fundamental) -----------------------------------------
 [





tramp-sh.el (not fixed yet):
 [emacsdev <at> ubuntu:~/freshemacs/emacs$ src/emacs -Q --eval "\
 [> (progn \
 [>    (setq pwd-a \"/\") \
 [>    (setq pwd-b \"/proc\")  \
 [>    (setq list-dir \"/ssh:emacsdev <at> localhost:~/\") \
 [>    (require 'tramp) \
 [>    (setq ls-lisp-use-insert-directory-program t) \
 [>    (with-current-buffer-window \"a\" nil nil \
 [>       (cd pwd-a) \
 [>       (insert-directory list-dir \"-l\" nil t) \
 [>       (insert (format \"listing of %s in cwd %s\" list-dir (pwd)))) \
 [>    (with-current-buffer-window \"b\" nil nil \
 [>       (cd pwd-b) \
 [>       (insert-directory list-dir \"-l\" nil t) \
 [>       (insert (format \"listing of %s in cwd %s\" list-dir (pwd)))) \
 [>    (switch-to-buffer \"a\") \
 [>    (switch-to-buffer-other-window \"b\"))"

 Resulting Emacs frame with different available space:
 [File Edit Options Buffers Tools Help
 [total used in directory 0 available 7.6 GiB
 [lrwxrwxrwx 1 emacsdev emacsdev 26 Sep 24 12:18 emacs -> /media/ubuntu/turkey/emacs
 [lrwxrwxrwx 1 emacsdev emacsdev 31 Sep 25 12:26 freshemacs -> /media/ubuntu/turkey/freshemacs
 [listing of /ssh:emacsdev <at> localhost:~/ in cwd Directory /
 [
 [
 [
 [
 [
 [
 [
 [-UUU:%%--F1  a              All L1     (Fundamental) -----------------------------------------
 [total used in directory 0 available 0 B
 [lrwxrwxrwx 1 emacsdev emacsdev 26 Sep 24 12:18 emacs -> /media/ubuntu/turkey/emacs
 [lrwxrwxrwx 1 emacsdev emacsdev 31 Sep 25 12:26 freshemacs -> /media/ubuntu/turkey/freshemacs
 [listing of /ssh:emacsdev <at> localhost:~/ in cwd Directory /proc/
 [
 [
 [
 [
 [
 [
 [-UUU:%%--F1  b              All L1     (Fundamental) -----------------------------------------
 [







Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 17:27:01 GMT) Full text and rfc822 format available.

Message #73 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: John Cummings <john <at> rootabega.net>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 20:26:22 +0300
> Date: Sat, 25 Sep 2021 17:15:21 +0000
> From: John Cummings <john <at> rootabega.net>
> Cc: 50630 <at> debbugs.gnu.org
> 
> >>  I and others found that the same problem was duplicated in ls-lisp.el
> >>  and tramp-sh.el, and I provided instructions to reproduce attached
> >>  earlier in the thread:
> >>  https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-09/msg01813.html
> 
> > That recipe invokes insert-directory in some arbitrary way, which is
> > probably different from what happens when one invokes Dired.  So I'm
> > not sure we should worry about that.  But anyway, I tried that recipe
> > on MS-Windows, and I don't see the problem: the values shown on the
> > first line reflect my home directory, not pwd-a or pwd-b.
> 
> I'm replying only to this part for now, because we need to agree on
> the facts in order to do anything else. If what you say is accurate,
> then this bug may not really exist as I've described it, but I think
> I've done everything reasonable to eliminate that possibility. Are you
> sure that you picked values for pwd-a and pwd-b that would have
> different amounts of available space on MS-Windows, like two drive
> letters on physically separate disks?

No, they were 2 different directories on the same drive.

Maybe I don't understand which part of the totals you want to fix:
there are two numbers there.

> I tried to supply values for pwd-a and pwd-b that would be under
> different mount points on a Unix-like system, / and /proc, but if your
> system is different, you may need to pick different Unix-like values
> for those, too.

I indeed provided different values, but not on different volumes.
What is the significance of a different volume for this purpose?

> I just built 285f59cbe230701f15d28dfe8036cf2feb9d1d31 (terminal only)
> from a brand new git clone on Ubuntu 20,and here are the results of
> the recipes. I'll use a "[" character to quote the text I copied off
> my terminal:

Thanks, but it's hard to interpret that.  What should I be looking at?
Do we agree that listing a given directory should show the same amount
of used and free space, not matter what is the default-directory of
the buffer from which you invoke insert-directory?  Because that's
what I saw after running your recipe.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 17:38:01 GMT) Full text and rfc822 format available.

Message #76 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 17:37:29 +0000
> What should I be looking at?

The "available" (or, loosely, "free") space reported.

> Do we agree that listing a given directory should show the same amount
> of used and free space, not matter what is the default-directory of
> the buffer from which you invoke insert-directory? Because that's
> what I saw after running your recipe.

Yes, I agree. If you saw the same amount of free space,
it's because you didn't make pwd-a and pwd-b be directories
on two different volumes that have different amounts of free space.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 17:45:01 GMT) Full text and rfc822 format available.

Message #79 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: John Cummings <john <at> rootabega.net>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 20:44:25 +0300
> Date: Sat, 25 Sep 2021 17:37:29 +0000
> From: John Cummings <john <at> rootabega.net>
> Cc: 50630 <at> debbugs.gnu.org
> 
> > Do we agree that listing a given directory should show the same amount
> > of used and free space, not matter what is the default-directory of
> > the buffer from which you invoke insert-directory? Because that's
> > what I saw after running your recipe.
> 
> Yes, I agree. If you saw the same amount of free space,
> it's because you didn't make pwd-a and pwd-b be directories
> on two different volumes that have different amounts of free space.

OK, so I'm saying that when invoking Dired, I see the same numbers
(both of them) no matter from which buffer I invoked Dired.  Even if
the default-directory of each buffer is on a different drive.

With your recipe, indeed the free space is not always correct.  But
why is that use case interesting?  insert-directory is not a normally
a user-visible function.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 18:02:02 GMT) Full text and rfc822 format available.

Message #82 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 18:01:26 +0000
> With your recipe, indeed the free space is not always correct. But
> why is that use case interesting? insert-directory is not a normally
> a user-visible function.

If being a user-visible function is necessary to consider it interesting,
then I reported this as a bug in the verbose `list-directory' output,
which is documented in the manual.

But I don't think it needs to be normally user-visible to be interesting.
Would programmers be welcome to use insert-directory in their code? It would
also be good to fix this so that any future Emacs functionality that used
insert-directory would have the correct output.

The help for that function says:

 insert-directory is a compiled Lisp function in
 ‘/media/ubuntu/turkey/freshemacs/emacs/lisp/files.el’.

 (insert-directory FILE SWITCHES &optional WILDCARD FULL-DIRECTORY-P)

 Insert directory listing for FILE, formatted according to SWITCHES.

I truncated that, but it doesn't say anything about depending on the
current default-directory. If the function performs a listing for FILE,
it shouldn't be possible for it to insert free space that is not correct
for FILE. If it's OK for insert-directory to rely on the current
default-directory, then the FILE argument itself seems unnecessary.









Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 18:45:02 GMT) Full text and rfc822 format available.

Message #85 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: John Cummings <john <at> rootabega.net>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 21:44:27 +0300
> Date: Sat, 25 Sep 2021 18:01:26 +0000
> From: John Cummings <john <at> rootabega.net>
> Cc: 50630 <at> debbugs.gnu.org
> 
> > With your recipe, indeed the free space is not always correct. But
> > why is that use case interesting? insert-directory is not a normally
> > a user-visible function.
> 
> If being a user-visible function is necessary to consider it interesting,
> then I reported this as a bug in the verbose `list-directory' output,
> which is documented in the manual.

Like I said before: let's pick this up when your test is installed,
and we can run it and debug it if needed.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 19:01:01 GMT) Full text and rfc822 format available.

Message #88 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 19:00:14 +0000
Eli Zaretskii <eliz <at> gnu.org> wrote:

> > > With your recipe, indeed the free space is not always correct. But
> > > why is that use case interesting? insert-directory is not a normally
> > > a user-visible function.
> >
> > If being a user-visible function is necessary to consider it interesting,
> > then I reported this as a bug in the verbose `list-directory' output,
> > which is documented in the manual.
>
> Like I said before: let's pick this up when your test is installed,
> and we can run it and debug it if needed.

I'd appreciate it if you continued this discussion until we can agree there
is actually a bug, or at least something that could be improved.  I don't
think we need the tests to be merged to be able to do that. In fact, I'd say
it's the other way around: if there's a chance that a maintainer might tell
me there is no bug, or that whatever I found is not worth changing, I would
decide not to invest any more time into it.

So are we in agreement about these 3 things below?

1. insert-directory in files.el used to be able to show the incorrect free
   space for its FILE arg. The same function in ls-lisp.el and tramp-sh.el
   continues to be able to show the incorrect free space today.

2. The recipes/instructions I sent will be able to confirm this

3. This should be changed because insert-directory should not be able to
   insert free space info that is not correct for its FILE arg.







Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 19:08:02 GMT) Full text and rfc822 format available.

Message #91 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: John Cummings <john <at> rootabega.net>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 22:07:39 +0300
> Date: Sat, 25 Sep 2021 19:00:14 +0000
> From: John Cummings <john <at> rootabega.net>
> Cc: 50630 <at> debbugs.gnu.org
> 
> So are we in agreement about these 3 things below?
> 
> 1. insert-directory in files.el used to be able to show the incorrect free
>    space for its FILE arg. The same function in ls-lisp.el and tramp-sh.el
>    continues to be able to show the incorrect free space today.
> 
> 2. The recipes/instructions I sent will be able to confirm this
> 
> 3. This should be changed because insert-directory should not be able to
>    insert free space info that is not correct for its FILE arg.

Yes.  But how exactly to fix this in ls-lisp remains to be seen.  I
think I know how, and I'm asking you to have your tests installed so I
could try my idea conveniently.

Thanks in advance.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Sat, 25 Sep 2021 19:59:02 GMT) Full text and rfc822 format available.

Message #94 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50630 <at> debbugs.gnu.org
Subject: Re: bug#50630: [PATCH] Add tests for insert-directory
Date: Sat, 25 Sep 2021 19:58:08 +0000
[Message part 1 (text/plain, inline)]
Here's the latest patch. If I should send it as a new email to the bug address, or create a new bug like Lars suggested, I can do that.
[0001-Add-tests-for-insert-directory.patch (text/x-patch, attachment)]

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 24 Oct 2021 11:24:07 GMT) Full text and rfc822 format available.

bug unarchived. Request was from John Cummings <john <at> rootabega.net> to control <at> debbugs.gnu.org. (Thu, 11 Nov 2021 01:41:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Thu, 11 Nov 2021 01:45:01 GMT) Full text and rfc822 format available.

Message #101 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: John Cummings <john <at> rootabega.net>
To: "50630 <at> debbugs.gnu.org" <50630 <at> debbugs.gnu.org>
Subject: [PATCH] Add tests for 'insert-directory'
Date: Thu, 11 Nov 2021 01:43:54 +0000
[Message part 1 (text/plain, inline)]
My copyright assignment is complete. Here is the patch to add tests,
signed from the same master key I used to sign the agreement:
D148 50A5 E6B1 45AE CD83  7983 16B9 CFB2 30B1 275C

This was generated against the emacs-28 branch. There will be a merge
conflict when merged to master, but these tests can just be added as
the last tests in files-tests.el
[0001-Add-tests-for-insert-directory.patch (text/x-patch, attachment)]
[0001-Add-tests-for-insert-directory.patch.asc (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#50630; Package emacs. (Thu, 11 Nov 2021 03:39:02 GMT) Full text and rfc822 format available.

Message #104 received at 50630 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: John Cummings <john <at> rootabega.net>
Cc: "50630 <at> debbugs.gnu.org" <50630 <at> debbugs.gnu.org>
Subject: Re: bug#50630: [PATCH] Add tests for 'insert-directory'
Date: Thu, 11 Nov 2021 04:38:01 +0100
John Cummings <john <at> rootabega.net> writes:

> My copyright assignment is complete. Here is the patch to add tests,
> signed from the same master key I used to sign the agreement:
> D148 50A5 E6B1 45AE CD83  7983 16B9 CFB2 30B1 275C

Thanks; applied to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 09 Dec 2021 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 100 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.