Revert "pushimage: add support for handling base image"

This reverts commit 01c965a7081505d8071c27b707c9b77776c78c1a.

BUG=chromium:527277
TEST=Unable to reproduce, so testing in production. (sigh)

Change-Id: If92f7d4821340d850e54b675a203e74bd13aece1
Reviewed-on: https://chromium-review.googlesource.com/296837
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Amey Deshpande <ameyd@google.com>
diff --git a/scripts/pushimage.py b/scripts/pushimage.py
index ae0ac6e..7b65993 100644
--- a/scripts/pushimage.py
+++ b/scripts/pushimage.py
@@ -12,6 +12,7 @@
 
 import ConfigParser
 import cStringIO
+import errno
 import getpass
 import os
 import re
@@ -38,14 +39,6 @@
 # Ketsets that are only valid in the above test bucket.
 TEST_KEYSETS = set(('test-keys-mp', 'test-keys-premp'))
 
-# Supported image types for signing.
-_SUPPORTED_IMAGE_TYPES = (
-    constants.IMAGE_TYPE_RECOVERY,
-    constants.IMAGE_TYPE_FACTORY,
-    constants.IMAGE_TYPE_FIRMWARE,
-    constants.IMAGE_TYPE_BASE,
-)
-
 
 class PushError(Exception):
   """When an (unknown) error happened while trying to push artifacts."""
@@ -67,13 +60,15 @@
 
     config = ConfigParser.ConfigParser()
     config.readfp(open(self.GetInsnFile('DEFAULT')))
-    # What pushimage internally refers to as 'recovery', are the basic signing
-    # instructions in practice, and other types are stacked on top.
-    input_insns = self.GetInsnFile(constants.IMAGE_TYPE_RECOVERY)
-    if not os.path.exists(input_insns):
-      # This board doesn't have any signing instructions.
-      raise MissingBoardInstructions(self.board)
-    config.readfp(open(input_insns))
+    try:
+      input_insn = self.GetInsnFile('recovery')
+      config.readfp(open(input_insn))
+    except IOError as e:
+      if e.errno == errno.ENOENT:
+        # This board doesn't have any signing instructions.
+        # This is normal for new or experimental boards.
+        raise MissingBoardInstructions(input_insn)
+      raise
     self.cfg = config
 
   def GetInsnFile(self, image_type):
@@ -88,8 +83,7 @@
     """
     if image_type == image_type.upper():
       name = image_type
-    elif image_type in (constants.IMAGE_TYPE_RECOVERY,
-                        constants.IMAGE_TYPE_BASE):
+    elif image_type == 'recovery':
       name = self.board
     else:
       name = '%s.%s' % (self.board, image_type)
@@ -209,9 +203,7 @@
   """
   # Whether we hit an unknown error.  If so, we'll throw an error, but only
   # at the end (so that we still upload as many files as possible).
-  # It's implemented using a list to deal with variable scopes in nested
-  # functions below.
-  unknown_error = [False]
+  unknown_error = False
 
   if versionrev is None:
     # Extract milestone/version from the directory name.
@@ -280,10 +272,6 @@
     lmid = ('%s-' % image_type) if image_type else ''
     return 'ChromeOS-%s%s-%s' % (lmid, versionrev, boardpath)
 
-  # These variables are defined outside the loop so that the nested functions
-  # below can access them without 'cell-var-from-loop' linter warning.
-  dst_path = ""
-  files_to_sign = []
   for channel in channels:
     logging.debug('\n\n#### CHANNEL: %s ####\n', channel)
     sect_insns['channel'] = channel
@@ -291,116 +279,84 @@
     dst_path = '%s/%s' % (gs_base, sub_path)
     logging.info('Copying images to %s', dst_path)
 
-    recovery_basename = _ImageNameBase(constants.IMAGE_TYPE_RECOVERY)
-    factory_basename = _ImageNameBase(constants.IMAGE_TYPE_FACTORY)
-    firmware_basename = _ImageNameBase(constants.IMAGE_TYPE_FIRMWARE)
-    test_basename = _ImageNameBase(constants.IMAGE_TYPE_TEST)
-    base_basename = _ImageNameBase(constants.IMAGE_TYPE_BASE)
+    recovery_base = _ImageNameBase('recovery')
+    factory_base = _ImageNameBase('factory')
+    firmware_base = _ImageNameBase('firmware')
+    test_base = _ImageNameBase('test')
     hwqual_tarball = 'chromeos-hwqual-%s-%s.tar.bz2' % (board, versionrev)
 
-    # The following build artifacts, if present, are always copied regardless of
-    # requested signing types.
-    files_to_copy_only = (
-        # (<src>, <dst>, <suffix>),
-        ('image.zip', _ImageNameBase(), 'zip'),
-        (constants.TEST_IMAGE_TAR, test_basename, 'tar.xz'),
-        ('debug.tgz', 'debug-%s' % boardpath, 'tgz'),
-        (hwqual_tarball, '', ''),
-        ('au-generator.zip', '', ''),
-        ('stateful.tgz', '', ''),
+    # Upload all the files first before flagging them for signing.
+    files_to_copy = (
+        # pylint: disable=bad-whitespace
+        # <src>                          <dst>
+        # <signing type>                 <sfx>
+        ('recovery_image.tar.xz',        recovery_base,          'tar.xz',
+         'recovery'),
+
+        ('factory_image.zip',            factory_base,           'zip',
+         'factory'),
+
+        ('firmware_from_source.tar.bz2', firmware_base,          'tar.bz2',
+         'firmware'),
+
+        ('image.zip',                    _ImageNameBase(),       'zip', ''),
+        ('chromiumos_test_image.tar.xz', test_base,              'tar.xz', ''),
+        ('debug.tgz',                    'debug-%s' % boardpath, 'tgz', ''),
+        (hwqual_tarball,                 '', '', ''),
+        ('au-generator.zip',             '', '', ''),
+        ('stateful.tgz',                 '', '', ''),
     )
-
-    # The following build artifacts, if present, are always copied.
-    # If |sign_types| is None, all of them are marked for signing, otherwise
-    # only the image types specified in |sign_types| are marked for signing.
-    files_to_copy_and_maybe_sign = (
-        # (<src>, <dst>, <suffix>, <signing type>),
-        (constants.RECOVERY_IMAGE_TAR, recovery_basename, 'tar.xz',
-         constants.IMAGE_TYPE_RECOVERY),
-
-        ('factory_image.zip', factory_basename, 'zip',
-         constants.IMAGE_TYPE_FACTORY),
-
-        ('firmware_from_source.tar.bz2', firmware_basename, 'tar.bz2',
-         constants.IMAGE_TYPE_FIRMWARE),
-    )
-
-    # The following build artifacts are copied and marked for signing, if
-    # they are present *and* if the image type is specified via |sign_types|.
-    files_to_maybe_copy_and_sign = (
-        # (<src>, <dst>, <suffix>, <signing type>),
-        (constants.BASE_IMAGE_TAR, base_basename, 'tar.xz',
-         constants.IMAGE_TYPE_BASE),
-    )
-
-    def _CopyFileToGS(src, dst, suffix):
-      """Returns |dst| file name if the copying was successful."""
+    files_to_sign = []
+    for src, dst, sfx, image_type in files_to_copy:
       if not dst:
         dst = src
-      elif suffix:
-        dst = '%s.%s' % (dst, suffix)
-      success = False
+      elif sfx:
+        dst += '.%s' % sfx
       try:
         ctx.Copy(os.path.join(src_path, src), os.path.join(dst_path, dst))
-        success = True
       except gs.GSNoSuchKey:
         logging.warning('Skipping %s as it does not exist', src)
+        continue
       except gs.GSContextException:
-        unknown_error[0] = True
+        unknown_error = True
         logging.error('Skipping %s due to unknown GS error', src, exc_info=True)
-      return dst if success else None
+        continue
 
-    for src, dst, suffix in files_to_copy_only:
-      _CopyFileToGS(src, dst, suffix)
-
-    # Clear the list of files to sign before adding new artifacts.
-    files_to_sign = []
-
-    def _AddToFilesToSign(image_type, dst, suffix):
-      # We cannot use os.path.splitext() here because the archive file name
-      # has '.<board>' prior to the file extension.
-      dst_base = dst.rstrip(suffix).rstrip('.')
-      assert dst == '%s.%s' % (dst_base, suffix), (
-          'dst: %s, dst_base: %s, suffix: %s' % (dst, dst_base, suffix))
-      files_to_sign.append([image_type, dst_base, suffix])
-
-    for src, dst, suffix, image_type in files_to_copy_and_maybe_sign:
-      dst = _CopyFileToGS(src, dst, suffix)
-      if dst and (sign_types is None or image_type in sign_types):
-        _AddToFilesToSign(image_type, dst, suffix)
-
-    for src, dst, suffix, image_type in files_to_maybe_copy_and_sign:
-      if sign_types and image_type in sign_types:
-        dst = _CopyFileToGS(src, dst, suffix)
-        if dst:
-          _AddToFilesToSign(image_type, dst, suffix)
+      if image_type:
+        dst_base = dst[:-(len(sfx) + 1)]
+        assert dst == '%s.%s' % (dst_base, sfx)
+        files_to_sign += [[image_type, dst_base, '.%s' % sfx]]
 
     # Now go through the subset for signing.
     for keyset in keysets:
       logging.debug('\n\n#### KEYSET: %s ####\n', keyset)
       sect_insns['keyset'] = keyset
       for image_type, dst_name, suffix in files_to_sign:
-        dst_archive = '%s.%s' % (dst_name, suffix)
+        dst_archive = '%s%s' % (dst_name, suffix)
         sect_general['archive'] = dst_archive
         sect_general['type'] = image_type
 
-        # In the default/automatic mode, only flag files for signing if the
-        # archives were actually uploaded in a previous stage. This additional
-        # check can be removed in future once |sign_types| becomes a required
-        # argument.
-        # TODO: Make |sign_types| a required argument.
-        gs_artifact_path = os.path.join(dst_path, dst_archive)
-        exists = False
-        try:
-          exists = ctx.Exists(gs_artifact_path)
-        except gs.GSContextException:
-          unknown_error[0] = True
-          logging.error('Unknown error while checking %s', gs_artifact_path,
-                        exc_info=True)
-        if not exists:
-          logging.info('%s does not exist.  Nothing to sign.',
-                       gs_artifact_path)
-          continue
+        # See if the caller has requested we only sign certain types.
+        if sign_types:
+          if not image_type in sign_types:
+            logging.info('Skipping %s signing as it was not requested',
+                         image_type)
+            continue
+        else:
+          # In the default/automatic mode, only flag files for signing if the
+          # archives were actually uploaded in a previous stage.
+          gs_artifact_path = os.path.join(dst_path, dst_archive)
+          try:
+            exists = ctx.Exists(gs_artifact_path)
+          except gs.GSContextException:
+            unknown_error = True
+            exists = False
+            logging.error('Unknown error while checking %s', gs_artifact_path,
+                          exc_info=True)
+          if not exists:
+            logging.info('%s does not exist.  Nothing to sign.',
+                         gs_artifact_path)
+            continue
 
         input_insn_path = input_insns.GetInsnFile(image_type)
         if not os.path.exists(input_insn_path):
@@ -422,7 +378,7 @@
           try:
             ctx.Copy(insns_path.name, gs_insns_path)
           except gs.GSContextException:
-            unknown_error[0] = True
+            unknown_error = True
             logging.error('Unknown error while uploading insns %s',
                           gs_insns_path, exc_info=True)
             continue
@@ -430,14 +386,14 @@
           try:
             MarkImageToBeSigned(ctx, tbs_base, gs_insns_path, priority)
           except gs.GSContextException:
-            unknown_error[0] = True
+            unknown_error = True
             logging.error('Unknown error while marking for signing %s',
                           gs_insns_path, exc_info=True)
             continue
           logging.info('Signing %s image %s', image_type, gs_insns_path)
           instruction_urls.setdefault(channel, []).append(gs_insns_path)
 
-  if unknown_error[0]:
+  if unknown_error:
     raise PushError('hit some unknown error(s)', instruction_urls)
 
   return instruction_urls
@@ -467,7 +423,7 @@
   parser.add_argument('--priority', type=int, default=50,
                       help='set signing priority (lower == higher prio)')
   parser.add_argument('--sign-types', default=None, nargs='+',
-                      choices=_SUPPORTED_IMAGE_TYPES,
+                      choices=('recovery', 'factory', 'firmware'),
                       help='only sign specified image types')
   parser.add_argument('--yes', action='store_true', default=False,
                       help='answer yes to all prompts')