target: Add Target.{push,pull}(globbing=False) parameter

When globbing=True, the source is interpreted as a globbing pattern on
the target and is expanded before pulling the files or folders.

This also aligns the behaviour of all targets:
    * adb connection was supported a limited form of globbing by default
      (only on the last component of the path)
    * SCP was supporting a limited form of globbing
    * GEM5 was not supporting globbing at all
    * paramiko was not supporting globbing at all

Also fix a race condition on push/pull as root, where pushing/pulling
the same file from multiple threads would have ended up using the same
temporary file.

devlib/host.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-from glob import iglob
+import glob
 import os
 import signal
 import shutil
@@ -64,17 +64,14 @@ class LocalConnection(ConnectionBase):
         self.unrooted = unrooted
         self.password = password
 
-    def push(self, source, dest, timeout=None, as_root=False):  # pylint: disable=unused-argument
+    def push(self, sources, dest, timeout=None, as_root=False):  # pylint: disable=unused-argument
-        self.logger.debug('cp {} {}'.format(source, dest))
+        self.logger.debug('copying {} to {}'.format(sources, dest))
+        for source in sources:
             shutil.copy(source, dest)
 
-    def pull(self, source, dest, timeout=None, as_root=False): # pylint: disable=unused-argument
+    def pull(self, sources, dest, timeout=None, as_root=False): # pylint: disable=unused-argument
-        self.logger.debug('cp {} {}'.format(source, dest))
+        for source in sources:
-        if ('*' in source or '?' in source) and os.path.isdir(dest):
+            self.logger.debug('copying {} to {}'.format(source, dest))
-            # Pull all files matching a wildcard expression
-            for each_source in iglob(source):
-                shutil.copy(each_source, dest)
-        else:
             if os.path.isdir(source):
                 # Use distutils to allow copying into an existing directory structure.
                 copy_tree(source, dest)

devlib/target.py

@@ -15,7 +15,9 @@
 
 import io
 import base64
+import functools
 import gzip
+import glob
 import os
 import re
 import time
@@ -26,6 +28,7 @@ import sys
 import tarfile
 import tempfile
 import threading
+import uuid
 import xml.dom.minidom
 import copy
 from collections import namedtuple, defaultdict
@@ -47,7 +50,7 @@ from devlib.platform import Platform
 from devlib.exception import (DevlibTransientError, TargetStableError,
                               TargetNotRespondingError, TimeoutError,
                               TargetTransientError, KernelConfigKeyError,
-                              TargetError) # pylint: disable=redefined-builtin
+                              TargetError, HostError) # pylint: disable=redefined-builtin
 from devlib.utils.ssh import SshConnection
 from devlib.utils.android import AdbConnection, AndroidProperties, LogcatMonitor, adb_command, adb_disconnect, INTENT_FLAGS
 from devlib.utils.misc import memoized, isiterable, convert_new_lines
@@ -364,25 +367,137 @@ class Target(object):
 
     # file transfer
 
-    def push(self, source, dest, as_root=False, timeout=None):  # pylint: disable=arguments-differ
+    @contextmanager
-        if not as_root:
+    def _xfer_cache_path(self, name):
-            self.conn.push(source, dest, timeout=timeout)
+        """
-        else:
+        Context manager to provide a unique path in the transfer cache with the
-            device_tempfile = self.path.join(self._file_transfer_cache, source.lstrip(self.path.sep))
+        basename of the given name.
-            self.execute("mkdir -p {}".format(quote(self.path.dirname(device_tempfile))))
+        """
-            self.conn.push(source, device_tempfile, timeout=timeout)
+        # Use a UUID to avoid race conditions on the target side
-            self.execute("cp {} {}".format(quote(device_tempfile), quote(dest)), as_root=True)
+        xfer_uuid = uuid.uuid4().hex
+        folder = self.path.join(self._file_transfer_cache, xfer_uuid)
+        # Make sure basename will work on folders too
+        name = os.path.normpath(name)
+        # Ensure the name is relative so that os.path.join() will actually
+        # join the paths rather than ignoring the first one.
+        name = './{}'.format(os.path.basename(name))
 
-    def pull(self, source, dest, as_root=False, timeout=None):  # pylint: disable=arguments-differ
+        check_rm = False
-        if not as_root:
+        try:
-            self.conn.pull(source, dest, timeout=timeout)
+            self.makedirs(folder)
+            # Don't check the exit code as the folder might not even exist
+            # before this point, if creating it failed
+            check_rm = True
+            yield self.path.join(folder, name)
+        finally:
+            self.execute('rm -rf -- {}'.format(quote(folder)), check_exit_code=check_rm)
+
+    def _prepare_xfer(self, action, sources, dest):
+        """
+        Check the sanity of sources and destination and prepare the ground for
+        transfering multiple sources.
+        """
+        if action == 'push':
+            src_excep = HostError
+            dst_excep = TargetStableError
+            dst_path_exists = self.file_exists
+            dst_is_dir = self.directory_exists
+            dst_mkdir = self.makedirs
+
+            for source in sources:
+                if not os.path.exists(source):
+                    raise HostError('No such file "{}"'.format(source))
         else:
-            device_tempfile = self.path.join(self._file_transfer_cache, source.lstrip(self.path.sep))
+            src_excep = TargetStableError
-            self.execute("mkdir -p {}".format(quote(self.path.dirname(device_tempfile))))
+            dst_excep = HostError
-            self.execute("cp -r {} {}".format(quote(source), quote(device_tempfile)), as_root=True)
+            dst_path_exists = os.path.exists
-            self.execute("chmod 0644 {}".format(quote(device_tempfile)), as_root=True)
+            dst_is_dir = os.path.isdir
-            self.conn.pull(device_tempfile, dest, timeout=timeout)
+            dst_mkdir = functools.partial(os.makedirs, exist_ok=True)
-            self.execute("rm -r {}".format(quote(device_tempfile)), as_root=True)
+
+        if not sources:
+            raise src_excep('No file matching: {}'.format(source))
+        elif len(sources) > 1:
+            if dst_path_exists(dest):
+                if not dst_is_dir(dest):
+                    raise dst_excep('A folder dest is required for multiple matches but destination is a file: {}'.format(dest))
+            else:
+                dst_makedirs(dest)
+
+
+    def push(self, source, dest, as_root=False, timeout=None, globbing=False):  # pylint: disable=arguments-differ
+        sources = glob.glob(source) if globbing else [source]
+        self._prepare_xfer('push', sources, dest)
+
+        def do_push(sources, dest):
+            return self.conn.push(sources, dest, timeout=timeout)
+
+        if as_root:
+            for source in sources:
+                with self._xfer_cache_path(source) as device_tempfile:
+                    do_push([source], device_tempfile)
+                    self.execute("mv -f -- {} {}".format(quote(device_tempfile), quote(dest)), as_root=True)
+        else:
+            do_push(sources, dest)
+
+    def _expand_glob(self, pattern, **kwargs):
+        """
+        Expand the given path globbing pattern on the target using the shell
+        globbing.
+        """
+        # Since we split the results based on new lines, forbid them in the
+        # pattern
+        if '\n' in pattern:
+            raise ValueError(r'Newline character \n are not allowed in globbing patterns')
+
+        # If the pattern is in fact a plain filename, skip the expansion on the
+        # target to avoid an unncessary command execution.
+        #
+        # fnmatch char list from: https://docs.python.org/3/library/fnmatch.html
+        special_chars = ['*', '?', '[', ']']
+        if not any(char in pattern for char in special_chars):
+            return [pattern]
+
+        # Characters to escape that are impacting parameter splitting, since we
+        # want the pattern to be given in one piece. Unfortunately, there is no
+        # fool-proof way of doing that without also escaping globbing special
+        # characters such as wildcard which would defeat the entire purpose of
+        # that function.
+        for c in [' ', "'", '"']:
+            pattern = pattern.replace(c, '\\' + c)
+
+        cmd = "exec printf '%s\n' {}".format(pattern)
+        # Make sure to use the same shell everywhere for the path globbing,
+        # ensuring consistent results no matter what is the default platform
+        # shell
+        cmd = '{} sh -c {} 2>/dev/null'.format(quote(self.busybox), quote(cmd))
+        # On some shells, match failure will make the command "return" a
+        # non-zero code, even though the command was not actually called
+        result = self.execute(cmd, strip_colors=False, check_exit_code=False, **kwargs)
+        paths = result.splitlines()
+        if not paths:
+            raise TargetStableError('No file matching: {}'.format(pattern))
+
+        return paths
+
+    def pull(self, source, dest, as_root=False, timeout=None, globbing=False):  # pylint: disable=arguments-differ
+        if globbing:
+            sources = self._expand_glob(source, as_root=as_root)
+        else:
+            sources = [source]
+
+        self._prepare_xfer('pull', sources, dest)
+
+        def do_pull(sources, dest):
+            self.conn.pull(sources, dest, timeout=timeout)
+
+        if as_root:
+            for source in sources:
+                with self._xfer_cache_path(source) as device_tempfile:
+                    self.execute("cp -r -- {} {}".format(quote(source), quote(device_tempfile)), as_root=True)
+                    self.execute("chmod 0644 -- {}".format(quote(device_tempfile)), as_root=True)
+                    do_pull([device_tempfile], dest)
+        else:
+            do_pull(sources, dest)
 
     def get_directory(self, source_dir, dest, as_root=False):
         """ Pull a directory from the device, after compressing dir """

devlib/utils/android.py

@@ -19,6 +19,7 @@ Utility functions for working with Android devices through adb.
 
 """
 # pylint: disable=E1103
+import glob
 import os
 import re
 import sys
@@ -288,28 +289,24 @@ class AdbConnection(ConnectionBase):
         self._setup_ls()
         self._setup_su()
 
-    def push(self, source, dest, timeout=None):
+    def _push_pull(self, action, sources, dest, timeout):
         if timeout is None:
             timeout = self.timeout
-        command = "push {} {}".format(quote(source), quote(dest))
-        if not os.path.exists(source):
-            raise HostError('No such file "{}"'.format(source))
-        return adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server)
 
-    def pull(self, source, dest, timeout=None):
+        paths = sources + [dest]
-        if timeout is None:
+
-            timeout = self.timeout
+        # Quote twice to avoid expansion by host shell, then ADB globbing
-        # Pull all files matching a wildcard expression
+        do_quote = lambda x: quote(glob.escape(x))
-        if os.path.isdir(dest) and \
+        paths = ' '.join(map(do_quote, paths))
-           ('*' in source or '?' in source):
+
-            command = 'shell {} {}'.format(self.ls_command, source)
+        command = "{} {}".format(action, paths)
-            output = adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server)
-            for line in output.splitlines():
-                command = "pull {} {}".format(quote(line.strip()), quote(dest))
         adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server)
-            return
+
-        command = "pull {} {}".format(quote(source), quote(dest))
+    def push(self, sources, dest, timeout=None):
-        return adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server)
+        return self._push_pull('push', sources, dest, timeout)
+
+    def pull(self, sources, dest, timeout=None):
+        return self._push_pull('pull', sources, dest, timeout)
 
     # pylint: disable=unused-argument
     def execute(self, command, timeout=None, check_exit_code=False,

devlib/utils/ssh.py

@@ -14,6 +14,7 @@
 #
 
 
+import glob
 import os
 import stat
 import logging
@@ -299,15 +300,21 @@ class SshConnectionBase(ConnectionBase):
         self.options = {}
         logger.debug('Logging in {}@{}'.format(username, host))
 
-    def push(self, source, dest, timeout=30):
+    def push(self, sources, dest, timeout=30):
-        dest = '{}@{}:{}'.format(self.username, self.host, dest)
+        # Quote the destination as SCP would apply globbing too
-        return self._scp(source, dest, timeout)
+        dest = '{}@{}:{}'.format(self.username, self.host, quote(dest))
+        paths = sources + [dest]
+        return self._scp(paths, timeout)
 
-    def pull(self, source, dest, timeout=30):
+    def pull(self, sources, dest, timeout=30):
-        source = '{}@{}:{}'.format(self.username, self.host, source)
+        # First level of escaping for the remote shell
-        return self._scp(source, dest, timeout)
+        sources = ' '.join(map(quote, sources))
+        # All the sources are merged into one scp parameter
+        sources = '{}@{}:{}'.format(self.username, self.host, sources)
+        paths = [sources, dest]
+        self._scp(paths, timeout)
 
-    def _scp(self, source, dest, timeout=30):
+    def _scp(self, paths, timeout=30):
         # NOTE: the version of scp in Ubuntu 12.04 occasionally (and bizarrely)
         # fails to connect to a device if port is explicitly specified using -P
         # option, even if it is the default port, 22. To minimize this problem,
@@ -316,12 +323,12 @@ class SshConnectionBase(ConnectionBase):
         keyfile_string = '-i {}'.format(quote(self.keyfile)) if self.keyfile else ''
         options = " ".join(["-o {}={}".format(key, val)
                             for key, val in self.options.items()])
-        command = '{} {} -r {} {} {} {}'.format(scp,
+        paths = ' '.join(map(quote, paths))
+        command = '{} {} -r {} {} {}'.format(scp,
                                                 options,
                                                 keyfile_string,
                                                 port_string,
-                                                quote(source),
+                                                paths)
-                                                quote(dest))
         command_redacted = command
         logger.debug(command)
         if self.password:
@@ -534,20 +541,22 @@ class SshConnection(SshConnectionBase):
             # Maybe that was a directory, so retry as such
             cls._pull_folder(sftp, src, dst)
 
-    def push(self, source, dest, timeout=30):
+    def push(self, sources, dest, timeout=30):
         # If using scp, use implementation from base class
         if self.use_scp:
-            super().push(source, dest, timeout)
+            super().push(sources, dest, timeout)
         else:
             with _handle_paramiko_exceptions(), self._get_sftp(timeout) as sftp:
+                for source in sources:
                     self._push_path(sftp, source, dest)
 
-    def pull(self, source, dest, timeout=30):
+    def pull(self, sources, dest, timeout=30):
         # If using scp, use implementation from base class
         if self.use_scp:
-            super().pull(source, dest, timeout)
+            super().pull(sources, dest, timeout)
         else:
             with _handle_paramiko_exceptions(), self._get_sftp(timeout) as sftp:
+                for source in sources:
                     self._pull_path(sftp, source, dest)
 
     def execute(self, command, timeout=None, check_exit_code=True,
@@ -1012,7 +1021,7 @@ class Gem5Connection(TelnetConnection):
                     .format(self.gem5_input_dir, indir))
         self.gem5_input_dir = indir
 
-    def push(self, source, dest, timeout=None):
+    def push(self, sources, dest, timeout=None):
         """
         Push a file to the gem5 device using VirtIO
 
@@ -1024,6 +1033,7 @@ class Gem5Connection(TelnetConnection):
         # First check if the connection is set up to interact with gem5
         self._check_ready()
 
+        for source in sources:
             filename = os.path.basename(source)
             logger.debug("Pushing {} to device.".format(source))
             logger.debug("gem5interactdir: {}".format(self.gem5_interact_dir))
@@ -1045,7 +1055,7 @@ class Gem5Connection(TelnetConnection):
             self._gem5_shell("ls -al {}".format(quote(self.gem5_input_dir)))
             logger.debug("Push complete.")
 
-    def pull(self, source, dest, timeout=0): #pylint: disable=unused-argument
+    def pull(self, sources, dest, timeout=0): #pylint: disable=unused-argument
         """
         Pull a file from the gem5 device using m5 writefile
 
@@ -1057,6 +1067,7 @@ class Gem5Connection(TelnetConnection):
         # First check if the connection is set up to interact with gem5
         self._check_ready()
 
+        for source in sources:
             result = self._gem5_shell("ls {}".format(source))
             files = strip_bash_colors(result).split()
 

doc/connection.rst

@@ -21,25 +21,25 @@ they do not derive from a common base. Instead, a :class:`Connection` is any
 class that implements the following methods.
 
 
-.. method:: push(self, source, dest, timeout=None)
+.. method:: push(self, sources, dest, timeout=None)
 
-   Transfer a file from the host machine to the connected device.
+   Transfer a list of files from the host machine to the connected device.
 
-   :param source: path of to the file on the host
+   :param sources: list of paths on the host
-   :param dest: path of to the file on the connected device.
+   :param dest: path to the file or folder on the connected device.
-   :param timeout: timeout (in seconds) for the transfer; if the transfer does
+   :param timeout: timeout (in seconds) for the transfer of each file; if the
-       not  complete within this period, an exception will be raised.
+       transfer does not complete within this period, an exception will be
+       raised.
 
-.. method:: pull(self, source, dest, timeout=None)
+.. method:: pull(self, sources, dest, timeout=None)
 
-   Transfer a file, or files matching a glob pattern, from the connected device
+   Transfer a list of files from the connected device to the host machine.
-   to the host machine.
 
-   :param source: path of to the file on the connected device. If ``dest`` is a
+   :param sources: list of paths on the connected device.
-       directory, may be a glob pattern.
+   :param dest: path to the file or folder on the host
-   :param dest: path of to the file on the host
+   :param timeout: timeout (in seconds) for the transfer for each file; if the
-   :param timeout: timeout (in seconds) for the transfer; if the transfer does
+       transfer does not complete within this period, an exception will be
-       not  complete within this period, an exception will be raised.
+       raised.
 
 .. method:: execute(self, command, timeout=None, check_exit_code=False, as_root=False, strip_colors=True, will_succeed=False)
 

doc/target.rst

@@ -218,7 +218,7 @@ Target
         operations during reboot process to detect if the reboot has failed and
         the device has hung.
 
-.. method:: Target.push(source, dest [,as_root , timeout])
+.. method:: Target.push(source, dest [,as_root , timeout, globbing])
 
    Transfer a file from the host machine to the target device.
 
@@ -227,8 +227,12 @@ Target
    :param as_root: whether root is required. Defaults to false.
    :param timeout: timeout (in seconds) for the transfer; if the transfer does
        not  complete within this period, an exception will be raised.
+   :param globbing: If ``True``, the ``source`` is interpreted as a globbing
+        pattern instead of being take as-is. If the pattern has mulitple
+        matches, ``dest`` must be a folder (or will be created as such if it
+        does not exists yet).
 
-.. method:: Target.pull(source, dest [, as_root, timeout])
+.. method:: Target.pull(source, dest [, as_root, timeout, globbing])
 
    Transfer a file from the target device to the host machine.
 
@@ -237,6 +241,10 @@ Target
    :param as_root: whether root is required. Defaults to false.
    :param timeout: timeout (in seconds) for the transfer; if the transfer does
        not  complete within this period, an exception will be raised.
+   :param globbing: If ``True``, the ``source`` is interpreted as a globbing
+        pattern instead of being take as-is. If the pattern has mulitple
+        matches, ``dest`` must be a folder (or will be created as such if it
+        does not exists yet).
 
 .. method:: Target.execute(command [, timeout [, check_exit_code [, as_root [, strip_colors [, will_succeed [, force_locale]]]]]])