[NaCl SDK] Refactor sdk_update*.

Still need to add tests and documentation, but this CL is getting large already.

BUG=156766
R=sbc@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/11228013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@164998 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 883 insertions, 709 deletions

diff --git a/native_client_sdk/PRESUBMIT.py b/native_client_sdk/PRESUBMIT.py
index 5faf369..f9967e3 100644
--- a/native_client_sdk/PRESUBMIT.py
+++ b/native_client_sdk/PRESUBMIT.py
@@ -18,7 +18,10 @@ def CommonChecks(input_api, output_api):
     'E1103',  # subprocess.communicate() generates these :(
     'R0201',  # method could be function (doesn't reference self)
   ]
-  black_list = [r'src[\\\/]build_tools[\\\/]tests[\\\/].*']
+  black_list = [
+    r'src[\\\/]build_tools[\\\/]tests[\\\/].*',
+    r'src[\\\/]build_tools[\\\/]sdk_tools[\\\/]third_party[\\\/].*',
+  ]
   canned = input_api.canned_checks
   output.extend(canned.RunPylint(input_api, output_api, black_list=black_list,
                 disabled_warnings=disabled_warnings))
diff --git a/native_client_sdk/src/build_tools/build_sdk.py b/native_client_sdk/src/build_tools/build_sdk.py
index 13cf633..be7f372 100755
--- a/native_client_sdk/src/build_tools/build_sdk.py
+++ b/native_client_sdk/src/build_tools/build_sdk.py
@@ -786,8 +786,8 @@ def BuildStepTestUpdater(platform, pepper_ver, revision, tarfile):
     naclsdk_sh = os.path.join(OUT_DIR, 'nacl_sdk', 'naclsdk')
     if platform == 'win':
       naclsdk_sh += '.bat'
-    buildbot_common.Run([naclsdk_sh, '-U',
-        server.GetURL(manifest_name), 'update', 'pepper_' + pepper_ver])
+    buildbot_common.Run([naclsdk_sh, 'update', 'pepper_' + pepper_ver,
+        '-U', server.GetURL(manifest_name), '-v'])
 
     # Return the new pepper directory as the one inside the downloaded SDK.
     return os.path.join(OUT_DIR, 'nacl_sdk', 'pepper_' + pepper_ver)
diff --git a/native_client_sdk/src/build_tools/build_updater.py b/native_client_sdk/src/build_tools/build_updater.py
index f454c86..e945359 100755
--- a/native_client_sdk/src/build_tools/build_updater.py
+++ b/native_client_sdk/src/build_tools/build_updater.py
@@ -11,6 +11,7 @@ tool users run to download new bundles, update existing bundles, etc.
 
 import buildbot_common
 import build_utils
+import glob
 import optparse
 import os
 import sys
@@ -38,18 +39,14 @@ UPDATER_FILES = [
 
   # SDK tools
   ('build_tools/sdk_tools/cacerts.txt', 'nacl_sdk/sdk_tools/cacerts.txt'),
-  ('build_tools/sdk_tools/sdk_update.py', 'nacl_sdk/sdk_tools/sdk_update.py'),
-  ('build_tools/sdk_tools/sdk_update_common.py',
-      'nacl_sdk/sdk_tools/sdk_update_common.py'),
-  ('build_tools/sdk_tools/sdk_update_main.py',
-      'nacl_sdk/sdk_tools/sdk_update_main.py'),
-  ('build_tools/manifest_util.py', 'nacl_sdk/sdk_tools/manifest_util.py'),
-  ('build_tools/sdk_tools/third_party/__init__.py',
-      'nacl_sdk/sdk_tools/third_party/__init__.py'),
-  ('build_tools/sdk_tools/third_party/fancy_urllib/__init__.py',
-      'nacl_sdk/sdk_tools/third_party/fancy_urllib/__init__.py'),
+  ('build_tools/sdk_tools/*.py', 'nacl_sdk/sdk_tools/'),
+  ('build_tools/sdk_tools/command/*.py', 'nacl_sdk/sdk_tools/command/'),
+  ('build_tools/sdk_tools/third_party/*.py', 'nacl_sdk/sdk_tools/third_party/'),
+  ('build_tools/sdk_tools/third_party/fancy_urllib/*.py',
+      'nacl_sdk/sdk_tools/third_party/fancy_urllib/'),
   ('build_tools/sdk_tools/third_party/fancy_urllib/README',
       'nacl_sdk/sdk_tools/third_party/fancy_urllib/README'),
+  ('build_tools/manifest_util.py', 'nacl_sdk/sdk_tools/manifest_util.py'),
   ('LICENSE', 'nacl_sdk/sdk_tools/LICENSE'),
   (CYGTAR, 'nacl_sdk/sdk_tools/cygtar.py'),
 ]
@@ -76,6 +73,30 @@ def MakeUpdaterFilesAbsolute(out_dir):
   return result
 
 
+def GlobFiles(files):
+  """Expand wildcards for 2-tuples of sources/destinations.
+
+  This function also will convert destinations from directories into filenames.
+  For example:
+    ('foo/*.py', 'bar/') => [('foo/a.py', 'bar/a.py'), ('foo/b.py', 'bar/b.py')]
+
+  Args:
+    files: A list of 2-tuples of (source, dest) paths.
+  Returns:
+    A new list of 2-tuples, after the sources have been wildcard-expanded, and
+    the destinations have been changed from directories to filenames.
+  """
+  result = []
+  for in_file_glob, out_file in files:
+    if out_file.endswith('/'):
+      for in_file in glob.glob(in_file_glob):
+        result.append((in_file,
+                      os.path.join(out_file, os.path.basename(in_file))))
+    else:
+      result.append((in_file_glob, out_file))
+  return result
+
+
 def CopyFiles(files):
   """Given a list of 2-tuples (source, dest), copy each source file to a dest
   file.
@@ -129,6 +150,7 @@ def BuildUpdater(out_dir, revision_number=None):
   buildbot_common.RemoveDir(os.path.join(out_dir, 'nacl_sdk'))
 
   updater_files = MakeUpdaterFilesAbsolute(out_dir)
+  updater_files = GlobFiles(updater_files)
 
   CopyFiles(updater_files)
   UpdateRevisionNumber(out_dir, revision_number)
diff --git a/native_client_sdk/src/build_tools/buildbot_common.py b/native_client_sdk/src/build_tools/buildbot_common.py
index cef170f..7aba249 100644
--- a/native_client_sdk/src/build_tools/buildbot_common.py
+++ b/native_client_sdk/src/build_tools/buildbot_common.py
@@ -49,7 +49,7 @@ def BuildStep(name):
 
 def Run(args, cwd=None, env=None, shell=False):
   """Start a process with the provided arguments.
-  
+
   Starts a process in the provided directory given the provided arguments. If
   shell is not False, the process is launched via the shell to provide shell
   interpretation of the arguments.  Shell behavior can differ between platforms
@@ -74,7 +74,7 @@ def CopyDir(src, dst, excludes=('.svn', '*/.svn')):
 
 
 def CopyFile(src, dst):
-  print 'cp -r %s %s' % (src, dst)
+  print 'cp %s %s' % (src, dst)
   if os.path.abspath(src) == os.path.abspath(dst):
     ErrorExit('ERROR: Copying file onto itself: ' + src)
   args = [src, dst]
diff --git a/native_client_sdk/src/build_tools/sdk_tools/command/__init__.py b/native_client_sdk/src/build_tools/sdk_tools/command/__init__.py
new file mode 100644
index 0000000..a7b1510
--- /dev/null
+++ b/native_client_sdk/src/build_tools/sdk_tools/command/__init__.py
@@ -0,0 +1,5 @@
+# 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.
+
+"""Package for all sdk_update commands."""
diff --git a/native_client_sdk/src/build_tools/sdk_tools/command/info.py b/native_client_sdk/src/build_tools/sdk_tools/command/info.py
new file mode 100644
index 0000000..d45c4ca5
--- /dev/null
+++ b/native_client_sdk/src/build_tools/sdk_tools/command/info.py
@@ -0,0 +1,34 @@
+# 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.
+
+import logging
+import manifest_util
+
+def Info(manifest, bundle_names):
+  valid_bundles = [bundle.name for bundle in manifest.GetBundles()]
+  valid_bundles = set(bundle_names) & set(valid_bundles)
+  invalid_bundles = set(bundle_names) - valid_bundles
+  if invalid_bundles:
+    logging.warn('Unknown bundle(s): %s\n' % (', '.join(invalid_bundles)))
+
+  for bundle_name in bundle_names:
+    if bundle_name not in valid_bundles:
+      continue
+
+    bundle = manifest.GetBundle(bundle_name)
+
+    print bundle.name
+    for key in sorted(bundle.iterkeys()):
+      value = bundle[key]
+      if key == manifest_util.ARCHIVES_KEY:
+        archive = bundle.GetHostOSArchive()
+        print '  Archive:'
+        if archive:
+          for archive_key in sorted(archive.iterkeys()):
+            print '    %s: %s' % (archive_key, archive[archive_key])
+        else:
+          print '    No archives for this host.'
+      elif key not in (manifest_util.ARCHIVES_KEY, manifest_util.NAME_KEY):
+        print '  %s: %s' % (key, value)
+    print
diff --git a/native_client_sdk/src/build_tools/sdk_tools/command/list.py b/native_client_sdk/src/build_tools/sdk_tools/command/list.py
new file mode 100644
index 0000000..da35058
--- /dev/null
+++ b/native_client_sdk/src/build_tools/sdk_tools/command/list.py
@@ -0,0 +1,48 @@
+# 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.
+
+def List(remote_manifest, local_manifest, display_revisions):
+  any_bundles_need_update = False
+  print 'Bundles:'
+  print ' I: installed\n *: update available\n'
+  for bundle in remote_manifest.GetBundles():
+    local_bundle = local_manifest.GetBundle(bundle.name)
+    needs_update = local_bundle and local_manifest.BundleNeedsUpdate(bundle)
+    if needs_update:
+      any_bundles_need_update = True
+
+    _PrintBundle(local_bundle, bundle, needs_update, display_revisions)
+
+  if not any_bundles_need_update:
+    print '\nAll installed bundles are up-to-date.'
+
+  local_only_bundles = set([b.name for b in local_manifest.GetBundles()])
+  local_only_bundles -= set([b.name for b in remote_manifest.GetBundles()])
+  if local_only_bundles:
+    print '\nBundles installed locally that are not available remotely:'
+    for bundle_name in local_only_bundles:
+      local_bundle = local_manifest.GetBundle(bundle_name)
+      _PrintBundle(local_bundle, None, False, display_revisions)
+
+
+def _PrintBundle(local_bundle, bundle, needs_update, display_revisions):
+  installed = local_bundle is not None
+  # If bundle is None, there is no longer a remote bundle with this name.
+  if bundle is None:
+    bundle = local_bundle
+
+  if display_revisions:
+    if needs_update:
+      revision = ' (r%s -> r%s)' % (local_bundle.revision, bundle.revision)
+    else:
+      revision = ' (r%s)' % (bundle.revision,)
+  else:
+    revision = ''
+
+  print ('  %s%s %s (%s)%s' % (
+    'I' if installed else ' ',
+    '*' if needs_update else ' ',
+    bundle.name,
+    bundle.stability,
+    revision))
diff --git a/native_client_sdk/src/build_tools/sdk_tools/command/sources.py b/native_client_sdk/src/build_tools/sdk_tools/command/sources.py
new file mode 100644
index 0000000..3a7c975
--- /dev/null
+++ b/native_client_sdk/src/build_tools/sdk_tools/command/sources.py
@@ -0,0 +1,29 @@
+# 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.
+
+import download
+from sdk_update_common import Error
+
+def AddSource(config, url):
+  try:
+    download.UrlOpen(url)
+  except Exception as e:
+    raise Error('Not adding %s, unable to load URL.\n  %s' % (url, e))
+  config.AddSource(url)
+
+
+def RemoveSource(config, url):
+  if url == 'all':
+    config.RemoveAllSources()
+  else:
+    config.RemoveSource(url)
+
+
+def ListSources(config):
+  if config.sources:
+    print 'Installed sources:'
+    for s in config.sources:
+      print '  ' + s
+  else:
+    print 'No external sources installed.'
diff --git a/native_client_sdk/src/build_tools/sdk_tools/command/update.py b/native_client_sdk/src/build_tools/sdk_tools/command/update.py
new file mode 100644
index 0000000..24389c9
--- /dev/null
+++ b/native_client_sdk/src/build_tools/sdk_tools/command/update.py
@@ -0,0 +1,238 @@
+# 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.
+
+import download
+import logging
+import os
+from sdk_update_common import Error
+import sdk_update_common
+import sys
+import urlparse
+import urllib2
+
+SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+PARENT_DIR = os.path.dirname(SCRIPT_DIR)
+sys.path.append(PARENT_DIR)
+try:
+  import cygtar
+except ImportError:
+  # Try to find this in the Chromium repo.
+  CHROME_SRC_DIR = os.path.abspath(
+      os.path.join(PARENT_DIR, '..', '..', '..', '..'))
+  sys.path.append(os.path.join(CHROME_SRC_DIR, 'native_client', 'build'))
+  import cygtar
+
+
+RECOMMENDED = 'recommended'
+SDK_TOOLS = 'sdk_tools'
+HTTP_CONTENT_LENGTH = 'Content-Length'  # HTTP Header field for content length
+
+
+class UpdateDelegate(object):
+  def BundleDirectoryExists(self, bundle_name):
+    raise NotImplementedError()
+
+  def DownloadToFile(self, url, dest_filename):
+    raise NotImplementedError()
+
+  def ExtractArchive(self, archive, extract_dir, rename_from_dir,
+                     rename_to_dir):
+    raise NotImplementedError()
+
+
+class RealUpdateDelegate(UpdateDelegate):
+  def __init__(self, user_data_dir, install_dir):
+    UpdateDelegate.__init__(self)
+    self.user_data_dir = user_data_dir
+    self.install_dir = install_dir
+
+  def BundleDirectoryExists(self, bundle_name):
+    bundle_path = os.path.join(self.install_dir, bundle_name)
+    return os.path.isdir(bundle_path)
+
+  def DownloadToFile(self, url, dest_filename):
+    sdk_update_common.MakeDirs(self.user_data_dir)
+    dest_path = os.path.join(self.user_data_dir, dest_filename)
+    out_stream = None
+    url_stream = None
+    try:
+      out_stream = open(dest_path, 'wb')
+      url_stream = download.UrlOpen(url)
+      content_length = int(url_stream.info()[HTTP_CONTENT_LENGTH])
+      progress = download.MakeProgressFunction(content_length)
+      sha1, size = download.DownloadAndComputeHash(url_stream, out_stream,
+                                                   progress)
+      return sha1, size
+    except urllib2.URLError as e:
+      raise Error('Unable to read from URL "%s".\n  %s' % (url, e))
+    except IOError as e:
+      raise Error('Unable to write to file "%s".\n  %s' % (dest_filename, e))
+    finally:
+      if url_stream:
+        url_stream.close()
+      if out_stream:
+        out_stream.close()
+
+  def ExtractArchive(self, archive, extract_dir, rename_from_dir,
+                     rename_to_dir):
+    tar_file = None
+
+    archive_path = os.path.join(self.user_data_dir, archive)
+    extract_path = os.path.join(self.install_dir, extract_dir)
+    rename_from_path = os.path.join(self.install_dir, rename_from_dir)
+    rename_to_path = os.path.join(self.install_dir, rename_to_dir)
+
+    # Extract to extract_dir, usually "<bundle name>_update".
+    # This way if the extraction fails, we haven't blown away the old bundle
+    # (if it exists).
+    sdk_update_common.RemoveDir(extract_path)
+    sdk_update_common.MakeDirs(extract_path)
+    curpath = os.getcwd()
+    tar_file = None
+
+    try:
+      try:
+        tar_file = cygtar.CygTar(archive_path, 'r', verbose=True)
+      except Exception as e:
+        raise Error('Can\'t open archive "%s".\n  %s' % (archive_path, e))
+
+      try:
+        logging.info('Changing the directory to %s' % (extract_path,))
+        os.chdir(extract_path)
+      except Exception as e:
+        raise Error('Unable to chdir into "%s".\n  %s' % (extract_path, e))
+
+      logging.info('Extracting to %s' % (extract_path,))
+      tar_file.Extract()
+
+      logging.info('Changing the directory to %s' % (curpath,))
+      os.chdir(curpath)
+
+      logging.info('Renaming %s->%s' % (rename_from_path, rename_to_path))
+      sdk_update_common.RenameDir(rename_from_path, rename_to_path)
+    finally:
+      # Change the directory back so we can remove the update directory.
+      os.chdir(curpath)
+
+      # Clean up the ..._update directory.
+      try:
+        sdk_update_common.RemoveDir(extract_path)
+      except Exception as e:
+        logging.error('Failed to remove directory \"%s\".  %s' % (
+            extract_path, e))
+
+      if tar_file:
+        tar_file.Close()
+
+    # Remove the archive.
+    os.remove(archive_path)
+
+
+def Update(delegate, remote_manifest, local_manifest, bundle_names, force):
+  valid_bundles = set([bundle.name for bundle in remote_manifest.GetBundles()])
+  requested_bundles = _GetRequestedBundlesFromArgs(remote_manifest,
+                                                   bundle_names)
+  invalid_bundles = requested_bundles - valid_bundles
+  if invalid_bundles:
+    logging.warn('Ignoring unknown bundle(s): %s' % (
+        ', '.join(invalid_bundles)))
+    requested_bundles -= invalid_bundles
+
+  if SDK_TOOLS in requested_bundles:
+    logging.warn('Updating sdk_tools happens automatically. '
+                 'Ignoring manual update request.')
+    requested_bundles.discard(SDK_TOOLS)
+
+  if requested_bundles:
+    for bundle_name in requested_bundles:
+      logging.info('Trying to update %s' % (bundle_name,))
+      UpdateBundleIfNeeded(delegate, remote_manifest, local_manifest,
+          bundle_name, force)
+  else:
+    logging.warn('No bundles to update.')
+
+
+def UpdateBundleIfNeeded(delegate, remote_manifest, local_manifest,
+                         bundle_name, force):
+  bundle = remote_manifest.GetBundle(bundle_name)
+  if bundle:
+    if _BundleNeedsUpdate(delegate, local_manifest, bundle):
+      _UpdateBundle(delegate, bundle, local_manifest, force)
+    else:
+      print '%s is already up-to-date.' % (bundle.name,)
+  else:
+    logging.error('Bundle %s does not exist.' % (bundle_name,))
+
+
+def _GetRequestedBundlesFromArgs(remote_manifest, requested_bundles):
+  requested_bundles = set(requested_bundles)
+  if RECOMMENDED in requested_bundles:
+    requested_bundles.discard(RECOMMENDED)
+    requested_bundles |= set(_GetRecommendedBundles(remote_manifest))
+
+  return requested_bundles
+
+
+def _GetRecommendedBundles(remote_manifest):
+  return [bundle for bundle in remote_manifest.GetBundles() if
+      bundle.recommended]
+
+
+def _BundleNeedsUpdate(delegate, local_manifest, bundle):
+  # Always update the bundle if the directory doesn't exist;
+  # the user may have deleted it.
+  if not delegate.BundleDirectoryExists(bundle.name):
+    return True
+
+  return local_manifest.BundleNeedsUpdate(bundle)
+
+
+def _UpdateBundle(delegate, bundle, local_manifest, force):
+  archive = bundle.GetHostOSArchive()
+  if not archive:
+    logging.warn('Bundle %s does not exist for this platform.' % (bundle.name,))
+    return
+
+  print 'Downloading bundle %s' % (bundle.name,)
+  dest_filename = _GetFilenameFromURL(archive.url)
+  sha1, size = delegate.DownloadToFile(archive.url, dest_filename)
+  _ValidateArchive(archive, sha1, size)
+
+  print 'Updating bundle %s to version %s, revision %s' % (
+      bundle.name, bundle.version, bundle.revision)
+  extract_dir = bundle.name + '_update'
+
+  repath_dir = bundle.get('repath', None)
+  if repath_dir:
+    # If repath is specified:
+    # The files are extracted to nacl_sdk/<bundle.name>_update/<repath>/...
+    # The destination directory is nacl_sdk/<repath>/...
+    rename_from_dir = os.path.join(extract_dir, repath_dir)
+    rename_to_dir = repath_dir
+  else:
+    # If no repath is specified:
+    # The files are extracted to nacl_sdk/<bundle.name>_update/...
+    # The destination directory is nacl_sdk/<bundle.name>/...
+    rename_from_dir = extract_dir
+    rename_to_dir = bundle.name
+
+  delegate.ExtractArchive(dest_filename, extract_dir, rename_from_dir,
+                          rename_to_dir)
+
+  logging.info('Updating local manifest to include bundle %s' % (bundle.name))
+  local_manifest.MergeBundle(bundle)
+
+
+def _GetFilenameFromURL(url):
+  _, _, path, _, _, _ = urlparse.urlparse(url)
+  return path.split('/')[-1]
+
+
+def _ValidateArchive(archive, actual_sha1, actual_size):
+  if actual_sha1 != archive.GetChecksum():
+    raise Error('SHA1 checksum mismatch on "%s".  Expected %s but got %s' % (
+        archive.name, archive.GetChecksum(), actual_sha1))
+  if actual_size != archive.size:
+    raise Error('Size mismatch on "%s".  Expected %s but got %s bytes' % (
+        archive.name, archive.size, actual_size))
diff --git a/native_client_sdk/src/build_tools/sdk_tools/config.py b/native_client_sdk/src/build_tools/sdk_tools/config.py
new file mode 100644
index 0000000..a2e69d7
--- /dev/null
+++ b/native_client_sdk/src/build_tools/sdk_tools/config.py
@@ -0,0 +1,62 @@
+# 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.
+
+import json
+import logging
+import urlparse
+
+SOURCE_WHITELIST = [
+  'https://commondatastorage.googleapis.com/nativeclient-mirror/nacl/nacl_sdk',
+]
+
+def IsSourceValid(url):
+  scheme, host, path, _, _, _ = urlparse.urlparse(url)
+  for allowed_url in SOURCE_WHITELIST:
+    allowed_scheme, allowed_host, allowed_path_prefix, _, _, _ = \
+        urlparse.urlparse(allowed_url)
+    if (scheme == allowed_scheme and host == allowed_host and
+        path.startswith(allowed_path_prefix)):
+      return True
+  return False
+
+
+class Config(dict):
+  def __init__(self, data=None):
+    dict.__init__(self)
+    if data:
+      self.update(data)
+    else:
+      self.sources = []
+
+  def ToJson(self):
+    return json.dumps(self, sort_keys=False, indent=2)
+
+  def __getattr__(self, name):
+    return self.__getitem__(name)
+
+  def __setattr__(self, name, value):
+    return self.__setitem__(name, value)
+
+  def AddSource(self, source):
+    if not IsSourceValid(source):
+      logging.warn('Only whitelisted sources are allowed. Ignoring \"%s\".' % (
+          source,))
+      return
+
+    if source in self.sources:
+      logging.info('Source \"%s\" already in Config.' % (source,))
+      return
+    self.sources.append(source)
+
+  def RemoveSource(self, source):
+    if source not in self.sources:
+      logging.warn('Source \"%s\" not in Config.' % (source,))
+      return
+    self.sources.remove(source)
+
+  def RemoveAllSources(self):
+    if not self.sources:
+      logging.info('No sources to remove.')
+      return
+    self.sources = []
diff --git a/native_client_sdk/src/build_tools/sdk_tools/download.py b/native_client_sdk/src/build_tools/sdk_tools/download.py
new file mode 100644
index 0000000..4f2cb3b
--- /dev/null
+++ b/native_client_sdk/src/build_tools/sdk_tools/download.py
@@ -0,0 +1,82 @@
+# 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.
+
+import hashlib
+import os
+import sys
+# when pylint runs the third_party module is the one from depot_tools
+# pylint: disable=E0611
+from third_party import fancy_urllib
+import urllib2
+
+
+SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+
+
+def UrlOpen(url):
+  request = fancy_urllib.FancyRequest(url)
+  ca_certs = os.path.join(SCRIPT_DIR, 'cacerts.txt')
+  request.set_ssl_info(ca_certs=ca_certs)
+  url_opener = urllib2.build_opener(
+      fancy_urllib.FancyProxyHandler(),
+      fancy_urllib.FancyRedirectHandler(),
+      fancy_urllib.FancyHTTPSHandler())
+  return url_opener.open(request)
+
+
+def MakeProgressFunction(file_size=0):
+  # An inner function can only read nonlocal variables, not assign them. We can
+  # work around this by using a list of one element.
+  dots = [0]
+  def ShowKnownProgress(progress):
+    '''Returns a progress function based on a known file size'''
+    if progress == 0:
+      sys.stdout.write('|%s|\n' % ('=' * 48))
+    else:
+      new_dots = progress * 50 / file_size - dots[0]
+      sys.stdout.write('.' * new_dots)
+      dots[0] += new_dots
+      if progress == file_size:
+        sys.stdout.write('\n')
+    sys.stdout.flush()
+
+  return ShowKnownProgress
+
+
+def DownloadAndComputeHash(from_stream, to_stream=None, progress_func=None):
+  '''Read from from-stream and generate sha1 and size info.
+
+  Args:
+    from_stream:   An input stream that supports read.
+    to_stream:     [optional] the data is written to to_stream if it is
+                   provided.
+    progress_func: [optional] A function used to report download progress. If
+                   provided, progress_func is called with progress=0 at the
+                   beginning of the download, periodically with progress=1
+                   during the download, and progress=100 at the end.
+
+  Return
+    A tuple (sha1, size) where sha1 is a sha1-hash for the archive data and
+    size is the size of the archive data in bytes.'''
+  # Use a no-op progress function if none is specified.
+  def progress_no_op(progress):
+    pass
+  if not progress_func:
+    progress_func = progress_no_op
+
+  sha1_hash = hashlib.sha1()
+  size = 0
+  progress_func(progress=0)
+  while(1):
+    data = from_stream.read(32768)
+    if not data:
+      break
+    sha1_hash.update(data)
+    size += len(data)
+    if to_stream:
+      to_stream.write(data)
+    progress_func(size)
+
+  progress_func(progress=100)
+  return sha1_hash.hexdigest(), size
diff --git a/native_client_sdk/src/build_tools/sdk_tools/sdk_update.py b/native_client_sdk/src/build_tools/sdk_tools/sdk_update.py
index 45c72f4..a28c4b9 100755
--- a/native_client_sdk/src/build_tools/sdk_tools/sdk_update.py
+++ b/native_client_sdk/src/build_tools/sdk_tools/sdk_update.py
@@ -52,7 +52,7 @@ def UpdateSDKTools(args):
   Returns:
     True if the sdk_tools bundle was updated.
   """
-  cmd = MakeSdkUpdateMainCmd(['--update-sdk-tools'] + args)
+  cmd = MakeSdkUpdateMainCmd(args + ['--update-sdk-tools'])
   process = subprocess.Popen(cmd, stdout=subprocess.PIPE)
   stdout, _ = process.communicate()
   if process.returncode == 0:
diff --git a/native_client_sdk/src/build_tools/sdk_tools/sdk_update_common.py b/native_client_sdk/src/build_tools/sdk_tools/sdk_update_common.py
index ad4175e..aa407c8 100644
--- a/native_client_sdk/src/build_tools/sdk_tools/sdk_update_common.py
+++ b/native_client_sdk/src/build_tools/sdk_tools/sdk_update_common.py
@@ -5,6 +5,7 @@
 """Utility functions for sdk_update.py and sdk_update_main.py."""
 
 import errno
+import logging
 import os
 import shutil
 import subprocess
@@ -17,6 +18,12 @@ class Error(Exception):
   pass
 
 
+def MakeDirs(directory):
+  if not os.path.exists(directory):
+    logging.info('Making directory %s' % (directory,))
+    os.makedirs(directory)
+
+
 def RemoveDir(outdir):
   """Removes the given directory
 
@@ -30,18 +37,46 @@ def RemoveDir(outdir):
     outdir: The directory to delete
 
   Raises:
-    CalledProcessError - if the delete operation fails on Windows
-    OSError - if the delete operation fails on Linux
+    Error - If this operation fails for any reason.
   """
 
-  try:
-    shutil.rmtree(outdir)
-  except OSError:
-    if not os.path.exists(outdir):
+  max_tries = 5
+  last_exception = None
+  for num_tries in xrange(max_tries):
+    try:
+      shutil.rmtree(outdir)
       return
-    # On Windows this could be an issue with junctions, so try again with rmdir
+    except OSError as e:
+      if not os.path.exists(outdir):
+        # The directory can't be removed because it doesn't exist.
+        return
+      last_exception = e
+
+    # On Windows this could be an issue with junctions, so try again with
+    # rmdir.
     if sys.platform == 'win32':
-      subprocess.check_call(['rmdir', '/S', '/Q', outdir], shell=True)
+      try:
+        cmd = ['rmdir', '/S', '/Q', outdir]
+        process = subprocess.Popen(cmd, stderr=subprocess.PIPE, shell=True)
+        _, stderr = process.communicate()
+        if process.returncode != 0:
+          raise Error('\"%s\" failed with code %d. Output:\n  %s' % (
+            ' '.join(cmd), process.returncode, stderr))
+        return
+        # Ignore failures, we'll just try again.
+      except subprocess.CalledProcessError as e:
+        # CalledProcessError has no error message, generate one.
+        last_exception = Error('\"%s\" failed with code %d.' % (
+          ' '.join(e.cmd), e.returncode))
+      except Error as e:
+        last_exception = e
+
+    # Didn't work, sleep and try again.
+    time.sleep(num_tries + 1)
+
+  # Failed.
+  raise Error('Unable to remove directory "%s"\n  %s' % (outdir,
+                                                         last_exception))
 
 
 def RenameDir(srcdir, destdir):
@@ -67,4 +102,4 @@ def RenameDir(srcdir, destdir):
   raise Error('Could not RenameDir %s => %s after %d tries.\n'
               'Please check that no shells or applications '
               'are accessing files in %s.'
-              % (srcdir, destdir, num_tries, destdir))
+              % (srcdir, destdir, num_tries + 1, destdir))
diff --git a/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py b/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py
index c16af4a..44fa811 100755
--- a/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py
+++ b/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py
@@ -3,760 +3,377 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-'''A simple tool to update the Native Client SDK to the latest version'''
+# CMD code copied from git_cl.py in depot_tools.
 
+import config
 import cStringIO
-import cygtar
+import download
 import json
-import manifest_util
+import logging
 import optparse
 import os
-from sdk_update_common import RenameDir, RemoveDir, Error
-import shutil
-import subprocess
+import re
+import sdk_update_common
+from sdk_update_common import Error
 import sys
-import tempfile
-# when pylint runs the third_party module is the one from depot_tools
-# pylint: disable=E0611
-from third_party import fancy_urllib
 import urllib2
-import urlparse
-
-# pylint: disable=C0301
 
-#------------------------------------------------------------------------------
-# Constants
+SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+PARENT_DIR = os.path.dirname(SCRIPT_DIR)
 
-# This revision number is autogenerated from the Chrome revision.
-REVISION = '{REVISION}'
-
-GLOBAL_HELP = '''Usage: naclsdk [options] command [command_options]
+sys.path.append(os.path.dirname(SCRIPT_DIR))
+import manifest_util
 
-naclsdk is a simple utility that updates the Native Client (NaCl)
-Software Developer's Kit (SDK).  Each component is kept as a 'bundle' that
-this utility can download as as subdirectory into the SDK.
 
-Commands:
-  help [command] - Get either general or command-specific help
-  info - Displays information about a bundle
-  list - Lists the available bundles
-  update/install - Updates/installs bundles in the SDK
-  sources - Manage external package sources
+# Import late so each command script can find our imports
+import command.info
+import command.list
+import command.sources
+import command.update
 
-Example Usage:
-  naclsdk info pepper_canary
-  naclsdk list
-  naclsdk update --force pepper_17
-  naclsdk install recommended
-  naclsdk help update
-  naclsdk sources --list'''
+# This revision number is autogenerated from the Chrome revision.
+REVISION = '{REVISION}'
 
+GSTORE_URL = 'https://commondatastorage.googleapis.com/nativeclient-mirror'
 CONFIG_FILENAME = 'naclsdk_config.json'
 MANIFEST_FILENAME = 'naclsdk_manifest2.json'
-SDK_TOOLS = 'sdk_tools'  # the name for this tools directory
-USER_DATA_DIR = 'sdk_cache'
-
-HTTP_CONTENT_LENGTH = 'Content-Length'  # HTTP Header field for content length
-
-
-#------------------------------------------------------------------------------
-# General Utilities
-
-
-_debug_mode = False
-_quiet_mode = False
+DEFAULT_SDK_ROOT = os.path.abspath(PARENT_DIR)
+USER_DATA_DIR = os.path.join(DEFAULT_SDK_ROOT, 'sdk_cache')
 
 
-def DebugPrint(msg):
-  '''Display a message to stderr if debug printing is enabled
+def usage(more):
+  def hook(fn):
+    fn.usage_more = more
+    return fn
+  return hook
 
-  Note: This function appends a newline to the end of the string
 
-  Args:
-    msg: A string to send to stderr in debug mode'''
-  if _debug_mode:
-    sys.stderr.write("%s\n" % msg)
-    sys.stderr.flush()
+def hide(fn):
+  fn.hide = True
+  return fn
 
 
-def InfoPrint(msg):
-  '''Display an informational message to stdout if not in quiet mode
-
-  Note: This function appends a newline to the end of the string
-
-  Args:
-    mgs: A string to send to stdio when not in quiet mode'''
-  if not _quiet_mode:
-    sys.stdout.write("%s\n" % msg)
-    sys.stdout.flush()
-
+def LoadConfig(raise_on_error=False):
+  path = os.path.join(USER_DATA_DIR, CONFIG_FILENAME)
+  if not os.path.exists(path):
+    return config.Config()
 
-def WarningPrint(msg):
-  '''Display an informational message to stderr.
+  try:
+    try:
+      with open(path) as f:
+        return config.Config(json.loads(f.read()))
+    except IOError as e:
+      raise Error('Unable to read config from "%s".\n  %s' % (path, e))
+    except Exception as e:
+      raise Error('Parsing config file from "%s" failed.\n  %s' % (path, e))
+  except Error as e:
+    if raise_on_error:
+      raise
+    else:
+      logging.warn(str(e))
+  return config.Config()
 
-  Note: This function appends a newline to the end of the string
 
-  Args:
-    mgs: A string to send to stderr.'''
-  sys.stderr.write("WARNING: %s\n" % msg)
-  sys.stderr.flush()
+def WriteConfig(cfg):
+  path = os.path.join(USER_DATA_DIR, CONFIG_FILENAME)
+  try:
+    sdk_update_common.MakeDirs(USER_DATA_DIR)
+  except Exception as e:
+    raise Error('Unable to create directory "%s".\n  %s' % (USER_DATA_DIR, e))
 
+  try:
+    cfg_json = cfg.ToJson()
+  except Exception as e:
+    raise Error('Json encoding error writing config "%s".\n  %s' % (path, e))
 
-def UrlOpen(url):
-  request = fancy_urllib.FancyRequest(url)
-  ca_certs = os.path.join(os.path.dirname(os.path.abspath(__file__)),
-                          'cacerts.txt')
-  request.set_ssl_info(ca_certs=ca_certs)
-  url_opener = urllib2.build_opener(
-      fancy_urllib.FancyProxyHandler(),
-      fancy_urllib.FancyRedirectHandler(),
-      fancy_urllib.FancyHTTPSHandler())
-  return url_opener.open(request)
+  try:
+    with open(path, 'w') as f:
+      f.write(cfg_json)
+  except IOError as e:
+    raise Error('Unable to write config to "%s".\n  %s' % (path, e))
 
-def ExtractInstaller(installer, outdir):
-  '''Extract the SDK installer into a given directory
 
-  If the outdir already exists, then this function deletes it
+def LoadLocalManifest(raise_on_error=False):
+  path = os.path.join(USER_DATA_DIR, MANIFEST_FILENAME)
+  manifest = manifest_util.SDKManifest()
+  try:
+    try:
+      with open(path) as f:
+        manifest_string = f.read()
+    except IOError as e:
+      raise Error('Unable to read manifest from "%s".\n  %s' % (path, e))
 
-  Args:
-    installer: full path of the SDK installer
-    outdir: output directory where to extract the installer
+    try:
+      manifest.LoadDataFromString(manifest_string)
+    except Exception as e:
+      raise Error('Parsing local manifest "%s" failed.\n  %s' % (path, e))
+  except Error as e:
+    if raise_on_error:
+      raise
+    else:
+      logging.warn(str(e))
+  return manifest
 
-  Raises:
-    CalledProcessError - if the extract operation fails'''
-  RemoveDir(outdir)
 
-  if os.path.splitext(installer)[1] == '.exe':
-    # If the installer has extension 'exe', assume it's a Windows NSIS-style
-    # installer that handles silent (/S) and relocated (/D) installs.
-    command = [installer, '/S', '/D=%s' % outdir]
-    subprocess.check_call(command)
-  else:
-    os.mkdir(outdir)
-    tar_file = None
-    curpath = os.getcwd()
-    try:
-      tar_file = cygtar.CygTar(installer, 'r', verbose=True)
-      if outdir:
-        os.chdir(outdir)
-      tar_file.Extract()
-    finally:
-      if tar_file:
-        tar_file.Close()
-      os.chdir(curpath)
-
-
-class ProgressFunction(object):
-  '''Create a progress function for a file with a given size'''
-
-  def __init__(self, file_size=0):
-    '''Constructor
-
-    Args:
-      file_size: number of bytes in file.  0 indicates unknown'''
-    self.dots = 0
-    self.file_size = int(file_size)
-
-  def GetProgressFunction(self):
-    '''Returns a progress function based on a known file size'''
-    def ShowKnownProgress(progress):
-      if progress == 0:
-        sys.stdout.write('|%s|\n' % ('=' * 48))
-      else:
-        new_dots = progress * 50 / self.file_size - self.dots
-        sys.stdout.write('.' * new_dots)
-        self.dots += new_dots
-        if progress == self.file_size:
-          sys.stdout.write('\n')
-      sys.stdout.flush()
-
-    return ShowKnownProgress
-
-
-def DownloadArchiveToFile(archive, dest_path):
-  '''Download the archive's data to a file at dest_path.
-
-  As a side effect, computes the sha1 hash and data size, both returned as a
-  tuple. Raises an Error if the url can't be opened, or an IOError exception if
-  dest_path can't be opened.
-
-  Args:
-    dest_path: Path for the file that will receive the data.
-  Return:
-    A tuple (sha1, size) with the sha1 hash and data size respectively.'''
-  sha1 = None
-  size = 0
-  with open(dest_path, 'wb') as to_stream:
-    from_stream = None
-    try:
-      from_stream = UrlOpen(archive.url)
-    except urllib2.URLError:
-      raise Error('Cannot open "%s" for archive %s' %
-          (archive.url, archive.host_os))
-    try:
-      content_length = int(from_stream.info()[HTTP_CONTENT_LENGTH])
-      progress_function = ProgressFunction(content_length).GetProgressFunction()
-      InfoPrint('Downloading %s' % archive.url)
-      sha1, size = manifest_util.DownloadAndComputeHash(
-          from_stream,
-          to_stream=to_stream,
-          progress_func=progress_function)
-      if size != content_length:
-        raise Error('Download size mismatch for %s.\n'
-                    'Expected %s bytes but got %s' %
-                    (archive.url, content_length, size))
-    finally:
-      if from_stream:
-        from_stream.close()
-  return sha1, size
-
-
-def LoadFromFile(path, obj):
-  '''Returns a manifest loaded from the JSON file at |path|.
-
-  If the path does not exist or is invalid, returns unmodified object.'''
-  methodlist = [m for m in dir(obj) if callable(getattr(obj, m))]
-  if 'LoadDataFromString' not in methodlist:
-    return obj
-  if not os.path.exists(path):
-    return obj
+def WriteLocalManifest(manifest):
+  path = os.path.join(USER_DATA_DIR, MANIFEST_FILENAME)
+  try:
+    sdk_update_common.MakeDirs(USER_DATA_DIR)
+  except Exception as e:
+    raise Error('Unable to create directory "%s".\n  %s' % (USER_DATA_DIR, e))
 
-  with open(path, 'r') as f:
-    json_string = f.read()
-  if not json_string:
-    return obj
+  try:
+    manifest_json = manifest.GetDataAsString()
+  except Exception as e:
+    raise Error('Error encoding manifest "%s" to JSON.\n  %s' % (path, e))
 
-  obj.LoadDataFromString(json_string)
-  return obj
+  try:
+    with open(path, 'w') as f:
+      f.write(manifest_json)
+  except IOError as e:
+    raise Error('Unable to write manifest to "%s".\n  %s' % (path, e))
 
 
-def LoadManifestFromURLs(urls):
-  '''Returns a manifest loaded from |urls|, merged into one manifest.'''
+def LoadRemoteManifest(url):
   manifest = manifest_util.SDKManifest()
-  for url in urls:
-    try:
-      url_stream = UrlOpen(url)
-    except urllib2.URLError as e:
-      raise Error('Unable to open %s. [%s]' % (url, e))
-
+  url_stream = None
+  try:
     manifest_stream = cStringIO.StringIO()
-    manifest_util.DownloadAndComputeHash(url_stream, manifest_stream)
-    temp_manifest = manifest_util.SDKManifest()
-    temp_manifest.LoadDataFromString(manifest_stream.getvalue())
+    url_stream = download.UrlOpen(url)
+    download.DownloadAndComputeHash(url_stream, manifest_stream)
+  except urllib2.URLError as e:
+    raise Error('Unable to read remote manifest from URL "%s".\n  %s' % (
+        url, e))
+  finally:
+    if url_stream:
+      url_stream.close()
 
-    manifest.MergeManifest(temp_manifest)
+  try:
+    manifest.LoadDataFromString(manifest_stream.getvalue())
+    return manifest
+  except manifest_util.Error as e:
+    raise Error('Parsing remote manifest from URL "%s" failed.\n  %s' % (
+        url, e,))
 
-  def BundleFilter(bundle):
-    # Only add this bundle if it's supported on this platform.
-    return bundle.GetHostOSArchive()
 
-  manifest.FilterBundles(BundleFilter)
+def LoadCombinedRemoteManifest(default_manifest_url, cfg):
+  manifest = LoadRemoteManifest(default_manifest_url)
+  for source in cfg.sources:
+    manifest.MergeManifest(LoadRemoteManifest(source))
   return manifest
 
 
-def WriteToFile(path, obj):
-  '''Write |manifest| to a JSON file at |path|.'''
-  methodlist = [m for m in dir(obj) if callable(getattr(obj, m))]
-  if 'GetDataAsString' not in methodlist:
-    raise Error('Unable to write object to file')
-  json_string = obj.GetDataAsString()
-
-  # Write the JSON data to a temp file.
-  temp_file_name = None
-  # TODO(dspringer): Use file locks here so that multiple sdk_updates can
-  # run at the same time.
-  with tempfile.NamedTemporaryFile(mode='w', delete=False) as f:
-    f.write(json_string)
-    temp_file_name = f.name
-  # Move the temp file to the actual file.
-  if os.path.exists(path):
-    os.remove(path)
-  shutil.move(temp_file_name, path)
-
-
-class SDKConfig(object):
-  '''This class contains utilities for manipulating an SDK config
-  '''
-
-  def __init__(self):
-    '''Create a new SDKConfig object with default contents'''
-    self._data = {
-        'sources': [],
-        }
-
-  def AddSource(self, string):
-    '''Add a source file to load packages from.
-
-    Args:
-      string: a URL to an external package manifest file.'''
-    # For now whitelist only the following location for external sources:
-    # https://commondatastorage.googleapis.com/nativeclient-mirror/nacl/nacl_sdk
-    (scheme, host, path, _, _, _) = urlparse.urlparse(string)
-    if (host != 'commondatastorage.googleapis.com' or
-        scheme != 'https' or
-        not path.startswith('/nativeclient-mirror/nacl/nacl_sdk')):
-      WarningPrint('Only whitelisted sources from '
-                   '\'https://commondatastorage.googleapis.com/nativeclient-'
-                   'mirror/nacl/nacl_sdk\' are currently allowed.')
-      return
-    if string in self._data['sources']:
-      WarningPrint('source \''+string+'\' already exists in config.')
-      return
-    try:
-      UrlOpen(string)
-    except urllib2.URLError:
-      WarningPrint('Unable to fetch manifest URL \'%s\'. Exiting...' % string)
-      return
-
-    self._data['sources'].append(string)
-    InfoPrint('source \''+string+'\' added to config.')
-
-  def RemoveSource(self, string):
-    '''Remove a source file to load packages from.
+# Commands #####################################################################
 
-    Args:
-      string: a URL to an external SDK manifest file.'''
-    if string not in self._data['sources']:
-      WarningPrint('source \''+string+'\' doesn\'t exist in config.')
-    else:
-      self._data['sources'].remove(string)
-      InfoPrint('source \''+string+'\' removed from config.')
-
-  def RemoveAllSources(self):
-    if len(self.GetSources()) == 0:
-      InfoPrint('There are no external sources to remove.')
-    # Copy the list because RemoveSource modifies the underlying list
-    sources = list(self.GetSources())
-    for source in sources:
-      self.RemoveSource(source)
-
-
-  def ListSources(self):
-    '''List all external sources in config.'''
-    if len(self._data['sources']):
-      InfoPrint('Installed sources:')
-      for s in self._data['sources']:
-        InfoPrint('  '+s)
-    else:
-      InfoPrint('No external sources installed')
-
-  def GetSources(self):
-    '''Return a list of external sources'''
-    return self._data['sources']
 
-  def LoadDataFromString(self, string):
-    ''' Load a JSON config string. Raises an exception if string
-        is not well-formed JSON.
+@usage('<bundle names...>')
+def CMDinfo(parser, args):
+  """display information about a bundle"""
+  options, args = parser.parse_args(args)
+  if len(args) == 0:
+    parser.error('No bundles given')
+    return 0
+  cfg = LoadConfig()
+  remote_manifest = LoadCombinedRemoteManifest(options.manifest_url, cfg)
+  command.info.Info(remote_manifest, args)
+  return 0
+
+
+def CMDlist(parser, args):
+  """list all available bundles"""
+  parser.add_option('-r', '--revision', action='store_true',
+                    help='display revision numbers')
+  options, args = parser.parse_args(args)
+  if args:
+    parser.error('Unsupported argument(s): %s' % ', '.join(args))
+  local_manifest = LoadLocalManifest()
+  cfg = LoadConfig()
+  remote_manifest = LoadCombinedRemoteManifest(options.manifest_url, cfg)
+  command.list.List(remote_manifest, local_manifest, options.revision)
+  return 0
+
+
+@usage('<bundle names...>')
+def CMDupdate(parser, args):
+  """update a bundle in the SDK to the latest version"""
+  parser.add_option(
+      '-F', '--force', action='store_true',
+      help='Force updating existing components that already exist')
+  options, args = parser.parse_args(args)
+  local_manifest = LoadLocalManifest()
+  cfg = LoadConfig()
+  remote_manifest = LoadCombinedRemoteManifest(options.manifest_url, cfg)
 
-    Args:
-      string: a JSON-formatted string containing the previous config'''
-    self._data = json.loads(string)
+  if not args:
+    args = [command.update.RECOMMENDED]
 
+  try:
+    delegate = command.update.RealUpdateDelegate(USER_DATA_DIR,
+                                                  DEFAULT_SDK_ROOT)
+    command.update.Update(delegate, remote_manifest, local_manifest, args,
+                           options.force)
+  finally:
+    # Always write out the local manifest, we may have successfully updated one
+    # or more bundles before failing.
+    try:
+      WriteLocalManifest(local_manifest)
+    except Error as e:
+      # Log the error writing to the manifest, but propagate the original
+      # exception.
+      logging.error(str(e))
 
-  def GetDataAsString(self):
-    '''Returns the current JSON manifest object, pretty-printed'''
-    pretty_string = json.dumps(self._data, sort_keys=False, indent=2)
-    # json.dumps sometimes returns trailing whitespace and does not put
-    # a newline at the end.  This code fixes these problems.
-    pretty_lines = pretty_string.split('\n')
-    return '\n'.join([line.rstrip() for line in pretty_lines]) + '\n'
+  return 0
 
 
-#------------------------------------------------------------------------------
-# Commands
+def CMDinstall(parser, args):
+  """install a bundle in the SDK"""
+  # For now, forward to CMDupdate. We may want different behavior for this
+  # in the future, though...
+  return CMDupdate(parser, args)
 
 
-def Info(options, argv, config):
-  '''Usage: %prof [global_options] info [options] bundle_names...
+def CMDsources(parser, args):
+  """manage external package sources"""
+  parser.add_option('-a', '--add', dest='url_to_add',
+                    help='Add an additional package source')
+  parser.add_option(
+      '-r', '--remove', dest='url_to_remove',
+      help='Remove package source (use \'all\' for all additional sources)')
+  parser.add_option('-l', '--list', dest='do_list', action='store_true',
+                    help='List additional package sources')
+  options, args = parser.parse_args(args)
 
-  Displays information about a SDK bundle.'''
+  cfg = LoadConfig(True)
+  write_config = False
+  if options.url_to_add:
+    command.sources.AddSource(cfg, options.url_to_add)
+    write_config = True
+  elif options.url_to_remove:
+    command.sources.RemoveSource(cfg, options.url_to_remove)
+    write_config = True
+  elif options.do_list:
+    command.sources.ListSources(cfg)
+  else:
+    parser.print_help()
 
-  DebugPrint("Running List command with: %s, %s" %(options, argv))
+  if write_config:
+    WriteConfig(cfg)
 
-  parser = optparse.OptionParser(usage=Info.__doc__)
-  (_, args) = parser.parse_args(argv)
+  return 0
 
-  if not args:
-    parser.print_help()
-    return
-
-  manifest = LoadManifestFromURLs([options.manifest_url] + config.GetSources())
-  valid_bundles = [bundle.name for bundle in manifest.GetBundles()]
-  valid_args = set(args) & set(valid_bundles)
-  invalid_args = set(args) - valid_args
-  if invalid_args:
-    InfoPrint('Unknown bundle(s): %s\n' % (', '.join(invalid_args)))
-
-  for bundle_name in args:
-    if bundle_name not in valid_args:
-      continue
-
-    bundle = manifest.GetBundle(bundle_name)
-
-    InfoPrint('%s' % bundle.name)
-    for key, value in bundle.iteritems():
-      if key == manifest_util.ARCHIVES_KEY:
-        archive = bundle.GetHostOSArchive()
-        InfoPrint('  Archive:')
-        for archive_key, archive_value in archive.iteritems():
-          InfoPrint('    %s: %s' % (archive_key, archive_value))
-      elif key not in (manifest_util.ARCHIVES_KEY, manifest_util.NAME_KEY):
-        InfoPrint('  %s: %s' % (key, value))
-    InfoPrint('')
-
-
-def List(options, argv, config):
-  '''Usage: %prog [global_options] list [options]
-
-  Lists the available SDK bundles that are available for download.'''
-
-  def PrintBundle(local_bundle, bundle, needs_update, display_revisions):
-    installed = local_bundle is not None
-    # If bundle is None, there is no longer a remote bundle with this name.
-    if bundle is None:
-      bundle = local_bundle
-
-    if display_revisions:
-      if needs_update:
-        revision = ' (r%s -> r%s)' % (local_bundle.revision, bundle.revision)
-      else:
-        revision = ' (r%s)' % (bundle.revision,)
-    else:
-      revision = ''
 
-    InfoPrint('  %s%s %s (%s)%s' % (
-      'I' if installed else ' ',
-      '*' if needs_update else ' ',
-      bundle.name,
-      bundle.stability,
-      revision))
+def CMDversion(parser, args):
+  """display version information"""
+  _, _ = parser.parse_args(args)
+  print "Native Client SDK Updater, version r%s" % REVISION
+  return 0
 
 
-  DebugPrint("Running List command with: %s, %s" %(options, argv))
+def CMDhelp(parser, args):
+  """print list of commands or help for a specific command"""
+  _, args = parser.parse_args(args)
+  if len(args) == 1:
+    return main(args + ['--help'])
+  parser.print_help()
+  return 0
 
-  parser = optparse.OptionParser(usage=List.__doc__)
-  parser.add_option(
-      '-r', '--revision', dest='revision',
-      default=False, action='store_true',
-      help='display revision numbers')
-  (list_options, _) = parser.parse_args(argv)
-
-  manifest = LoadManifestFromURLs([options.manifest_url] + config.GetSources())
-  manifest_path = os.path.join(options.user_data_dir, options.manifest_filename)
-  local_manifest = LoadFromFile(manifest_path, manifest_util.SDKManifest())
-
-  any_bundles_need_update = False
-  InfoPrint('Bundles:')
-  InfoPrint(' I: installed\n *: update available\n')
-  for bundle in manifest.GetBundles():
-    local_bundle = local_manifest.GetBundle(bundle.name)
-    needs_update = local_bundle and local_manifest.BundleNeedsUpdate(bundle)
-    if needs_update:
-      any_bundles_need_update = True
-
-    PrintBundle(local_bundle, bundle, needs_update, list_options.revision)
-
-  if not any_bundles_need_update:
-    InfoPrint('\nAll installed bundles are up-to-date.')
-
-  local_only_bundles = set([b.name for b in local_manifest.GetBundles()])
-  local_only_bundles -= set([b.name for b in manifest.GetBundles()])
-  if local_only_bundles:
-    InfoPrint('\nBundles installed locally that are not available remotely:')
-    for bundle_name in local_only_bundles:
-      local_bundle = local_manifest.GetBundle(bundle_name)
-      PrintBundle(local_bundle, None, False, list_options.revision)
-
-
-def Update(options, argv, config):
-  '''Usage: %prog [global_options] update [options] [target]
-
-  Updates the Native Client SDK to a specified version.  By default, this
-  command updates all the recommended components.  The update process works
-  like this:
-    1. Fetch the manifest from the mirror.
-    2. Load manifest from USER_DATA_DIR - if there is no local manifest file,
-       make an empty manifest object.
-    3. Update each the bundle:
-      for bundle in bundles:
-        # Compare bundle versions & revisions.
-        # Test if local version.revision < mirror OR local doesn't exist.
-        if local_manifest < mirror_manifest:
-          update(bundle)
-          update local_manifest with mirror_manifest for bundle
-          write manifest to disk.  Use locks.
-        else:
-          InfoPrint('bundle is up-to-date')
-
-  Targets:
-    recommended: (default) Install/Update all recommended components
-    all:         Install/Update all available components
-    bundle_name: Install/Update only the given bundle
-  '''
-  DebugPrint("Running Update command with: %s, %s" % (options, argv))
-  ALL = 'all'  # Update all bundles
-  RECOMMENDED = 'recommended'  # Only update the bundles with recommended=yes
-
-  parser = optparse.OptionParser(usage=Update.__doc__)
-  parser.add_option(
-      '-F', '--force', dest='force',
-      default=False, action='store_true',
-      help='Force updating existing components that already exist')
-  (update_options, args) = parser.parse_args(argv)
 
-  if len(args) == 0:
-    args = [RECOMMENDED]
-
-  manifest = LoadManifestFromURLs([options.manifest_url] + config.GetSources())
-  bundles = manifest.GetBundles()
-  local_manifest_path = os.path.join(options.user_data_dir,
-                                     options.manifest_filename)
-  local_manifest = LoadFromFile(local_manifest_path,
-                                manifest_util.SDKManifest())
-
-  # Validate the arg list against the available bundle names.  Raises an
-  # error if any invalid bundle names or args are detected.
-  valid_args = set([ALL, RECOMMENDED] + [bundle.name for bundle in bundles])
-  bad_args = set(args) - valid_args
-  if len(bad_args) > 0:
-    raise Error("Unrecognized bundle name or argument: '%s'" %
-                ', '.join(bad_args))
-
-  if SDK_TOOLS in args and not options.update_sdk_tools:
-    # We only want sdk_tools to be updated by sdk_update.py. If the user
-    # tries to update directly, we just ignore the request.
-    InfoPrint('Updating sdk_tools happens automatically.\n'
-        'Ignoring manual update request.')
-    args.remove(SDK_TOOLS)
-
-  for bundle in bundles:
-    bundle_path = os.path.join(options.sdk_root_dir, bundle.name)
-    bundle_update_path = '%s_update' % bundle_path
-    if not (bundle.name in args or
-            ALL in args or (RECOMMENDED in args and
-                            bundle[RECOMMENDED] == 'yes')):
-      continue
-
-    if bundle.name == SDK_TOOLS and not options.update_sdk_tools:
-      continue
-
-    def UpdateBundle():
-      '''Helper to install a bundle'''
-      archive = bundle.GetHostOSArchive()
-      (_, _, path, _, _, _) = urlparse.urlparse(archive['url'])
-      dest_filename = os.path.join(options.user_data_dir, path.split('/')[-1])
-      sha1, size = DownloadArchiveToFile(archive, dest_filename)
-      if sha1 != archive.GetChecksum():
-        raise Error("SHA1 checksum mismatch on '%s'.  Expected %s but got %s" %
-                    (bundle.name, archive.GetChecksum(), sha1))
-      if size != archive.size:
-        raise Error("Size mismatch on Archive.  Expected %s but got %s bytes" %
-                    (archive.size, size))
-      InfoPrint('Updating bundle %s to version %s, revision %s' % (
-                (bundle.name, bundle.version, bundle.revision)))
-      ExtractInstaller(dest_filename, bundle_update_path)
-      if bundle.name != SDK_TOOLS:
-        repath = bundle.get('repath', None)
-        if repath:
-          bundle_move_path = os.path.join(bundle_update_path, repath)
-        else:
-          bundle_move_path = bundle_update_path
-        RenameDir(bundle_move_path, bundle_path)
-        if os.path.exists(bundle_update_path):
-          RemoveDir(bundle_update_path)
-      os.remove(dest_filename)
-      local_manifest.MergeBundle(bundle)
-      WriteToFile(local_manifest_path, local_manifest)
-    # Test revision numbers, update the bundle accordingly.
-    # TODO(dspringer): The local file should be refreshed from disk each
-    # iteration thought this loop so that multiple sdk_updates can run at the
-    # same time.
-    if local_manifest.BundleNeedsUpdate(bundle):
-      if (not update_options.force and os.path.exists(bundle_path) and
-          bundle.name != SDK_TOOLS):
-        WarningPrint('%s already exists, but has an update available.\n'
-                     'Run update with the --force option to overwrite the '
-                     'existing directory.\nWarning: This will overwrite any '
-                     'modifications you have made within this directory.'
-                     % bundle.name)
-      else:
-        UpdateBundle()
-    else:
-      InfoPrint('%s is already up-to-date.' % bundle.name)
+def Command(name):
+  return globals().get('CMD' + name, None)
 
-def Sources(options, argv, config):
-  '''Usage: %prog [global_options] sources [options] [--list,--add URL,--remove URL]
 
-  Manage additional package sources.  URL should point to a valid package
-  manifest file for download.
-  '''
-  DebugPrint("Running Sources command with: %s, %s" % (options, argv))
+def GenUsage(parser, cmd):
+  """Modify an OptParse object with the function's documentation."""
+  obj = Command(cmd)
+  more = getattr(obj, 'usage_more', '')
+  if cmd == 'help':
+    cmd = '<command>'
+  else:
+    # OptParser.description prefer nicely non-formatted strings.
+    parser.description = re.sub('[\r\n ]{2,}', ' ', obj.__doc__)
+  parser.set_usage('usage: %%prog %s [options] %s' % (cmd, more))
 
-  parser = optparse.OptionParser(usage=Sources.__doc__)
-  parser.add_option(
-      '-a', '--add', dest='url_to_add',
-      default=None,
-      help='Add additional package source')
-  parser.add_option(
-      '-r', '--remove', dest='url_to_remove',
-      default=None,
-      help='Remove package source (use \'all\' for all additional sources)')
-  parser.add_option(
-      '-l', '--list', dest='do_list',
-      default=False, action='store_true',
-      help='List additional package sources')
-  source_options, _ = parser.parse_args(argv)
 
-  write_config = False
-  if source_options.url_to_add:
-    config.AddSource(source_options.url_to_add)
-    write_config = True
-  elif source_options.url_to_remove:
-    if source_options.url_to_remove == 'all':
-      config.RemoveAllSources()
-    else:
-      config.RemoveSource(source_options.url_to_remove)
-    write_config = True
-  elif source_options.do_list:
-    config.ListSources()
-  else:
-    parser.print_help()
+def UpdateSDKTools(options, args):
+  """update the sdk_tools bundle"""
 
-  if write_config:
-    WriteToFile(os.path.join(options.user_data_dir, options.config_filename),
-                config)
+  local_manifest = LoadLocalManifest()
+  cfg = LoadConfig()
+  remote_manifest = LoadCombinedRemoteManifest(options.manifest_url, cfg)
 
-#------------------------------------------------------------------------------
-# Command-line interface
+  try:
+    delegate = command.update.RealUpdateDelegate(USER_DATA_DIR,
+                                                  DEFAULT_SDK_ROOT)
+    command.update.UpdateBundleIfNeeded(
+        delegate,
+        remote_manifest,
+        local_manifest,
+        command.update.SDK_TOOLS,
+        force=True)
+  finally:
+    # Always write out the local manifest, we may have successfully updated one
+    # or more bundles before failing.
+    WriteLocalManifest(local_manifest)
+  return 0
 
 
 def main(argv):
-  '''Main entry for the sdk_update utility'''
-  parser = optparse.OptionParser(usage=GLOBAL_HELP, add_help_option=False)
-  DEFAULT_SDK_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
-
-  # Manually add help options so we can ignore it when auto-updating.
-  parser.add_option(
-      '-h', '--help', dest='help', action='store_true',
-      help='show this help message and exit')
-  parser.add_option(
-      '-U', '--manifest-url', dest='manifest_url',
-      default='https://commondatastorage.googleapis.com/nativeclient-mirror/'
-              'nacl/nacl_sdk/%s' % MANIFEST_FILENAME,
-      help='override the default URL for the NaCl manifest file')
+  # Get all commands...
+  cmds = [fn[3:] for fn in dir(sys.modules[__name__]) if fn.startswith('CMD')]
+  # Remove hidden commands...
+  cmds = filter(lambda fn: not getattr(Command(fn), 'hide', 0), cmds)
+  # Format for CMDhelp usage.
+  CMDhelp.usage_more = ('\n\nCommands are:\n' + '\n'.join([
+      '  %-10s %s' % (fn, Command(fn).__doc__.split('\n')[0].strip())
+      for fn in cmds]))
+
+  # Create the option parse and add --verbose support.
+  parser = optparse.OptionParser()
   parser.add_option(
-      '-d', '--debug', dest='debug',
-      default=False, action='store_true',
-      help='enable displaying debug information to stderr')
-  parser.add_option(
-      '-q', '--quiet', dest='quiet',
-      default=False, action='store_true',
-      help='suppress displaying informational prints to stdout')
-  parser.add_option(
-      '-u', '--user-data-dir', dest='user_data_dir',
-      # TODO(mball): the default should probably be in something like
-      # ~/.naclsdk (linux), or ~/Library/Application Support/NaClSDK (mac),
-      # or %HOMEPATH%\Application Data\NaClSDK (i.e., %APPDATA% on windows)
-      default=os.path.join(DEFAULT_SDK_ROOT, USER_DATA_DIR),
-      help="specify location of NaCl SDK's data directory")
-  parser.add_option(
-      '-s', '--sdk-root-dir', dest='sdk_root_dir',
-      default=DEFAULT_SDK_ROOT,
-      help="location where the SDK bundles are installed")
-  parser.add_option(
-      '-v', '--version', dest='show_version',
-      action='store_true',
-      help='show version information and exit')
-  parser.add_option(
-      '-m', '--manifest', dest='manifest_filename',
-      default=MANIFEST_FILENAME,
-      help="name of local manifest file relative to user-data-dir")
-  parser.add_option(
-      '-c', '--config', dest='config_filename',
-      default=CONFIG_FILENAME,
-      help="name of the local config file relative to user-data-dir")
-  parser.add_option(
-      '--update-sdk-tools', dest='update_sdk_tools',
-      default=False, action='store_true')
-
-
-  COMMANDS = {
-      'info': Info,
-      'list': List,
-      'update': Update,
-      'install': Update,
-      'sources': Sources,
-      }
-
-  # Separate global options from command-specific options
-  global_argv = argv
-  command_argv = []
-  for index, arg in enumerate(argv):
-    if arg in COMMANDS:
-      global_argv = argv[:index]
-      command_argv = argv[index:]
-      break
-
-  (options, args) = parser.parse_args(global_argv)
-  args += command_argv
-
-  global _debug_mode, _quiet_mode
-  _debug_mode = options.debug
-  _quiet_mode = options.quiet
-
-  def PrintHelpAndExit(unused_options=None, unused_args=None):
-    parser.print_help()
-    exit(1)
+      '-v', '--verbose', action='count', default=0,
+      help='Use 2 times for more debugging info')
+  parser.add_option('-U', '--manifest-url', dest='manifest_url',
+      default=GSTORE_URL + '/nacl/nacl_sdk/' + MANIFEST_FILENAME,
+      metavar='URL', help='override the default URL for the NaCl manifest file')
+  parser.add_option('--update-sdk-tools', action='store_true',
+                    dest='update_sdk_tools', help=optparse.SUPPRESS_HELP)
+
+  old_parser_args = parser.parse_args
+  def Parse(args):
+    options, args = old_parser_args(args)
+    if options.verbose >= 2:
+      loglevel = logging.DEBUG
+    elif options.verbose:
+      loglevel = logging.INFO
+    else:
+      loglevel = logging.WARNING
 
-  if options.update_sdk_tools:
-    # Ignore all other commands, and just update the sdk tools.
-    args = ['update', 'sdk_tools']
-    # Leave the rest of the options alone -- they may be needed to update
-    # correctly.
-    options.show_version = False
-    options.sdk_root_dir = DEFAULT_SDK_ROOT
+    fmt = '%(levelname)s:%(message)s'
+    logging.basicConfig(stream=sys.stdout, level=loglevel, format=fmt)
 
-  if options.show_version:
-    print "Native Client SDK Updater, version r%s" % (REVISION,)
-    exit(0)
+    # If --update-sdk-tools is passed, circumvent any other command running.
+    if options.update_sdk_tools:
+      UpdateSDKTools(options, args)
+      sys.exit(1)
 
+    return options, args
+  parser.parse_args = Parse
 
-  if not args:
-    print "Need to supply a command"
-    PrintHelpAndExit()
-
-  if options.help:
-    PrintHelpAndExit()
-
-  def DefaultHandler(unused_options=None, unused_args=None, unused_config=None):
-    print "Unknown Command: %s" % args[0]
-    PrintHelpAndExit()
-
-  def InvokeCommand(args):
-    command = COMMANDS.get(args[0], DefaultHandler)
-    # Load the config file before running commands
-    config = LoadFromFile(os.path.join(options.user_data_dir,
-                                       options.config_filename),
-                          SDKConfig())
-    command(options, args[1:], config)
-
-  if args[0] == 'help':
-    if len(args) == 1:
-      PrintHelpAndExit()
-    else:
-      InvokeCommand([args[1], '-h'])
-  else:
-    # Make sure the user_data_dir exists.
-    if not os.path.exists(options.user_data_dir):
-      os.makedirs(options.user_data_dir)
-    InvokeCommand(args)
+  if argv:
+    cmd = Command(argv[0])
+    if cmd:
+      # "fix" the usage and the description now that we know the subcommand.
+      GenUsage(parser, argv[0])
+      return cmd(parser, argv[1:])
 
-  return 0  # Success
+  # Not a known command. Default to help.
+  GenUsage(parser, 'help')
+  return CMDhelp(parser, argv)
 
 
 if __name__ == '__main__':
   try:
     sys.exit(main(sys.argv[1:]))
-  except Error as error:
-    print "Error: %s" % error
+  except Error as e:
+    logging.error(str(e))
     sys.exit(1)
diff --git a/native_client_sdk/src/build_tools/tests/test_sdktools.py b/native_client_sdk/src/build_tools/tests/test_sdktools.py
index c453f24..0f3731c 100755
--- a/native_client_sdk/src/build_tools/tests/test_sdktools.py
+++ b/native_client_sdk/src/build_tools/tests/test_sdktools.py
@@ -112,15 +112,16 @@ class SdkToolsTestCase(unittest.TestCase):
     naclsdk_shell_script = os.path.join(self.basedir, 'nacl_sdk', 'naclsdk')
     if getos.GetPlatform() == 'win':
       naclsdk_shell_script += '.bat'
-    cmd = [naclsdk_shell_script, '-U', self.server.GetURL(MANIFEST_BASENAME)]
+    cmd = [naclsdk_shell_script]
     cmd.extend(args)
+    cmd.extend(['-U', self.server.GetURL(MANIFEST_BASENAME)])
     process = subprocess.Popen(cmd, stdout=subprocess.PIPE)
     stdout, _ = process.communicate()
     self.assertEqual(process.returncode, 0)
     return stdout
-  
+
   def _RunAndExtractRevision(self):
-    stdout = self._Run(['-v'])
+    stdout = self._Run(['version'])
     match = re.search('version r(\d+)', stdout)
     self.assertTrue(match is not None)
     return int(match.group(1))
@@ -195,11 +196,12 @@ class TestAutoUpdateSdkTools(SdkToolsTestCase):
     self.sdk_tools_bundle.revision = new_revision
     self._WriteManifest()
 
-    stdout = self._Run(['update', 'sdk_tools'])
-    self.assertTrue(stdout.find('Ignoring manual update request.') != -1)
     sdk_tools_update_dir = os.path.join(self.basedir, 'nacl_sdk',
         'sdk_tools_update')
     self.assertFalse(os.path.exists(sdk_tools_update_dir))
+    stdout = self._Run(['update', 'sdk_tools'])
+    self.assertTrue(stdout.find('Ignoring manual update request.') != -1)
+    self.assertFalse(os.path.exists(sdk_tools_update_dir))
 
 
 class TestAutoUpdateSdkToolsDifferentFilesystem(TestAutoUpdateSdkTools):
@@ -211,10 +213,7 @@ class TestAutoUpdateSdkToolsDifferentFilesystem(TestAutoUpdateSdkTools):
 
 
 def main():
-  suite = unittest.defaultTestLoader.loadTestsFromModule(sys.modules[__name__])
-  result = unittest.TextTestRunner(verbosity=2).run(suite)
-
-  return int(not result.wasSuccessful())
+  unittest.main()
 
 if __name__ == '__main__':
   sys.exit(main())