Re: Connection-local login programs

--=-=-=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

Eshel Yaron <[email protected]> writes:

Hi Eshel,

>> That's not an abuse. Overwriting tramp-login-program is a common (and
>> documented, I believe) use case. Another use case is to overwrite
>> tramp-connection-timeout for sudo-like methods. And in general it can b=
e
>> used to tweak tramp-login-args. People do so.
>
> The people who do so are users, right? =20
> My point was that here we'd be relying on tramp-connection-properties as
> part of the method definition, not as something that users configure.
> Namely, when users will run kubed-tramp-enable to enable the kubedv2
> method, it'll modify tramp-connection-properties at the same time as it
> adds the method to tramp-methods.  (Note that this means that the
> tramp-login-program in tramp-methods is never actually used.)
> Are there any packages/methods that modify tramp-connection-properties?
> I searched a bit and couldn't find any.

Right. There's no backend (AFAIK), which uses
tramp-connection-properties for this purpose.

> What I meant was allowing tramp-login-program in tramp-methods to be a
> function, instead of doing that in tramp-connection-properties. =20
> That way we wouldn't have to touch tramp-connection-properties in the
> method definition, just tramp-methods.

Thanks for the heads-up, this is the right direction. We have already
format characters in tramp-methods, which are expanded. I have appended
a patch which does it also for tramp-login-program.

The second patch is for kubed-tramp.el. It simply adds a new format
character (I've decided for ?1, but it doesn't matter), which calls
kubed-tramp-get-login-program (derived from
kubed-tramp-get-method-parameter-advice). This should be all.

Completely untested, pls tell me how it goes.

> Thanks,
>
> Eshel

Best regards, Michael.


--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment
Content-Transfer-Encoding: quoted-printable

diff --git a/lisp/tramp-container.el b/lisp/tramp-container.el
index 91d9b239..fec2e16a 100644
=2D-- a/lisp/tramp-container.el
+++ b/lisp/tramp-container.el
@@ -266,7 +266,7 @@ BODY is the backend specific code."
 		    tramp--last-hop-directory)
 	       tramp-compat-temporary-file-directory))
 	  (program (let ((tramp-verbose 0))
-		     (tramp-get-method-parameter
+		     (tramp-expand-args
 		      (make-tramp-file-name :method ,method)
 		      'tramp-login-program)))
 	  (vec (when (tramp-tramp-file-p default-directory)
@@ -656,10 +656,9 @@ see its function help for a description of the format=
."
    '((tramp-config-check . tramp-kubernetes--current-context-data)
      ;; This variable will be eval'ed in `tramp-expand-args'.
      (tramp-extra-expand-args
-      . (?a (tramp-kubernetes--container (car tramp-current-connection))
-	 ?h (tramp-kubernetes--pod (car tramp-current-connection))
-	 ?x (tramp-kubernetes--context-namespace
-	     (car tramp-current-connection)))))
+      ?a (tramp-kubernetes--container (car tramp-current-connection))
+      ?h (tramp-kubernetes--pod (car tramp-current-connection))
+      ?x (tramp-kubernetes--context-namespace (car tramp-current-connecti=
on))))
    "Default connection-local variables for remote kubernetes connections.=
")
=20
  (connection-local-set-profile-variables
diff --git a/lisp/tramp-sh.el b/lisp/tramp-sh.el
index 08a44c81..9aec9e38 100644
=2D-- a/lisp/tramp-sh.el
+++ b/lisp/tramp-sh.el
@@ -5037,7 +5037,7 @@ Goes through the list `tramp-inline-compress-command=
s'."
    ;; Use plink options.
    ((string-match-p
      (rx "plink" (? ".exe") eol)
-     (tramp-get-method-parameter vec 'tramp-login-program))
+     (tramp-expand-args vec 'tramp-login-program))
     (concat
      (if (eq tramp-use-connection-share 'suppress)
 	 "-noshare" "-share")
@@ -5398,7 +5398,7 @@ connection if a previous connection has died for som=
e reason."
 			   hop 'tramp-connection-timeout
 			   tramp-connection-timeout))
 			 (command
-			  (tramp-get-method-parameter
+			  (tramp-expand-args
 			   hop 'tramp-login-program))
 			 ;; We don't create the temporary file.  In
 			 ;; fact, it is just a prefix for the
diff --git a/lisp/tramp-sshfs.el b/lisp/tramp-sshfs.el
index 2cb5b5b1..f4073158 100644
=2D-- a/lisp/tramp-sshfs.el
+++ b/lisp/tramp-sshfs.el
@@ -269,7 +269,7 @@ arguments to pass to the OPERATION."
       (setq ret
 	    (apply
 	     #'tramp-call-process
-	     v (tramp-get-method-parameter v 'tramp-login-program)
+	     v (tramp-expand-args v 'tramp-login-program)
 	     nil outbuf display
 	     (tramp-expand-args
 	      v 'tramp-login-args nil
diff --git a/lisp/tramp.el b/lisp/tramp.el
index d67d77fa..8b393e7a 100644
=2D-- a/lisp/tramp.el
+++ b/lisp/tramp.el
@@ -5325,7 +5325,7 @@ Do not set it manually, it is used buffer-local in `=
tramp-get-lock-pid'.")
 (defvar tramp-extra-expand-args nil
   "Method specific arguments.")
=20
-(defun tramp-expand-args (vec parameter default &rest spec-list)
+(defun tramp-expand-args (vec parameter &optional default &rest spec-list=
)
   "Expand login arguments as given by PARAMETER in `tramp-methods'.
 PARAMETER is a symbol like `tramp-login-args', denoting a list of
 list of strings from `tramp-methods', containing %-sequences for
@@ -5348,12 +5348,14 @@ a connection-local variable."
       (setq spec-list (cddr spec-list)))
     (setq spec (apply #'format-spec-make extra-spec-list))
     ;; Expand format spec.
-    (flatten-tree
-     (mapcar
-      (lambda (x)
-	(setq x (mapcar (lambda (y) (tramp-format-spec y spec)) x))
-	(unless (member "" x) x))
-      args))))
+    (if (atom args)
+	(tramp-format-spec args spec)
+      (flatten-tree
+       (mapcar
+	(lambda (x)
+	  (setq x (mapcar (lambda (y) (tramp-format-spec y spec)) x))
+	  (unless (member "" x) x))
+	args)))))
=20
 (defun tramp-post-process-creation (proc vec)
   "Apply actions after creation of process PROC."
@@ -5476,7 +5478,7 @@ processes."
                  `(,(string-join command " ")))
 	      command))
 	   (login-program
-	    (tramp-get-method-parameter v 'tramp-login-program))
+	    (tramp-expand-args v 'tramp-login-program))
 	   ;; We don't create the temporary file.  In fact, it is just
 	   ;; a prefix for the ControlPath option of ssh; the real
 	   ;; temporary file has another name, and it is created and

--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment
Content-Transfer-Encoding: quoted-printable

diff --git a/kubed-tramp.el b/kubed-tramp.el
index 6cfeb6bca6..894a4ac7aa 100644
=2D-- a/kubed-tramp.el
+++ b/kubed-tramp.el
@@ -121,19 +121,15 @@
 	  ?h (or (tramp-file-name-host vec) "")))))
     tramp-default-proxies-alist)))
=20
-(defun kubed-tramp-get-method-parameter-advice
-    (of vec param &rest rest)
+(defun kubed-tramp-get-login-program (vec)
   "Respect connection-local value of `kubed-kubectl-program'."
-  (if (and (eq param 'tramp-login-program)
-           (equal (tramp-file-name-method vec) kubed-tramp-method))
-      ;; When Tramp asks how to invoke kubectl on our behalf,
-      ;; point it to the up-to-date (and possibly connection-local)
-      ;; value of `kubed-kubectl-program'.
-      (if-let ((hop-dir (kubed-tramp--previous-hop vec)))
-          (let ((default-directory hop-dir))
-            (connection-local-value kubed-kubectl-program 'kubed))
-        kubed-kubectl-program)
-    (apply of vec param rest)))
+  ;; When Tramp asks how to invoke kubectl on our behalf,
+  ;; point it to the up-to-date (and possibly connection-local)
+  ;; value of `kubed-kubectl-program'.
+  (if-let ((hop-dir (kubed-tramp--previous-hop vec)))
+      (let ((default-directory hop-dir))
+        (connection-local-value kubed-kubectl-program 'kubed))
+    kubed-kubectl-program))
=20
 ;;;###autoload
 (defun kubed-tramp-enable ()
@@ -148,7 +144,7 @@
                     ;; that gives the up-to-date (and possibly
                     ;; connection-local, for remote hosts) value of
                     ;; `kubed-kubectl-program' when Tramp asks.
-                    (tramp-login-program ,kubed-kubectl-program)
+                    (tramp-login-program "%1")
                     (tramp-login-args (("exec")
                                        ("--context" "%x")
                                        ("--namespace" "%y")
@@ -168,6 +164,7 @@
       (connection-local-set-profile-variables
        'kubed-tramp-connection-local-default-profile
        '((tramp-extra-expand-args
+          ?1 (kubed-tramp-get-login-program (car tramp-current-connection=
))
           ?a (kubed-tramp--container  (car tramp-current-connection))
           ?h (kubed-tramp--pod        (car tramp-current-connection))
           ?x (kubed-tramp--context    (car tramp-current-connection))
@@ -183,6 +180,7 @@
       (connection-local-set-profile-variables
        'kubed-tramp-v2-connection-local-default-profile
        '((tramp-extra-expand-args
+          ?1 (kubed-tramp-get-login-program (car tramp-current-connection=
))
           ?a (kubed-tramp--container  (car tramp-current-connection))
           ?h (kubed-tramp--pod        (car tramp-current-connection))
           ?x (kubed-tramp--v2-context (car tramp-current-connection))
@@ -190,11 +188,7 @@
=20
       (connection-local-set-profiles
        `(:application tramp :protocol ,kubed-tramp-method)
-       'kubed-tramp-v2-connection-local-default-profile))
-
-    (advice-add 'tramp-get-method-parameter :around
-                #'kubed-tramp-get-method-parameter-advice
-                '((name . kubed)))))
+       'kubed-tramp-v2-connection-local-default-profile))))
=20
 ;;;###autoload (with-eval-after-load 'tramp (kubed-tramp-enable))
=20

--=-=-=--