GNU bug report logs - #54001
29.0.50; abbreviate-file-name has side-effects

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 54001 in the body.
You can then email your comments to 54001 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; abbreviate-file-name has side-effects
Date: Mon, 14 Feb 2022 12:19:57 -0500

Package: Emacs
Version: 29.0.50


Since:

    commit bf505a63f98ed61934a8fb81ec65c96859606b6e
    Author: Jim Porter <jporterbugs <at> gmail.com>
    Date:   Mon Nov 15 13:33:07 2021 +0100

    Support abbreviating home directory of Tramp filenames

`abbreviate-file-name` has significantly changed in its behavior:
- it's slower (because it goes through file-name-handlers)
- it can have very visible side effects like prompting the user for a password.

I haven't measured the slowdown, so I'll assume it's acceptable, but
asking for a password (or contacting a remote host) is not.

I suggest we take a step back and think of how to get that feature
without having to contact any remote host during `abbreviate-file-name`.
Maybe we can do that by making Tramp opportunistically add entries to
`directory-abbrev-alist` when it performs expansion?


        Stefan

Message #8 received at 54001 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, 54001 <at> debbugs.gnu.org
Cc: Daniel Mendler <mail <at> daniel-mendler.de>
Subject: Re: bug#54001: 29.0.50; abbreviate-file-name has side-effects
Date: Mon, 14 Feb 2022 09:52:32 -0800

On 2/14/2022 9:19 AM, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> Package: Emacs
> Version: 29.0.50
> 
> 
> Since:
> 
>      commit bf505a63f98ed61934a8fb81ec65c96859606b6e
>      Author: Jim Porter <jporterbugs <at> gmail.com>
>      Date:   Mon Nov 15 13:33:07 2021 +0100
> 
>      Support abbreviating home directory of Tramp filenames
> 
> `abbreviate-file-name` has significantly changed in its behavior:
> - it's slower (because it goes through file-name-handlers)
> - it can have very visible side effects like prompting the user for a password.
> 
> I haven't measured the slowdown, so I'll assume it's acceptable, but
> asking for a password (or contacting a remote host) is not.

Sorry about that. I did what I could to minimize the slowdown (including 
some more general optimizations to make Tramp faster). There are some 
benchmarks in the original bug here (these are with 1000 iterations; 
you'll want to compare the first section with the last): 
<https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-11/msg01293.html>.

> I suggest we take a step back and think of how to get that feature
> without having to contact any remote host during `abbreviate-file-name`.
> Maybe we can do that by making Tramp opportunistically add entries to
> `directory-abbrev-alist` when it performs expansion?

I think Michael Albinus suggested doing that in the original bug, 
although I was concerned about modifying defcustoms invisibly like that. 
Is that ok to do? Another option might be to store the abbreviations for 
a given file-name-handler somewhere internally and consult that when 
calling that file-name-handler's implementation of `abbreviate-file-name'.

Maybe this patch should be backed out for now; it shouldn't be 
interrupting the user. (I thought I'd tested that, but maybe it was on 
an earlier revision of the patch.) I'll probably have time to look into 
a new solution in a few weeks, but anyone else who's interested should 
feel free to fix it in the meantime.

Message #11 received at 54001 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Daniel Mendler <mail <at> daniel-mendler.de>, 54001 <at> debbugs.gnu.org
Subject: Re: bug#54001: 29.0.50; abbreviate-file-name has side-effects
Date: Mon, 14 Feb 2022 14:48:41 -0500

>> I suggest we take a step back and think of how to get that feature
>> without having to contact any remote host during `abbreviate-file-name`.
>> Maybe we can do that by making Tramp opportunistically add entries to
>> `directory-abbrev-alist` when it performs expansion?
>
> I think Michael Albinus suggested doing that in the original bug, although
> I was concerned about modifying defcustoms invisibly like that. Is that ok
> to do?

No it's not, but we can add a non-defcustom'd variable holding
additional entries to circumvent this problem.

> Another option might be to store the abbreviations for a given
> file-name-handler somewhere internally and consult that when calling that
> file-name-handler's implementation of `abbreviate-file-name'.

That would work as well.


        Stefan

Message #14 received at 54001 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Daniel Mendler <mail <at> daniel-mendler.de>, 54001 <at> debbugs.gnu.org
Subject: Re: bug#54001: 29.0.50; abbreviate-file-name has side-effects
Date: Mon, 14 Feb 2022 12:52:22 -0800

On 2/14/2022 11:48 AM, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
>>> I suggest we take a step back and think of how to get that feature
>>> without having to contact any remote host during `abbreviate-file-name`.
>>> Maybe we can do that by making Tramp opportunistically add entries to
>>> `directory-abbrev-alist` when it performs expansion?
[snip]
>> Another option might be to store the abbreviations for a given
>> file-name-handler somewhere internally and consult that when calling that
>> file-name-handler's implementation of `abbreviate-file-name'.
> 
> That would work as well.

Looking at my original patch again, this value is already cached as a 
Tramp connection property named "home-directory". If the code were 
changed to set that connection property in a more-appropriate place, and 
`tramp-handle-abbreviate-file-name' simply *reads* that connection 
property, then I think that should resolve this issue.

Message #17 received at 54001 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Daniel Mendler <mail <at> daniel-mendler.de>, 54001 <at> debbugs.gnu.org
Subject: Re: bug#54001: 29.0.50; abbreviate-file-name has side-effects
Date: Mon, 14 Feb 2022 16:00:34 -0500

> Looking at my original patch again, this value is already cached as a Tramp
> connection property named "home-directory". If the code were changed to set
> that connection property in a more-appropriate place, and
> `tramp-handle-abbreviate-file-name' simply *reads* that connection property,
> then I think that should resolve this issue.

Sounds like it,


        Stefan

Message #20 received at 54001 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Daniel Mendler <mail <at> daniel-mendler.de>, 54001 <at> debbugs.gnu.org
Subject: Re: bug#54001: 29.0.50; abbreviate-file-name has side-effects
Date: Mon, 14 Feb 2022 13:38:58 -0800

[Message part 1 (text/plain, inline)]

On 2/14/2022 1:00 PM, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
>> Looking at my original patch again, this value is already cached as a Tramp
>> connection property named "home-directory". If the code were changed to set
>> that connection property in a more-appropriate place, and
>> `tramp-handle-abbreviate-file-name' simply *reads* that connection property,
>> then I think that should resolve this issue.
> 
> Sounds like it,

I've only lightly tested this patch, but I think it should resolve the 
issue. It only sets the "home-directory" connection property when a 
connection is already established. Otherwise, it just uses the cached 
value (if any).

Maybe there's a cleaner way to do this though; opportunistically setting 
a connection property like this seems like something there might be a 
special function for in Tramp, but I didn't see one after a bit of 
looking so this is what I went with.

[0001-Don-t-attempt-to-connect-to-a-remote-server-during-a.patch (text/plain, attachment)]

Message #23 received at 54001 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: Jim Porter <jporterbugs <at> gmail.com>, Daniel Mendler <mail <at> daniel-mendler.de>,
 54001 <at> debbugs.gnu.org
Subject: Re: bug#54001: 29.0.50; abbreviate-file-name has side-effects
Date: Mon, 14 Feb 2022 16:51:14 -0500

Jim Porter [2022-02-14 13:38:58] wrote:
> I've only lightly tested this patch, but I think it should resolve the
> issue. It only sets the "home-directory" connection property when
> a connection is already established. Otherwise, it just uses the cached
> value (if any).
>
> Maybe there's a cleaner way to do this though; opportunistically setting
> a connection property like this seems like something there might be
> a special function for in Tramp, but I didn't see one after a bit of looking
> so this is what I went with.

Thanks Jim.

Michael, I assume you'll want to take care of this patch.  Let me know
if you prefer that I install it, of course,


        Stefan

Message #26 received at 54001 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Jim Porter <jporterbugs <at> gmail.com>, Daniel Mendler <mail <at> daniel-mendler.de>,
 54001 <at> debbugs.gnu.org
Subject: Re: bug#54001: 29.0.50; abbreviate-file-name has side-effects
Date: Tue, 15 Feb 2022 09:52:43 +0100

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

Hi Jim, Stefan,

>> I've only lightly tested this patch, but I think it should resolve the
>> issue. It only sets the "home-directory" connection property when
>> a connection is already established. Otherwise, it just uses the cached
>> value (if any).
>
> Thanks Jim.
>
> Michael, I assume you'll want to take care of this patch.  Let me know
> if you prefer that I install it, of course,

Yes, it is OK. Please install.

>> Maybe there's a cleaner way to do this though; opportunistically setting
>> a connection property like this seems like something there might be
>> a special function for in Tramp, but I didn't see one after a bit of looking
>> so this is what I went with.

I've recently made some changes for home directories in the other Tramp
backends. Likely, I need to revise these settings once the patch has
arrived on master.

>         Stefan

Best regards, Michael.

Message #31 received at 54001-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Daniel Mendler <mail <at> daniel-mendler.de>, 54001-done <at> debbugs.gnu.org
Subject: Re: bug#54001: 29.0.50; abbreviate-file-name has side-effects
Date: Tue, 15 Feb 2022 08:11:35 -0500

> I've only lightly tested this patch, but I think it should resolve the
> issue. It only sets the "home-directory" connection property when
> a connection is already established. Otherwise, it just uses the cached
> value (if any).

Thanks, pushed to master.  And closing.
Daniel, feel to ping us back if the fix isn't good enough.


        Stefan

Message #34 received at 54001-done <at> debbugs.gnu.org (full text, mbox):

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Jim Porter <jporterbugs <at> gmail.com>
Cc: 54001-done <at> debbugs.gnu.org
Subject: Re: bug#54001: 29.0.50; abbreviate-file-name has side-effects
Date: Tue, 15 Feb 2022 14:26:21 +0100

On 2/15/22 14:11, Stefan Monnier wrote:
>> I've only lightly tested this patch, but I think it should resolve the
>> issue. It only sets the "home-directory" connection property when
>> a connection is already established. Otherwise, it just uses the cached
>> value (if any).
> 
> Thanks, pushed to master.  And closing.
> Daniel, feel to ping us back if the fix isn't good enough.

Thanks! I will if the issue comes up again.

Daniel

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.