pack_firmware: Stop putting image version info in SFX header

The `futility` now has a much better way of reporting image versions and
contents using `--manifest`. We should stop keeping version information
in SFX, which was hard to maintain when updating updater contents.

BUG=chromium:882445
TEST=./pack_firmware_unittest.py; ./pack_firmware_functest.py

Change-Id: Ic57f98cbeb01402c03c3d6348cdf466940cb0de1
Reviewed-on: https://chromium-review.googlesource.com/1343619
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Evan Hernandez <evanhernandez@chromium.org>
diff --git a/pack/sfx.sh b/pack/sfx.sh
index 2a99a08..49eaf4f 100755
--- a/pack/sfx.sh
+++ b/pack/sfx.sh
@@ -3,25 +3,6 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-# This is replaced by 'UNIBUILD=' for non-unibuild, or 'UNIBUILD="yes"' for
-# unibuild by pack_firmware.py. In the unibuild case the variables below are
-# not used, but are set later by the setvars.sh script, since we don't have
-# access to the configuration until we unpack the archive.
-REPLACE_UNIBUILD
-
-if [ -z "${UNIBUILD}" ]; then
-  # Version information. Please keep this in head of the updater.
-  TARGET_RO_FWID="REPLACE_TARGET_RO_FWID"
-  TARGET_FWID="REPLACE_TARGET_FWID"
-  TARGET_ECID="REPLACE_TARGET_ECID"
-  TARGET_PDID="REPLACE_TARGET_PDID"
-  TARGET_PLATFORM="REPLACE_TARGET_PLATFORM"
-fi
-
-# Export all version information
-export TARGET_FWID TARGET_RO_FWID TARGET_ECID TARGET_PDID
-export TARGET_PLATFORM
-export UNIBUILD MODEL_DIR SIGNATURE_ID
 set -e
 
 # Global variables
@@ -94,8 +75,8 @@
     die "Cannot extract bundle content to: $destination"
 }
 
-# Executes a script in bundle
-exec_bundle_script() {
+# Executes the updater bundle
+exec_bundle() {
   local rc=0
   make_exec_temp
   extract_bundle "$TMP_DIR"
@@ -125,113 +106,16 @@
   set -- "-c"  # Force shar to overwrite files
 }
 
-find_version_string() {
-  local filename="$1"
-  local pattern="$2"
-  local section="$3"
-  if [ ! -s "$filename" ]; then
-    return
-  fi
-
-  # Chrome OS & chroot has futility, and standard Linux desktop has strings.
-  if type futility >/dev/null 2>&1; then
-
-    local tmpdir="$(mktemp -d)"
-    local filepath="$(readlink -f "$filename")"
-    (cd "$tmpdir"; futility dump_fmap -x "$filepath" "$section") >/dev/null 2>&1
-    cat "$tmpdir/$section" #2>/dev/null
-    rm -rf "$tmpdir"
-
-  elif type strings >/dev/null 2>&1; then
-
-    local versions=$( (strings "$filename" | grep "$pattern") || true)
-    local version="$(echo "$versions" | uniq)"
-
-    local num_versions="$(echo "$versions" | wc -l)"
-    local num_version="$(echo "$version" | wc -l)"
-
-    # To deal with firmware having RO != RW, we need to find difference
-    # between RO_FRID, RW_FWID_A and RW_FWID_B; and unfortunately that is not
-    # possible if there's just strings, since we don't know the ordering of
-    # RO/RW sections. The hack here is, if there are three versions with only
-    # 2 different values, we assume the duplicated = RW and unique = RO.
-    if [ "$num_version" -eq 1 ]; then
-      echo "$version"
-    elif [ "$num_version" -eq 2 -a "$num_versions" -eq 3 ]; then
-      case "$section" in
-        RO_FRID)
-          echo "$versions" | uniq -u
-          ;;
-        RW_FWID_*)
-          echo "$versions" | uniq -d
-          ;;
-        *)
-          echo "WARNING: Unknown firmware versions for $filename." >&2
-          ;;
-      esac
-    else
-      echo "WARNING: cannot identify firmware version for $filename." >&2
-    fi
-
-  else
-
-    (echo "WARNING: 'strings' and 'futility' are both not available."
-     echo "         TARGET_{FW,EC,PD}ID can't be updated."
-     echo "         You have to manually change that or repack on a desktop."
-     ) >&2
-
-  fi
-}
-
 # Repacks current file ($SELF) by given source folder.
 perform_shar_repack() {
   local new_source="$1"
   local cut_mark="$(sed -n '/^##CUTHERE##/=' "$SELF")"
   local manifest="${new_source}/manifest.json"
-  local ro_fw_ver="$(find_version_string "$new_source/bios.bin" \
-                  '^Google_' RO_FRID)"
-  local fw_ver="$(find_version_string "$new_source/bios.bin" \
-                  '^Google_' RW_FWID_A)"
-  local ec_ver="$(find_version_string "$new_source/ec.bin" \
-                  '^[a-zA-Z0-9]*_v[0-9\.]*-[a-z0-9]*$' RO_FRID)"
-  local pd_ver="$(find_version_string "$new_source/pd.bin" \
-                  '^[a-zA-Z0-9]*_v[0-9\.]*-[a-z0-9]*$' RO_FRID)"
-
-  # Since mosys r430, trailing spaces reported by mosys is always scrubbed.
-  ec_ver="$(echo "$ec_ver" | sed 's/ *$//')"
 
   [ "$cut_mark" -gt 0 ] || die "File corrupted: $SELF"
   sed -i "$((cut_mark + 1)),\$d" "$SELF" ||
     die "Failed to truncate existing data in $SELF"
 
-  # Try to update firmware version if available. This doesn't work with
-  # unified builds since the variables are in separate files.
-  if [ -z "${UNIBUILD}" ]; then
-    if [ -n "$fw_ver" ]; then
-      sed -i 's/^\( *TARGET_FWID=\)".*"/\1"'"$fw_ver"'"/' "$SELF" &&
-        echo "Changed TARGET_FWID to $fw_ver"
-      sed -i 's/^\( *TARGET_RO_FWID=\)".*"/\1"'"$ro_fw_ver"'"/' \
-          "$SELF" &&
-        echo "Changed TARGET_ROFWID to $ro_fw_ver"
-      if [ -s "$new_source/VERSION" ]; then
-        sed -i "s/^\(BIOS version: *\).*/\1$ro_fw_ver/;
-                s/^\(BIOS .RW. version: *\).*/\1$fw_ver/" "$new_source/VERSION"
-      fi
-    fi
-    if [ -n "$ec_ver" ]; then
-      sed -i 's/^\( *TARGET_ECID=\)".*"/\1"'"$ec_ver"'"/' "$SELF" &&
-        echo "Changed TARGET_ECID to $ec_ver"
-      [ -s "$new_source/VERSION" ] &&
-        sed -i "s/^\(EC version:*\).*/\1$ec_ver/" "$new_source/VERSION"
-    fi
-    if [ -n "$pd_ver" ]; then
-      sed -i 's/^\( *TARGET_PDID=\)".*"/\1"'"$pd_ver"'"/' "$SELF" &&
-        echo "Changed TARGET_PDID to $pd_ver"
-      [ -s "$new_source/VERSION" ] &&
-        sed -i "s/^\(PD version:*\).*/\1$pd_ver/" "$new_source/VERSION"
-    fi
-  fi
-
   if type futility >/dev/null 2>&1; then
     futility update -a "${new_source}" --manifest >"${manifest}" ||
       rm -f "${manifest}"
@@ -328,7 +212,7 @@
       "
       # Invoke script with -h for usage help
       IS_IGNORE_RC=TRUE
-      exec_bundle_script "-h"
+      exec_bundle "-h"
       ;;
 
     --force)
@@ -356,7 +240,11 @@
     exit 0
   fi
 
-  exec_bundle_script "$@"
+  if type futility >/dev/null 2>&1; then
+    exec_bundle "$@"
+  else
+    die "Need 'futility' in PATH to execute updater."
+  fi
 }
 
 main "$@"
diff --git a/pack_firmware.py b/pack_firmware.py
index 932a2fa..a3bac90 100755
--- a/pack_firmware.py
+++ b/pack_firmware.py
@@ -704,31 +704,6 @@
     osutils.WriteFile(outfile, data)
     os.chmod(outfile, os.stat(outfile).st_mode | 0555)
 
-  def _WriteUpdateScript(self, replace_dict):
-    """Create and write the update script which will run on the device.
-
-    This generates the beginnings of the output file (shellball) based on the
-    template. Normally all the required settings for firmware update are at the
-    top of the script. With unified builds only TARGET_SCRIPT is used: the
-    rest are unset and the model-specific setvars.sh files contain the rest
-    of the settings.
-
-    Args:
-      replace_dict: Modified by this function to add the script. Dict with:
-          key: String to replace.
-          value: Value to replace with.
-        Note: For unified builds this should be empty.
-    """
-    if replace_dict:
-      unibuild = ''
-    else:
-      unibuild = '"yes"'
-      for key in REPLACE_VARS:
-        replace_dict['REPLACE_' + key] = '<unused with unified builds>'
-    replace_dict['REPLACE_UNIBUILD'] = 'UNIBUILD=%s' % unibuild
-    self._CreateFileFromTemplate(self._sfx_file, self._args.output,
-                                 replace_dict)
-
   def _WriteVersionFile(self):
     """Write out the VERSION file with our collected version information."""
     print(file=self._versions)
@@ -737,10 +712,12 @@
   def _BuildShellball(self):
     """Build a shell-ball containing the firmware update.
 
-    Add our files to the shell-ball, and display all version information.
+    Create a new shell-ball by copying from SFX file, add our files to the
+    shell-ball, and display all version information.
     """
+    self._CopyFile(self._sfx_file, self._args.output, mode=CHMOD_ALL_EXEC)
     cros_build_lib.RunCommand(
-        ['sh', self._args.output, '--sb_repack', self._basedir],
+        ['sh', self._args.output, '--repack', self._basedir],
         quiet=self._args.quiet, mute_output=not self._args.quiet)
     if not self._args.quiet:
       for fname in glob.glob(self._BaseDirPath('VERSION*')):
@@ -990,9 +967,8 @@
         for model in models:
           model_details[model] = self._GenerateOneModel(
               firmware_info[model], args, model_details)
-        replace_dict = {}
       else:
-        image_files = self._ProcessModel(
+        self._ProcessModel(
             model='',
             bios_image=args.bios_image,
             bios_rw_image=args.bios_rw_image,
@@ -1000,8 +976,6 @@
             pd_image=args.pd_image,
             shared_image_files=None,
             target_dir=self._basedir)
-        replace_dict = self._GetReplaceDict(image_files)
-      self._WriteUpdateScript(replace_dict)
       self._WriteVersionFile()
       if model_details:
         self._WriteSignerInstructions(model_details)
diff --git a/pack_firmware_functest.py b/pack_firmware_functest.py
index 35834c2..8ca2d5d 100755
--- a/pack_firmware_functest.py
+++ b/pack_firmware_functest.py
@@ -161,7 +161,7 @@
     os.environ['SYSROOT'] = 'test'
     os.environ['FILESDIR'] = 'test'
     self.packer.Start(argv)
-    cros_build_lib.RunCommand([outfile, '--sb_extract', self.unpackdir],
+    cros_build_lib.RunCommand([outfile, '--unpack', self.unpackdir],
                               quiet=True, mute_output=True)
     files = []
     for dirpath, _, fnames in os.walk(self.unpackdir):
@@ -195,7 +195,6 @@
     self.assertEqual('', versions['TARGET_ECID'])
     self.assertEqual('', versions['TARGET_PDID'])
     self.assertEqual('Google_Reef', versions['TARGET_PLATFORM'])
-    self.assertEqual('', versions['UNIBUILD'])
 
   def _CheckVersionsReef(self, versions):
     """Check the versions match expectations for reef.
@@ -322,22 +321,18 @@
       if assertion:
         assertion(version_lines[i])
 
-    self.assertEqual('yes', versions['UNIBUILD'])
-    for var in pack_firmware.REPLACE_VARS:
-      self.assertEqual('<unused with unified builds>', versions[var])
-
-    versions = pack_firmware_utils.ReadVersions(
+    versions = pack_firmware_utils.ReadSetVars(
         os.path.join(self.unpackdir, MODELS_DIR, 'reef/setvars.sh'))
     self.assertEqual(9, len(versions))
     self._CheckVersionsReef(versions)
     self._CheckVars(versions, 'models/reef/%s', 'reef')
 
-    versions = pack_firmware_utils.ReadVersions(
+    versions = pack_firmware_utils.ReadSetVars(
         os.path.join(self.unpackdir, MODELS_DIR, 'electro/setvars.sh'))
     self._CheckVersionsReef(versions)
     self._CheckVars(versions, 'models/reef/%s', 'electro')
 
-    versions = pack_firmware_utils.ReadVersions(
+    versions = pack_firmware_utils.ReadSetVars(
         os.path.join(self.unpackdir, MODELS_DIR, 'pyro/setvars.sh'))
     self.assertEqual('Google_Pyro.9042.41.0', versions['TARGET_FWID'])
     self._CheckVars(versions, 'models/pyro/%s', 'pyro')
@@ -400,7 +395,7 @@
 
     # Extract the file into a directory, then overwrite various files with new
     # images with a different IDs.
-    cros_build_lib.RunCommand([outfile, '--sb_extract', self.unpackdir],
+    cros_build_lib.RunCommand([outfile, '--unpack', self.unpackdir],
                               quiet=True, mute_output=True)
     ro_id = 'Google_Veyron_Mickey.6588.197.0'
     rw_id = 'Google_Veyron_Mickey.6588.197.1'
@@ -412,7 +407,7 @@
     self._MakeImage('pd.bin', pd_id)
 
     # Repack the file and make sure that the versions update.
-    cros_build_lib.RunCommand([outfile, '--sb_repack', self.unpackdir],
+    cros_build_lib.RunCommand([outfile, '--repack', self.unpackdir],
                               quiet=True, mute_output=True)
     versions = pack_firmware_utils.ReadVersions(outfile)
     self.assertEqual(ro_id, versions['TARGET_RO_FWID'])
@@ -611,12 +606,12 @@
     Returns:
       List of sums for each key in key_ids
     """
-    cros_build_lib.RunCommand([outfile, '--sb_extract', self.unpackdir],
+    cros_build_lib.RunCommand([outfile, '--unpack', self.unpackdir],
                               quiet=True, mute_output=True)
     keydir = os.path.join(self.unpackdir, 'keyset')
     osutils.SafeMakedirs(keydir)
     key_sums = [self._AddKeys(keydir, x) for x in key_ids]
-    cros_build_lib.RunCommand([outfile, '--sb_repack', self.unpackdir],
+    cros_build_lib.RunCommand([outfile, '--repack', self.unpackdir],
                               quiet=True, mute_output=True)
     return key_sums
 
@@ -661,7 +656,7 @@
         assertion(lines[i])
 
     # Check that the setvars.sh file is correct (points to sand).
-    versions = pack_firmware_utils.ReadVersions(
+    versions = pack_firmware_utils.ReadSetVars(
         os.path.join(self.unpackdir, MODELS_DIR, 'whitelabel-test/setvars.sh'))
     self.assertEqual('Google_Reef.9000.0.0', versions['TARGET_FWID'])
     self._CheckVars(versions, 'models/sand/%s', 'whitelabel-test')
@@ -720,7 +715,7 @@
       self.assertNotExists(os.path.join(self.unpackdir, MODELS_DIR, wl_model))
 
     # Check that the setvars.sh file is correct (points to sand).
-    versions = pack_firmware_utils.ReadVersions(
+    versions = pack_firmware_utils.ReadSetVars(
         os.path.join(self.unpackdir, MODELS_DIR, '%s/setvars.sh' % model))
     self.assertEqual('Google_Reef.9042.50.0', versions['TARGET_FWID'])
     self._CheckVars(versions, 'models/reef/%s', 'sig-id-in-customization-id')
diff --git a/pack_firmware_unittest.py b/pack_firmware_unittest.py
index 104270e..236bd12 100755
--- a/pack_firmware_unittest.py
+++ b/pack_firmware_unittest.py
@@ -410,7 +410,7 @@
     rc.AddCmdResult(
         partial_mock.ListRegex('futility vbutil_firmware'),
         returncode=0, output=VBUTIL_OUTPUT)
-    rc.AddCmdResult(partial_mock.ListRegex('--sb_repack'), returncode=0)
+    rc.AddCmdResult(partial_mock.ListRegex('--repack'), returncode=0)
     rc.AddCmdResult(partial_mock.ListRegex('futility dump_fmap -x .*ec.bin'),
                     side_effect=_CopySections, returncode=0)
     rc.AddCmdResult(partial_mock.ListRegex('futility dump_fmap -x .*pd.bin'),
@@ -589,10 +589,6 @@
     self.assertNotIn('EC version', result)
     self.assertEqual(4, len(result.splitlines()))
 
-    # In the script, the EC version should be ''.
-    lines = osutils.ReadFile(self.output).splitlines()
-    self.assertIn('', self._FindLineInList(lines, 'TARGET_ECID'))
-
 
 if __name__ == '__main__':
   cros_test_lib.main(module=__name__)
diff --git a/pack_firmware_utils.py b/pack_firmware_utils.py
index cd9a08e..76b4a17 100644
--- a/pack_firmware_utils.py
+++ b/pack_firmware_utils.py
@@ -10,13 +10,14 @@
 
 from __future__ import print_function
 
-import os
+import json
 import re
+import subprocess
 
 # We are looking for KEY="VALUE", or KEY=
 RE_KEY_VALUE = re.compile('(?P<key>[A-Z_]+)=("(?P<value>.*)")?$')
 
-def ReadVersions(fname):
+def ReadSetVars(fname):
   """Read the start of the supplied script to get the version information.
 
   This picks up various shell variable assignments from the script and
@@ -35,8 +36,7 @@
   # Use strip() where needed since some lines are indented.
   prefixes = ['TARGET', 'IMAGE', 'SIGNATURE']
   lines = [line.strip() for line in lines
-           if line.strip().split('_')[0] in prefixes or
-           line.startswith('UNIBUILD')]
+           if line.strip().split('_')[0] in prefixes]
   versions = {}
   for line in lines:
     m = RE_KEY_VALUE.match(line)
@@ -44,6 +44,38 @@
     versions[m.group('key')] = value if value else ''
   return versions
 
+def ReadVersions(fname):
+  """Read the version of images in an updater archive.
+
+  Args:
+    fname: Filename of script file.
+
+  Returns:
+    Dict with:
+        key: Version name.
+        value: Version value.
+  """
+  raw_data = subprocess.check_output(['sh', fname, '--manifest'])
+  data = json.loads(raw_data)
+  if len(data) == 1:
+    data = data.values()[0]
+    ro_fwid = data['host']['versions']['ro']
+    rw_fwid = data['host']['versions']['rw']
+    platform = ro_fwid.partition('.')[0]
+    ec_id = data['ec']['versions']['ro'] if 'ec' in data else ''
+    pd_id = data['pd']['versions']['ro'] if 'pd' in data else ''
+  else:
+    ro_fwid = rw_fwid = platform = ec_id = pd_id = ''
+
+  versions = {
+      'TARGET_RO_FWID': ro_fwid,
+      'TARGET_FWID': rw_fwid,
+      'TARGET_ECID': ec_id,
+      'TARGET_PDID': pd_id,
+      'TARGET_PLATFORM': platform,
+  }
+  return versions
+
 def MakeTestFiles():
   """Create test files that we need."""
-  os.system('./make_test_files.sh')
+  subprocess.check_call(['./make_test_files.sh'], shell=True)