Refactor and rename install-build-deps-android-sdk

build/install_android_sdk_ndk.py

Index: build/install_android_sdk_ndk.py
diff --git a/build/install_android_sdk_ndk.py b/build/install_android_sdk_ndk.py
new file mode 100755
index 0000000000000000000000000000000000000000..ac410d2088dfa24f98767dbf1fef5af4116366c4
--- /dev/null
+++ b/build/install_android_sdk_ndk.py
@@ -0,0 +1,197 @@
+#!/usr/bin/python
+#
+# 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.
+"""A script to automatically install the android sdk and ndk."""
+
+import optparse
+import os
+import re
+import shutil
+import subprocess
+import sys
+import tarfile
+
+SDK_CONFIG = {'varname': 'ANDROID_SDK_ROOT',
+              'default_dir': '/usr/local/google/android-sdk-linux',
+              'url': 'http://dl.google.com/android/android-sdk_r20-linux.tgz',
+              'tar_md5': 0x22a81cf1d4a951c62f71a8758290e9bb,
+              'version': 1}

[bulach, 2012/07/17 11:27:20]

nit: "version" here seems related to the package version itself... is it
necessary per package?
how about a single global SCRIPT_REVISION, with comments saying something like 
"# Bump this number when changing the configs to trigger a re-installation."

[Isaac (away), 2012/07/18 09:45:05]

Will fix.

+SDK_TARGET_ID = 'android-15'
+
+
+# Do not change without updating the 64-bit linker!
+NDK_CONFIG = {'varname': 'ANDROID_NDK_ROOT',
+              'default_dir': '/usr/local/google/android-ndk-r7',
+              'url': ('http://dl.google.com/android/ndk'
+                      '/android-ndk-r7-linux-x86.tar.bz2'),
+              'tar_md5': 0xbf15e6b47bf50824c4b96849bf003ca3,
+              'version': 1}
+NDK_LINKER = 'arm-linux-androideabi-ld.e4df3e0a5bb640ccfa2f30ee67fe9b3146b152d6'
+
+
+def GetCmdOutput(cmd, shell=False):
+  proc = subprocess.Popen(cmd, shell=shell, stdout=subprocess.PIPE)
+  return proc.communicate()[0]
+
+
+def GetRoot(config):

[bulach, 2012/07/17 11:27:20]

nit: maybe clearer as "GetInstallPathForConfig". more below.

[Isaac (away), 2012/07/18 09:45:05]

OK will adjust

+  env_var_name = config['varname']
+  default_path = config['default_dir']
+  path = os.environ.get(env_var_name)
+  if not path:
+    raw_input('%s not set.  Hit enter to use %s' % (env_var_name,

[bulach, 2012/07/17 11:27:20]

nit: since this is going to be used by bots, it'd be better to avoid any
interaction.. it'd be simpler as:

path = os.environ.get(env_var_name, default_path)

[Isaac (away), 2012/07/17 16:35:30]

If a user calls the script I want to confirm before using certain paths, but the
bot should be setting up their environment properly.  

On 2012/07/17 11:27:20, bulach wrote:

[John Grabowski, 2012/07/17 18:38:15]

Strongly agree.  Raw input is bad form.

[Isaac (away), 2012/07/18 09:45:05]

I want to distinguish between an automated 'bot mode' and user mode.  For users
running this script to install the sdk and ndk, a confirmation before setting up
and installing to a default folder seems reasonable.

For bots, this will result in a timeout but should never happen as long as we
don't mess up scripts to not set path.  But I could add another option --auto
and fail rather than raw_input here.

[John Grabowski, 2012/07/18 20:48:43]

No.  No option.  Just no raw input.  If user doesn't want the behavior of the
script, don't run it.

+                                                    default_path))
+    path = default_path
+  base_dir = os.path.dirname(path)
+  if not os.path.exists(base_dir):
+    print '%s does not exist, creating...' % base_dir
+    os.makedirs(base_dir, 0755)
+  return path
+
+
+def VersionedInstall(root, version, installer, options):

[bulach, 2012/07/17 11:27:20]

nit: rather than root, perhaps install_dir would be clearer (root looks like
chrome/src or CLANK_DIR)..

also, as above, the "version" seems conflicting with the actual package
version.. it seems that this script has two modes:
1. "forced" (default): will overinstall regardless.
2. "refresh" (aka --version): will re-install only if this script has changed..

would it be clearer to call this "RefreshInstall" ? my point is that it's always
"VersionedInstall" because we're installing a very specific version of the
packages. :)

also, if we don't need separate "versions" for sdk / ndk, it'd be simpler to
have something like this on main():

if not options.refresh or NeedsRefresh():
  InstallSdk(sdk_root, options)
  InstallNdk(ndk_root, options)

so that this function only checks for a SCRIPT_REVISION file and returns True
whether or not we should refresh the installation..

[John Grabowski, 2012/07/17 18:38:15]

Looks like you're adding undocumented features that can cause mysterious
behavior on bots.  I don't like this either.

[Isaac (away), 2012/07/18 09:45:05]

Well, I'm happy to document it before landing.  forced and refresh are quite
different paradigms than what's going on here:

- in 'versioned', we try to ensure that all bots have exactly the same SDK and
NDK folders, rather than the current state which accumulates SDK targets in the
older bots.  We also will not auto upgrade the sdk tools without a version
number bump.  If the SDK tools releases an r21 which breaks our build, I don't
want all the bots to go red.  This version number prevents that (although
deploying new bots would be broken until underlying issues are fixed)

- In unversioned, we don't delete the sdk directory, since the user might want
the targets for other applications.  We always upgrade to the latest version of
things.

[John Grabowski, 2012/07/18 20:48:43]

You missed the point.
This is unnecessary complexity.
Don't do it.

+  version_filename = os.path.join(root, 'SCRIPT_VERSION')
+  try:
+    with open(version_filename) as f:
+      if int(f.read()) == version:
+        print 'Skipping %s (version is up to date).' % installer.__name__
+        return
+  except IOError:

[bulach, 2012/07/17 11:27:20]

probably need ValueError as well for the int cast.

[Isaac (away), 2012/07/17 16:35:30]

IOError is expected because file might not exist.  ValueError should never
happen because file is created inside this script.  Not sure want to catch that.

+    pass
+
+  if os.path.exists(root):
+    print 'Removing %s for reinstall' % root
+    shutil.rmtree(root)
+
+  success = installer(root, options)
+
+  if success:
+    with open(version_filename, 'w') as f:
+      f.write('%d' % version)
+
+
+def Download(base_dir, config, options):
+  url = config['url']
+  md5 = config['tar_md5']
+  tar_filepath = os.path.join(base_dir, os.path.basename(url))
+  if not options.dry_run:
+    subprocess.check_call(['wget', url, '-O', tar_filepath])
+  assert os.path.exists(tar_filepath)
+  md5str = GetCmdOutput(['md5sum', tar_filepath])
+  assert md5 == int('0x' + md5str.split()[0], 16), 'md5 mismatch!'
+  return tar_filepath
+
+
+def ExtractTgz(path, tar_filepath, options):
+  base_dir = os.path.dirname(path)
+  print 'extracting %s...' % tar_filepath
+  if options.dry_run:
+    if not os.path.exists(path):
+      os.mkdir(path)
+    return
+
+  f = tarfile.open(tar_filepath)
+  f.extractall(path=base_dir)
+  extract_prefix = os.path.commonprefix(f.getnames()).split('/')
+  f.close()
+  extract_path = os.path.join(base_dir, extract_prefix[0])
+  if os.path.abspath(path) != os.path.abspath(extract_path):
+    print 'Moving files from %s to %s' % (extract_path, path)
+    os.rename(extract_path, path)
+  os.remove(tar_filepath)
+
+
+def InstallSdk(sdk_root, options):
+  if not os.path.exists(sdk_root):
+    tar_filepath = Download(os.path.dirname(sdk_root), SDK_CONFIG, options)
+    ExtractTgz(sdk_root, tar_filepath, options)
+  android_bin = os.path.join(sdk_root, 'tools/android')
+
+  def Android(args, stdin=None):

[bulach, 2012/07/17 11:27:20]

nit: remove the default value, there's only one caller that needs to specify..

+    if options.dry_run:
+      print 'android', args
+    else:
+      proc = subprocess.Popen([android_bin] + args, stdin=subprocess.PIPE)
+      proc.communicate(stdin)
+
+  print 'Getting available targets...'
+  raw_targets = GetCmdOutput([android_bin, 'list', 'sdk', '--extended'])
+  targets = re.findall('^id:[\w ]*"([-\w_]+)"$', raw_targets, re.MULTILINE)
+
+  def AndroidUpdate(sdk_filter):
+    if any(t in targets for t in sdk_filter.split(',')):
+      Android(['update', 'sdk', '--no-ui', '--filter', sdk_filter])
+    else:
+      print 'SDK targets %s already up to date' % sdk_filter
+
+  AndroidUpdate('platform-tool,tool,%s' % SDK_TARGET_ID)
+
+  if options.emulator:
+    AndroidUpdate('system-image')
+
+    def CreateAvd(name, abi):
+      Android(['create', 'avd', '--name', name, '--abi', abi,
+               '--target', SDK_TARGET_ID, '--force'], stdin='no\n')
+    CreateAvd('avd-armeabi', 'armeabi-v7a')
+    CreateAvd('avd-x86', 'x86')
+
+  return True
+
+
+def ReplaceBinary(source_file, dest_file):
+  dest_dir = os.path.dirname(dest_file)
+  backup_file = dest_file + '.orig'
+  if not os.path.exists(backup_file):
+    os.rename(dest_file, backup_file)
+  print 'replacing %s with %s' % (dest_file, source_file)
+  shutil.copyfile(source_file, dest_file)
+  with open(os.path.join(dest_dir, 'README.PATCH'), 'w') as f:
+    f.write('%s replaced by %s' % (dest_file, source_file))
+
+
+def InstallNdk(ndk_root, options):
+  if os.path.exists(ndk_root):
+    print 'NDK directory already exists, skipping install'
+    return True
+
+  tar_filepath = Download(os.path.dirname(ndk_root), NDK_CONFIG, options)
+  ExtractTgz(ndk_root, tar_filepath, options)
+
+  new_linker = os.path.join(options.src_root, 'third_party/aosp', NDK_LINKER)
+  if not os.path.exists(new_linker):
+    print 'Error installing NDK: linker %s does not exist' % new_linker
+    return False
+  linker_base = 'toolchains/arm-linux-androideabi-4.4.3/prebuilt/linux-x86'
+  ReplaceBinary(new_linker, os.path.join(ndk_root, linker_base,
+                                         'bin/arm-linux-androideabi-ld'))
+  ReplaceBinary(new_linker, os.path.join(ndk_root, linker_base,
+                                         'arm-linux-androideabi/bin/ld'))
+  return True
+
+
+def main(argv):
+  parser = optparse.OptionParser()
+  parser.add_option('--versioned', action='store_true')

[bulach, 2012/07/17 11:27:20]

nit: see above, I think this would be clearer as --refresh..

+  parser.add_option('--emulator', action='store_true')
+  parser.add_option('--dry-run', action='store_true')
+  parser.add_option('--src-root', default=os.path.realpath(__file__ + '/../..'))

[bulach, 2012/07/17 11:27:20]

nit: probably best to keep as dry_run and src_root so a simple grep can find all
usages..

+
+  options, args = parser.parse_args(argv)
+  if args[1:]:
+    parser.error('Unknown options %s' % args[1:])
+
+  sdk_root = GetRoot(SDK_CONFIG)
+  ndk_root = GetRoot(NDK_CONFIG)
+
+  if options.versioned:
+    VersionedInstall(sdk_root, SDK_CONFIG['version'], InstallSdk, options)
+    VersionedInstall(ndk_root, NDK_CONFIG['version'], InstallNdk, options)
+  else:
+    InstallSdk(sdk_root, options)
+    InstallNdk(ndk_root, options)
+
+
+if __name__ == '__main__':
+  sys.exit(main(sys.argv))