Factored out the code to copy an image and modify it for test

common.sh

 # Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # Common constants for build scripts
 # This must evaluate properly for both /bin/bash and /bin/sh
 
 # All scripts should die on error unless commands are specifically excepted
 # by prefixing with '!' or surrounded by 'set +e' / 'set -e'.
 # TODO: Re-enable this once shflags is less prone to dying.
 #set -e
 
 # The number of jobs to pass to tools that can run in parallel (such as make
 # and dpkg-buildpackage
 NUM_JOBS=`grep -c "^processor" /proc/cpuinfo`
 
+# True if we have the 'pv' utility - also set up PV_CAT for convenience
+PV_OK=1

[sosa, 2010/11/30 22:01:43]

These vars are pretty short, watch out for name conflicts.  Might be better to
prepend with something more unique .. i.e. COMMON_

[sjg, 2010/12/01 00:57:05]

Done

+PV_CAT=pv
+pv -V >/dev/null || PV_OK=0
+if [ $PV_OK -eq 0 ]; then
+  PV_CAT=cat
+fi
+
 # Store location of the calling script.
 TOP_SCRIPT_DIR="${TOP_SCRIPT_DIR:-$(dirname $0)}"
 
 # Find root of source tree
 if [ "x$GCLIENT_ROOT" != "x" ]
 then
   # GCLIENT_ROOT already set, so we're done
   true
 elif [ "x$COMMON_SH" != "x" ]
 then
@@ skipping 93 matching lines @@
 fi
 
 # Detect whether we're inside a chroot or not
 if [ -e /etc/debian_chroot ]
 then
   INSIDE_CHROOT=1
 else
   INSIDE_CHROOT=0
 fi
 
+
+# Standard filenames

[sosa, 2010/11/30 22:01:43]

Check for name collisions in other files

+CHROMEOS_IMAGE_NAME="chromiumos_image.bin"
+CHROMEOS_TEST_IMAGE_NAME="chromiumos_test_image.bin"
+

[sjg, 2010/12/01 00:57:05]

There are none.

cd .../src/scripts
grep CHROMEOS_IMAGE
grep CHROMEOS_TEST *

+
 # Directory locations inside the dev chroot
 CHROOT_TRUNK_DIR="/home/$USER/trunk"
 
 # Install make for portage ebuilds.  Used by build_image and gmergefs.
 # TODO: Is /usr/local/autotest-chrome still used by anyone?
 DEFAULT_INSTALL_MASK="/usr/include /usr/man /usr/share/man /usr/share/doc \
   /usr/share/gtk-doc /usr/share/gtk-2.0 /usr/lib/gtk-2.0/include \
   /usr/share/info /usr/share/aclocal /usr/lib/gcc /usr/lib/pkgconfig \
   /usr/share/pkgconfig /usr/share/gettext /usr/share/readline /etc/runlevels \
   /usr/share/openrc /lib/rc *.a *.la /etc/init.d /usr/lib/debug
@@ skipping 379 matching lines @@
 chroot_hacks_from_outside() {
   # Give args better names.
   local chroot_dir="${1}"
 
   # Add root as a sudoer if not already done.
   if ! sudo grep -q '^root ALL=(ALL) ALL$' "${chroot_dir}/etc/sudoers" ; then
     info "Upgrading old chroot (pre 2010-10-19) - adding root to sudoers"
     sudo bash -c "echo root ALL=\(ALL\) ALL >> \"${chroot_dir}/etc/sudoers\""
   fi
 }
+
+# This function converts a chromiumos image into a test image, either
+# in place or by copying to a new test image filename first. It honors
+# the following flags (see mod_image_for_test.sh)
+#
+#   --factory
+#   --factory_install
+#   --force_copy
+#
+# On entry, SRC_IMAGE should contain the image pathname to modify
+# On exit, SRC_IMAGE contains the pathname of the resulting test image

[sosa, 2010/11/30 22:01:43]

Treat this more like  a function ... i.e. don't modify SRC_IMAGE in place rather
take it as an arg ... i.e. arg 1 ... and echo the new path at the end ... that
way callees can do

SRC_IMAGE="$(prepare_test_image)" 

Rather than rely on some underlying mechanism to change a global var

[sjg, 2010/12/01 00:57:05]

Actually this is hard, because mod_image_for_test generates lots of output. It
means that I need to redirect it and then perhaps print it afterwards on
failure. Are you sure this is the best approach?

[sjg, 2010/12/01 20:02:31]

OK have piped the script to stderr as you suggest. It seems fine.

+
+prepare_test_image() {

[sosa, 2010/11/30 22:01:43]

Indent

+# If we're asked to modify the image for test, then let's make a copy and
+# modify that instead.
+# Check for manufacturing image.
+if [ ${FLAGS_factory} -eq ${FLAGS_TRUE} ] ; then
+  EXTRA_ARGS="--factory"

[sosa, 2010/11/30 22:01:43]

use locals

+fi
+
+# Check for install shim.
+if [ ${FLAGS_factory_install} -eq ${FLAGS_TRUE} ] ; then

[sosa, 2010/11/30 22:01:43]

style is no space between ] and ;.  This has changed so some other places might
be wrong

+  EXTRA_ARGS="--factory_install"
+fi
+
+# Check for forcing copy of image
+if [ ${FLAGS_force_copy} -eq ${FLAGS_TRUE} ] ; then
+  EXTRA_ARGS+="--force_copy"

[sosa, 2010/11/30 22:01:43]

Does this work?  +=?  Is that a bashism?  Why not EXTRA_ARGS="${EXTRA_ARGS}
--force_copy"

+fi
+
+# modified the image for test, creating a new test image

[sosa, 2010/11/30 22:01:43]

s/m/M

+"${SCRIPTS_DIR}/mod_image_for_test.sh" --board=${FLAGS_board} --image \

[sosa, 2010/11/30 22:01:43]

Use = consistently in options

+  "${SRC_IMAGE}" --noinplace ${EXTRA_ARGS}
+
+# from now on we use the just-created test image

[sosa, 2010/11/30 22:01:43]

s/f/F

+SRC_IMAGE="${FLAGS_from}/${CHROMEOS_TEST_IMAGE_NAME}"
+echo "Source test image is: ${SRC_IMAGE}"
+}