[android] Push only updated files in PushIfNeeded when few files have changed.

build/android/pylib/android_commands.py

 # Copyright (c) 2012 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 an interface to communicate with the device via the adb command.
 
 Assumes adb binary is currently on system path.
 """
 
 import collections
@@ skipping 166 matching lines @@
       if isinstance(utc_offset, str) and len(utc_offset) == 5:
         utc_delta = datetime.timedelta(hours=int(utc_offset[1:3]),
                                        minutes=int(utc_offset[3:5]))
         if utc_offset[0:1] == '-':
           utc_delta = -utc_delta
         lastmod -= utc_delta
       files[filename] = (int(file_match.group('size')), lastmod)
   return files
 
 
 def _ComputeFileListHash(md5sum_output):

[frankf, 2013/08/06 00:17:23]

Rename this to _ParseMd5sumOutput()

   """Returns a list of tuples from the provided md5sum output.
 
   Args:
     md5sum_output: output directly from md5sum binary.
 
   Returns:
     List of namedtuples (hash, path).

[frankf, 2013/08/06 00:17:23]

What is path? Is it absolute or relative? Provide an example

[craigdh, 2013/08/06 21:17:29]

Done.

   """
   HashAndPath = collections.namedtuple('HashAndPath', ['hash', 'path'])
   split_lines = [line.split('  ') for line in md5sum_output]
   return [HashAndPath._make(s) for s in split_lines if len(s) == 2]
 
 
 def _HasAdbPushSucceeded(command_output):
   """Returns whether adb push has succeeded from the provided output."""
   # TODO(frankf): We should look at the return code instead of the command
   # output for many of the commands in this file.
@@ skipping 526 matching lines @@
       self.RunShellCommand('pm clear ' + package)
 
   def SendKeyEvent(self, keycode):
     """Sends keycode to the device.
 
     Args:
       keycode: Numeric keycode to send (see "enum" at top of file).
     """
     self.RunShellCommand('input keyevent %d' % keycode)
 
-  def CheckMd5Sum(self, local_path, device_path):
-    """Compares the md5sum of a local path against a device path.
+  def _RunMd5Sum(self, host_path, device_path):

[frankf, 2013/08/06 00:17:23]

Why not inline this in GetFilesChanged?

[craigdh, 2013/08/06 21:17:29]

Could, this seems reusable and keeps the functions each to readable lengths
though.

+    """Gets the md5sum of a host path and device path.
 
     Args:
-      local_path: Path (file or directory) on the host.
+      host_path: Path (file or directory) on the host.
       device_path: Path on the device.
 
     Returns:
-      True if the md5sums match.
+      A tuple containing lists of the host and device md5sum results as
+      created by _ComputeFileListHash().
     """
     if not self._md5sum_build_dir:
       default_build_type = os.environ.get('BUILD_TYPE', 'Debug')
       build_dir = '%s/%s/' % (
           cmd_helper.OutDirectory().get(), default_build_type)
       md5sum_dist_path = '%s/md5sum_dist' % build_dir
       if not os.path.exists(md5sum_dist_path):
         build_dir = '%s/Release/' % cmd_helper.OutDirectory().get()
         md5sum_dist_path = '%s/md5sum_dist' % build_dir
         assert os.path.exists(md5sum_dist_path), 'Please build md5sum.'
       command = 'push %s %s' % (md5sum_dist_path, MD5SUM_DEVICE_FOLDER)
       assert _HasAdbPushSucceeded(self._adb.SendCommand(command))
       self._md5sum_build_dir = build_dir
 
     cmd = (MD5SUM_LD_LIBRARY_PATH + ' ' + self._util_wrapper + ' ' +
            MD5SUM_DEVICE_PATH + ' ' + device_path)
     device_hash_tuples = _ComputeFileListHash(
         self.RunShellCommand(cmd, timeout_time=2 * 60))
-    assert os.path.exists(local_path), 'Local path not found %s' % local_path
+    assert os.path.exists(host_path), 'Local path not found %s' % host_path
     md5sum_output = cmd_helper.GetCmdOutput(
-        ['%s/md5sum_bin_host' % self._md5sum_build_dir, local_path])
+        ['%s/md5sum_bin_host' % self._md5sum_build_dir, host_path])
     host_hash_tuples = _ComputeFileListHash(md5sum_output.splitlines())
+    return (host_hash_tuples, device_hash_tuples)
+
+  def GetFilesChanged(self, host_path, device_path):
+    """Compares the md5sum of a host path against a device path.
+
+    Note: Ignores extra files on the device.
+
+    Args:
+      host_path: Path (file or directory) on the host.
+      device_path: Path on the device.
+
+    Returns:
+      A list of tuples of the form (host_path, device_path) for files whose
+      md5sums do not match.
+    """
+    host_hash_tuples, device_hash_tuples = self._RunMd5Sum(
+        host_path, device_path)
 
     # Ignore extra files on the device.
     if len(device_hash_tuples) > len(host_hash_tuples):
       host_files = [os.path.relpath(os.path.normpath(p.path),
-                    os.path.normpath(local_path)) for p in host_hash_tuples]
+                    os.path.normpath(host_path)) for p in host_hash_tuples]
 
-      def _host_has(fname):
+      def HostHas(fname):
         return any(path in fname for path in host_files)
 
-      hashes_on_device = [h.hash for h in device_hash_tuples if
-                         _host_has(h.path)]
-    else:
-      hashes_on_device = [h.hash for h in device_hash_tuples]
+      device_hash_tuples = [h for h in device_hash_tuples if HostHas(h.path)]
 
-    # Compare md5sums between host and device files.
-    hashes_on_host = [h.hash for h in host_hash_tuples]
-    hashes_on_device.sort()
-    hashes_on_host.sort()
-    return hashes_on_device == hashes_on_host
+    # Constructs the target device path from a given host path. Don't use when
+    # only a single file is given as the base name given in device_path may
+    # differ from that in host_path.
+    def HostToDevicePath(host_path):
+      return os.path.join(os.path.dirname(device_path), os.path.relpath(
+          host_path, os.path.dirname(os.path.normpath(host_path))))
 
-  def PushIfNeeded(self, local_path, device_path):
-    """Pushes |local_path| to |device_path|.
+    device_hashes = [h.hash for h in device_hash_tuples]
+    return [(t.path, HostToDevicePath(t.path) if os.path.isdir(host_path) else
+             device_path)
+            for t in host_hash_tuples if t.hash not in device_hashes]
+
+  def CheckMd5Sum(self, host_path, device_path):

[frankf, 2013/08/06 00:17:23]

Can you get rid of this and just call GetFilesChanged?

[craigdh, 2013/08/06 21:17:29]

Done.

+    """Compares the md5sum of a host path against a device path.
+
+    Args:
+      host_path: Path (file or directory) on the host.
+      device_path: Path on the device.
+
+    Returns:
+      True if the md5sums match.
+    """
+    return not self.GetFilesChanged(host_path, device_path)
+
+  def PushIfNeeded(self, host_path, device_path):
+    """Pushes |host_path| to |device_path|.
 
     Works for files and directories. This method skips copying any paths in
     |test_data_paths| that already exist on the device with the same hash.
 
     All pushed files can be removed by calling RemovePushedFiles().
     """
-    assert os.path.exists(local_path), 'Local path not found %s' % local_path
-    size = int(cmd_helper.GetCmdOutput(['du', '-sb', local_path]).split()[0])
+    MAX_INDIVIDUAL_PUSHES = 50
+    assert os.path.exists(host_path), 'Local path not found %s' % host_path
+
+    def GetHostSize(path):
+      return int(cmd_helper.GetCmdOutput(['du', '-sb', path]).split()[0])
+
+    size = GetHostSize(host_path)
     self._pushed_files.append(device_path)
     self._potential_push_size += size
 
-    if self.CheckMd5Sum(local_path, device_path):
+    missing_files = self.GetFilesChanged(host_path, device_path)

[frankf, 2013/08/06 00:17:23]

this is misleading -> changed_files

[craigdh, 2013/08/06 21:17:29]

Done.

+    if not missing_files:
       return
 
-    self._actual_push_size += size
-    # They don't match, so remove everything first and then create it.
-    if os.path.isdir(local_path):
-      self.RunShellCommand('rm -r %s' % device_path, timeout_time=2 * 60)
-      self.RunShellCommand('mkdir -p %s' % device_path)
+    def Push(host, device):
+      # NOTE: We can't use adb_interface.Push() because it hardcodes a timeout
+      # of 60 seconds which isn't sufficient for a lot of users of this method.
+      push_command = 'push %s %s' % (host, device)
+      self._LogShell(push_command)
 
-    # NOTE: We can't use adb_interface.Push() because it hardcodes a timeout of
-    # 60 seconds which isn't sufficient for a lot of users of this method.
-    push_command = 'push %s %s' % (local_path, device_path)
-    self._LogShell(push_command)
+      # Retry push with increasing backoff if the device is busy.
+      retry = 0
+      while True:
+        output = self._adb.SendCommand(push_command, timeout_time=30 * 60)
+        if _HasAdbPushSucceeded(output):
+          return
+        if 'resource busy' in output and retry < 3:
+          retry += 1
+          wait_time = 5 * retry
+          logging.error('Push failed, retrying in %d seconds: %s' %
+                        (wait_time, output))
+          time.sleep(wait_time)
+        else:
+          raise Exception('Push failed: %s' % output)
 
-    # Retry push with increasing backoff if the device is busy.
-    retry = 0
-    while True:
-      output = self._adb.SendCommand(push_command, timeout_time=30 * 60)
-      if _HasAdbPushSucceeded(output):
-        return
-      if 'resource busy' in output and retry < 3:
-        retry += 1
-        wait_time = 5 * retry
-        logging.error('Push failed, retrying in %d seconds: %s' %
-                      (wait_time, output))
-        time.sleep(wait_time)
-      else:
-        raise Exception('Push failed: %s' % output)
+    diff_size = 0
+    if len(missing_files) <= MAX_INDIVIDUAL_PUSHES:
+      diff_size = sum(GetHostSize(f[0]) for f in missing_files)
+
+    # TODO(craigdh): Replace this educated guess with a heuristic that
+    # approximates the push time for each method.

[frankf, 2013/08/06 00:17:23]

Can you look how adb sync operates:
https://android.googlesource.com/platform/system/core/+/master/adb/file_sync_...

[craigdh, 2013/08/06 21:17:29]

Seems to just compare the timestamps. From what I gather it doesn't use adb push
as it is directly reading in the files and sending data to adbd on the device to
write out.

+    if len(missing_files) > MAX_INDIVIDUAL_PUSHES or diff_size > 0.5 * size:
+      # We're pushing everything, remove everything first and then create it.
+      self._actual_push_size += size
+      if os.path.isdir(host_path):
+        self.RunShellCommand('rm -r %s' % device_path, timeout_time=2 * 60)
+        self.RunShellCommand('mkdir -p %s' % device_path)
+      Push(host_path, device_path)
+    else:
+      for f in missing_files:
+        Push(f[0], f[1])
+      self._actual_push_size += diff_size
 
   def GetPushSizeInfo(self):
     """Get total size of pushes to the device done via PushIfNeeded()
 
     Returns:
       A tuple:
         1. Total size of push requests to PushIfNeeded (MB)
         2. Total size that was actually pushed (MB)
     """
     return (self._potential_push_size, self._actual_push_size)
@@ skipping 629 matching lines @@
   """
   def __init__(self, output):
     self._output = output
 
   def write(self, data):
     data = data.replace('\r\r\n', '\n')
     self._output.write(data)
 
   def flush(self):
     self._output.flush()