Get rid of device/host clock synchronization in android_commands.py.

build/android/pylib/android_commands.py

Index: build/android/pylib/android_commands.py
diff --git a/build/android/pylib/android_commands.py b/build/android/pylib/android_commands.py
index e665a656634d8989dad238b77d764cd392f63088..dc6cf21f129f2e3e42033873a19a901b279786af 100644
--- a/build/android/pylib/android_commands.py
+++ b/build/android/pylib/android_commands.py
@@ -22,10 +22,12 @@ import time
 import pexpect
 import io_stats_parser
 
-# adb_interface.py is under ../../../third_party/android_testrunner/
-sys.path.append(os.path.join(os.path.abspath(os.path.dirname(__file__)), '..',
-                             '..', '..', 'third_party', 'android_testrunner'))
+CHROME_SRC = os.path.join(
+    os.path.abspath(os.path.dirname(__file__)), '..', '..', '..')
+
+sys.path.append(os.path.join(CHROME_SRC, 'third_party', 'android_testrunner'))
 import adb_interface
+
 import cmd_helper
 import errors  #  is under ../../../third_party/android_testrunner/errors.py
 
@@ -66,6 +68,7 @@ KEYCODE_DPAD_RIGHT = 22
 KEYCODE_ENTER = 66
 KEYCODE_MENU = 82
 
+MD5SUM_DEVICE_PATH = '/data/local/tmp/md5sum'
 
 def GetEmulators():
   """Returns a list of emulators.  Does not filter by status (e.g. offline).
@@ -113,42 +116,6 @@ def GetAttachedDevices():
     devices.insert(0, preferred_device)
   return devices
 
-
-def _GetHostFileInfo(file_name):
-  """Returns a tuple containing size and modified UTC time for file_name."""
-  # The time accuracy on device is only to minute level, remove the second and
-  # microsecond from host results.
-  utc_time = datetime.datetime.utcfromtimestamp(os.path.getmtime(file_name))
-  time_delta = datetime.timedelta(seconds=utc_time.second,
-                                  microseconds=utc_time.microsecond)
-  return os.path.getsize(file_name), utc_time - time_delta
-
-
-def ListHostPathContents(path):
-  """Lists files in all subdirectories of |path|.
-
-  Args:
-    path: The path to list.
-
-  Returns:
-    A dict of {"name": (size, lastmod), ...}.
-  """
-  if os.path.isfile(path):
-    return {os.path.basename(path): _GetHostFileInfo(path)}
-  ret = {}
-  for root, dirs, files in os.walk(path):
-    for d in dirs:
-      if d.startswith('.'):
-        dirs.remove(d)  # Prune the dir for subsequent iterations.
-    for f in files:
-      if f.startswith('.'):
-        continue
-      full_file_name = os.path.join(root, f)
-      file_name = os.path.relpath(full_file_name, path)
-      ret[file_name] = _GetHostFileInfo(full_file_name)
-  return ret
-
-
 def _GetFilesFromRecursiveLsOutput(path, ls_output, re_file, utc_offset=None):
   """Gets a list of files from `ls` command output.
 
@@ -201,6 +168,10 @@ def _GetFilesFromRecursiveLsOutput(path, ls_output, re_file, utc_offset=None):
       files[filename] = (int(file_match.group('size')), lastmod)
   return files
 
+def _ComputeFileListHash(md5sum_output):
+  """Returns a list of MD5 strings from the provided md5sum output."""
+  return [line.split('  ')[0] for line in md5sum_output]
+
 
 def GetLogTimestamp(log_line, year):
   """Returns the timestamp of the given |log_line| in the given year."""
@@ -228,6 +199,17 @@ class AndroidCommands(object):
     self._original_governor = None
     self._pushed_files = []
     self._device_utc_offset = self.RunShellCommand('date +%z')[0]
+    self._md5sum_path = '%s/out/Debug/md5sum' % (CHROME_SRC)
+    if not os.path.exists(self._md5sum_path):
+      self._md5sum_path = '%s/out/Release/md5sum' % (chrome_src_path)
+      if not os.path.exists(self._md5sum_path):
+        print >>sys.stderr, 'Please build md5sum (\'make md5sum\')'

[Isaac (away), 2012/08/30 06:50:57]

nit: you can remove the backslash escape chars if you change the outer quotes to
double quotes.

[Philippe, 2012/08/30 08:22:12]

Done.

+        sys.exit(1)

[Isaac (away), 2012/08/30 06:50:57]

Most functions in android_commands do not require md5sum_path.  Is there a way
to not crash if this is missing?  Otherwise users of this class with stale out
directories could get a runtime exception until they rebuild

[Philippe, 2012/08/30 08:22:12]

Good point. I moved this check to PushIfNeeded().

+    # Push the md5sum binary to the device if needed.
+    if not self.FileExistsOnDevice(MD5SUM_DEVICE_PATH):
+      command = 'push %s %s' % (self._md5sum_path, MD5SUM_DEVICE_PATH)
+      logging.info(command)
+      assert self._adb.SendCommand(command)

[Isaac (away), 2012/08/30 06:50:57]

Is this assert being used correctly?  it will success as long as there is output
from _adb.SendCommand()

[Philippe, 2012/08/30 08:22:12]

Indeed. I moved the code we had in PushIfNeeded() to a new helper function,
_HasAdbPushSucceeded() so that I can reuse it here.

 
   def Adb(self):
     """Returns our AdbInterface to avoid us wrapping all its methods."""
@@ -264,10 +246,6 @@ class AndroidCommands(object):
         self.RestartShell()
     raise last_err  # Only reached after max retries, re-raise the last error.
 
-  def SynchronizeDateTime(self):
-    """Synchronize date/time between host and device."""
-    self._adb.SendShellCommand('date -u %f' % time.time())
-
   def RestartShell(self):
     """Restarts the shell on the device. Does not block for it to return."""
     self.RunShellCommand('stop')
@@ -552,26 +530,20 @@ class AndroidCommands(object):
     """Pushes |local_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 timestamp
-    and size.
+    |test_data_paths| that already exist on the device with the same md5.
 
     All pushed files can be removed by calling RemovePushedFiles().
     """
     assert os.path.exists(local_path), 'Local path not found %s' % local_path
     self._pushed_files.append(device_path)
 
-    # If the path contents are the same, there's nothing to do.
-    local_contents = ListHostPathContents(local_path)
-    device_contents = self.ListPathContents(device_path)
-    # Only compare the size and timestamp if only copying a file because
-    # the filename on device can be renamed.
-    if os.path.isfile(local_path):
-      assert len(local_contents) == 1
-      is_equal = local_contents.values() == device_contents.values()
-    else:
-      is_equal = local_contents == device_contents
-    if is_equal:
-      logging.info('%s is up-to-date. Skipping file push.', device_path)
+    hashes_on_device = _ComputeFileListHash(
+        self.RunShellCommand(MD5SUM_DEVICE_PATH + ' ' + device_path))
+    assert os.path.exists(local_path), 'Local path not found %s' % local_path
+    hashes_on_host = _ComputeFileListHash(
+        cmd_helper.GetCmdOutput(
+            '%s_host_bin %s' % (self._md5sum_path, local_path), shell=True))
+    if hashes_on_device == hashes_on_host:
       return
 
     # They don't match, so remove everything first and then create it.
@@ -994,6 +966,20 @@ class AndroidCommands(object):
     logging.info('PidsUsingDevicePort: %s', pids)
     return pids
 
+  def FileExistsOnDevice(self, file_name):
+    """Checks whether the given (regular) file exists on the device.
+
+    Args:
+      file_name: Full path of file to check.
+
+    Returns:
+      True if the file exists, False otherwise.
+    """
+    assert '"' not in file_name, 'file_name cannot contain double quotes'
+    status = self._adb.SendShellCommand(
+        '\'test -f "%s"; echo $?\'' % (file_name))

[Philippe, 2012/08/29 08:36:45]

Do we still need a comment here? I think it's much simpler now.

[Isaac (away), 2012/08/30 06:50:57]

this is fine, thanks

[Philippe, 2012/08/30 08:22:12]

Done.

+    return int(status) == 0
+
   def RunMonkey(self, package_name, category=None, throttle=100, seed=None,
                 event_count=10000, verbosity=1, extra_args=''):
     """Runs monkey test for a given package.