bug#69592: 29.2; eshell rm doesn't expand wildcard over TRAMP

bug#69592: 29.2; eshell rm doesn't expand wildcard over TRAMP

[Antero Mejr]

When deleting multiple files with the * wildcard expansion in eshell
over TRAMP, like this:

rm /ssh:some-machine:path/*

nothing is deleted. However, when running the same command locally:

rm path/*

the files under path are deleted. Deleting single files over TRAMP
does work, however.

Could eshell/rm support wild card expansions on remote machines?


In GNU Emacs 29.2 (build 2, aarch64-apple-darwin23.2.0, NS
 appkit-2487.30 Version 14.2.1 (Build 23C71)) of 2024-01-18 built on
 sonomaa.local
Windowing system distributor 'Apple', version 10.3.2487
System Description:  macOS 14.3.1

Configured using:
 'configure --prefix=/opt/local --disable-silent-rules --without-dbus
 --without-gconf --without-libotf --without-m17n-flt --with-libgmp
 --with-gnutls --with-json --with-xml2 --with-modules --with-sqlite3
 --with-webp --infodir /opt/local/share/info/emacs --with-ns
 --with-lcms2 --without-harfbuzz --without-imagemagick --without-xaw3d
 --with-rsvg --with-native-compilation=aot --with-tree-sitter
 'CFLAGS=-pipe -Os -Wno-attributes
 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk -arch
 arm64' 'CPPFLAGS=-I/opt/local/include
 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk'
 'LDFLAGS=-L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-rpath
 /opt/local/lib/gcc13 -Wl,-no_pie
 -Wl,-syslibroot,/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk
 -arch arm64''

Configured features:
ACL GIF GLIB GMP GNUTLS JPEG JSON LCMS2 LIBXML2 MODULES NATIVE_COMP
NOTIFY KQUEUE NS PDUMPER PNG RSVG SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM ZLIB

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

Major mode: ELisp/l

Minor modes in effect:
  bug-reference-prog-mode: t
  display-fill-column-indicator-mode: t
  pixel-scroll-precision-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  global-auto-revert-mode: t
  server-mode: t
  override-global-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  column-number-mode: t
  line-number-mode: t
  auto-fill-function: do-auto-fill
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/opt/local/share/emacs/site-lisp/site-start hides /Applications/MacPorts/Emacs.app/Contents/Resources/site-lisp/site-start
/Users/a/.emacs.d/elpa/transient-0.5.3/transient hides /Applications/MacPorts/Emacs.app/Contents/Resources/lisp/transient

Features:
(shadow sort mail-extr emacsbug vc cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs cl-print shortdoc
delsel rect face-remap help-fns radix-tree bug-reference conf-mode
pcmpl-git misearch multi-isearch vc-hg vc-git vc-bzr vc-dispatcher
python project pcase treesit dired-aux tramp-cache time-stamp tramp-sh
gomoku nndraft nnmh utf-7 network-stream nsm nnfolder nnnil gnus-agent
gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-art mm-uu
mml2015 mm-view mml-smime smime gnutls dig gnus-cache gnus-sum shr
pixel-fill kinsoku url-file svg dom gnus-demon nntp gnus-group gnus-undo
gnus-start gnus-dbus dbus xml gnus-cloud nnimap nnmail mail-source utf7
gnus-spec gnus-win nnoo gnus-int gnus-range gnus nnheader range wid-edit
pcmpl-unix em-unix em-term term disp-table ehelp em-script em-prompt
em-ls em-hist em-pred em-glob em-extpipe em-cmpl em-dirs esh-var
em-basic em-banner em-alias em-tramp tramp tramp-loaddefs trampver
tramp-integration tramp-compat parse-time iso8601 esh-mode eshell
esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg esh-module esh-groups
esh-util files-x display-fill-column-indicator pixel-scroll cua-base
markdown-mode color thingatpt magit-submodule magit-obsolete magit-blame
magit-stash magit-reflog magit-bisect magit-push magit-pull magit-fetch
magit-clone magit-remote magit-commit magit-sequence magit-notes
magit-worktree magit-tag magit-merge magit-branch magit-reset
magit-files magit-refs magit-status magit magit-repos magit-apply
magit-wip magit-log which-func imenu magit-diff smerge-mode diff
diff-mode git-commit log-edit message sendmail yank-media puny dired
dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068 epg-config
gnus-util text-property-search mm-decode mm-bodies mm-encode mail-parse
rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev
mail-utils gmm-utils mailheader pcvs-util add-log magit-core
magit-autorevert magit-margin magit-transient magit-process with-editor
comp comp-cstr warnings shell magit-mode transient edmacro kmacro compat
magit-git magit-section magit-utils crm dash cl-extra help-mode advice
autorevert filenotify server ox-man ox-odt rng-loc rng-uri rng-parse
rng-match rng-dt rng-util rng-pttrn nxml-parse nxml-ns nxml-enc xmltok
nxml-util ox-latex ox-icalendar org-agenda ox-html table ox-ascii
ox-publish ox org-element org-persist xdg org-id org-refile org ob
ob-tangle ob-ref ob-lob ob-table org-macro org-src ob-comint
org-pcomplete pcomplete comint ansi-osc ansi-color org-list org-footnote
org-faces org-entities time-date noutline outline icons ob-emacs-lisp
org-table org-keys org-loaddefs find-func cal-menu calendar cal-loaddefs
ring avl-tree generator ol rx oc ob-exp ob-core org-cycle org-fold
org-fold-core org-compat ob-eval org-version org-macs format-spec
use-package use-package-ensure use-package-delight use-package-diminish
use-package-bind-key bind-key easy-mmode use-package-core
magit-autoloads git-commit-autoloads magit-section-autoloads
dash-autoloads markdown-mode-autoloads transient-autoloads
with-editor-autoloads info compat-autoloads package browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/ns-win ns-win
ucs-normalize mule-util term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-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 nadvice seq simple cl-generic
indonesian philippine 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 emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads kqueue cocoa ns lcms2
multi-tty make-network-process native-compile emacs)

Memory information:
((conses 16 544232 61786)
 (symbols 48 35992 0)
 (strings 32 133712 7121)
 (string-bytes 1 4708176)
 (vectors 16 72291)
 (vector-slots 8 1252694 70109)
 (floats 8 609 445)
 (intervals 56 23468 177)
 (buffers 984 23))

bug#69592: 29.2; eshell rm doesn't expand wildcard over TRAMP

[Jim Porter]

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

On 3/6/2024 11:50 AM, Antero Mejr wrote:
> 
> When deleting multiple files with the * wildcard expansion in eshell
> over TRAMP, like this:
> 
> rm /ssh:some-machine:path/*
> 
> nothing is deleted.

Interesting. I didn't realize this case wasn't already covered in 
Eshell. Does the attached patch fix things for you?

[-- Attachment #2: 0001-Support-expanding-Eshell-globs-for-remote-file-names.patch --]
[-- Type: text/plain, Size: 8308 bytes --]

From 480802d91185fb863785feb33e3308e6977e7d21 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 6 Mar 2024 13:27:07 -0800
Subject: [PATCH] Support expanding Eshell globs for remote file names

* lisp/eshell/em-glob.el (eshell-glob-chars-regexp): New function...
(eshell-glob-regexp): ... use it.
(eshell-glob-p): New function...
(eshell-glob-convert): ... use it, and return the deepest start
directory possible.

* lisp/eshell/esh-util.el (eshell-split-path): Rename to...
(eshell-split-path): ... this, and account for remote file names.

* test/lisp/eshell/em-glob-tests.el
(em-glob-test/convert/current-start-directory)
(em-glob-test/convert/relative-start-directory)
(em-glob-test/convert/absolute-start-directory)
(em-glob-test/convert/remote-start-directory): New tests (bug#69592).
---
 lisp/eshell/em-glob.el            | 34 +++++++++++++--------
 lisp/eshell/esh-util.el           | 51 +++++++++++++++++--------------
 test/lisp/eshell/em-glob-tests.el | 30 ++++++++++++++++++
 3 files changed, 79 insertions(+), 36 deletions(-)

diff --git a/lisp/eshell/em-glob.el b/lisp/eshell/em-glob.el
index b0c3e6e7a11..a358acc970b 100644
--- a/lisp/eshell/em-glob.el
+++ b/lisp/eshell/em-glob.el
@@ -190,6 +190,11 @@ eshell-glob-recursive-alist
   '(("**/" . recurse)
     ("***/" . recurse-symlink)))
 
+(defsubst eshell-glob-chars-regexp ()
+  (or eshell-glob-chars-regexp
+      (setq-local eshell-glob-chars-regexp
+		  (format "[%s]+" (apply 'string eshell-glob-chars-list)))))
+
 (defun eshell-glob-regexp (pattern)
   "Convert glob-pattern PATTERN to a regular expression.
 The basic syntax is:
@@ -210,11 +215,8 @@ eshell-glob-regexp
 resulting regular expression."
   (let ((matched-in-pattern 0)          ; How much of PATTERN handled
 	regexp)
-    (while (string-match
-	    (or eshell-glob-chars-regexp
-                (setq-local eshell-glob-chars-regexp
-		     (format "[%s]+" (apply 'string eshell-glob-chars-list))))
-	    pattern matched-in-pattern)
+    (while (string-match (eshell-glob-chars-regexp)
+                         pattern matched-in-pattern)
       (let* ((op-begin (match-beginning 0))
 	     (op-char (aref pattern op-begin)))
 	(setq regexp
@@ -239,6 +241,9 @@ eshell-glob-regexp
 	    (regexp-quote (substring pattern matched-in-pattern))
 	    "\\'")))
 
+(defun eshell-glob-p (pattern)
+  (string-match (eshell-glob-chars-regexp) pattern))
+
 (defun eshell-glob-convert-1 (glob &optional last)
   "Convert a GLOB matching a single element of a file name to regexps.
 If LAST is non-nil, this glob is the last element of a file name.
@@ -291,14 +296,13 @@ eshell-glob-convert
      symlinks.
 
 3. A boolean indicating whether to match directories only."
-  (let ((globs (eshell-split-path glob))
-        (isdir (eq (aref glob (1- (length glob))) ?/))
+  (let ((globs (eshell-split-filename glob))
+        (isdir (string-suffix-p "/" glob))
         start-dir result last-saw-recursion)
     (if (and (cdr globs)
              (file-name-absolute-p (car globs)))
-        (setq start-dir (car globs)
-              globs (cdr globs))
-      (setq start-dir "."))
+        (setq start-dir (pop globs))
+      (setq start-dir (file-name-as-directory ".")))
     (while globs
       (if-let ((recurse (cdr (assoc (car globs)
                                     eshell-glob-recursive-alist))))
@@ -306,11 +310,15 @@ eshell-glob-convert
               (setcar result recurse)
             (push recurse result)
             (setq last-saw-recursion t))
-        (push (eshell-glob-convert-1 (car globs) (null (cdr globs)))
-              result)
+        (if (or result (eshell-glob-p (car globs)))
+            (push (eshell-glob-convert-1 (car globs) (null (cdr globs)))
+                  result)
+          ;; We haven't seen a glob yet, so instead append to the start
+          ;; directory.
+          (setq start-dir (file-name-concat start-dir (car globs))))
         (setq last-saw-recursion nil))
       (setq globs (cdr globs)))
-    (list (file-name-as-directory start-dir)
+    (list start-dir
           (nreverse result)
           isdir)))
 
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index f0acfecb701..129134814e3 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -447,29 +447,34 @@ eshell-parse-colon-path
 	 (parse-colon-path path-env))
       (parse-colon-path path-env))))
 
-(defun eshell-split-path (path)
-  "Split a path into multiple subparts."
-  (let ((len (length path))
-	(i 0) (li 0)
-	parts)
-    (if (and (eshell-under-windows-p)
-	     (> len 2)
-	     (eq (aref path 0) ?/)
-	     (eq (aref path 1) ?/))
-	(setq i 2))
-    (while (< i len)
-      (if (and (eq (aref path i) ?/)
-	       (not (get-text-property i 'escaped path)))
-	  (setq parts (cons (if (= li i) "/"
-			      (substring path li (1+ i))) parts)
-		li (1+ i)))
-      (setq i (1+ i)))
-    (if (< li i)
-	(setq parts (cons (substring path li i) parts)))
-    (if (and (eshell-under-windows-p)
-	     (string-match "\\`[A-Za-z]:\\'" (car (last parts))))
-	(setcar (last parts) (concat (car (last parts)) "/")))
-    (nreverse parts)))
+(defun eshell-split-filename (filename)
+  "Split a FILENAME into a list of file/directory components."
+  (let* ((remote (file-remote-p filename))
+         (filename (file-local-name filename))
+         (len (length filename))
+         (index 0) (curr-start 0)
+         parts)
+    (when (and (eshell-under-windows-p)
+               (string-prefix-p "//" filename))
+      (setq index 2))
+    (while (< index len)
+      (when (and (eq (aref filename index) ?/)
+                 (not (get-text-property index 'escaped filename)))
+        (push (if (= curr-start index) "/"
+                (substring filename curr-start (1+ index)))
+              parts)
+        (setq curr-start (1+ index)))
+      (setq index (1+ index)))
+    (when (< curr-start len)
+      (push (substring filename curr-start) parts))
+    (setq parts (nreverse parts))
+    (when (and (eshell-under-windows-p)
+               (string-match "\\`[A-Za-z]:\\'" (car parts)))
+      (setcar parts (concat (car parts) "/")))
+    (if remote (cons remote parts) parts)))
+
+(define-obsolete-function-alias 'eshell-split-path
+  'eshell-split-filename "30.1")
 
 (defun eshell-to-flat-string (value)
   "Make value a string.  If separated by newlines change them to spaces."
diff --git a/test/lisp/eshell/em-glob-tests.el b/test/lisp/eshell/em-glob-tests.el
index 6d922666ea3..fc460a59eed 100644
--- a/test/lisp/eshell/em-glob-tests.el
+++ b/test/lisp/eshell/em-glob-tests.el
@@ -61,6 +61,9 @@ with-fake-files
 
 ;;; Tests:
 
+\f
+;; Glob expansion
+
 (ert-deftest em-glob-test/expand/splice-results ()
   "Test that globs are spliced into the argument list when
 `eshell-glob-splice-results' is non-nil."
@@ -115,6 +118,33 @@ em-glob-test/expand/explicitly-listify-results
           (eshell-command-result-equal "list ${listify *.no}"
                                        '(("*.no"))))))))
 
+\f
+;; Glob conversion
+
+(ert-deftest em-glob-test/convert/current-start-directory ()
+  "Test converting a glob starting in the current directory."
+  (should (equal (eshell-glob-convert "*.el")
+                 '("./" (("\\`.*\\.el\\'" . "\\`\\.")) nil))))
+
+(ert-deftest em-glob-test/convert/relative-start-directory ()
+  "Test converting a glob starting in a relative directory."
+  (should (equal (eshell-glob-convert "some/where/*.el")
+                 '("./some/where/" (("\\`.*\\.el\\'" . "\\`\\.")) nil))))
+
+(ert-deftest em-glob-test/convert/absolute-start-directory ()
+  "Test converting a glob starting in an absolute directory."
+  (should (equal (eshell-glob-convert "/some/where/*.el")
+                 '("/some/where/" (("\\`.*\\.el\\'" . "\\`\\.")) nil))))
+
+(ert-deftest em-glob-test/convert/remote-start-directory ()
+  "Test converting a glob starting in a remote directory."
+  (should (equal (eshell-glob-convert "/ssh:nowhere.invalid:some/where/*.el")
+                 '("/ssh:nowhere.invalid:/some/where/"
+                   (("\\`.*\\.el\\'" . "\\`\\.")) nil))))
+
+\f
+;; Glob matching
+
 (ert-deftest em-glob-test/match-any-string ()
   "Test that \"*\" pattern matches any string."
   (with-fake-files '("a.el" "b.el" "c.txt" "dir/a.el")
-- 
2.25.1

bug#69592: 29.2; eshell rm doesn't expand wildcard over TRAMP

[Antero Mejr]

Jim Porter <jporterbugs@gmail.com> writes:

> On 3/6/2024 11:50 AM, Antero Mejr wrote:
>> When deleting multiple files with the * wildcard expansion in eshell
>> over TRAMP, like this:
>> rm /ssh:some-machine:path/*
>> nothing is deleted.
>
> Interesting. I didn't realize this case wasn't already covered in Eshell. Does
> the attached patch fix things for you?

Yes, that patch fixes it. The em-glob-tests pass on my machine as well.
Thanks!

bug#69592: 29.2; eshell rm doesn't expand wildcard over TRAMP

[Jim Porter]

On 3/7/2024 7:46 AM, Antero Mejr wrote:
> Yes, that patch fixes it. The em-glob-tests pass on my machine as well.
> Thanks!

Thanks for testing. I've now merged this to the master branch as 
e42f14f0e03, so closing this bug.