Always download google_play_services zip package instead of rsync.

build/download_sdk_extras.py

Index: build/download_sdk_extras.py
diff --git a/build/download_sdk_extras.py b/build/download_sdk_extras.py
index 1149d9d29641a7814b6271bb53c76acdabc69db5..96b1ba5b3287edcf4938b59da284ae9dc419f16b 100755
--- a/build/download_sdk_extras.py
+++ b/build/download_sdk_extras.py
@@ -5,10 +5,9 @@
 
 """Script to download sdk/extras packages on the bots from google storage.
 
-The script expects an argument that specifies the packet name in the following
-format: <dir_in_sdk_extras>_<package_name>_<version>. There will be a
-correpsonding bucket in google storage with that name, and it will be downloaded
-to android_tools/sdk/extras/.
+The script expects arguments that specify zips file in the google storage
+bucket named: <dir in SDK extras>_<package name>_<version>.zip. The file will
+be extracted in the android_tools/sdk/extras directory.
 """
 
 import os
@@ -24,41 +23,47 @@ SDK_EXTRAS_BUCKET = 'gs://chrome-sdk-extras'
 SDK_EXTRAS_PATH = os.path.join(constants.ANDROID_SDK_ROOT, 'extras')
 
 
-def GetCmdOutputAndStatus(cmd_lst):
-  process = subprocess.Popen(cmd_lst, stdout=subprocess.PIPE)
-  stdout, _ = process.communicate()
-  return stdout, process.returncode
+def clean_and_extract(zip_file):
+  # Extract the directory name and package name from zip file name. The format
+  # of the zip file is <dir in sdk/extras>_<package_name>_<version>.zip.
+  dir_name = zip_file[:zip_file.index('_')]
+  package_name = zip_file[zip_file.index('_')+1:zip_file.rindex('_')]
+  local_dir = '%s/%s/%s' % (SDK_EXTRAS_PATH, dir_name, package_name)
+  cmd_lst = ['rm', '-rf', local_dir]

[cjhopman, 2015/01/06 02:04:04]

instead of subbing out to 'rm -rf', you can do:

if os.path.exists(local_dir):
  shutil.rmtree(local_dir)

but either is fine with me.

[navabi, 2015/01/06 19:10:13]

Done.

+  rc = subprocess.check_call(cmd_lst)

[cjhopman, 2015/01/06 02:04:04]

check_call throws an exception if the returncode is non-zero, so you don't need
to handle that case for all of these

[navabi, 2015/01/06 19:10:13]

Acknowledged.

+  if rc:
+    return rc
+  local_zip = '%s/%s' % (SDK_EXTRAS_PATH, zip_file)
+  cmd_lst = ['unzip', '-u', local_zip, '-d', SDK_EXTRAS_PATH]

[cjhopman, 2015/01/06 02:04:04]

You could use python's zipfile here

something like:

with zipfile.ZipFile(local_zip) as z:
  z.extractall(path=SDK_EXTRAS_PATH)

[navabi, 2015/01/06 19:10:13]

Done.

+  return subprocess.check_call(cmd_lst)
 
-def is_android_buildbot_checkout():
-  if not os.path.exists(GSUTIL_PATH) or not os.path.exists(SDK_EXTRAS_PATH):
-    return False
-  stdout, rc = GetCmdOutputAndStatus([GSUTIL_PATH, 'ls', SDK_EXTRAS_BUCKET])
-  # If successfully read bucket, then this must be a bot with permissions
-  return not rc
 
 def main(args):
-  if is_android_buildbot_checkout():
-    success = True
-    for arg in args[1:]:
-      # Package is named <folder>_<package_name>_<version>
-      first_underscore = arg.find('_')
-      last_underscore = arg.rfind('_')
-      folder = arg[0:first_underscore]
-      package = arg[first_underscore+1:last_underscore]
-      # Package bucket is <SDK_EXTRAS_BUCKET>/<folder>_<package_name>_<version>
-      # and in that bucket will be the directory <folder>/<package_name> to cp.
-      package_bucket = '%s/%s/%s/%s' % (SDK_EXTRAS_BUCKET, arg, folder, package)
-      package_dir = '%s/%s/%s' % (SDK_EXTRAS_PATH, folder, package)
-      if not os.path.exists(package_dir):
-        os.makedirs(package_dir)
-      # rsync is only supported by gsutil version 4.x
-      cmd_lst = [GSUTIL_PATH, '--force-version', '4.6', '-m', 'rsync', '-r',
-                 '-d', package_bucket, package_dir]
-      stdout, rc = GetCmdOutputAndStatus(cmd_lst)
-      success = (rc == 0) and success
-    if not success:
-      return 1
-  return 0
+  if not os.path.exists(GSUTIL_PATH) or not os.path.exists(SDK_EXTRAS_PATH):
+    # This is not a buildbot checkout.
+    return 0
+  success = True
+  for arg in args[1:]:
+    local_zip = '%s/%s' % (SDK_EXTRAS_PATH, arg)
+    if os.path.exists(local_zip):

[cjhopman, 2015/01/06 02:04:04]

this if/else can just be:

if not os.path.exists(local_zip):
  package_zip = ...
  subprocess.check_call(...stat...)
  subprocess.check_call(...cp...)
return clean_and_extract(arg)

[navabi, 2015/01/06 19:10:13]

Done. (and there is really no point in doing the stat. It's to check if cp is
going to work, but we may as well just try to cp).

+      # Always clean dir and extract zip to ensure correct contents.
+      clean_and_extract(arg)
+    else:
+      # Package zip is named <folder>_<package_name>_<version>.zip.
+      package_zip = '%s/%s' % (SDK_EXTRAS_BUCKET, arg)
+      rc = subprocess.check_call([GSUTIL_PATH, '--force-version', '4.7', 'stat',
+                                  package_zip])
+      if rc:
+        return 1
+      rc = subprocess.check_call([GSUTIL_PATH, '--force-version', '4.7', 'cp',
+                                  package_zip, local_zip])
+      if not rc:
+        rc = clean_and_extract(arg)
+        success = (rc == 0) and success
+      else:
+       success = False
+  if not success:
+    return 1
 
 
 if __name__ == '__main__':