device_utils.Install: Calculate host md5s at the same time as device md5s

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=unused-argument
 
 import collections
 import contextlib
 import itertools
 import logging
 import multiprocessing
 import os
 import posixpath
 import re
 import shutil
 import sys
 import tempfile
+import threading
 import time
 import zipfile
 
 import pylib.android_commands
 from pylib import cmd_helper
 from pylib import constants
 from pylib import device_signal
 from pylib.constants import keyevent
 from pylib.device import adb_wrapper
 from pylib.device import decorators
@@ skipping 1053 matching lines @@
   def _ComputeDeviceChecksumsForApks(self, package_name):
     ret = self._cache['package_apk_checksums'].get(package_name)
     if ret is None:
       device_paths = self._GetApplicationPathsInternal(package_name)
       file_to_checksums = md5sum.CalculateDeviceMd5Sums(device_paths, self)
       ret = set(file_to_checksums.values())
       self._cache['package_apk_checksums'][package_name] = ret
     return ret
 
   def _ComputeStaleApks(self, package_name, host_apk_paths):
-    host_checksums = md5sum.CalculateHostMd5Sums(host_apk_paths)
+    host_checksums_holder = [None]
+    def DoComputeHostChecksums():

[jbudorick, 2015/08/11 18:06:40]

nit: I've generally been trying to keep local functions within pylib as
lower_case_with_underscores to distinguish them.

[agrieve, 2015/08/11 18:15:00]

Done.

+      host_checksums_holder[0] = md5sum.CalculateHostMd5Sums(host_apk_paths)

[jbudorick, 2015/08/11 18:06:40]

nit: line break after function definition

[agrieve, 2015/08/11 18:15:00]

Done.

+    host_thread = threading.Thread(target=DoComputeHostChecksums)

[jbudorick, 2015/08/11 18:06:40]

I'd prefer it if you used a timeout_retry thread:
https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli...

(this doesn't need retries, but a timeout would be nice in case something
strange happens)

[agrieve, 2015/08/11 18:15:00]

Since this is a private helper, and all public methods use their own timeout
logic, having a timeout within a timeout seems like overkill.

[jbudorick, 2015/08/11 18:16:54]

ah, right.

+    host_thread.start()
     device_checksums = self._ComputeDeviceChecksumsForApks(package_name)
-    stale_apks = [k for (k, v) in host_checksums.iteritems()
+    host_thread.join()
+    stale_apks = [k for (k, v) in host_checksums_holder[0].iteritems()
                   if v not in device_checksums]
-    return stale_apks, set(host_checksums.values())
+    return stale_apks, set(host_checksums_holder[0].values())
 
   def _PushFilesImpl(self, host_device_tuples, files):
     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))
@@ skipping 770 matching lines @@
     return [cls(adb) 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()