The patch is updated based on your comments.
On Mon, Oct 13, 2014 at 12:28 AM, Michael Albinus <michael.albi...@gmx.de>
wrote:
> Ashi <ashi08...@gmail.com> writes:
>
> > Hi, all,
>
> Hi Zhongwei,
>
> > In this patch, I enabled the adb port access in tramp adb module. The
> > patch is attached, please help review, thanks a lot!
>
> Thanks for your contribution!
>
> I have had a look on this. No real test, just code reading.
>
> > +(defun tramp-adb-get-real-host (host)
> > + "Returns real host (with port or without port) from tramp host format
> > +e.g. input: 192.168.1.1#5555 return 192.168.1.1:5555
> > + input: R38273882DE return R38273882DE"
> > + (let ((l-port)
> > + (l-host)
> > + (real-host))
> > + (if (string-match tramp-host-with-port-regexp host)
> > + (setq l-port (match-string 2 host)
> > + l-host (match-string 1 host)
> > + real-host (concat l-host ":" l-port))
> > + (setq real-host host))))
> > +
>
> Well, this function ought to convert a host name in tramp syntax into a
> host name which shall be used by the adb command. First comment is, that
> in Tramp we usually take VEC as argument for such kind of functions. By
> this, you could apply all helper functions for parsing file name
> components.
>
> Second remark is, that "*-real-host" function names in Tramp are used to
> extract just the host name of a remote file name, without anything
> else like the port. So this function name would be confusing.
>
> Third (minor comment) is, that according to Emacs conventions the first
> line of doc strings shall consist of a sentence with a final period. So
> I would rewrite the function as (untested)
>
> (defun tramp-adb-get-host-for-execution (vec)
> "Returns host name and port from VEC to be used in shell exceution.
> E.g. a host name \"192.168.1.1#5555\" returns \"192.168.1.1:5555\"
> a host name \"R38273882DE\" returns \"R38273882DE\"."
> (if (tramp-file-name-port vec)
> (format
> "%s:%s" (tramp-file-name-real-host vec) (tramp-file-name-port vec))
> (tramp-file-name-host vec)))
>
Thanks, the code is much cleaner now!
>
> > (defun tramp-adb-execute-adb-command (vec &rest args)
> > "Returns nil on success error-output on failure."
> > - (when (> (length (tramp-file-name-host vec)) 0)
> > - (setq args (append (list "-s" (tramp-file-name-host vec)) args)))
> > + (let ((host (tramp-file-name-host vec)))
> > + (when (> (length host) 0)
> > + (setq args (append (list "-s" (tramp-adb-get-real-host host))
> args))))
> > (with-temp-buffer
>
> This shall be replaced by the call of (tramp-adb-get-host-for-execution
> vec)
> However, since this is the only place this function is called, you could
> use the code of the function directly.
>
> > + (let* ((host (tramp-file-name-host vec))
> > + (real-host (tramp-adb-get-real-host host)))
>
> [...]
>
> Well, I don't understand why this is needed. Since the devices variable
> is just a temporary one, which keeps known adb targets, it doesn't
> matter whether this is a list of ("192.168.1.1#5555" "R38273882DE")
> or ("192.168.1.1:5555" "R38273882DE")
>
> Maybe tramp-adb-parse-device-names must be adapted. Or do I miss something?
>
Yes, part of the problem is caused by tramp-adb-parse-device-names.
And another one is the host name in "args" which is needed by adb command.
This patch is only tested by hand. I tried to run "make check", but there
are many cases fails (26/35 fails) on current master. What can I do for
this?
>
> > Best regards,
> > Zhongwei
>
> Best regards, Michael.
>
From 38f90ec1fbdec7f45a6047d6aaf9a0c458be3167 Mon Sep 17 00:00:00 2001
From: Zhongwei Yao <ashi08...@gmail.com>
Date: Wed, 24 Sep 2014 17:38:39 +0800
Subject: [PATCH] enable adb access with port format, e.g.
/adb:164.2.168.1#5555:/
---
lisp/tramp-adb.el | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/lisp/tramp-adb.el b/lisp/tramp-adb.el
index 8c1cbbe..444f901 100644
--- a/lisp/tramp-adb.el
+++ b/lisp/tramp-adb.el
@@ -988,11 +988,20 @@ PRESERVE-UID-GID and PRESERVE-EXTENDED-ATTRIBUTES are completely ignored."
(tramp-set-connection-property v "process-buffer" nil))))))
;; Helper functions.
+(defun tramp-adb-get-host-for-execution (vec)
+ "Returns host name and port from VEC to be used in shell exceution.
+E.g. a host name \"192.168.1.1#5555\" returns \"192.168.1.1:5555\"
+ a host name \"R38273882DE\" returns \"R38273882DE\"."
+ (if (tramp-file-name-port vec)
+ (format
+ "%s:%s" (tramp-file-name-real-host vec) (tramp-file-name-port vec))
+ (tramp-file-name-host vec)))
(defun tramp-adb-execute-adb-command (vec &rest args)
"Returns nil on success error-output on failure."
- (when (> (length (tramp-file-name-host vec)) 0)
- (setq args (append (list "-s" (tramp-file-name-host vec)) args)))
+ (let ((host (tramp-file-name-host vec)))
+ (when (> (length host) 0)
+ (setq args (append (list "-s" (tramp-adb-get-host-for-execution vec)) args))))
(with-temp-buffer
(prog1
(unless
@@ -1093,6 +1102,7 @@ connection if a previous connection has died for some reason."
(let* ((buf (tramp-get-connection-buffer vec))
(p (get-buffer-process buf))
(host (tramp-file-name-host vec))
+ (exe-host (tramp-adb-get-host-for-execution vec))
(user (tramp-file-name-user vec))
devices)
@@ -1109,7 +1119,7 @@ connection if a previous connection has died for some reason."
(setq devices (mapcar 'cadr (tramp-adb-parse-device-names nil)))
(if (not devices)
(tramp-error vec 'file-error "No device connected"))
- (if (and (> (length host) 0) (not (member host devices)))
+ (if (and (> (length host) 0) (not (member exe-host devices)))
(tramp-error vec 'file-error "Device %s not connected" host))
(if (and (> (length devices) 1) (zerop (length host)))
(tramp-error
@@ -1119,7 +1129,7 @@ connection if a previous connection has died for some reason."
(let* ((coding-system-for-read 'utf-8-dos) ;is this correct?
(process-connection-type tramp-process-connection-type)
(args (if (> (length host) 0)
- (list "-s" host "shell")
+ (list "-s" exe-host "shell")
(list "shell")))
(p (let ((default-directory
(tramp-compat-temporary-file-directory)))
--
1.9.1
_______________________________________________
Tramp-devel mailing list
Tramp-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/tramp-devel
