From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Felix Dietrich Newsgroups: gmane.emacs.help Subject: Re: Error with tramp-archive-autoload-file-name-handler Date: Wed, 06 Apr 2022 10:49:42 +0200 Message-ID: <87lewiy4t5.fsf@sperrhaken.name> References: <87bkxtzjf1.fsf@web.de> <871qyphzv1.fsf@gmx.de> <87r16o4jxo.fsf@web.de> <871qyouuls.fsf@web.de> <87y20vg5gu.fsf@gmx.de> <87fsn2q0mk.fsf@web.de> <87o81qwqwi.fsf@sperrhaken.name> <87h77ifk6w.fsf@gmx.de> <87v8vx7gsz.fsf@web.de> <87wngddxm0.fsf@gmx.de> <87pmm11sdw.fsf@web.de> <877d873dak.fsf@sperrhaken.name> <87v8vre7y6.fsf@gmx.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="23550"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) Cc: Michael Albinus To: help-gnu-emacs@gnu.org Original-X-From: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane-mx.org@gnu.org Wed Apr 06 10:51:53 2022 Return-path: Envelope-to: geh-help-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nc1OK-0005yh-W4 for geh-help-gnu-emacs@m.gmane-mx.org; Wed, 06 Apr 2022 10:51:53 +0200 Original-Received: from localhost ([::1]:39812 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nc1OJ-0003E3-VL for geh-help-gnu-emacs@m.gmane-mx.org; Wed, 06 Apr 2022 04:51:51 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:57978) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nc1MP-0003Db-Ds for help-gnu-emacs@gnu.org; Wed, 06 Apr 2022 04:49:53 -0400 Original-Received: from mout.kundenserver.de ([212.227.17.10]:47957) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nc1MM-0001cr-N8 for help-gnu-emacs@gnu.org; Wed, 06 Apr 2022 04:49:53 -0400 Original-Received: from localhost ([78.54.126.235]) by mrelayeu.kundenserver.de (mreue106 [212.227.15.183]) with ESMTPSA (Nemesis) id 1MaIKJ-1nWBwn081N-00WILG; Wed, 06 Apr 2022 10:49:48 +0200 In-Reply-To: <87v8vre7y6.fsf@gmx.de> (Michael Albinus's message of "Sat, 02 Apr 2022 19:00:49 +0200") X-Provags-ID: V03:K1:a3w46MmISPuR1NlmcWyyYBLKw0katkYCV1KSAQEnqmeMnfowJ3Q JpFmSNAl/3h5lW6WyrPKEwv16cy7QXV5pk0Z2CqZw5ArjXoycFKBgkGCPAbJD5EP7IzuGPW auA1UPAqtAqilM1DlE3zYXNmrRfF9SOq/KVkO2kwDSvnoCyYgrevvLpBf4eiLrk/XixFQVK 1LVV5ZZApV5AzF3vf/+rw== X-UI-Out-Filterresults: notjunk:1;V03:K0:AaaEfHCWWFU=:hCD6cuchwxuFr0jziB/oB0 IzKxOWq+chvJdIycbVK+OoZVFehtpZmk7J4Z6UzE2EYAhzqaFiHEfxaZEXZ7B0uskWFpHtkSS gUzOXLw60CqNb3E1zVe2pO/o3hc/zwvfjsmmGIVVsBbPUHxXNpH8MwD3NfNrlGuyQfiVYY8pM EAKeE2AB+7ADEQw4s1dj1uVguRd6FNFidJknUiS08plU6EOdgGzXmjKI2FUjbylwTFczsaU/Z 2cWX9QbavwCXHe6RDiDOCgsXJ6rZ9zZ+0k7NDD5HBS4VBPHKSKsFtFoPuhF5AyqhBICRPCtZx GBHpfnMabHnoQgdBKloajqsYLXVnD6Noroxg++lNnWIdSBoy2m48GXajW/Oe++YdXdtNY0dIE 1/cpn4VLVOB+gCDo0+Llzf/gOC/5zU2rB/1lqUHLzZA1bnIQnLMzjeEHzW6aIpNkTTe1sGKeX vku5lpvHXQb6OxuOMFnMdiYvzWMU5Hca1gkF8zBbJ1Gxi/wXRcCJh/QNuf5fseQMKEuD6nL/v vShbYyW+mWCijBo03IZLVuoy+XdAlgP4+JDCrjFdUtX6u2e4tX9nHWr+n6W8HHgyinfqspSOw 3uWjraeop1MHOf4niH0LnXBA8wGIfdXXjZOn3lJGORcIcTOksf36ffsuQOH2NM233piif5J0m cyhmldDcAQFGvj1bAoAskGvu6aPd0pYpoGon8yPBrBUv7jzYZL3+5IdQKIJUBUr8djBtYPzzy 5PWcq91OGxugPybg Received-SPF: none client-ip=212.227.17.10; envelope-from=felix.dietrich@sperrhaken.name; helo=mout.kundenserver.de X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_NONE=0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: help-gnu-emacs@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Users list for the GNU Emacs text editor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "help-gnu-emacs" Xref: news.gmane.io gmane.emacs.help:136876 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Michael Albinus writes: > Felix Dietrich 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 =E2=80=9Ctramp file handler=E2=80=9D here instead of =E2=80=9Ct= ramp error handler=E2=80=9D =E2=80=93 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 =E2=80=9Cgit bisect=E2=80=9D between the tag =E2=80=9Cema= cs-27.1=E2=80=9D and your commit a5841b196f with the attached script to find the offending commit introducing the issue: 4db69b32b8 (=E2=80=9CFix bug#48476=E2=80=9D) [2]. --=-=-= Content-Type: text/x-sh; charset=utf-8 Content-Disposition: attachment; filename=bisect.sh Content-Transfer-Encoding: quoted-printable Content-Description: Bisection Script #!/bin/sh # Usage: this_script [recompile [JOBS]] # # Without =E2=80=9Crecompile=E2=80=9D this script only removes # =E2=80=9Clisp/net/tramp-archive.elc=E2=80=9D to ensure that Emacs uses the # =E2=80=9Ctramp-archive=E2=80=9D version of the current revision; you have= to have # built Emacs yourself before running this script. # # With =E2=80=9Crecompile=E2=80=9D the repository is cleaned, the build con= figured, # and then Emacs build, before the test is run. JOBS is bassed to the # =E2=80=9C-j=E2=80=9D switch of =E2=80=9Cmake=E2=80=9D, that is: it =E2=80= =9Cspecifies the number of jobs to # run simultaneously=E2=80=9D. # Emacs executable to use emacs=3Dsrc/emacs skip_commit () { exit 125; } # Can =E2=80=9Cgit bisect=E2=80=9D really be properly and reliably # interupted this way? interrupt_bisect () { exit 255; } mark_good () { exit 0; } mark_bad () { exit 1; } if [ "$1" =3D "recompile" ] then jobs=3D${2:+-j$2} { git clean -dxf \ && ./autogen.sh \ && ./configure --without-x --without-gnutls \ && make $jobs src } || interrupt_bisect else # Ensure that =E2=80=9Ctramp-archive=E2=80=9D 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 =E2=80=9Cbuild-aux/install-sh=E2=80=9D to the version in the curr= ent HEAD. # Otherwise, =E2=80=9Cgit bisect=E2=80=9D 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 # ################################################## # 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 =E2=80=98file-directory-p=E2=80=99 (setq ret 0))) (kill-emacs ret))) " ret=3D$? case $ret in 0) mark_good ;; 1) mark_bad ;; 255) # For some revisions the =E2=80=98kill-emacs=E2=80=99 in the unwin= d-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 =E2=80=98condition-case=E2=80=99 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 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable At a quick glance, this commit changed the implementation of =E2=80=98tramp-archive-autload-file-name-handler=E2=80=99 and might be miss= ing an else-form. This causes the autoload handler to return nil when =E2=80=98tramp-archive-enabled=E2=80=99 is nil, and, at a slightly longer g= lance, for some file name handlers this is wrong and an error. The attached patch changes =E2=80=98tramp-archive-autoload-file-name-handler=E2=80=99 to alway= s call =E2=80=98tramp-autoload-file-name-handler=E2=80=99, setting =E2=80=98tramp-= archive-autoload=E2=80=99 appropriately. This may cause a regression, though, of bug #48476 [3] (=E2=80=9CEmacs hangs with 100% cpu if started within a current directory t= hat has a name ending with ".tar"=E2=80=9D). I have not tested that or spent t= ime on understanding that bug. Maybe you still remember enough details of the bug to judge this. --=-=-= Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename=0001-tramp-archive-Always-call-tramp-autoload-file-name-h.patch Content-Transfer-Encoding: quoted-printable Content-Description: Patch to always call =?utf-8?Q?=E2=80=98tramp-autoload-file-name-handler?= =?utf-8?Q?=E2=80=99?= >From 282fe5b2af1422236b213732d31a176fdea904ed Mon Sep 17 00:00:00 2001 From: Felix Dietrich Date: Mon, 4 Apr 2022 18:45:17 +0200 Subject: [PATCH] =3D?UTF-8?q?tramp-archive:=3D20Always=3D20call=3D20=3DE2= =3D80=3D98tramp?=3D =3D?UTF-8?q?-autoload-file-name-handler=3DE2=3D80=3D99?=3D MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit Otherwise, not calling =E2=80=98tramp-autoload-file-name-handler=E2=80=99 in =E2=80=98tramp-archive-autoload-file-name-handler=E2=80=99 when =E2=80=98tramp-archive-enabled=E2=80=99 is nil and =E2=80=98tramp-archive-autoload-file-name-handler=E2=80=99 is in the =E2=80=98file-name-handler-alist=E2=80=99 results in an error =E2=80=9CInva= lid handler in =E2=80=98file-name-handler-alist=E2=80=9D once Emacs calls =E2=80=98tramp-archive-autoload-file-name-handler=E2=80=99 with a handler t= hat 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 ar= gs) "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)))) =20 (put #'tramp-archive-autoload-file-name-handler 'tramp-autoload t) =20 --=20 2.35.1 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable At a much longer glance, in this particular case it is not actually =E2=80=98Ffile_directory_p=E2=80=99 (the C function in fileio.c) directly t= hat has an issue with nil, but, instead, the error is signalled in =E2=80=98Fexpand_file_name=E2=80=99, called indirectly by =E2=80=98Ffile_di= rectory_p=E2=80=99: - Ffile_directory_p - expand_and_dir_to_file - Fexpand_file_name =E2=80=98expand_file_name=E2=80=99 expects a string as the return value fro= m a magic file name handler; nil, and everything else, is an error. Now, why is it a problem to add =E2=80=98tramp-archive-autoload-file-name-handler=E2=80=99 to =E2=80=98file= -name-handler-alist=E2=80=99 if =E2=80=98tramp-archive-file-name-handler=E2=80=99 is already there? Why= does the following snipped still fail even though =E2=80=98tramp-archive-file-name-handler=E2=80=99 comes first in the handle= r 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 =E2=80=98tramp-archive-file-name-handler=E2=80=99 has its =E2=80=98operatio= ns=E2=80=99 property set, which does not include =E2=80=98expand-file-name=E2=80=99; the autoload han= dler has not, and so, when =E2=80=98file-directory-p=E2=80=99 is called, =E2=80=98find-fi= le-name-handler=E2=80=99 picks for the =E2=80=98expand-file-name=E2=80=99 operation initiated by =E2=80=98file-directory-p=E2=80=99 the autoload handler, because the operat= ions property of =E2=80=98tramp-archive-file-name-handler=E2=80=99 indicates that =E2=80=98tramp-archive-file-name-handler=E2=80=99 is not applicable. With =E2=80=98tramp-archive-enabled=E2=80=99 bound to nil, the autoload handler = then returns nil and, by that, causes =E2=80=98Fexpand_file_name=E2=80=99 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 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 =E2=80=9CTWICE=E2=80=9D: > But evaluating this TWICE (the `add-to-list' form is from > `tramp-register-archive-file-name-handler') does: >=20 > #+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.is= o/") > |--> (error "Invalid handler in =E2=80=98file-name-handler-alist=E2=80= =99") > #+end_src >=20 > I think that adding `tramp-archive-autoload-file-name-handler' > multiple times could be related. For the first time: When =E2=80=98tramp-archive-enabled=E2=80=99 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 =E2=80=98tramp-archive-autoload-file-name-handler=E2=80=99 will initia= lise tramp by calling =E2=80=98tramp-autoload-file-name-handler=E2=80=99 which will first= load =E2=80=9Ctramp-archive=E2=80=9D, because =E2=80=98tramp-archive-enabled=E2= =80=99 is t; this can potentially set =E2=80=98tramp-archive-enabled=E2=80=99 to nil: #+begin_src emacs-lisp ;; In tramp-archive.el, ;; right after the (defvar tramp-archive-enabled=E2=80=A6 (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 =E2=80=9Ctramp-archive=E2=80=9D, =E2=80=98tramp-autoload-file= -name-handler=E2=80=99 continues with loading =E2=80=9Ctramp=E2=80=9D; this will call, via the =E2=80=98tramp--startup-hook=E2=80=99 =E2=80=98tramp-register-file-name-han= dlers=E2=80=99. This function will, depending on the value of =E2=80=98tramp-archive-enabled=E2= =80=99, also add the =E2=80=98tramp-archive-file-name-handler=E2=80=99 to the =E2=80=98file-name-handler-alist=E2=80=99. For the second time: If =E2=80=98tramp-archive-enabled=E2=80=99 is nil now, and you add =E2=80=98tramp-archive-autoload-file-name-handler=E2=80=99 again to =E2=80=98file-name-handler-alist=E2=80=99, you arrive at the above describe= d problem: =E2=80=98Fexpand_file_name=E2=80=99 called from =E2=80=98Ffile_directory_p= =E2=80=99 will signal an error because the autoload handler returns nil. If =E2=80=98tramp-archive-enabled=E2=80=99 is t after the first time, the r= egistration process is simply done a second time; no error occurs. Footnotes: [1] commit a5841b196f12894df4c1bb073f28ddadb6faa3cf Author: Michael Albinus 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 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. =20=20=20=20 * lisp/net/tramp-integration.el (tramp-rename-files) (tramp-rename-these-files): Declare them. =20=20=20=20 * lisp/net/tramp.el (tramp-autoload-file-name-handler): Load tramp-archive.el if needed. (Bug#48476) =20=20=20=20 * test/lisp/net/tramp-archive-tests.el (tramp-archive-test45-auto-load): Extend test. =20=20=20=20 Use #' syntax for function symbols. [3] --=20 Felix Dietrich --=-=-=--