Defer running of install_commands.Installed() until necessary

build/android/devil/android/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=unused-argument
 
@@ skipping 1110 matching lines @@
 
   def _ComputeStaleApks(self, package_name, host_apk_paths):
     host_checksums, device_checksums = reraiser_thread.RunAsync((
         lambda: md5sum.CalculateHostMd5Sums(host_apk_paths),
         lambda: self._ComputeDeviceChecksumsForApks(package_name)))
     stale_apks = [k for (k, v) in host_checksums.iteritems()
                   if v not in device_checksums]
     return stale_apks, set(host_checksums.values())
 
   def _PushFilesImpl(self, host_device_tuples, files):
+    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
     for h, _ in host_device_tuples:
       if os.path.isdir(h):
         dir_file_count += sum(len(f) for _r, _d, f in os.walk(h))
       else:
         dir_file_count += 1
 
     push_duration = self._ApproximateDuration(
         file_count, file_count, size, False)
     dir_push_duration = self._ApproximateDuration(
         len(host_device_tuples), dir_file_count, dir_size, False)
     zip_duration = self._ApproximateDuration(1, 1, size, True)
 
-    self._InstallCommands()
-
-    if dir_push_duration < push_duration and (
-        dir_push_duration < zip_duration or not self._commands_installed):
+    if dir_push_duration < push_duration and dir_push_duration < zip_duration:
       self._PushChangedFilesIndividually(host_device_tuples)
-    elif push_duration < zip_duration or not self._commands_installed:
+    elif push_duration < zip_duration:
+      self._PushChangedFilesIndividually(files)
+    elif self._commands_installed is False:
+      # Already tried and failed to install unzip command.
       self._PushChangedFilesIndividually(files)
     else:
-      self._PushChangedFilesZipped(files)
-      self.RunShellCommand(
-          ['chmod', '-R', '777'] + [d for _, d in host_device_tuples],
-          as_root=True, check_return=True)
+      self._PushChangedFilesZipped(files, [d for _, d in host_device_tuples])
 
-  def _InstallCommands(self):
+  def _MaybeInstallCommands(self):
     if self._commands_installed is None:
-      try:

[jbudorick, 2015/09/04 16:09:11]

What happened to the exception handler on installation failure?

[agrieve, 2015/09/04 17:21:23]

Yikes! Removed it when debugging. put it back.

-        if not install_commands.Installed(self):
-          install_commands.InstallCommands(self)
-        self._commands_installed = True
-      except Exception as e:
-        logging.warning('unzip not available: %s' % str(e))
-        self._commands_installed = False
+      if not install_commands.Installed(self):
+        install_commands.InstallCommands(self)
+      self._commands_installed = True
+    return self._commands_installed
 
   @staticmethod
   def _ApproximateDuration(adb_calls, file_count, byte_count, is_zipping):
     # We approximate the time to push a set of files to a device as:
     #   t = c1 * a + c2 * f + c3 + b / c4 + b / (c5 * c6), where
     #     t: total time (sec)
     #     c1: adb call time delay (sec)
     #     a: number of times adb is called (unitless)
     #     c2: push time delay (sec)
     #     f: number of files pushed via adb (unitless)
@@ skipping 18 matching lines @@
       transfer_time = byte_count / (TRANSFER_RATE * COMPRESSION_RATIO)
     else:
       zip_time = 0
       transfer_time = byte_count / TRANSFER_RATE
     return adb_call_time + adb_push_setup_time + zip_time + transfer_time
 
   def _PushChangedFilesIndividually(self, files):
     for h, d in files:
       self.adb.Push(h, d)
 
-  def _PushChangedFilesZipped(self, files):
-    if not files:
-      return
-
+  def _PushChangedFilesZipped(self, files, dirs):
     with tempfile.NamedTemporaryFile(suffix='.zip') as zip_file:
       zip_proc = multiprocessing.Process(
           target=DeviceUtils._CreateDeviceZip,
           args=(zip_file.name, files))
       zip_proc.start()
-      zip_proc.join()
+      try:
+        # While it's zipping, ensure the unzip command exists on the device.

[jbudorick, 2015/09/04 16:09:11]

I'm not crazy about this flow... PuchChangedFilesZipped -> start zipping ->
can't unzip, so kill zipping -> PushChangedFilesIndividually is a pretty nasty
flow. I think I'd prefer if the fallback logic were handled at a higher level.

[agrieve, 2015/09/04 17:21:23]

Fair. Moved it up a level.

+        if not self._MaybeInstallCommands():
+          zip_proc.terminate()
+          self._PushChangedFilesIndividually(files)
+          return
 
-      zip_on_device = '%s/tmp.zip' % self.GetExternalStoragePath()
-      try:
-        self.adb.Push(zip_file.name, zip_on_device)
-        self.RunShellCommand(
-            ['unzip', zip_on_device],
-            as_root=True,
-            env={'PATH': '%s:$PATH' % install_commands.BIN_DIR},
-            check_return=True)
+        # Warm up NeedsSU cache while we're still zipping.
+        self.NeedsSU()
+        external_dir = self.GetExternalStoragePath()
+        with device_temp_file.DeviceTempFile(
+            self.adb, suffix='.zip', dir=external_dir) as device_temp:
+          zip_proc.join()
+          self.adb.Push(zip_file.name, device_temp.name)
+          quoted_dirs = ' '.join(cmd_helper.SingleQuote(d) for d in dirs)
+          self.RunShellCommand(
+              'unzip %s&&chmod -R 777 %s' % (device_temp.name, quoted_dirs),
+              as_root=True,
+              env={'PATH': '%s:$PATH' % install_commands.BIN_DIR},
+              check_return=True)
       finally:
         if zip_proc.is_alive():
           zip_proc.terminate()
-        if self.IsOnline():
-          self.RunShellCommand(['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:
         zip_utils.WriteToZipFile(zip_file, host_path, device_path)
 
   # TODO(nednguyen): remove this and migrate the callsite to PathExists().
   def FileExists(self, device_path, timeout=None, retries=None):
     """Checks whether the given file exists on the device.
 
     Arguments are the same as PathExists.
     """
     return self.PathExists(device_path, timeout=timeout, retries=retries)
 
-  @decorators.WithTimeoutAndRetriesFromInstance()
-  def PathExists(self, device_path, timeout=None, retries=None):
-    """Checks whether the given path exists on the device.
+  def PathExists(self, device_paths, timeout=None, retries=None):
+    """Checks whether the given path(s) exists on the device.
 
     Args:
       device_path: A string containing the absolute path to the file on the
-                   device.
+                   device, or an iterable of paths to check.
       timeout: timeout in seconds
       retries: number of retries
 
     Returns:
-      True if the file exists on the device, False otherwise.
+      True if the all given paths exist on the device, False otherwise.
 
     Raises:
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
-    try:
-      self.RunShellCommand(['test', '-e', device_path], check_return=True)
-      return True
-    except device_errors.AdbCommandFailedError:
-      return False
+    paths = device_paths
+    if isinstance(paths, basestring):
+      paths = (paths,)
+    condition = ' -a '.join('-e %s' % cmd_helper.SingleQuote(p) for p in paths)
+    cmd = 'test %s;echo $?' % condition
+    result = self.RunShellCommand(cmd, check_return=True, timeout=timeout,
+                                  retries=retries)
+    return '0' == result[0]
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def PullFile(self, device_path, host_path, timeout=None, retries=None):
     """Pull a file from the device.
 
     Args:
       device_path: A string containing the absolute path of the file to pull
                    from the device.
       host_path: A string containing the absolute path of the destination on
                  the host.
@@ skipping 651 matching lines @@
     return [cls(adb, **kwargs) for adb in adb_wrapper.AdbWrapper.Devices()
             if not blacklisted(adb)]
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def RestartAdbd(self, timeout=None, retries=None):
     logging.info('Restarting adbd on device.')
     with device_temp_file.DeviceTempFile(self.adb, suffix='.sh') as script:
       self.WriteFile(script.name, _RESTART_ADBD_SCRIPT)
       self.RunShellCommand(['source', script.name], as_root=True)
       self.adb.WaitForDevice()