bug#70959: Tramp connection property can interact weirdly with cache

bug#70959: Tramp connection property can interact weirdly with cache

[Dmitry Gutov]

I've used the recipe for "direct-async-processes" as suggested:

(add-to-list 'tramp-connection-properties
              (list (regexp-quote "/ssh:user@host:")
                    "direct-async-process" t))

and at first (the connection was already open) it didn't take effect. I 
think I had to kill the connection buffer for the new property to be 
used. Would be nice if this was instant.

But also, after that I tried altering tramp-connection-properties back 
to disable direct-async-process for that host (for the purposes of 
testing and measuring things more precisely), and found that changing 
the variable doesn't bring the new behavior back. Restarting Emacs 
didn't bring the old (non-direct) behavior back either - I had to edit 
the ~/.emacs.d/tramp file by hand to remove that property.

I think that's something that might confuse other users.

bug#70959: Tramp connection property can interact weirdly with cache

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> I've used the recipe for "direct-async-processes" as suggested:
>
> (add-to-list 'tramp-connection-properties
>              (list (regexp-quote "/ssh:user@host:")
>                    "direct-async-process" t))
>
> and at first (the connection was already open) it didn't take
> effect. I think I had to kill the connection buffer for the new
> property to be used. Would be nice if this was instant.
>
> But also, after that I tried altering tramp-connection-properties back
> to disable direct-async-process for that host (for the purposes of
> testing and measuring things more precisely), and found that changing
> the variable doesn't bring the new behavior back. Restarting Emacs
> didn't bring the old (non-direct) behavior back either - I had to edit
> the ~/.emacs.d/tramp file by hand to remove that property.
>
> I think that's something that might confuse other users.

If you want to remove a property from the cache, you need to call one of
the tramp-cleanup* commands.  See (info "(tramp) Cleanup remote connections")

bug#70959: Tramp connection property can interact weirdly with cache

[Dmitry Gutov]

On 16/05/2024 18:45, Michael Albinus wrote:
> Dmitry Gutov<dmitry@gutov.dev>  writes:
> 
>> I've used the recipe for "direct-async-processes" as suggested:
>>
>> (add-to-list 'tramp-connection-properties
>>               (list (regexp-quote "/ssh:user@host:")
>>                     "direct-async-process" t))
>>
>> and at first (the connection was already open) it didn't take
>> effect. I think I had to kill the connection buffer for the new
>> property to be used. Would be nice if this was instant.
>>
>> But also, after that I tried altering tramp-connection-properties back
>> to disable direct-async-process for that host (for the purposes of
>> testing and measuring things more precisely), and found that changing
>> the variable doesn't bring the new behavior back. Restarting Emacs
>> didn't bring the old (non-direct) behavior back either - I had to edit
>> the ~/.emacs.d/tramp file by hand to remove that property.
>>
>> I think that's something that might confuse other users.
> If you want to remove a property from the cache, you need to call one of
> the tramp-cleanup* commands.  See (info "(tramp) Cleanup remote connections")

Thank you. I suppose this is well-documented. But as a user, it's not 
apparent that the change in the variable wouldn't take effect right away.

Would it be difficult to have this connection property (perhaps all such 
properties?) to take effect right away? Does caching it improve 
performance in any realistic scenario?

bug#70959: Tramp connection property can interact weirdly with cache

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

Hi Dmitry,

> Would it be difficult to have this connection property (perhaps all
> such properties?) to take effect right away? Does caching it improve
> performance in any realistic scenario?

Caching is just a side effect. The idea for this connection property is
to have it connection-wise.

Best regards, Michael.

bug#70959: Tramp connection property can interact weirdly with cache

[Dmitry Gutov]

On 16/05/2024 21:39, Michael Albinus wrote:
> Hi Dmitry,
> 
>> Would it be difficult to have this connection property (perhaps all
>> such properties?) to take effect right away? Does caching it improve
>> performance in any realistic scenario?
> Caching is just a side effect. The idea for this connection property is
> to have it connection-wise.

IMHO if it were possible (and easy enough to implement) to have the 
property connection-wide without caching it, it would make for a better 
user experience.

bug#70959: Tramp connection property can interact weirdly with cache

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

Hi Dmitry,

>>> Would it be difficult to have this connection property (perhaps all
>>> such properties?) to take effect right away? Does caching it improve
>>> performance in any realistic scenario?
>> Caching is just a side effect. The idea for this connection property is
>> to have it connection-wise.
>
> IMHO if it were possible (and easy enough to implement) to have the
> property connection-wide without caching it, it would make for a
> better user experience.

What about to make it a connection-local variable?

Best regards, Michael.

bug#70959: Tramp connection property can interact weirdly with cache

[Dmitry Gutov]

On 17/05/2024 17:40, Michael Albinus wrote:
> Hi Dmitry,
> 
>>>> Would it be difficult to have this connection property (perhaps all
>>>> such properties?) to take effect right away? Does caching it improve
>>>> performance in any realistic scenario?
>>> Caching is just a side effect. The idea for this connection property is
>>> to have it connection-wise.
>> IMHO if it were possible (and easy enough to implement) to have the
>> property connection-wide without caching it, it would make for a
>> better user experience.
> What about to make it a connection-local variable?

Maybe? How will that look in customization?

Will we set some variable with 'setopt', or setq, or just have to do 
this through Customize?

bug#70959: Tramp connection property can interact weirdly with cache

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

Hi Dmitry,

>>> IMHO if it were possible (and easy enough to implement) to have the
>>> property connection-wide without caching it, it would make for a
>>> better user experience.
>> What about to make it a connection-local variable?
>
> Maybe? How will that look in customization?
>
> Will we set some variable with 'setopt', or setq, or just have to do
> this through Customize?

You would do something like

--8<---------------cut here---------------start------------->8---
(connection-local-set-profile-variables 'remote-direct-async
   '((tramp-direct-async-process . t)))

(connection-local-set-profiles
   '(:application tramp :machine "randomhost") 'remote-direct-async)
--8<---------------cut here---------------end--------------->8---

`remote-direct-async' is a profile name you could choose
yourself. `tramp-direct-async-process' would be the respective Tramp
variable.

Best regards, Michael.

bug#70959: Tramp connection property can interact weirdly with cache

[Dmitry Gutov]

Hi Michael,

On 18/05/2024 13:57, Michael Albinus wrote:

>>>> IMHO if it were possible (and easy enough to implement) to have the
>>>> property connection-wide without caching it, it would make for a
>>>> better user experience.
>>> What about to make it a connection-local variable?
>> Maybe? How will that look in customization?
>>
>> Will we set some variable with 'setopt', or setq, or just have to do
>> this through Customize?
> You would do something like
> 
> --8<---------------cut here---------------start------------->8---
> (connection-local-set-profile-variables 'remote-direct-async
>     '((tramp-direct-async-process . t)))
> 
> (connection-local-set-profiles
>     '(:application tramp :machine "randomhost") 'remote-direct-async)
> --8<---------------cut here---------------end--------------->8---
> 
> `remote-direct-async' is a profile name you could choose
> yourself. `tramp-direct-async-process' would be the respective Tramp
> variable.

That looks very reasonable. One bonus is that the machine is specified 
by the hostname only.

Which would avoid the difficulty of tweaking the regexp (e.g. I had a 
problem - on a different machine - that my url included a port, and that 
needed to be reflected in the regexp as well).

bug#70959: Tramp connection property can interact weirdly with cache

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

>> You would do something like
>> --8<---------------cut here---------------start------------->8---
>> (connection-local-set-profile-variables 'remote-direct-async
>>     '((tramp-direct-async-process . t)))
>> (connection-local-set-profiles
>>     '(:application tramp :machine "randomhost") 'remote-direct-async)
>> --8<---------------cut here---------------end--------------->8---
>> `remote-direct-async' is a profile name you could choose
>> yourself. `tramp-direct-async-process' would be the respective Tramp
>> variable.
>
> That looks very reasonable. One bonus is that the machine is specified
> by the hostname only.
>
> Which would avoid the difficulty of tweaking the regexp (e.g. I had a
> problem - on a different machine - that my url included a port, and
> that needed to be reflected in the regexp as well).

Try the following patch and the config as shown above.


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

diff --git a/lisp/tramp.el b/lisp/tramp.el
index f024ebec..814d33e6 100644
--- a/lisp/tramp.el
+++ b/lisp/tramp.el
@@ -4847,6 +4847,8 @@ a connection-local variable."
   (when (process-command proc)
     (tramp-message vec 6 "%s" (string-join (process-command proc) " "))))

+(defvar tramp-direct-async-process nil)
+
 (defun tramp-direct-async-process-p (&rest args)
   "Whether direct async `make-process' can be called."
   (let ((v (tramp-dissect-file-name default-directory))
@@ -4855,7 +4857,7 @@ a connection-local variable."
     (and ;; The method supports it.
          (tramp-get-method-parameter v 'tramp-direct-async)
 	 ;; It has been indicated.
-         (tramp-get-connection-property v "direct-async-process")
+	 (connection-local-value tramp-direct-async-process)
 	 ;; There's no multi-hop.
 	 (or (not (tramp-multi-hop-p v))
 	     (null (cdr (tramp-compute-multi-hops v))))

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


Best regards, Michael.

bug#70959: Tramp connection property can interact weirdly with cache

[Dmitry Gutov]

On 18/05/2024 21:43, Michael Albinus wrote:
>>> You would do something like
>>> --8<---------------cut here---------------start------------->8---
>>> (connection-local-set-profile-variables 'remote-direct-async
>>>      '((tramp-direct-async-process . t)))
>>> (connection-local-set-profiles
>>>      '(:application tramp :machine "randomhost") 'remote-direct-async)
>>> --8<---------------cut here---------------end--------------->8---
>>> `remote-direct-async' is a profile name you could choose
>>> yourself. `tramp-direct-async-process' would be the respective Tramp
>>> variable.
>> That looks very reasonable. One bonus is that the machine is specified
>> by the hostname only.
>>
>> Which would avoid the difficulty of tweaking the regexp (e.g. I had a
>> problem - on a different machine - that my url included a port, and
>> that needed to be reflected in the regexp as well).
> Try the following patch and the config as shown above.

Seems to be working well, thank you.

Except in my case tramp.el was inside directory lisp/net.

And I guess the code will need some backward compatibility path since 
the connection property was there since Emacs 28? ;-(

bug#70959: Tramp connection property can interact weirdly with cache

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Version: 30.1

Dmitry Gutov <dmitry@gutov.dev> writes:

Hi Dmitry,

>> Try the following patch and the config as shown above.
>
> Seems to be working well, thank you.

Thanks for the feedback, I've pushed an extended version to master.

> Except in my case tramp.el was inside directory lisp/net.

Well, my major development happens in the Tramp git repo. It has a
different subdirectory structure, compared to the Emacs repo.

> And I guess the code will need some backward compatibility path since
> the connection property was there since Emacs 28? ;-(

Sure. I have marked the connection property "direct-async-process" as
deprecated, the user will see a warning.

Closing the bug.

Best regards, Michael.

bug#70959: Tramp connection property can interact weirdly with cache

[Dmitry Gutov]

On 19/05/2024 15:18, Michael Albinus wrote:
> Version: 30.1
> 
> Dmitry Gutov <dmitry@gutov.dev> writes:
> 
> Hi Dmitry,
> 
>>> Try the following patch and the config as shown above.
>>
>> Seems to be working well, thank you.
> 
> Thanks for the feedback, I've pushed an extended version to master.

Thank you, this seems like a solid improvement.

>> Except in my case tramp.el was inside directory lisp/net.
> 
> Well, my major development happens in the Tramp git repo. It has a
> different subdirectory structure, compared to the Emacs repo.

All right.

>> And I guess the code will need some backward compatibility path since
>> the connection property was there since Emacs 28? ;-(
> 
> Sure. I have marked the connection property "direct-async-process" as
> deprecated, the user will see a warning.

Nice.

bug#70959: Tramp connection property can interact weirdly with cache

[Dmitry Gutov]

Hi Michael,

On 18/05/2024 21:43, Michael Albinus wrote:
> Try the following patch and the config as shown above.
> 
> 
> diff --git a/lisp/tramp.el b/lisp/tramp.el
> index f024ebec..814d33e6 100644
> --- a/lisp/tramp.el
> +++ b/lisp/tramp.el
> @@ -4847,6 +4847,8 @@ a connection-local variable."
>     (when (process-command proc)
>       (tramp-message vec 6 "%s" (string-join (process-command proc) " "))))
> 
> +(defvar tramp-direct-async-process nil)
> +
>   (defun tramp-direct-async-process-p (&rest args)
>     "Whether direct async `make-process' can be called."
>     (let ((v (tramp-dissect-file-name default-directory))
> @@ -4855,7 +4857,7 @@ a connection-local variable."
>       (and ;; The method supports it.
>            (tramp-get-method-parameter v 'tramp-direct-async)
>   	 ;; It has been indicated.
> -         (tramp-get-connection-property v "direct-async-process")
> +	 (connection-local-value tramp-direct-async-process)

Do you think we could also make it so

  (setq tramp-direct-async-process t)

in the user config also works? Affecting all connections, naturally.

Not sure if we'll want to advertise it as a proper way to apply this 
config, but I'd probably use this shortcut myself, and especially during 
testing.

E.g. the change below (or equivalent):

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 18116229337..1864ef6541d 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -4878,9 +4878,7 @@ tramp-direct-async-process-p
           (tramp-get-method-parameter v 'tramp-direct-async)
  	 ;; It has been indicated.  We don't use the global value of
  	 ;; `tramp-direct-async-process'.
-	 (or (and (tramp-compat-connection-local-p tramp-direct-async-process)
-		  (tramp-compat-connection-local-value
-		   tramp-direct-async-process))
+	 (or (with-connection-local-variables tramp-direct-async-process)
  	     ;; Deprecated setting.
               (tramp-get-connection-property v "direct-async-process"))
  	 ;; There's no multi-hop.

bug#70959: Tramp connection property can interact weirdly with cache

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

> Do you think we could also make it so
>
>  (setq tramp-direct-async-process t)

I don't want. It is too easy. People will set it, and months later they
report that direct async processes won't work, and they've forgotten
this setting, and there's a new host which needs an interactive password.

You can achieve the same effect, if you set the connection-local profile
w/o any :protocol, :user or :machine, just the :application. Something like

--8<---------------cut here---------------start------------->8---
     (connection-local-set-profiles
      '(:application tramp) 'remote-direct-async-process)
--8<---------------cut here---------------end--------------->8---

And be prepared that I'll blame you in case you report an error because
of this :-)

Best regards, Michael.

bug#70959: Tramp connection property can interact weirdly with cache

[Dmitry Gutov]

On 25/05/2024 17:58, Michael Albinus wrote:
> I don't want. It is too easy. People will set it, and months later they
> report that direct async processes won't work, and they've forgotten
> this setting, and there's a new host which needs an interactive password.
> 
> You can achieve the same effect, if you set the connection-local profile
> w/o any :protocol, :user or :machine, just the :application. Something like
> 
> --8<---------------cut here---------------start------------->8---
>       (connection-local-set-profiles
>        '(:application tramp) 'remote-direct-async-process)
> --8<---------------cut here---------------end--------------->8---
> 
> And be prepared that I'll blame you in case you report an error because
> of this 🙂

All right. Fair enough. :-)