[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Error with tramp-archive-autoload-file-name-handler
From: |
Felix Dietrich |
Subject: |
Re: Error with tramp-archive-autoload-file-name-handler |
Date: |
Wed, 06 Apr 2022 10:49:42 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
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].
bisect.sh
Description: Bisection Script
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.
0001-tramp-archive-Always-call-tramp-autoload-file-name-h.patch
Description: Patch to always call ‘tramp-autoload-file-name-handler’
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, Felix Dietrich, 2022/04/02
- Re: Error with tramp-archive-autoload-file-name-handler, Michael Albinus, 2022/04/02
- Re: Error with tramp-archive-autoload-file-name-handler,
Felix Dietrich <=
- Re: Error with tramp-archive-autoload-file-name-handler, Michael Albinus, 2022/04/06
- Re: Error with tramp-archive-autoload-file-name-handler, Michael Albinus, 2022/04/07
- Re: Error with tramp-archive-autoload-file-name-handler, Felix Dietrich, 2022/04/07
- Re: Error with tramp-archive-autoload-file-name-handler, Michael Albinus, 2022/04/08
- Re: Error with tramp-archive-autoload-file-name-handler, Michael Heerdegen, 2022/04/12
Re: Error with tramp-archive-autoload-file-name-handler, Michael Heerdegen, 2022/04/02