bug#71655: Eshell external commands do not work under GNU Emacs for Windows

[Jim Porter <jporterbugs@gmail.com>]

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

On 6/20/2024 12:45 AM, Eli Zaretskii wrote:
>> Date: Wed, 19 Jun 2024 22:34:02 -0700
>> Cc: 71655@debbugs.gnu.org, james@literate-devops.io
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> My version of ls reports it as a symlink, interestingly enough.
> 
> It isn't a symlink.  It is a reparse point of type APPEXECLINK, which
> has different attributes and different data structure describing the
> target.  We could represent it as a symlink (since Posix has no direct
> equivalent), but the implementation under the hood will need to be
> different.

Right. This was just (what I thought was) an interesting observation 
about how other POSIX-based tools treat these reparse points.

> I agree that all those other conditions (including the .exe test) seem
> to be reasonable, in addition to zero-size.

Do you have a preference between either of these patches? They either 
check for zero-size or ignore file errors when trying to insert.

I don't have a strong preference myself, but the latter seems 
ever-so-slightly safer to me. This bug happened because we can't read 
the file when trying to insert it, so ignoring file errors would cover 
any other situations we haven't predicted. On the other hand, maybe 
there's a case where we *want* the 'insert-file-contents-literally' 
error to signal so that we don't try to execute the file normally (I 
can't think of any such cases, though).

[-- Attachment #2: 0001-Use-file-attribute-size.patch --]
[-- Type: text/plain, Size: 1605 bytes --]

From 4521f82dcef0253564574012c571564111e06c4a Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 22 Jun 2024 12:45:19 -0700
Subject: [PATCH] Fix execution of MS-Windows app execution aliases in Eshell

* lisp/eshell/esh-ext.el (eshell-script-interpreter): Check for 0-size
files.
---
 lisp/eshell/esh-ext.el | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lisp/eshell/esh-ext.el b/lisp/eshell/esh-ext.el
index 3c4deb32601..cf93d2904da 100644
--- a/lisp/eshell/esh-ext.el
+++ b/lisp/eshell/esh-ext.el
@@ -301,7 +301,17 @@ eshell-script-interpreter
   (INTERPRETER [ARGS] FILE)"
   (let ((maxlen eshell-command-interpreter-max-length))
     (if (and (file-readable-p file)
-	     (file-regular-p file))
+	     (file-regular-p file)
+             ;; If the file is zero bytes, it can't possibly have a
+             ;; shebang.  This check may seem redundant, but we can
+             ;; encounter files that Emacs considers both readable and
+             ;; regular, but which aren't *actually* readable.  This can
+             ;; happen, for example, with certain kinds of reparse
+             ;; points like APPEXECLINK on NTFS filesystems (MS-Windows
+             ;; uses these for "app execution aliases").  In these
+             ;; cases, the file size is 0, so this check protects us
+             ;; from errors.
+             (> (file-attribute-size (file-attributes file)) 0))
 	(with-temp-buffer
 	  (insert-file-contents-literally file nil 0 maxlen)
 	  (if (looking-at "#![ \t]*\\([^ \r\t\n]+\\)\\([ \t]+\\(.+\\)\\)?")
-- 
2.25.1


[-- Attachment #3: 0001-Use-ignore-error.patch --]
[-- Type: text/plain, Size: 2091 bytes --]

From 55cf3088d0075f7d453eecb1e03683de4444eb4f Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 22 Jun 2024 12:41:25 -0700
Subject: [PATCH] Fix execution of MS-Windows app execution aliases in Eshell

* lisp/eshell/esh-ext.el (eshell-script-interpreter): Return nil if we
can't actually read the file.
---
 lisp/eshell/esh-ext.el | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/lisp/eshell/esh-ext.el b/lisp/eshell/esh-ext.el
index 3c4deb32601..6fb0579ed14 100644
--- a/lisp/eshell/esh-ext.el
+++ b/lisp/eshell/esh-ext.el
@@ -299,18 +299,23 @@ eshell-script-interpreter
 Return nil, or a list of the form:
 
   (INTERPRETER [ARGS] FILE)"
-  (let ((maxlen eshell-command-interpreter-max-length))
-    (if (and (file-readable-p file)
-	     (file-regular-p file))
-	(with-temp-buffer
-	  (insert-file-contents-literally file nil 0 maxlen)
-	  (if (looking-at "#![ \t]*\\([^ \r\t\n]+\\)\\([ \t]+\\(.+\\)\\)?")
-	      (if (match-string 3)
-		  (list (match-string 1)
-			(match-string 3)
-			file)
-		(list (match-string 1)
-		      file)))))))
+  (when (and (file-readable-p file)
+             (file-regular-p file))
+    (let ((maxlen eshell-command-interpreter-max-length))
+      ;; If we encounter a file error, assume that FILE doesn't have a
+      ;; shebang.  This can happen, for example, with certain kinds of
+      ;; reparse points like APPEXECLINK on NTFS filesystems (MS-Windows
+      ;; uses these for "app execution aliases").
+      (ignore-error 'file-error
+        (with-temp-buffer
+          (insert-file-contents-literally file nil 0 maxlen)
+          (if (looking-at "#![ \t]*\\([^ \r\t\n]+\\)\\([ \t]+\\(.+\\)\\)?")
+              (if (match-string 3)
+                  (list (match-string 1)
+                        (match-string 3)
+                        file)
+                (list (match-string 1)
+                      file))))))))
 
 (defun eshell-find-interpreter (file args &optional no-examine-p)
   "Find the command interpreter with which to execute FILE.
-- 
2.25.1