Check in tofactory script.

scripts/image_signing/common.sh

Index: scripts/image_signing/common.sh
diff --git a/scripts/image_signing/common.sh b/scripts/image_signing/common.sh
index 5a3e600b316c86c622d418a1364ec0ad07f15386..b7a55046e245d56882a7c47057659d143d6aa7a9 100755
--- a/scripts/image_signing/common.sh
+++ b/scripts/image_signing/common.sh
@@ -4,6 +4,9 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+# Globals
+# ----------------------------------------------------------------------------
+
 # Determine script directory
 SCRIPT_DIR=$(dirname $0)
 PROG=$(basename $0)
@@ -12,6 +15,13 @@ GPT=cgpt
 # The tag when the rootfs is changed.
 TAG_NEEDS_TO_BE_SIGNED="/root/.need_to_be_signed"
 
+# List of Temporary files and mount points.
+TEMP_FILE_LIST=$(mktemp)
+TEMP_DIR_LIST=$(mktemp)
+
+# Functions
+# ----------------------------------------------------------------------------
+
 # Finds and loads the 'shflags' library, or return as failed.
 load_shflags() {
   # Load shflags
@@ -25,11 +35,76 @@ load_shflags() {
     echo "ERROR: Cannot find the required shflags library."
     return 1
   fi
+
+  # Add debug option for debug output below
+  DEFINE_boolean debug $FLAGS_FALSE "Provide debug messages" "d"
 }
 
-# List of Temporary files and mount points.
-TEMP_FILE_LIST=$(mktemp)
-TEMP_DIR_LIST=$(mktemp)
+# Functions for debug output
+# ----------------------------------------------------------------------------
+
+# Reports error message and exit(1)
+# Args: error message
+err_die() {
+  echo "ERROR: $*" 1>&2
+  exit 1
+}
+
+# Returns true if we're running in debug mode
+is_debug_mode() {
+  [ "$FLAGS_debug" = $FLAGS_TRUE ]

[gauravsh, 2010/12/17 01:24:37]

This and the debug_msg below were intentionally not moved to common.sh earlier
since they are shflags specific. 

My suggestion would be to either move it back or make is_debug_mode dependent on
another global defined in common.sh that must be set to value of FLAGS_debug in
any script that uses shflags is (as in make_dev_ssd or the script you just
added). 

[Randall Spangler, 2010/12/21 22:52:35]

I've added a default value for the check so that it will default to false if
neither FLAGS_debug nor FLAGS_TRUE are defined.  This way it'll work if the
caller doesn't call load_shflags().

If the caller doesn't use shflags but wants to use debug_msg, all the caller
needs to do is define FLAGS_TRUE and FLAGS_debug.

+}
+
+# Prints messages (in parameters) in debug mode
+# Args: debug message
+debug_msg() {
+  if is_debug_mode; then
+    echo "DEBUG: $*" 1>&2
+  fi
+}
+
+# Functions for temporary files and directories
+# ----------------------------------------------------------------------------
+
+# Create a new temporary file and return its name.
+# File is automatically cleaned when cleanup_temps_and_mounts() is called.
+make_temp_file() {
+  local tempfile=$(mktemp)
+  echo "$tempfile" >> $TEMP_FILE_LIST
+  echo $tempfile
+}
+
+# Create a new temporary directory and return its name.
+# Directory is automatically deleted and any filesystem mounted on it unmounted
+# when cleanup_temps_and_mounts() is called.
+make_temp_dir() {
+  local tempdir=$(mktemp -d)
+  echo "$tempdir" >> $TEMP_DIR_LIST
+  echo $tempdir
+}
+
+cleanup_temps_and_mounts() {
+  for i in "$(cat $TEMP_FILE_LIST)"; do

[Bill Richardson, 2010/12/17 01:10:53]

No quotes here. "${cat file}" will return the contents of the file as a single
long string. 

[gauravsh, 2010/12/17 01:24:37]

Please, if you make this change as Bill suggested, make sure you test that this
does not break sign_official_build.sh and other scripts. Bash quoting is
somewhat confusing - and I may have added the extra quotes because something
wasn't working as expected.

[Randall Spangler, 2010/12/21 22:52:35]

I did some experimentation; the quotes will indeed mess things up.


rrs@a5:~/b/tmp$ for i in $(echo 1 2 3); do echo $i; done
1
2
3
rrs@a5:~/b/tmp$ for i in "$(echo 1 2 3)"; do echo $i; done
1 2 3

[gauravsh, 2010/12/22 23:04:02]

ahh, ok. good to know. thanks! :)

+    rm -f $i

[Randall Spangler, 2010/12/21 22:52:35]

Note that since $i is used UNquoted here, and you can specify multiple files to
rm, this undoes the damage from the previous extra quotes...

+  done
+  set +e  # umount may fail for unmounted directories
+  for i in "$(cat $TEMP_DIR_LIST)"; do

[Bill Richardson, 2010/12/17 01:10:53]

No quotes here, either.

+    if [ -n "$i" ]; then
+      if has_needs_to_be_resigned_tag "$i"; then

[Randall Spangler, 2010/12/21 22:52:35]

If there's more than one temp dir, this check will return false, so the warning
won't show up.

+        echo "Warning: image may be modified. Please resign image."
+      fi
+      sudo umount -d $i 2>/dev/null

[Randall Spangler, 2010/12/21 22:52:35]

This still works just like line 88, because umount can also take multiple mount
points on one command line.

+      rm -rf $i
+    fi
+  done
+  set -e
+  rm -rf $TEMP_DIR_LIST $TEMP_FILE_LIST
+}
+
+trap "cleanup_temps_and_mounts" EXIT
+
+# Functions for partition management
+# ----------------------------------------------------------------------------
 
 # Read GPT table to find the starting location of a specific partition.
 # Args: DEVICE PARTNUM
@@ -116,54 +191,6 @@ replace_image_partition() {
   dd if=$input_file of=$image bs=512 seek=$offset count=$size conv=notrunc
 }
 
-# Create a new temporary file and return its name.
-# File is automatically cleaned when cleanup_temps_and_mounts() is called.
-make_temp_file() {
-  local tempfile=$(mktemp)
-  echo "$tempfile" >> $TEMP_FILE_LIST
-  echo $tempfile
-}
-
-# Create a new temporary directory and return its name.
-# Directory is automatically deleted and any filesystem mounted on it unmounted
-# when cleanup_temps_and_mounts() is called.
-make_temp_dir() {
-  local tempdir=$(mktemp -d)
-  echo "$tempdir" >> $TEMP_DIR_LIST
-  echo $tempdir
-}
-
-cleanup_temps_and_mounts() {
-  for i in "$(cat $TEMP_FILE_LIST)"; do
-    rm -f $i
-  done
-  set +e  # umount may fail for unmounted directories
-  for i in "$(cat $TEMP_DIR_LIST)"; do
-    if [ -n "$i" ]; then
-      if has_needs_to_be_resigned_tag "$i"; then
-        echo "Warning: image may be modified. Please resign image."
-      fi
-      sudo umount -d $i 2>/dev/null
-      rm -rf $i
-    fi
-  done
-  set -e
-  rm -rf $TEMP_DIR_LIST $TEMP_FILE_LIST
-}
-
-# Returns true if all files in parameters exist.
-ensure_files_exist() {
-  local filename return_value=0
-  for filename in "$@"; do
-    if [ ! -f "$filename" -a ! -b "$filename" ]; then
-      echo "ERROR: Cannot find required file: $filename"
-      return_value=1
-    fi
-  done
-
-  return $return_value
-}
-
 # For details, see crosutils.git/common.sh
 enable_rw_mount() {
   local rootfs="$1"
@@ -238,12 +265,28 @@ rw_mount_disabled() {
   return 1
 }
 
+# Misc functions
+# ----------------------------------------------------------------------------
+
+# Returns true if all files in parameters exist.
+# Args: List of files
+ensure_files_exist() {
+  local filename return_value=0
+  for filename in "$@"; do
+    if [ ! -f "$filename" -a ! -b "$filename" ]; then
+      echo "ERROR: Cannot find required file: $filename"
+      return_value=1
+    fi
+  done
+
+  return $return_value
+}
+
 # Check if the 'chronos' user already has a password
-# ARGS: rootfs
+# Args: rootfs
 no_chronos_password() {
   local rootfs=$1
   sudo grep -q '^chronos:\*:' "$rootfs/etc/shadow"
 }
 
-trap "cleanup_temps_and_mounts" EXIT