Error with tramp-archive-autoload-file-name-handler

Error with tramp-archive-autoload-file-name-handler

[Michael Heerdegen]

Hello,

one user (in the technical sense) of my computer sometimes gets "Invalid
handler in `file-name-handler-alist' errors.  Here is one example:

(file-directory-p
  "/home/micha/software/emacs/test/lisp/net/tramp-archive-resources/foo.iso/")

Evaluating that gives the error.  For another user, I get just `t'.

The main difference seems to be that the first entry of
`file-name-handler-alist' is an entry referring to
`tramp-archive-autoload-file-name-handler' for the user that gets the
error.  That entry is missing for the user where it "works".  For the
user where it doesn't work, I see that
`tramp-archive-autoload-file-name-handler' succeeds and returns nil
(normally) but seems Emacs doesn't like that result.  I'm not sure where
in the code the error is actually raised.

Both users share more or less the same Emacs configuration.

tramp-archive-enabled -> nil for both users.

When I remove the `tramp-archive-autoload-file-name-handler' entry,
result is `t` for both users.

Any hints?

TIA,

Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Hello,

Hi Michael,

> one user (in the technical sense) of my computer sometimes gets "Invalid
> handler in `file-name-handler-alist' errors.  Here is one example:
>
> (file-directory-p
>   "/home/micha/software/emacs/test/lisp/net/tramp-archive-resources/foo.iso/")
>
> Evaluating that gives the error.  For another user, I get just `t'.
>
> The main difference seems to be that the first entry of
> `file-name-handler-alist' is an entry referring to
> `tramp-archive-autoload-file-name-handler' for the user that gets the
> error.  That entry is missing for the user where it "works".  For the
> user where it doesn't work, I see that
> `tramp-archive-autoload-file-name-handler' succeeds and returns nil
> (normally) but seems Emacs doesn't like that result.  I'm not sure where
> in the code the error is actually raised.
>
> Both users share more or less the same Emacs configuration.
>
> tramp-archive-enabled -> nil for both users.
>
> When I remove the `tramp-archive-autoload-file-name-handler' entry,
> result is `t` for both users.
>
> Any hints?

Does this happen with "emacs -Q"?

> TIA,
>
> Michael.

Best regards, Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Heerdegen]

Michael Albinus <michael.albinus@gmx.de> writes:

> > Any hints?
>
> Does this happen with "emacs -Q"?

No.

Also for the affected user, it doesn't happen always.  I started several
session with init files loaded, and sometimes the problem occurred,
sometimes not.  Maybe it depends on the load order of libraries (dired,
helm, ...), I have no clue.

Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> > Does this happen with "emacs -Q"?
> No.

But evaluating this TWICE (the `add-to-list' form is from
`tramp-register-archive-file-name-handler') does:

 #+begin_src emacs-lisp
(add-to-list 'file-name-handler-alist
	         (cons (tramp-archive-autoload-file-name-regexp)
		       #'tramp-archive-autoload-file-name-handler))
(file-directory-p
 "/home/micha/software/emacs/test/lisp/net/tramp-archive-resources/foo.iso/")
  |--> (error "Invalid handler in ‘file-name-handler-alist’")
#+end_src

Hope that helps, I still have no complete overview what is evaluated
when and why, but I think that adding
`tramp-archive-autoload-file-name-handler' multiple times could be
related.

BTW, one difference, maybe a crucial one, between the two users is the
value of `tramp-archive-enabled': Here I have `nil` after starting
Emacs, the user where the problem occurs gets `t` after starting.

Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

Michael Heerdegen <michael_heerdegen@web.de> writes:

Hi Michael.

>> Does this happen with "emacs -Q"?
>
> No.
>
> Also for the affected user, it doesn't happen always.  I started several
> session with init files loaded, and sometimes the problem occurred,
> sometimes not.  Maybe it depends on the load order of libraries (dired,
> helm, ...), I have no clue.

Hmm, hard to investigate. Are you able to provide a simple recipe,
starting with "emacs -Q"?

Or, alternatively, would it be possible to bisect the relevant ~/.emacs,
in order to find out what triggers the problem?

> Michael.

Best regards, Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

Michael Heerdegen <michael_heerdegen@web.de> writes:


> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> > Does this happen with "emacs -Q"?
>> No.
>
> But evaluating this TWICE (the `add-to-list' form is from
> `tramp-register-archive-file-name-handler') does:
>
>  #+begin_src emacs-lisp
> (add-to-list 'file-name-handler-alist
> 	         (cons (tramp-archive-autoload-file-name-regexp)
> 		       #'tramp-archive-autoload-file-name-handler))
> (file-directory-p
>  "/home/micha/software/emacs/test/lisp/net/tramp-archive-resources/foo.iso/")
>   |--> (error "Invalid handler in ‘file-name-handler-alist’")
> #+end_src
>
> Hope that helps, I still have no complete overview what is evaluated
> when and why, but I think that adding
> `tramp-archive-autoload-file-name-handler' multiple times could be
> related.

I cannot reproduce. The following works fine:

--8<---------------cut here---------------start------------->8---
$ emacs -Q -l tramp --eval "(add-to-list 'file-name-handler-alist (cons (tramp-archive-autoload-file-name-regexp) #'tramp-archive-autoload-file-name-handler))" --eval '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'
--8<---------------cut here---------------end--------------->8---

> BTW, one difference, maybe a crucial one, between the two users is the
> value of `tramp-archive-enabled': Here I have `nil` after starting
> Emacs, the user where the problem occurs gets `t` after starting.

That makes a difference, indeed.

> Michael.

Best regards, Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Heerdegen]

Michael Albinus <michael.albinus@gmx.de> writes:

> I cannot reproduce. The following works fine:
>
> $ emacs -Q -l tramp --eval "(add-to-list 'file-name-handler-alist (cons (tramp-archive-autoload-file-name-regexp) #'tramp-archive-autoload-file-name-handler))" --eval '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'

You missed the TWICE part in my last message.  Just eval the above
twice:

$ emacs -Q -l tramp --eval "(add-to-list 'file-name-handler-alist (cons (tramp-archive-autoload-file-name-regexp) #'tramp-archive-autoload-file-name-handler))" --eval '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'\
                    --eval "(add-to-list 'file-name-handler-alist (cons (tramp-archive-autoload-file-name-regexp) #'tramp-archive-autoload-file-name-handler))" --eval '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'

Sorry, but that's the best recipe I can offer.  And I guess something
like this is actually happening.  AFAIU, Tramp can call
`tramp-register-archive-file-name-handler' several times - see
tramp-archive.el line 394:

```
;; In older Emacsen (prior 27.1), the autoload above does not exist.
;; So we call it again; it doesn't hurt.
(tramp-register-archive-file-name-handler)
```

and my interpretation is that at least in my case that does hurt.

But let's first see if you can reproduce the recipe now, and interpret
the result later...


Thanks,

Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Felix Dietrich]

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Michael Albinus <michael.albinus@gmx.de> writes:
>
>> I cannot reproduce. The following works fine:
>>
>> […]
>
> You missed the TWICE part in my last message.  Just eval the above
> twice:
>
> $ emacs -Q -l tramp --eval "(add-to-list 'file-name-handler-alist
> (cons (tramp-archive-autoload-file-name-regexp)
> #'tramp-archive-autoload-file-name-handler))" --eval
> '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'\
>                     --eval "(add-to-list 'file-name-handler-alist
> (cons (tramp-archive-autoload-file-name-regexp)
> #'tramp-archive-autoload-file-name-handler))" --eval
> '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'
>
> Sorry, but that's the best recipe I can offer.  And I guess something
> like this is actually happening.  AFAIU, Tramp can call
> `tramp-register-archive-file-name-handler' several times - see
> tramp-archive.el line 394:

With Emacs 27.1, I ran the test in the attached script a couple of times
and could *not* reproduce your error.


[-- Attachment #2: Script to test the issue with tramp --]
[-- Type: text/x-sh, Size: 2600 bytes --]

#!/bin/sh

# Usage: testscript ARCHIVE_PATH [COUNT]
#
# Start Emacs and test ‘file-directory-p’ on ARCHIVE_PATH.
#
# Each time before using ‘file-directory-p’, add
#
#     (cons (tramp-archive-autoload-file-name-handler)
#           #'tramp-archive-autoload-file-name-handler)
#
# to ‘file-name-handler-alist’.  ‘file-directory-p’ is called twice.
#
# Do this for COUNT times or just once if COUNT is not provided
#
# ARCHIVE_PATH defaults to
# ~/"src/emacs/test/lisp/net/tramp-archive-resources/foo.iso/".


# Emacs -nw switch: do not create a GUI frame.
nw=-nw

archive_path=${1:-~/"src/emacs/test/lisp/net/tramp-archive-resources/foo.iso/"}
if ! [ -e "$archive_path" ]
then
    printf '"%s" does not exist' "$archive_path" >&2
    exit 66 # EX_NOINPUT
fi
count=${2:-1}

run_emacs ()
{
  emacs $nw -Q -l tramp \
    --eval \
        '(setq command-error-function
               ;; Catch an error in a process filter.
               (lambda () (kill-emacs 1)))'                                \
    --eval \
        "(add-to-list 'file-name-handler-alist
                      (cons (tramp-archive-autoload-file-name-regexp)
                             #'tramp-archive-autoload-file-name-handler))" \
    --eval \
        '(let ((ret 2))
          (unwind-protect
              (setq ret (if (file-directory-p "'"$archive_path"'")
                          ;; either result counts as success
                          0 0))
            (kill-emacs ret)))'                                            \
    --eval \
        "(add-to-list 'file-name-handler-alist
                      (cons (tramp-archive-autoload-file-name-regexp)
  		                    #'tramp-archive-autoload-file-name-handler))"  \
    --eval \
        '(let ((ret 3))
          (unwind-protect
              (setq ret (if (file-directory-p "'"$archive_path"'")
                          ;; either result counts as success
                          0 0))
            (kill-emacs ret)))'
}


i=0
while [ "$i" -lt "$count" ]
do
    i=$(( i + 1 ))
    run_emacs
    ret=$?
	case $ret in
        0) # All good, just
           continue
           ;;
	    1) echo "‘command-error-function’ killed emacs" >&2
           exit $ret
           ;;
        2) echo "1. ‘unwind-protect’ killed emacs" >&2
           exit $ret
           ;;
		3) echo "2. ‘unwind-protect’ killed emacs" >&2
           exit $ret
           ;;
        *) echo "Unexpected return code: $ret" >&2
           exit $ret
           ;;
    esac
done

[-- Attachment #3: Type: text/plain, Size: 21 bytes --]



-- 
Felix Dietrich

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

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

Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

Hi,

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> Michael Albinus <michael.albinus@gmx.de> writes:
>>
>>> I cannot reproduce. The following works fine:
>>>
>>> […]
>>
>> You missed the TWICE part in my last message.  Just eval the above
>> twice:
>>
>> $ emacs -Q -l tramp --eval "(add-to-list 'file-name-handler-alist
>> (cons (tramp-archive-autoload-file-name-regexp)
>> #'tramp-archive-autoload-file-name-handler))" --eval
>> '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'\
>>                     --eval "(add-to-list 'file-name-handler-alist
>> (cons (tramp-archive-autoload-file-name-regexp)
>> #'tramp-archive-autoload-file-name-handler))" --eval
>> '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'
>>
>> Sorry, but that's the best recipe I can offer.  And I guess something
>> like this is actually happening.  AFAIU, Tramp can call
>> `tramp-register-archive-file-name-handler' several times - see
>> tramp-archive.el line 394:
>
> With Emacs 27.1, I ran the test in the attached script a couple of times
> and could *not* reproduce your error.

Same here, I cannot reproduce it with Emacs 27, Emacs 28, Emacs 29. Btw,
I believe a more realistic check would be

--8<---------------cut here---------------start------------->8---
$ emacs -Q -f tramp-register-archive-file-name-handler --eval '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'\
           -f tramp-register-archive-file-name-handler --eval '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'
--8<---------------cut here---------------end--------------->8---

It is tramp-register-archive-file-name-handler which might be invoked
several times, and which causes the problem. Could *you* (Michael)
reproduce the problem with this recipe?

Anyway, I've pushed the appended patch to the repositories. It should fix
this problem, hopefully.

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1724 bytes --]

diff --git a/lisp/net/tramp-archive.el b/lisp/net/tramp-archive.el
index 788e457367..890c8dbb75 100644
--- a/lisp/net/tramp-archive.el
+++ b/lisp/net/tramp-archive.el
@@ -374,7 +374,9 @@ tramp-archive-file-name-handler
 ;;;###autoload
 (progn (defun tramp-register-archive-file-name-handler ()
   "Add archive file name handler to `file-name-handler-alist'."
-  (when tramp-archive-enabled
+  (when (and tramp-archive-enabled
+             (not
+              (rassq #'tramp-archive-file-name-handler file-name-handler-alist)))
     (add-to-list 'file-name-handler-alist
                 (cons (tramp-archive-autoload-file-name-regexp)
                       #'tramp-archive-autoload-file-name-handler))
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 0192a63a10..580cfea1f8 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -2756,10 +2756,11 @@ tramp-completion-file-name-handler
 ;;;###autoload
 (progn (defun tramp-register-autoload-file-name-handlers ()
   "Add Tramp file name handlers to `file-name-handler-alist' during autoload."
-  (add-to-list 'file-name-handler-alist
-              (cons tramp-autoload-file-name-regexp
-                    #'tramp-autoload-file-name-handler))
-  (put #'tramp-autoload-file-name-handler 'safe-magic t)))
+  (unless (rassq #'tramp-file-name-handler file-name-handler-alist)
+    (add-to-list 'file-name-handler-alist
+                (cons tramp-autoload-file-name-regexp
+                      #'tramp-autoload-file-name-handler))
+    (put #'tramp-autoload-file-name-handler 'safe-magic t))))

 (put #'tramp-register-autoload-file-name-handlers 'tramp-autoload t)
 ;;;###autoload (tramp-register-autoload-file-name-handlers)

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Heerdegen]

Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

> With Emacs 27.1, I ran the test in the attached script a couple of times
> and could *not* reproduce your error.

With your script I can also *not* reproduce it.

I'm sorry, I'm not very good in reading shell scripts, so I didn't try
to "fix" it.

Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Heerdegen]

Michael Albinus <michael.albinus@gmx.de> writes:

> Same here, I cannot reproduce it with Emacs 27, Emacs 28, Emacs 29. Btw,
> I believe a more realistic check would be
>
> $ emacs -Q -f tramp-register-archive-file-name-handler --eval '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'\
>            -f tramp-register-archive-file-name-handler --eval '(file-directory-p "/home/albinus/tmp/out.tar.xz/")'

Dunno if it's more realistic, but:

> It is tramp-register-archive-file-name-handler which might be invoked
> several times, and which causes the problem. Could *you* (Michael)
> reproduce the problem with this recipe?

no, not with that recipe.  Mine worked, but not that one.

Sorry, can we please stop suggesting shell scripts to reproduce the
issue?  I can't read them.

Here is a recipe for emacs -Q:

M-:

(dotimes (_ 2)
  (add-to-list 'file-name-handler-alist
	       (cons (tramp-archive-autoload-file-name-regexp)
		     #'tramp-archive-autoload-file-name-handler))
  (file-directory-p
   "/home/micha/software/emacs/test/lisp/net/tramp-archive-resources/foo.iso/"))

RET

gives me:

Debugger entered--Lisp error: (error "Invalid handler in ‘file-name-handler-alist’")
  file-directory-p("/home/micha/software/emacs/test/lisp/net/tramp-arc...")
  (let ((_ --dotimes-counter--)) (add-to-list 'file-name-handler-alist (cons (concat "\\`" "\\(" ".+" "\\." (regexp-opt tramp-archive-suffixes) "\\(?:" "\\." (regexp-opt tramp-archive-compression-suffixes) "\\)*" "\\)" "\\(" "/" ".*" "\\)" "\\'") #'tramp-archive-autoload-file-name-handler)) (file-directory-p "/home/micha/software/emacs/test/lisp/net/tramp-arc..."))
  (while (< --dotimes-counter-- --dotimes-limit--) (let ((_ --dotimes-counter--)) (add-to-list 'file-name-handler-alist (cons (concat "\\`" "\\(" ".+" "\\." (regexp-opt tramp-archive-suffixes) "\\(?:" "\\." (regexp-opt tramp-archive-compression-suffixes) "\\)*" "\\)" "\\(" "/" ".*" "\\)" "\\'") #'tramp-archive-autoload-file-name-handler)) (file-directory-p "/home/micha/software/emacs/test/lisp/net/tramp-arc...")) (setq --dotimes-counter-- (1+ --dotimes-counter--)))
  (let ((--dotimes-limit-- 2) (--dotimes-counter-- 0)) (while (< --dotimes-counter-- --dotimes-limit--) (let ((_ --dotimes-counter--)) (add-to-list 'file-name-handler-alist (cons (concat "\\`" "\\(" ".+" "\\." (regexp-opt tramp-archive-suffixes) "\\(?:" "\\." (regexp-opt tramp-archive-compression-suffixes) "\\)*" "\\)" "\\(" "/" ".*" "\\)" "\\'") #'tramp-archive-autoload-file-name-handler)) (file-directory-p "/home/micha/software/emacs/test/lisp/net/tramp-arc...")) (setq --dotimes-counter-- (1+ --dotimes-counter--))))
  eval((let ((--dotimes-limit-- 2) (--dotimes-counter-- 0)) (while (< --dotimes-counter-- --dotimes-limit--) (let ((_ --dotimes-counter--)) (add-to-list 'file-name-handler-alist (cons (concat "\\`" "\\(" ".+" "\\." (regexp-opt tramp-archive-suffixes) "\\(?:" "\\." (regexp-opt tramp-archive-compression-suffixes) "\\)*" "\\)" "\\(" "/" ".*" "\\)" "\\'") #'tramp-archive-autoload-file-name-handler)) (file-directory-p "/home/micha/software/emacs/test/lisp/net/tramp-arc...")) (setq --dotimes-counter-- (1+ --dotimes-counter--)))) t)
  eval-expression((dotimes (_ 2) (add-to-list 'file-name-handler-alist (cons (tramp-archive-autoload-file-name-regexp) #'tramp-archive-autoload-file-name-handler)) (file-directory-p "/home/micha/software/emacs/test/lisp/net/tramp-arc...")) nil nil 127)
  funcall-interactively(eval-expression (dotimes (_ 2) (add-to-list 'file-name-handler-alist (cons (tramp-archive-autoload-file-name-regexp) #'tramp-archive-autoload-file-name-handler)) (file-directory-p "/home/micha/software/emacs/test/lisp/net/tramp-arc...")) nil nil 127)
  call-interactively(eval-expression nil nil)
  command-execute(eval-expression)

Is that acceptable?

> Anyway, I've pushed the appended patch to the repositories. It should fix
> this problem, hopefully.

Hmm - that actually fixes the real-life issue I see - but not the above
recipe.


Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

Michael Heerdegen <michael_heerdegen@web.de> writes:

Hi Michael.

> Here is a recipe for emacs -Q:
>
> M-:
>
> (dotimes (_ 2)
>   (add-to-list 'file-name-handler-alist
> 	       (cons (tramp-archive-autoload-file-name-regexp)
> 		     #'tramp-archive-autoload-file-name-handler))
>   (file-directory-p
>    "/home/micha/software/emacs/test/lisp/net/tramp-archive-resources/foo.iso/"))
>
> RET
>
> gives me:
>
> Debugger entered--Lisp error: (error "Invalid handler in ‘file-name-handler-alist’")
>   file-directory-p("/home/micha/software/emacs/test/lisp/net/tramp-arc...")
>
> Is that acceptable?

I'm afraid, this still doesn't raise any error here (tested with Emacs
git master and Emacs 27).

>> Anyway, I've pushed the appended patch to the repositories. It should fix
>> this problem, hopefully.
>
> Hmm - that actually fixes the real-life issue I see - but not the above
> recipe.

Of course! My patch avoids several calls of that add-to-list. So I guess
we can regard the problem as fixed, because the real-life issue is fixed
for you as well.

> Michael.

Thanks again for reporting, and for patient testing. Best regards, Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Heerdegen]

Michael Albinus <michael.albinus@gmx.de> writes:

> Of course! My patch avoids several calls of that add-to-list. So I guess
> we can regard the problem as fixed, because the real-life issue is fixed
> for you as well.

Sure - and thanks.

I've found out a bit more background I think:

When I start Emacs, tramp-archive-enabled is t.  But loading
tramp-archive.el sets it to nil (probably because some dependencies are
missing).  I guess that for you it is still t, and that's the reason why
the recipe(s) didn't work for you.

Anyway, when `tramp-archive-autoload-file-name-handler' is present after
tramp-archive-enabled has become nil, it is used and called but doesn't
handle the request (i.e. returns nil).  AFAIU, that was what I have been
seeing.

Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Felix Dietrich]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> I've found out a bit more background I think:
>
> When I start Emacs, tramp-archive-enabled is t.  But loading
> tramp-archive.el sets it to nil (probably because some dependencies are
> missing).  I guess that for you it is still t, and that's the reason why
> the recipe(s) didn't work for you.

Did you compile Emacs without D-Bus support and/or is Emacs unable to
run gvfsd-fuse?  This seems to be what decides the value of
‘tramp-gvfs-enabled’, which is, at a quick glance, used to set
‘tramp-archive-enabled’ (in tramp-archive.el and tramp-gvfs.el,
respectively).

> Anyway, when `tramp-archive-autoload-file-name-handler' is present after
> tramp-archive-enabled has become nil, it is used and called but doesn't
> handle the request (i.e. returns nil).  AFAIU, that was what I have been
> seeing.

Could the tramp error handler provide a more helpful error message in
cases where it currently just returns nil?  Or are there limitations
prohibiting this, for example in how Emacs processes file handlers?

-- 
Felix Dietrich

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

Hi Felix,

>> Anyway, when `tramp-archive-autoload-file-name-handler' is present after
>> tramp-archive-enabled has become nil, it is used and called but doesn't
>> handle the request (i.e. returns nil).  AFAIU, that was what I have been
>> seeing.
>
> Could the tramp error handler provide a more helpful error message in
> cases where it currently just returns nil?  Or are there limitations
> prohibiting this, for example in how Emacs processes file handlers?

tramp-archive-autoload-file-name-handler is just a temporary setting, it
doesn't get used after being replaced by tramp-archive-file-name-handler.

Since this bug is fixed now, I don't believe it is worthful to invest
more code there.

Best regards, Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Heerdegen]

Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

> Did you compile Emacs without D-Bus support and/or is Emacs unable to
> run gvfsd-fuse?  This seems to be what decides the value of
> ‘tramp-gvfs-enabled’, which is, at a quick glance, used to set
> ‘tramp-archive-enabled’ (in tramp-archive.el and tramp-gvfs.el,
> respectively).

Yes, this fails for me:

  (tramp-compat-funcall 'dbus-get-unique-name :session)

But that's probably expected: I had uninstalled the package
"dbus-user-session" because, well, it caused problems with user sessions
(when using multiple sessions for different users at the same time).

I don't need to get tramp-archive working, I only found this issue while
doing other stuff.  And I wanted to give some more insights here so that
everybody can be sure that Michael's fix is good enough to avoid that
kind of problem (I cannot estimate that).

Danke soweit und Grüße,

Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

Michael Heerdegen <michael_heerdegen@web.de> writes:

Hi Michael.

> But that's probably expected: I had uninstalled the package
> "dbus-user-session" because, well, it caused problems with user sessions
> (when using multiple sessions for different users at the same time).

Being curious: what's this package? I don't find it in * ELPA archives.

> Danke soweit und Grüße,
>
> Michael.

LG, Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Heerdegen]

Michael Albinus <michael.albinus@gmx.de> writes:

> > But that's probably expected: I had uninstalled the package
> > "dbus-user-session" because, well, it caused problems with user sessions
> > (when using multiple sessions for different users at the same time).
>
> Being curious: what's this package? I don't find it in * ELPA archives.

What? - It's a Debian package! ;-)

Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

Michael Heerdegen <michael_heerdegen@web.de> writes:

Hi Michael,

>> > But that's probably expected: I had uninstalled the package
>> > "dbus-user-session" because, well, it caused problems with user sessions
>> > (when using multiple sessions for different users at the same time).
>>
>> Being curious: what's this package? I don't find it in * ELPA archives.
>
> What? - It's a Debian package! ;-)

D'oh! :-)

> Michael.

Best regards, Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Felix Dietrich]

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

Michael Albinus <michael.albinus@gmx.de> writes:

> Felix Dietrich <felix.dietrich@sperrhaken.name> writes:
>
>>> Anyway, when `tramp-archive-autoload-file-name-handler' is present after
>>> tramp-archive-enabled has become nil, it is used and called but doesn't
>>> handle the request (i.e. returns nil).  AFAIU, that was what I have been
>>> seeing.
>>
>> Could the tramp error handler provide a more helpful error message

Oh, I meant “tramp file handler” here instead of “tramp error handler” –
although I had made wrong assumptions; therefore, this is irrelevant.

> Since this bug is fixed now, I don't believe it is worthful to invest
> more code there.

I am *not* convinced that the reason for this bug has been correctly
identified.  After spending some more time with it, I have come up with
the following test case with which I am now able to produce the error
reliably with revision a5841b196f [1] (that is your patch):

  #+begin_src emacs-lisp
	(let* ((emacs-repo "~/src/emacs/")
		   (foo.iso (concat (file-name-as-directory emacs-repo)
							"test/lisp/net/tramp-archive-resources/foo.iso/")))
	  (load "tramp-archive")
	  (let ((file-name-handler-alist
			 (list (cons (tramp-archive-autoload-file-name-regexp)
						 #'tramp-archive-autoload-file-name-handler)))
			(tramp-archive-enabled nil))
		(file-directory-p foo.iso))))
  #+end_src

With Emacs 27.1, however, I was not able to reproduce the issue.
Therefore, I used “git bisect” between the tag “emacs-27.1” and your
commit a5841b196f with the attached script to find the offending commit
introducing the issue: 4db69b32b8 (“Fix bug#48476”) [2].


[-- Attachment #2: Bisection Script --]
[-- Type: text/x-sh, Size: 2839 bytes --]

#!/bin/sh

# Usage: this_script [recompile [JOBS]]
#
# Without “recompile” this script only removes
# “lisp/net/tramp-archive.elc” to ensure that Emacs uses the
# “tramp-archive” version of the current revision; you have to have
# built Emacs yourself before running this script.
#
# With “recompile” the repository is cleaned, the build configured,
# and then Emacs build, before the test is run.  JOBS is bassed to the
# “-j” switch of “make”, that is: it “specifies the number of jobs to
# run simultaneously”.


# Emacs executable to use
emacs=src/emacs

skip_commit () { exit 125; }
# Can “git bisect” really be properly and reliably
# interupted this way?
interrupt_bisect () { exit 255; }
mark_good () { exit 0; }
mark_bad () { exit 1; }


if [ "$1" = "recompile" ]
then
    jobs=${2:+-j$2}
    {
      git clean -dxf \
      && ./autogen.sh \
      && ./configure --without-x --without-gnutls \
      && make $jobs src
    } || interrupt_bisect
else
    # Ensure that “tramp-archive” from the current revision is used.
    # The byte-compiled version is from the revision where the
    # bisection started and would be preferred by Emacs.
    rm -f lisp/net/tramp-archive.elc
fi

# Restore “build-aux/install-sh” to the version in the current HEAD.
# Otherwise, “git bisect” would abort after the script has finished
# (at least at some revisions) as the checkout of the next revision
# would fail because of changes to this file.
git checkout -- build-aux/install-sh



# \f
##################################################
# The actual Test.
##################################################

"$emacs" -Q --batch --eval \
"
(let ((ret 1))
  (unwind-protect
      (progn
        (load \"tramp-archive\")
        (let ((file-name-handler-alist
               (list (cons (tramp-archive-autoload-file-name-regexp)
                           #'tramp-archive-autoload-file-name-handler)))
              (tramp-archive-enabled nil))
          (file-directory-p
           \"~/emacs/test/lisp/net/tramp-archive-resources/foo.iso/\")
          ;; No error during ‘file-directory-p’
          (setq ret 0)))
    (kill-emacs ret)))
"

ret=$?
case $ret in
    0) mark_good
       ;;
    1) mark_bad
       ;;
    255) # For some revisions the ‘kill-emacs’ in the unwind-form does
         # not work as expected and Emacs instead exits with 255, or
         # maybe that is -1, (as it does on erros in batch mode).
         # Using ‘condition-case’ would proberly fix that (and make a
         # more robust test), but this is quicker and good enough.
        mark_bad
        ;;
    *)
        printf "Unexpected return code by Emacs: %i\n" $ret >&2
        interrupt_bisect
        ;;
esac

[-- Attachment #3: Type: text/plain, Size: 817 bytes --]



At a quick glance, this commit changed the implementation of
‘tramp-archive-autload-file-name-handler’ and might be missing an
else-form.  This causes the autoload handler to return nil when
‘tramp-archive-enabled’ is nil, and, at a slightly longer glance, for
some file name handlers this is wrong and an error.  The attached patch
changes ‘tramp-archive-autoload-file-name-handler’ to always call
‘tramp-autoload-file-name-handler’, setting ‘tramp-archive-autoload’
appropriately.  This may cause a regression, though, of bug #48476 [3]
(“Emacs hangs with 100% cpu if started within a current directory that
has a name ending with ".tar"”).  I have not tested that or spent time
on understanding that bug.  Maybe you still remember enough details of
the bug to judge this.


[-- Attachment #4: Patch to always call ‘tramp-autoload-file-name-handler’ --]
[-- Type: text/x-diff, Size: 2340 bytes --]

From 282fe5b2af1422236b213732d31a176fdea904ed Mon Sep 17 00:00:00 2001
From: Felix Dietrich <felix.dietrich@sperrhaken.name>
Date: Mon, 4 Apr 2022 18:45:17 +0200
Subject: [PATCH] =?UTF-8?q?tramp-archive:=20Always=20call=20=E2=80=98tramp?=
 =?UTF-8?q?-autoload-file-name-handler=E2=80=99?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Otherwise, not calling ‘tramp-autoload-file-name-handler’ in
‘tramp-archive-autoload-file-name-handler’ when
‘tramp-archive-enabled’ is nil and
‘tramp-archive-autoload-file-name-handler’ is in the
‘file-name-handler-alist’ results in an error “Invalid handler in
‘file-name-handler-alist” once Emacs calls
‘tramp-archive-autoload-file-name-handler’ with a handler that does
not expect nil.  Always returning nil is also false in general.
---
 lisp/net/tramp-archive.el | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/lisp/net/tramp-archive.el b/lisp/net/tramp-archive.el
index 890c8dbb75..e7ffe2d4f4 100644
--- a/lisp/net/tramp-archive.el
+++ b/lisp/net/tramp-archive.el
@@ -360,14 +360,13 @@ arguments to pass to the OPERATION."
 (progn (defun tramp-archive-autoload-file-name-handler (operation &rest args)
   "Load Tramp archive file name handler, and perform OPERATION."
   (defvar tramp-archive-autoload)
-  (when tramp-archive-enabled
-    ;; We cannot use `tramp-compat-temporary-file-directory' here due
-    ;; to autoload.  When installing Tramp's GNU ELPA package, there
-    ;; might be an older, incompatible version active.  We try to
-    ;; overload this.
-    (let ((default-directory temporary-file-directory)
-          (tramp-archive-autoload t))
-      (apply #'tramp-autoload-file-name-handler operation args)))))
+  (let (;; We cannot use `tramp-compat-temporary-file-directory' here due
+        ;; to autoload.  When installing Tramp's GNU ELPA package, there
+        ;; might be an older, incompatible version active.  We try to
+        ;; overload this.
+        (default-directory temporary-file-directory)
+        (tramp-archive-autoload tramp-archive-enabled))
+    (apply #'tramp-autoload-file-name-handler operation args))))
 
 (put #'tramp-archive-autoload-file-name-handler 'tramp-autoload t)
 
-- 
2.35.1


[-- Attachment #5: Type: text/plain, Size: 7197 bytes --]



At a much longer glance, in this particular case it is not actually
‘Ffile_directory_p’ (the C function in fileio.c) directly that has an
issue with nil, but, instead, the error is signalled in
‘Fexpand_file_name’, called indirectly by ‘Ffile_directory_p’:

    - Ffile_directory_p
      - expand_and_dir_to_file
        - Fexpand_file_name

‘expand_file_name’ expects a string as the return value from a magic
file name handler; nil, and everything else, is an error.

Now, why is it a problem to add
‘tramp-archive-autoload-file-name-handler’ to ‘file-name-handler-alist’
if ‘tramp-archive-file-name-handler’ is already there?  Why does the
following snipped still fail even though
‘tramp-archive-file-name-handler’ comes first in the handler alist?

   #+begin_src emacs-lisp
     (let* ((emacs-repo "~/src/emacs/")
            (foo.iso
             (concat (file-name-as-directory emacs-repo)
                     "test/lisp/net/tramp-archive-resources/foo.iso/")))
       (load "tramp-archive")
       (let ((tramp-archive-enabled nil)
             (file-name-handler-alist
              (list (cons tramp-archive-file-name-regexp
                          #'tramp-archive-file-name-handler)
                    (cons (tramp-archive-autoload-file-name-regexp)
                          #'tramp-archive-autoload-file-name-handler))))
         (file-directory-p foo.iso)))
   #+end_src

The answer is that the real file handler
‘tramp-archive-file-name-handler’ has its ‘operations’ property set,
which does not include ‘expand-file-name’; the autoload handler has not,
and so, when ‘file-directory-p’ is called, ‘find-file-name-handler’
picks for the ‘expand-file-name’ operation initiated by
‘file-directory-p’ the autoload handler, because the operations property
of ‘tramp-archive-file-name-handler’ indicates that
‘tramp-archive-file-name-handler’ is not applicable.  With
‘tramp-archive-enabled’ bound to nil, the autoload handler then returns
nil and, by that, causes ‘Fexpand_file_name’ to signal the error.  (As
my test above shows, having multiple entries is not significant, it is
the autoload handler that causes the error.)

The erroneous autoload handler:

  #+begin_src emacs-lisp
    (defun tramp-archive-autoload-file-name-handler (operation &rest args)
      "Load Tramp archive file name handler, and perform OPERATION."
      (defvar tramp-archive-autoload)
      (when tramp-archive-enabled
        ;; We cannot use `tramp-compat-temporary-file-directory' here due
        ;; to autoload.  When installing Tramp's GNU ELPA package, there
        ;; might be an older, incompatible version active.  We try to
        ;; overload this.
        (let ((default-directory temporary-file-directory)
              (tramp-archive-autoload t))
          (apply #'tramp-autoload-file-name-handler operation args))))
  #+end_src


I guess other Michael was on the right track:

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Anyway, when `tramp-archive-autoload-file-name-handler' is present after
> tramp-archive-enabled has become nil, it is used and called but doesn't
> handle the request (i.e. returns nil).  AFAIU, that was what I have been
> seeing.

Now, to explain this part and especially the “TWICE”:

> But evaluating this TWICE (the `add-to-list' form is from
> `tramp-register-archive-file-name-handler') does:
> 
>  #+begin_src emacs-lisp
> (add-to-list 'file-name-handler-alist
> 	         (cons (tramp-archive-autoload-file-name-regexp)
> 		       #'tramp-archive-autoload-file-name-handler))
> (file-directory-p
>  "/home/micha/software/emacs/test/lisp/net/tramp-archive-resources/foo.iso/")
>   |--> (error "Invalid handler in ‘file-name-handler-alist’")
> #+end_src
> 
> I think that adding `tramp-archive-autoload-file-name-handler'
> multiple times could be related.

For the first time:

When ‘tramp-archive-enabled’ is initially set to t:

  #+begin_src emacs-lisp
    ;;;###autoload
    (defvar tramp-archive-enabled (featurep 'dbusbind)
      "Non-nil when file archive support is available.")
  #+end_src

then ‘tramp-archive-autoload-file-name-handler’ will initialise tramp by
calling ‘tramp-autoload-file-name-handler’ which will first load
“tramp-archive”, because ‘tramp-archive-enabled’ is t; this can
potentially set ‘tramp-archive-enabled’ to nil:

  #+begin_src emacs-lisp
    ;; In tramp-archive.el,
    ;; right after the (defvar tramp-archive-enabled…
    (setq tramp-archive-enabled tramp-gvfs-enabled)
  #+end_src

  #+begin_src emacs-lisp
    (defconst tramp-gvfs-enabled
    (ignore-errors
      (and (featurep 'dbusbind)
       (autoload 'zeroconf-init "zeroconf")
       (tramp-compat-funcall 'dbus-get-unique-name :system)
       (tramp-compat-funcall 'dbus-get-unique-name :session)
       (or (tramp-process-running-p "gvfs-fuse-daemon")
           (tramp-process-running-p "gvfsd-fuse"))))
    "Non-nil when GVFS is available.")
  #+end_src

After loading “tramp-archive”, ‘tramp-autoload-file-name-handler’
continues with loading “tramp”; this will call, via the
‘tramp--startup-hook’ ‘tramp-register-file-name-handlers’.  This
function will, depending on the value of ‘tramp-archive-enabled’, also
add the ‘tramp-archive-file-name-handler’ to the
‘file-name-handler-alist’.

For the second time:

If ‘tramp-archive-enabled’ is nil now, and you add
‘tramp-archive-autoload-file-name-handler’ again to
‘file-name-handler-alist’, you arrive at the above described problem:
‘Fexpand_file_name’ called from ‘Ffile_directory_p’ will signal an error
because the autoload handler returns nil.

If ‘tramp-archive-enabled’ is t after the first time, the registration
process is simply done a second time; no error occurs.



Footnotes:
[1] commit a5841b196f12894df4c1bb073f28ddadb6faa3cf
    Author: Michael Albinus <michael.albinus@gmx.de>
    Date:   Mon Mar 28 12:02:23 2022 +0200
    Subject:  Do not register Tramp file name handlers twice

    * lisp/net/tramp.el (tramp-register-autoload-file-name-handlers):
    * lisp/net/tramp-archive.el (tramp-register-archive-file-name-handler):
    Check, whether the real file name handler is already registered.


[2] commit 4db69b32b835a833168982b0f11a43d7f62ba8b2
    Author: Michael Albinus <michael.albinus@gmx.de>
    Date:   Sat May 22 17:51:07 2021 +0200
    Subject: Fix bug#48476

    * lisp/net/tramp-archive.el (tramp-archive-autoload-file-name-handler):
    Add implementation.
    
    * lisp/net/tramp-integration.el (tramp-rename-files)
    (tramp-rename-these-files): Declare them.
    
    * lisp/net/tramp.el (tramp-autoload-file-name-handler):
    Load tramp-archive.el if needed.  (Bug#48476)
    
    * test/lisp/net/tramp-archive-tests.el (tramp-archive-test45-auto-load):
    Extend test.
    
    Use #' syntax for function symbols.


[3]  <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=48476>

-- 
Felix Dietrich

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

Hi Felix,

>>>> Anyway, when `tramp-archive-autoload-file-name-handler' is present after
>>>> tramp-archive-enabled has become nil, it is used and called but doesn't
>>>> handle the request (i.e. returns nil).  AFAIU, that was what I have been
>>>> seeing.
>>>
>>> Could the tramp error handler provide a more helpful error message
>
> Oh, I meant “tramp file handler” here instead of “tramp error handler” –
> although I had made wrong assumptions; therefore, this is irrelevant.

tramp-archive-autoload-file-name-handler is a function with a very short
life time. It is supposed to be called only once, and it shall disappear
afterwards. So I don't believe it is worth to be extended too much with
error handling.

>> Since this bug is fixed now, I don't believe it is worthful to invest
>> more code there.
>
> I am *not* convinced that the reason for this bug has been correctly
> identified.

First of all, many thanks you've analyzed this in deep detail! Not so
many people do this with Tramp.

> At a quick glance, this commit changed the implementation of
> ‘tramp-archive-autload-file-name-handler’ and might be missing an
> else-form.  This causes the autoload handler to return nil when
> ‘tramp-archive-enabled’ is nil, and, at a slightly longer glance, for
> some file name handlers this is wrong and an error.  The attached patch
> changes ‘tramp-archive-autoload-file-name-handler’ to always call
> ‘tramp-autoload-file-name-handler’, setting ‘tramp-archive-autoload’
> appropriately.

Yes, this works, thanks.

> This may cause a regression, though, of bug #48476 [3]
> (“Emacs hangs with 100% cpu if started within a current directory that
> has a name ending with ".tar"”).  I have not tested that or spent time
> on understanding that bug.  Maybe you still remember enough details of
> the bug to judge this.

I've shortly tested the recipe given in that bug, and everything seems
to be OK with your patch. I will apply your patch in your name to the
emacs-28 branch, after I have merged it with other pending Tramp
patches. Likely tomorrow. Perhaps you could provide a ChangeLog-style
commit message?

> Now, why is it a problem to add
> ‘tramp-archive-autoload-file-name-handler’ to ‘file-name-handler-alist’
> if ‘tramp-archive-file-name-handler’ is already there?  Why does the
> following snipped still fail even though
> ‘tramp-archive-file-name-handler’ comes first in the handler alist?

With my previous patch, this shouldn't happen
anymore. Both tramp-archive-autoload-file-name-handler and
tramp-archive-file-name-handler shouldn't coexist in
file-name-handler-alist. Do you still see this after your patch has been
applied?

Best regards, Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Felix,

> I've shortly tested the recipe given in that bug, and everything seems
> to be OK with your patch. I will apply your patch in your name to the
> emacs-28 branch, after I have merged it with other pending Tramp
> patches.

Done.

Best regards, Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Felix Dietrich]

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

Hi Michael,

Michael Albinus <michael.albinus@gmx.de> writes:

> Michael Albinus <michael.albinus@gmx.de> writes:

>> I've shortly tested the recipe given in that bug, and everything seems
>> to be OK with your patch. I will apply your patch in your name to the
>> emacs-28 branch, after I have merged it with other pending Tramp
>> patches.
>
> Done.

>> Perhaps you could provide a ChangeLog-style commit message?

Thanks for taking care of fixing the style of my commit message. :)


Hm, I had wanted to add that now ‘tramp-autoload-file-name-handler’ and
‘tramp-archive-autoload-file-name-handler’ look nearly the same, and
that a nicer solution would be to revert the changes of 4db69b32b8 (“Fix
bug#48476”) to tramp-archive.el and make the archive autoload handler an
alias again to ‘tramp-autoload-file-name-handler’ as well as changing
the latter to use ‘tramp-archive-enabled’ directly instead of via
‘tramp-archive-autoload’.  Interestingly, this does not work (almost
sent it without testing): it causes a similar issue as bug #48476 with
an error message “Recursive load” (no hang due to an infinite loop
though).  Do you remember why using ‘defalias’ here causes an infinite
recursion?

The attached patch shows the erroneous changes described above.  Here
the steps from bug #48476 to reproduce the issue:

    mkdir foo.tar
    cd foo.tar
    emacs

I also forced ‘tramp-gvfs-enabled’ to t because its checks do not seem
to take the DBUS_SESSION_BUS_ADDRESS environment variable into account
(which was set by dbus-run-session).  Maybe this had a negative
influence.


[-- Attachment #2: Patch with changes that result in a recursive load --]
[-- Type: text/x-diff, Size: 2587 bytes --]

From d1182bca9737129c0b517b4370f0ddc52f11dcad Mon Sep 17 00:00:00 2001
From: Felix Dietrich <felix.dietrich@sperrhaken.name>
Date: Thu, 7 Apr 2022 17:43:10 +0200
Subject: [PATCH] Make tramp-archive-autoload-file-name-handler an alias
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/net/tramp-archive.el (tramp-archive-autoload-file-name-handler):
Make ‘tramp-archive-autoload-file-name-handler’ an alias of
‘tramp-autoload-file-name-handler’.  This reverts the changes to this
file introduced by “Fix bug#48476” on 22. May 2021.
* lisp/net/tramp.el (tramp-autoload-file-name-handler):
Use ‘tramp-archive-enabled’ instead of ‘tramp-archive-autoload’.
---
 lisp/net/tramp-archive.el | 12 ++----------
 lisp/net/tramp.el         |  2 +-
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/lisp/net/tramp-archive.el b/lisp/net/tramp-archive.el
index 4b649edaab..cc7d6b3694 100644
--- a/lisp/net/tramp-archive.el
+++ b/lisp/net/tramp-archive.el
@@ -353,16 +353,8 @@ arguments to pass to the OPERATION."
 	      (tramp-archive-run-real-handler operation args)))))))
 
 ;;;###autoload
-(progn (defun tramp-archive-autoload-file-name-handler (operation &rest args)
-  "Load Tramp archive file name handler, and perform OPERATION."
-  (defvar tramp-archive-autoload)
-  (let (;; We cannot use `tramp-compat-temporary-file-directory' here
-	;; due to autoload.  When installing Tramp's GNU ELPA package,
-	;; there might be an older, incompatible version active.  We
-	;; try to overload this.
-        (default-directory temporary-file-directory)
-        (tramp-archive-autoload tramp-archive-enabled))
-    (apply #'tramp-autoload-file-name-handler operation args))))
+(defalias 'tramp-archive-autoload-file-name-handler
+  #'tramp-autoload-file-name-handler)
 
 (put #'tramp-archive-autoload-file-name-handler 'tramp-autoload t)
 
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index a24d83f876..72ce862dc4 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -2678,7 +2678,7 @@ Falls back to normal file name handler if no Tramp file name handler exists."
     ;; might be an older, incompatible version active.  We try to
     ;; overload this.
     (let ((default-directory temporary-file-directory))
-      (when (bound-and-true-p tramp-archive-autoload)
+      (when (bound-and-true-p tramp-archive-enabled)
 	(load "tramp-archive" 'noerror 'nomessage))
       (load "tramp" 'noerror 'nomessage)))
   (apply operation args)))
-- 
2.35.1


[-- Attachment #3: Type: text/plain, Size: 1183 bytes --]



> Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

>> Now, why is it a problem to add
>> ‘tramp-archive-autoload-file-name-handler’ to ‘file-name-handler-alist’
>> if ‘tramp-archive-file-name-handler’ is already there?  Why does the
>> following snipped still fail even though
>> ‘tramp-archive-file-name-handler’ comes first in the handler alist?

> With my previous patch, this shouldn't happen
> anymore. Both tramp-archive-autoload-file-name-handler and
> tramp-archive-file-name-handler shouldn't coexist in
> file-name-handler-alist. Do you still see this after your patch has been
> applied?

I havenʼt looked particular hard, but no, I do not see both the autoload
and the normal handler in the ‘file-name-handler-alist’ at the same time
after a normal start and load of tramp – well, if I donʼt put them there
myself, that is.  My intention with this part was to show the issue I
described was connected to and indeed the cause for the problem Michael
Heerdegen had encountered, and also just to provide further details (I
was really confused about the example in that part for quite a while).

-- 
Felix Dietrich

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Albinus]

Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

> Hi Michael,

Hi Felix,

> Hm, I had wanted to add that now ‘tramp-autoload-file-name-handler’ and
> ‘tramp-archive-autoload-file-name-handler’ look nearly the same, and
> that a nicer solution would be to revert the changes of 4db69b32b8 (“Fix
> bug#48476”) to tramp-archive.el and make the archive autoload handler an
> alias again to ‘tramp-autoload-file-name-handler’ as well as changing
> the latter to use ‘tramp-archive-enabled’ directly instead of via
> ‘tramp-archive-autoload’.  Interestingly, this does not work (almost
> sent it without testing): it causes a similar issue as bug #48476 with
> an error message “Recursive load” (no hang due to an infinite loop
> though).  Do you remember why using ‘defalias’ here causes an infinite
> recursion?

Not really. I guess it is because this function was called twice
(replacing tramp-autoload-file-name-handler and
tramp-archive-autoload-file-name-handler in file-name-handler-alist),
which results in infinite recursion.

> The attached patch shows the erroneous changes described above.  Here
> the steps from bug #48476 to reproduce the issue:
>
>     mkdir foo.tar
>     cd foo.tar
>     emacs

This case is already handled in tramp-archive-test45-auto-load (emacs-28
branch) resp tramp-archive-test47-auto-load (master branch). I've
extended it slightly in the emacs-28 branch in order to test also the
case tramp-archive-enabled is nil, should arrive in master branch after
next merge.

. If you like you could play with this test case, perhaps more tests
could be applied (for example existence of
tramp-archive-autoload-file-name-handler and
tramp-archive-file-name-handler in file-name-handler-alist).

> I also forced ‘tramp-gvfs-enabled’ to t because its checks do not seem
> to take the DBUS_SESSION_BUS_ADDRESS environment variable into account
> (which was set by dbus-run-session).  Maybe this had a negative
> influence.

This could also be tested.

>> With my previous patch, this shouldn't happen
>> anymore. Both tramp-archive-autoload-file-name-handler and
>> tramp-archive-file-name-handler shouldn't coexist in
>> file-name-handler-alist. Do you still see this after your patch has been
>> applied?
>
> I havenʼt looked particular hard, but no, I do not see both the autoload
> and the normal handler in the ‘file-name-handler-alist’ at the same time
> after a normal start and load of tramp – well, if I donʼt put them there
> myself, that is.  My intention with this part was to show the issue I
> described was connected to and indeed the cause for the problem Michael
> Heerdegen had encountered, and also just to provide further details (I
> was really confused about the example in that part for quite a while).

Yes, thanks! But I believe we don't need to care further this.

Best regards, Michael.

Re: Error with tramp-archive-autoload-file-name-handler

[Michael Heerdegen]

Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

> >> Perhaps you could provide a ChangeLog-style commit message?
>
> Thanks for taking care of fixing the style of my commit message. :)

And thank you for your help and your patch!

> an error message “Recursive load” (no hang due to an infinite loop
> though).  Do you remember why using ‘defalias’ here causes an infinite
> recursion?

I think the debugger should be able to catch that one, and with Elisp
sources loaded the backtrace should provide enough information to find
the reason.

Michael.