master 8aaf13e: Rearrange argument handling in Tramp scp calls.

[Michael Albinus]

branch: master
commit 8aaf13eae6a0edcaf6528ba0490ed351ef00e2b3
Author: Michael Albinus <michael.albinus@gmx.de>
Commit: Michael Albinus <michael.albinus@gmx.de>

    Rearrange argument handling in Tramp scp calls.
    
    * lisp/net/tramp-sh.el (tramp-do-copy-or-rename-file-out-of-band):
    Use `start-process' instead of `start-process-shell-command'.
    (tramp-make-copy-program-file-name): Do not quote `localname'.
    
    * test/lisp/net/tramp-tests.el (tramp-method-out-of-band-p): Declare.
    (tramp--test-windows-nt-and-batch-p)
    (tramp--test-windows-nt-and-pscp-psftp-p): Remove, and also all callees.
    (tramp--test-windows-nt-and-out-of-band-p)
    (tramp--test-windows-nt-and-scp-p): New defuns.
    (tramp-test17-dired-with-wildcards)
    (tramp-test40-special-characters)
    (tramp-test40-special-characters-with-stat)
    (tramp-test40-special-characters-with-perl)
    (tramp-test40-special-characters-with-ls, tramp-test41-utf8)
    (tramp-test41-utf8-with-stat, tramp-test41-utf8-with-perl)
    (tramp-test41-utf8-with-ls): Use them.
---
 lisp/net/tramp-sh.el         | 68 +++++++++++++++++++++++---------------------
 test/lisp/net/tramp-tests.el | 40 +++++++++++++-------------
 2 files changed, 55 insertions(+), 53 deletions(-)

diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 0e6a2bb..651444b 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -2218,7 +2218,7 @@ The method used must be an out-of-band method."
         (t2 (tramp-tramp-file-p newname))
         (orig-vec (tramp-dissect-file-name (if t1 filename newname)))
         copy-program copy-args copy-env copy-keep-date listener spec
-        options source target remote-copy-program remote-copy-args)
+        options source target remote-copy-program remote-copy-args p)
 
     (with-parsed-tramp-file-name (if t1 filename newname) nil
       (if (and t1 t2)
@@ -2253,10 +2253,10 @@ The method used must be an out-of-band method."
                        #'identity)
                      (if t1
                          (tramp-make-copy-program-file-name v)
-                       (tramp-unquote-shell-quote-argument filename)))
+                       (tramp-compat-file-name-unquote filename)))
              target (if t2
                         (tramp-make-copy-program-file-name v)
-                      (tramp-unquote-shell-quote-argument newname)))
+                      (tramp-compat-file-name-unquote newname)))
 
        ;; Check for user.  There might be an interactive setting.
        (setq user (or (tramp-file-name-user v)
@@ -2288,6 +2288,13 @@ The method used must be an out-of-band method."
              ;; keep-date argument is non-nil), or a replacement for
              ;; the whole keep-date sublist.
              (delete " " (apply #'tramp-expand-args v 'tramp-copy-args spec))
+             ;; `tramp-ssh-controlmaster-options' is a string instead
+             ;; of a list.  Unflatten it.
+             copy-args
+             (tramp-compat-flatten-tree
+              (mapcar
+               (lambda (x) (if (string-match-p " " x) (split-string x) x))
+               copy-args))
              copy-env (apply #'tramp-expand-args v 'tramp-copy-env spec)
              remote-copy-program
              (tramp-get-method-parameter v 'tramp-remote-copy-program)
@@ -2349,31 +2356,26 @@ The method used must be an out-of-band method."
                  copy-args
                  (if remote-copy-program
                      (list (if t1 (concat ">" target) (concat "<" source)))
-                   (list source target))))
-
-               ;; Use an asynchronous process.  By this, password can
-               ;; be handled.  We don't set a timeout, because the
-               ;; copying of large files can last longer than 60 secs.
-               (let* ((command
-                       (mapconcat
-                        #'identity (append (list copy-program) copy-args)
-                        " "))
-                      (p (let ((default-directory
-                                 (tramp-compat-temporary-file-directory)))
-                           (start-process-shell-command
-                            (tramp-get-connection-name v)
-                            (tramp-get-connection-buffer v)
-                            command))))
-                 (tramp-message orig-vec 6 "%s" command)
-                 (process-put p 'vector orig-vec)
-                 (process-put p 'adjust-window-size-function #'ignore)
-                 (set-process-query-on-exit-flag p nil)
-
-                 ;; We must adapt `tramp-local-end-of-line' for
-                 ;; sending the password.
-                 (let ((tramp-local-end-of-line tramp-rsh-end-of-line))
-                   (tramp-process-actions
-                    p v nil tramp-actions-copy-out-of-band))))
+                   (list source target)))
+                ;; Use an asynchronous process.  By this, password
+                ;; can be handled.  We don't set a timeout, because
+                ;; the copying of large files can last longer than 60
+                ;; secs.
+                p (apply
+                   #'start-process
+                   (tramp-get-connection-name v)
+                   (tramp-get-connection-buffer v)
+                   copy-program copy-args))
+               (tramp-message orig-vec 6 "%s" (string-join (process-command p) 
" "))
+               (process-put p 'vector orig-vec)
+               (process-put p 'adjust-window-size-function #'ignore)
+               (set-process-query-on-exit-flag p nil)
+
+               ;; We must adapt `tramp-local-end-of-line' for
+               ;; sending the password.
+               (let ((tramp-local-end-of-line tramp-rsh-end-of-line))
+                 (tramp-process-actions
+                  p v nil tramp-actions-copy-out-of-band)))
 
            ;; Reset the transfer process properties.
            (tramp-flush-connection-property v "process-name")
@@ -5221,15 +5223,17 @@ Return ATTR."
         (directory-file-name (tramp-file-name-unquote-localname vec))))
     (when (string-match-p tramp-ipv6-regexp host)
       (setq host (format "[%s]" host)))
+    ;; This does not work yet for MS Windows scp, if there are
+    ;; characters to be quoted.  Win32 OpenSSH 7.9 is said to support
+    ;; this, see
+    ;; 
<https://github.com/PowerShell/Win32-OpenSSH/releases/tag/v7.9.0.0p1-Beta>
     (unless (string-match-p "ftp$" method)
       (setq localname (tramp-shell-quote-argument localname)))
     (cond
      ((tramp-get-method-parameter vec 'tramp-remote-copy-program)
       localname)
-     ((not (zerop (length user)))
-      (format
-       "%s@%s:%s" user host (tramp-unquote-shell-quote-argument localname)))
-     (t (format "%s:%s" host (tramp-unquote-shell-quote-argument 
localname))))))
+     ((zerop (length user)) (format "%s:%s" host localname))
+     (t (format "%s@%s:%s" user host localname)))))
 
 (defun tramp-method-out-of-band-p (vec size)
   "Return t if this is an out-of-band method, nil otherwise."
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index be428fc..1eb0d0e 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -59,6 +59,7 @@
 (declare-function tramp-get-remote-perl "tramp-sh")
 (declare-function tramp-get-remote-stat "tramp-sh")
 (declare-function tramp-list-tramp-buffers "tramp-cmds")
+(declare-function tramp-method-out-of-band-p "tramp-sh")
 (declare-function tramp-smb-get-localname "tramp-smb")
 (defvar ange-ftp-make-backup-files)
 (defvar auto-save-file-name-transforms)
@@ -3097,6 +3098,7 @@ This tests also `file-directory-p' and 
`file-accessible-directory-p'."
   (skip-unless (tramp--test-enabled))
   (skip-unless (tramp--test-sh-p))
   (skip-unless (not (tramp--test-rsync-p)))
+  (skip-unless (not (tramp--test-windows-nt-and-scp-p)))
   ;; Wildcards are not supported in tramp-crypt.el.
   (skip-unless (not (tramp--test-crypt-p)))
   ;; Since Emacs 26.1.
@@ -4369,7 +4371,7 @@ This tests also `make-symbolic-link', `file-truename' and 
`add-name-to-file'."
          (delete-file tmp-name))))))
 
 (defun tramp--test-shell-file-name ()
-  "Return default remote shell.."
+  "Return default remote shell."
   (if (tramp--test-adb-p) "/system/bin/sh" "/bin/sh"))
 
 (ert-deftest tramp-test28-process-file ()
@@ -5838,18 +5840,18 @@ This requires restrictions of file name syntax."
   "Check, whether the locale host runs MS Windows."
   (eq system-type 'windows-nt))
 
-(defun tramp--test-windows-nt-and-batch-p ()
-  "Check, whether the locale host runs MS Windows in batch mode.
-This does not support special characters."
-  (and (eq system-type 'windows-nt) noninteractive))
+(defun tramp--test-windows-nt-and-out-of-band-p ()
+  "Check, whether the locale host runs MS Windows and an out-of-band method.
+This does not support utf8 based file transfer."
+  (and (eq system-type 'windows-nt)
+       (tramp-method-out-of-band-p tramp-test-vec 1)))
 
-(defun tramp--test-windows-nt-and-pscp-psftp-p ()
-  "Check, whether the locale host runs MS Windows, and ps{cp,ftp} is used.
+(defun tramp--test-windows-nt-and-scp-p ()
+  "Check, whether the locale host runs MS Windows, and scpx? is used.
 This does not support utf8 based file transfer."
   (and (eq system-type 'windows-nt)
        (string-match-p
-       (regexp-opt '("pscp" "psftp"))
-       (file-remote-p tramp-test-temporary-file-directory 'method))))
+       "^scpx?" (file-remote-p tramp-test-temporary-file-directory 'method))))
 
 (defun tramp--test-windows-nt-or-smb-p ()
   "Check, whether the locale or remote host runs MS Windows.
@@ -6112,7 +6114,7 @@ This requires restrictions of file name syntax."
   "Check special characters in file names."
   (skip-unless (tramp--test-enabled))
   (skip-unless (not (tramp--test-rsync-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-pscp-psftp-p)))
+  (skip-unless (not (tramp--test-windows-nt-and-scp-p)))
   (skip-unless (or (tramp--test-emacs26-p) (not (tramp--test-rclone-p))))
 
   (tramp--test-special-characters))
@@ -6124,7 +6126,7 @@ Use the `stat' command."
   (skip-unless (tramp--test-enabled))
   (skip-unless (tramp--test-sh-p))
   (skip-unless (not (tramp--test-rsync-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-pscp-psftp-p)))
+  (skip-unless (not (tramp--test-windows-nt-and-scp-p)))
   ;; We cannot use `tramp-test-vec', because this fails during compilation.
   (with-parsed-tramp-file-name tramp-test-temporary-file-directory nil
     (skip-unless (tramp-get-remote-stat v)))
@@ -6143,7 +6145,7 @@ Use the `perl' command."
   (skip-unless (tramp--test-enabled))
   (skip-unless (tramp--test-sh-p))
   (skip-unless (not (tramp--test-rsync-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-pscp-psftp-p)))
+  (skip-unless (not (tramp--test-windows-nt-and-scp-p)))
   ;; We cannot use `tramp-test-vec', because this fails during compilation.
   (with-parsed-tramp-file-name tramp-test-temporary-file-directory nil
     (skip-unless (tramp-get-remote-perl v)))
@@ -6165,7 +6167,7 @@ Use the `ls' command."
   (skip-unless (tramp--test-enabled))
   (skip-unless (tramp--test-sh-p))
   (skip-unless (not (tramp--test-rsync-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-pscp-psftp-p)))
+  (skip-unless (not (tramp--test-windows-nt-and-scp-p)))
 
   (let ((tramp-connection-properties
         (append
@@ -6230,8 +6232,7 @@ Use the `ls' command."
   (skip-unless (tramp--test-enabled))
   (skip-unless (not (tramp--test-docker-p)))
   (skip-unless (not (tramp--test-rsync-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-batch-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-pscp-psftp-p)))
+  (skip-unless (not (tramp--test-windows-nt-and-out-of-band-p)))
   (skip-unless (not (tramp--test-ksh-p)))
   (skip-unless (not (tramp--test-gdrive-p)))
   (skip-unless (not (tramp--test-crypt-p)))
@@ -6247,8 +6248,7 @@ Use the `stat' command."
   (skip-unless (tramp--test-sh-p))
   (skip-unless (not (tramp--test-docker-p)))
   (skip-unless (not (tramp--test-rsync-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-batch-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-pscp-psftp-p)))
+  (skip-unless (not (tramp--test-windows-nt-and-out-of-band-p)))
   (skip-unless (not (tramp--test-ksh-p)))
   (skip-unless (not (tramp--test-crypt-p)))
   ;; We cannot use `tramp-test-vec', because this fails during compilation.
@@ -6270,8 +6270,7 @@ Use the `perl' command."
   (skip-unless (tramp--test-sh-p))
   (skip-unless (not (tramp--test-docker-p)))
   (skip-unless (not (tramp--test-rsync-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-batch-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-pscp-psftp-p)))
+  (skip-unless (not (tramp--test-windows-nt-and-out-of-band-p)))
   (skip-unless (not (tramp--test-ksh-p)))
   (skip-unless (not (tramp--test-crypt-p)))
   ;; We cannot use `tramp-test-vec', because this fails during compilation.
@@ -6296,8 +6295,7 @@ Use the `ls' command."
   (skip-unless (tramp--test-sh-p))
   (skip-unless (not (tramp--test-docker-p)))
   (skip-unless (not (tramp--test-rsync-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-batch-p)))
-  (skip-unless (not (tramp--test-windows-nt-and-pscp-psftp-p)))
+  (skip-unless (not (tramp--test-windows-nt-and-out-of-band-p)))
   (skip-unless (not (tramp--test-ksh-p)))
   (skip-unless (not (tramp--test-crypt-p)))