Reland of 'New run shell implementation for DeviceUtils'

build/android/pylib/device/device_utils.py

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Provides a variety of device interactions based on adb.
 
 Eventually, this will be based on adb_wrapper.
 """
 # pylint: disable=W0613
 
 import logging
 import multiprocessing
 import os
-import pipes
+import re
 import sys
 import tempfile
 import time
 import zipfile
 
 import pylib.android_commands
+from pylib import cmd_helper
 from pylib.device import adb_wrapper
 from pylib.device import decorators
 from pylib.device import device_errors
 from pylib.device.commands import install_commands
 from pylib.utils import apk_helper
 from pylib.utils import host_utils
 from pylib.utils import parallelizer
 
 _DEFAULT_TIMEOUT = 30
 _DEFAULT_RETRIES = 3
@@ skipping 16 matching lines @@
   """Restarts the adb server.
 
   Raises:
     CommandFailedError if we fail to kill or restart the server.
   """
   pylib.android_commands.AndroidCommands().RestartAdbServer()
 
 
 class DeviceUtils(object):
 
+  _VALID_SHELL_VARIABLE = re.compile('^[a-zA-Z_][a-zA-Z0-9_]*$')
+
   def __init__(self, device, default_timeout=_DEFAULT_TIMEOUT,
                default_retries=_DEFAULT_RETRIES):
     """DeviceUtils constructor.
 
     Args:
       device: Either a device serial, an existing AdbWrapper instance, an
               an existing AndroidCommands instance, or nothing.
       default_timeout: An integer containing the default number of seconds to
                        wait for an operation to complete if no explicit value
                        is provided.
@@ skipping 13 matching lines @@
       self.adb = adb_wrapper.AdbWrapper(device.GetDevice())
       self.old_interface = device
     elif not device:
       self.adb = adb_wrapper.AdbWrapper('')
       self.old_interface = pylib.android_commands.AndroidCommands()
     else:
       raise ValueError('Unsupported type passed for argument "device"')
     self._commands_installed = None
     self._default_timeout = default_timeout
     self._default_retries = default_retries
+    self._cache = {}
     assert(hasattr(self, decorators.DEFAULT_TIMEOUT_ATTR))
     assert(hasattr(self, decorators.DEFAULT_RETRIES_ATTR))
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def IsOnline(self, timeout=None, retries=None):
     """Checks whether the device is online.
 
     Args:
       timeout: timeout in seconds
       retries: number of retries
@@ skipping 20 matching lines @@
     Returns:
       True if adbd has root privileges, False otherwise.
 
     Raises:
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
     return self._HasRootImpl()
 
   def _HasRootImpl(self):
-    return self.old_interface.IsRootEnabled()
+    try:
+      self._RunShellCommandImpl('ls /root', check_return=True)
+      return True
+    except device_errors.AdbShellCommandFailedError:
+      return False
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def EnableRoot(self, timeout=None, retries=None):
     """Restarts adbd with root privileges.
 
     Args:
       timeout: timeout in seconds
       retries: number of retries
 
     Raises:
@@ skipping 34 matching lines @@
       The device's path to its SD card.
 
     Raises:
       CommandFailedError if the external storage path could not be determined.
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
     return self._GetExternalStoragePathImpl()
 
   def _GetExternalStoragePathImpl(self):
-    try:
-      return self.old_interface.GetExternalStorage()
-    except AssertionError as e:
-      raise device_errors.CommandFailedError(
-          str(e), device=str(self)), None, sys.exc_info()[2]
+    if 'external_storage' in self._cache:
+      return self._cache['external_storage']
+
+    value = self._RunShellCommandImpl('echo $EXTERNAL_STORAGE',
+                                      single_line=True,
+                                      check_return=True)
+    if not value:
+      raise device_errors.CommandFailedError('$EXTERNAL_STORAGE is not set',
+                                             str(self))
+    self._cache['external_storage'] = value
+    return value
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def WaitUntilFullyBooted(self, wifi=False, timeout=None, retries=None):
     """Wait for the device to fully boot.
 
     This means waiting for the device to boot, the package manager to be
     available, and the SD card to be ready. It can optionally mean waiting
     for wifi to come up, too.
 
     Args:
       wifi: A boolean indicating if we should wait for wifi to come up or not.
       timeout: timeout in seconds
       retries: number of retries
 
     Raises:
       CommandFailedError on failure.
       CommandTimeoutError if one of the component waits times out.
       DeviceUnreachableError if the device becomes unresponsive.
     """
     self._WaitUntilFullyBootedImpl(wifi=wifi, timeout=timeout)
 
   def _WaitUntilFullyBootedImpl(self, wifi=False, timeout=None):
     if timeout is None:
       timeout = self._default_timeout
     self.old_interface.WaitForSystemBootCompleted(timeout)
     self.old_interface.WaitForDevicePm()
     self.old_interface.WaitForSdCardReady(timeout)
     if wifi:
       while not 'Wi-Fi is enabled' in (
-          self._RunShellCommandImpl('dumpsys wifi')):
+          self.old_interface.RunShellCommand('dumpsys wifi')):
         time.sleep(1)
 
   REBOOT_DEFAULT_TIMEOUT = 10 * _DEFAULT_TIMEOUT
   REBOOT_DEFAULT_RETRIES = _DEFAULT_RETRIES
 
   @decorators.WithTimeoutAndRetriesDefaults(
       REBOOT_DEFAULT_TIMEOUT,
       REBOOT_DEFAULT_RETRIES)
   def Reboot(self, block=True, timeout=None, retries=None):
     """Reboot the device.
@@ skipping 55 matching lines @@
         out = self.old_interface.Install(apk_path, reinstall=reinstall)
         for line in out.splitlines():
           if 'Failure' in line:
             raise device_errors.CommandFailedError(
                 line.strip(), device=str(self))
       except AssertionError as e:
         raise device_errors.CommandFailedError(
             str(e), device=str(self)), None, sys.exc_info()[2]
 
   @decorators.WithTimeoutAndRetriesFromInstance()
-  def RunShellCommand(self, cmd, check_return=False, as_root=False, cwd=None,
-                      env=None, timeout=None, retries=None):
+  def RunShellCommand(self, cmd, check_return=False, cwd=None, env=None,
+                      as_root=False, single_line=False,
+                      timeout=None, retries=None):
     """Run an ADB shell command.
 
-    TODO(jbudorick) Switch the default value of check_return to True after
-    AndroidCommands is gone.
+    The command to run |cmd| should be a sequence of program arguments or else
+    a single string.
+
+    When |cmd| is a sequence, it is assumed to contain the name of the command
+    to run followed by its arguments. In this case, arguments are passed to the
+    command exactly as given, without any further processing by the shell. This
+    allows to easily pass arguments containing spaces or special characters
+    without having to worry about getting quoting right. Whenever possible, it
+    is recomended to pass |cmd| as a sequence.
+
+    When |cmd| is given as a string, it will be interpreted and run by the
+    shell on the device.
+
+    This behaviour is consistent with that of command runners in cmd_helper as
+    well as Python's own subprocess.Popen.
+
+    TODO(perezju) Change the default of |check_return| to True when callers
+      have switched to the new behaviour.
 
     Args:
-      cmd: A list containing the command to run on the device and any arguments.
+      cmd: A string with the full command to run on the device, or a sequence
+        containing the command and its arguments.
       check_return: A boolean indicating whether or not the return code should
-                    be checked.
-      as_root: A boolean indicating whether the shell command should be run
-               with root privileges.
+        be checked.
       cwd: The device directory in which the command should be run.
       env: The environment variables with which the command should be run.
+      as_root: A boolean indicating whether the shell command should be run
+        with root privileges.
+      single_line: A boolean indicating if a single line of output is expected,
+        and the caller wants to retrieve the value of that line. The default
+        behaviour is to return a list of output lines.
       timeout: timeout in seconds
       retries: number of retries
 
     Returns:
-      The output of the command.
+      The output of the command either as list of lines or, when single_line is
+        True, the value contained in the single expected line of output.
 
     Raises:
-      CommandFailedError if check_return is True and the return code is nozero.
+      AdbShellCommandFailedError if check_return is True and the exit code of
+        the command run on the device is non-zero.
+      CommandFailedError if single_line is True but the output consists of
+        either zero or more than one lines.
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
-    return self._RunShellCommandImpl(
-        cmd, check_return=check_return, as_root=as_root, cwd=cwd, env=env,
-        timeout=timeout)
+    return self._RunShellCommandImpl(cmd, check_return=check_return, cwd=cwd,
+        env=env, as_root=as_root, single_line=single_line)
 
-  def _RunShellCommandImpl(self, cmd, check_return=False, as_root=False,
-                           cwd=None, env=None, timeout=None):
-    # TODO(jbudorick): Remove the timeout parameter once this is no longer
-    # backed by AndroidCommands.
-    if isinstance(cmd, list):
-      cmd = ' '.join(cmd)
+  def _RunShellCommandImpl(self, cmd, check_return=False, cwd=None, env=None,
+                           as_root=False, single_line=False):
+    def env_quote(key, value):
+      if not DeviceUtils._VALID_SHELL_VARIABLE.match(key):
+        raise KeyError('Invalid shell variable name %r' % key)
+      # using double quotes here to allow interpolation of shell variables
+      return '%s=%s' % (key, cmd_helper.DoubleQuote(value))
+
+    if not isinstance(cmd, basestring):
+      cmd = ' '.join(cmd_helper.SingleQuote(s) for s in cmd)
     if as_root and not self._HasRootImpl():
       cmd = 'su -c %s' % cmd
     if env:
-      cmd = '%s %s' % (
-          ' '.join('%s=%s' % (k, v) for k, v in env.iteritems()), cmd)
+      env = ' '.join(env_quote(k, v) for k, v in env.iteritems())
+      cmd = '%s %s' % (env, cmd)
     if cwd:
-      cmd = 'cd %s && %s' % (cwd, cmd)
-    if check_return:
-      code, output = self.old_interface.GetShellCommandStatusAndOutput(
-          cmd, timeout_time=timeout)
-      if int(code) != 0:
-        raise device_errors.AdbCommandFailedError(
-            cmd.split(), 'Nonzero exit code (%d)' % code, device=str(self))
+      cmd = 'cd %s && %s' % (cmd_helper.SingleQuote(cwd), cmd)
+
+    try:
+      # TODO(perezju) still need to make sure that we call a version of
+      # adb.Shell without a timeout-and-retries wrapper.
+      output = self.adb.Shell(cmd, expect_rc=0)
+    except device_errors.AdbShellCommandFailedError as e:
+      if check_return:
+        raise
+      else:
+        output = e.output
+
+    output = output.splitlines()
+    if single_line:
+      if len(output) != 1:
+        msg = 'exactly one line of output expected, but got: %s'
+        raise device_errors.CommandFailedError(msg % output)
+      return output[0]
     else:
-      output = self.old_interface.RunShellCommand(cmd, timeout_time=timeout)
-    return output
+      return output
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def KillAll(self, process_name, signum=9, as_root=False, blocking=False,
               timeout=None, retries=None):
     """Kill all processes with the given name on the device.
 
     Args:
       process_name: A string containing the name of the process to kill.
       signum: An integer containing the signal number to send to kill. Defaults
               to 9 (SIGKILL).
       as_root: A boolean indicating whether the kill should be executed with
                root privileges.
       blocking: A boolean indicating whether we should wait until all processes
                 with the given |process_name| are dead.
       timeout: timeout in seconds
       retries: number of retries
 
     Raises:
       CommandFailedError if no process was killed.
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
     pids = self._GetPidsImpl(process_name)
     if not pids:
       raise device_errors.CommandFailedError(
           'No process "%s"' % process_name, device=str(self))
 
-    cmd = 'kill -%d %s' % (signum, ' '.join(pids.values()))
-    self._RunShellCommandImpl(cmd, as_root=as_root)
+    cmd = ['kill', '-%d' % signum] + pids.values()
+    self._RunShellCommandImpl(cmd, as_root=as_root, check_return=True)
 
     if blocking:
       wait_period = 0.1
       while self._GetPidsImpl(process_name):
         time.sleep(wait_period)
 
     return len(pids)
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def StartActivity(self, intent, blocking=False, trace_file_name=None,
@@ skipping 130 matching lines @@
 
     Raises:
       CommandFailedError on failure.
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
 
     files = []
     for h, d in host_device_tuples:
       if os.path.isdir(h):
-        self._RunShellCommandImpl(['mkdir', '-p', '"%s"' % d],
-                                  check_return=True)
+        self._RunShellCommandImpl(['mkdir', '-p', d], check_return=True)
       files += self._GetChangedFilesImpl(h, d)
 
     if not files:
       return
 
     size = sum(host_utils.GetRecursiveDiskUsage(h) for h, _ in files)
     file_count = len(files)
     dir_size = sum(host_utils.GetRecursiveDiskUsage(h)
                    for h, _ in host_device_tuples)
     dir_file_count = 0
@@ skipping 13 matching lines @@
 
     if dir_push_duration < push_duration and (
         dir_push_duration < zip_duration or not self._commands_installed):
       self._PushChangedFilesIndividually(host_device_tuples)
     elif push_duration < zip_duration or not self._commands_installed:
       self._PushChangedFilesIndividually(files)
     else:
       self._PushChangedFilesZipped(files)
       self._RunShellCommandImpl(
           ['chmod', '-R', '777'] + [d for _, d in host_device_tuples],
-          as_root=True)
+          as_root=True, check_return=True)
 
   def _GetChangedFilesImpl(self, host_path, device_path):
     real_host_path = os.path.realpath(host_path)
     try:
       real_device_path = self._RunShellCommandImpl(
-          ['realpath', device_path], check_return=True)
-      real_device_path = real_device_path[0]
+          ['realpath', device_path], single_line=True, check_return=True)
     except device_errors.CommandFailedError:
       return [(host_path, device_path)]
 
     # TODO(jbudorick): Move the md5 logic up into DeviceUtils or base
     # this function on mtime.
     # pylint: disable=W0212
     host_hash_tuples, device_hash_tuples = self.old_interface._RunMd5Sum(
         real_host_path, real_device_path)
     # pylint: enable=W0212
 
@@ skipping 71 matching lines @@
           target=DeviceUtils._CreateDeviceZip,
           args=(zip_file.name, files))
       zip_proc.start()
       zip_proc.join()
 
       zip_on_device = '%s/tmp.zip' % self._GetExternalStoragePathImpl()
       try:
         self.adb.Push(zip_file.name, zip_on_device)
         self._RunShellCommandImpl(
             ['unzip', zip_on_device],
-            as_root=True, check_return=True,
-            env={'PATH': '$PATH:%s' % install_commands.BIN_DIR})
+            as_root=True,
+            env={'PATH': '$PATH:%s' % install_commands.BIN_DIR},
+            check_return=True)
       finally:
         if zip_proc.is_alive():
           zip_proc.terminate()
         if self._IsOnlineImpl():
-          self._RunShellCommandImpl(['rm', zip_on_device])
+          self._RunShellCommandImpl(['rm', zip_on_device], check_return=True)
 
   @staticmethod
   def _CreateDeviceZip(zip_path, host_device_tuples):
     with zipfile.ZipFile(zip_path, 'w') as zip_file:
       for host_path, device_path in host_device_tuples:
         if os.path.isfile(host_path):
           zip_file.write(host_path, device_path, zipfile.ZIP_DEFLATED)
         else:
           for hd, _, files in os.walk(host_path):
             dd = '%s/%s' % (device_path, os.path.relpath(host_path, hd))
@@ skipping 115 matching lines @@
       as_root: A boolean indicating whether the write should be executed with
                root privileges.
       timeout: timeout in seconds
       retries: number of retries
 
     Raises:
       CommandFailedError if the file could not be written on the device.
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
-    self._RunShellCommandImpl('echo {1} > {0}'.format(device_path,
-        pipes.quote(text)), check_return=True, as_root=as_root)
+    cmd = 'echo %s > %s' % (cmd_helper.SingleQuote(text),
+                            cmd_helper.SingleQuote(device_path))
+    self._RunShellCommandImpl(cmd, as_root=as_root, check_return=True)
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def Ls(self, device_path, timeout=None, retries=None):
     """Lists the contents of a directory on the device.
 
     Args:
       device_path: A string containing the path of the directory on the device
                    to list.
       timeout: timeout in seconds
       retries: number of retries
@@ skipping 96 matching lines @@
       provided |process_name|.
 
     Raises:
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
     return self._GetPidsImpl(process_name)
 
   def _GetPidsImpl(self, process_name):
     procs_pids = {}
-    for line in self._RunShellCommandImpl('ps'):
+    for line in self._RunShellCommandImpl('ps', check_return=True):
       try:
         ps_data = line.split()
         if process_name in ps_data[-1]:
           procs_pids[ps_data[-1]] = ps_data[1]
       except IndexError:
         pass
     return procs_pids
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def TakeScreenshot(self, host_path=None, timeout=None, retries=None):
@@ skipping 77 matching lines @@
     Returns:
       A Parallelizer operating over |devices|.
     """
     if not devices or len(devices) == 0:
       devices = pylib.android_commands.GetAttachedDevices()
     parallelizer_type = (parallelizer.Parallelizer if async
                          else parallelizer.SyncParallelizer)
     return parallelizer_type([
         d if isinstance(d, DeviceUtils) else DeviceUtils(d)
         for d in devices])