New utility to tag/stamp image

Add a utility to tag/stamp image

There are several procedures in Chrome OS post-processing before being released:
stamping, tagging, mod image for URLs, ... and signing.
We need an integrated script to handle all the stamping / tagging.

This CL can handle empty tag files like /root/.force_update_firmware
or /root/.dev_mode.

This CL deprecates http://codereview.chromium.org/3421040 and moved script
from crosutils to vboot_reference. In the future we may isolate the non-signing
post-processing scripts (set_lsb, tag_image, remove_label, ...) into crosutils.

BUG=none
TEST=manually:
(1) Build a general dev image without firmware updates (default behavior of build_image for x86-generic ToT)
(2) Enter chroot and then execute:
    cd ~/trunk/src/platform/vboot_reference/scripts;
    ./tag_image.sh \
      --from ~/trunk/src/build/images/x86-generic/latest/chromiumos_image.bin
    Expected: output message:
      Update Firmware: disabled
      Developer Mode: Enabled
(3) ./tag_image.sh --update_firmware=1 --dev_mode=0 \
      --from ~/trunk/src//build/images/x86-generic/latest/chromiumos_image.bin
    Expected: output message:
      Update Firmware: disabled => Enabled
      Developer Mode: Enabled => disabled

    Manually verify:
      pushd ../../build/images/x86-generic/latest
      unpack_partitions.sh chromiumos_image.bin
      sudo mount -o loop,ro part_3 rootfs
      ls -l rootfs/root/.force_update_firmware # this file should exist
      ls -l rootfs/root/.dev_mode # this file should NOT exist (i.e., error)
      sudo umount rootfs

(4) ./tag_image.sh --update_firmware=0 --dev_mod=1 \
      --from ~/trunk/src/build/images/x86-generic/latest/chromiumos_image.bin
    Expected: output message:
      Update Firmware: Enabled => disabled
      Developer Mode: disabled => Enabled

    Manually verify:
      pushd ../../build/images/x86-generic/latest
      unpack_partitions.sh chromiumos_image.bin
      sudo mount -o loop,ro part_3 rootfs
      ls -l rootfs/root/.force_update_firmware # this file should NOT exist (i.e., error)
      ls -l rootfs/root/.dev_mode # this file should exist
      sudo umount rootfs

Change-Id: I96af3c7201372bb904426d10cff142467a1fa2e7

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=04c00e1

[gauravsh, 2010-09-30 04:28:48]

http://codereview.chromium.org/3604001/diff/3001/4004
File scripts/image_signing/tag_image.sh (right):

http://codereview.chromium.org/3604001/diff/3001/4004#newcode15
scripts/image_signing/tag_image.sh:15: DEFINE_boolean enable_update_firmware
${FLAGS_FALSE} \
could we combine these 2 flags under [no]enable_update_firmware

http://codereview.chromium.org/3604001/diff/3001/4004#newcode19
scripts/image_signing/tag_image.sh:19: DEFINE_boolean enable_dev_mode
${FLAGS_FALSE} \
ditto for the dev_mode flag. this will then follow normal convention that is
followed for optional arguments in other cros scripts.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode40
scripts/image_signing/tag_image.sh:40: # syntax: process_tag name root
tag_file_path enable disable
nit: just for consistency, syntax comment goes right before the "function
process_tag()" line.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode40
scripts/image_signing/tag_image.sh:40: # syntax: process_tag name root
tag_file_path enable disable
also, it is not clear from the comment what each of these arguments mean or do.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode43
scripts/image_signing/tag_image.sh:43: if [ $4 = ${FLAGS_TRUE} -a $5 =
${FLAGS_TRUE} ]; then
nit: instantiate local vars with the passed in function arguments. this makes it
easy to see what exactly is being checked.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode48
scripts/image_signing/tag_image.sh:48: if [ -f "$2$3" ]; then
again, this is confusing. what exactly is being done here?

http://codereview.chromium.org/3604001/diff/3001/4004#newcode48
scripts/image_signing/tag_image.sh:48: if [ -f "$2$3" ]; then
again, this is confusing. what exactly is being done here?

http://codereview.chromium.org/3604001/diff/3001/4004#newcode80
scripts/image_signing/tag_image.sh:80: if ! [[ -x ./unpack_partitions.sh && -x
./pack_partitions.sh ]]; then
I would rather that you didn't depend on these associated scripts to do the
work. Most times one just wants to tag/stamp a given .bin image - the associated
pack_partitions and unpack_partitions may not be present.

 You can use the functions from common.sh to extract/replace/manipulate specific
partitions.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode86
scripts/image_signing/tag_image.sh:86: mkdir -p ./rootfs
use a temp directory here. common.sh has functions to do tracked mounting and
umounting for you.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode120
scripts/image_signing/tag_image.sh:120: echo "Image is modified. Please remember
to resign your image."
This got me thinking. It would be a good idea that whenever a tagging/stamping
is performed, we should create a file somewhere well known on the rootfs that
says - .needs_to_be_signed. This file should be removed when the signing is
performed. If we have a standard set of stamping/tagging scripts that anyone
uses, this would be good. I see often that people modify images (say, change the
lsb-release-description) and don't realize that the image needs to be re-signed
(even if using devkeys).

What do you think?

[Hung-Te, 2010-09-30 07:48:00]

http://codereview.chromium.org/3604001/diff/3001/4004
File scripts/image_signing/tag_image.sh (right):

http://codereview.chromium.org/3604001/diff/3001/4004#newcode15
scripts/image_signing/tag_image.sh:15: DEFINE_boolean enable_update_firmware
${FLAGS_FALSE} \
It was made as 2 param because I wanted to have report,enable,disable to be
explicitly assigned.

I agree combining to only one param is better so I'll change that to tag="1" /
"0" format.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode19
scripts/image_signing/tag_image.sh:19: DEFINE_boolean enable_dev_mode
${FLAGS_FALSE} \
On 2010/09/30 04:28:48, gauravsh wrote:
> ditto for the dev_mode flag. this will then follow normal convention that is
> followed for optional arguments in other cros scripts.

Done.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode40
scripts/image_signing/tag_image.sh:40: # syntax: process_tag name root
tag_file_path enable disable
On 2010/09/30 04:28:48, gauravsh wrote:
> also, it is not clear from the comment what each of these arguments mean or
do.

Done.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode43
scripts/image_signing/tag_image.sh:43: if [ $4 = ${FLAGS_TRUE} -a $5 =
${FLAGS_TRUE} ]; then
On 2010/09/30 04:28:48, gauravsh wrote:
> nit: instantiate local vars with the passed in function arguments. this makes
it
> easy to see what exactly is being checked.

Done.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode48
scripts/image_signing/tag_image.sh:48: if [ -f "$2$3" ]; then
My bad. the params used in this function was very simple at first which drove me
to $1 $2 $3 etc.

Changed to meaningful variables.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode80
scripts/image_signing/tag_image.sh:80: if ! [[ -x ./unpack_partitions.sh && -x
./pack_partitions.sh ]]; then
Using pack/unpack_partitions.sh prevents the dependency of cgpt, which was
discussed in some factory scripts in crosutils.

Since we're in vboot_ref folder, I'll follow the convention here and change to
mount_image_partition.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode86
scripts/image_signing/tag_image.sh:86: mkdir -p ./rootfs
That portion of code was copied from crosutils... anyway since we're in
vboot_referenece now, I'll follow the convention here.

http://codereview.chromium.org/3604001/diff/3001/4004#newcode120
scripts/image_signing/tag_image.sh:120: echo "Image is modified. Please remember
to resign your image."
I like the idea. Added modification to support this in common.sh and to clear
the tag in sign_official_build.sh

[gauravsh, 2010-09-30 07:59:43]

lgtm with some minor nits. Thanks Hung-Te!

http://codereview.chromium.org/3604001/diff/8001/9004
File scripts/image_signing/sign_official_build.sh (right):

http://codereview.chromium.org/3604001/diff/8001/9004#newcode130
scripts/image_signing/sign_official_build.sh:130: sudo umount "${rootfs_dir}"
-d here to free the loop device

http://codereview.chromium.org/3604001/diff/8001/9004#newcode134
scripts/image_signing/sign_official_build.sh:134: sudo umount "${rootfs_dir}"
-d

http://codereview.chromium.org/3604001/diff/8001/9005
File scripts/image_signing/tag_image.sh (right):

http://codereview.chromium.org/3604001/diff/8001/9005#newcode86
scripts/image_signing/tag_image.sh:86: if [ "$do_enable" = ${FLAGS_TRUE} ]; then
nit: for consistency enclose var names within parentheses { }

Same comment for the rest of the file.

[Hung-Te, 2010-09-30 08:13:08]

http://codereview.chromium.org/3604001/diff/8001/9004
File scripts/image_signing/sign_official_build.sh (right):

http://codereview.chromium.org/3604001/diff/8001/9004#newcode130
scripts/image_signing/sign_official_build.sh:130: sudo umount "${rootfs_dir}"
On 2010/09/30 07:59:44, gauravsh wrote:
> -d here to free the loop device

Done.

http://codereview.chromium.org/3604001/diff/8001/9004#newcode134
scripts/image_signing/sign_official_build.sh:134: sudo umount "${rootfs_dir}"
On 2010/09/30 07:59:44, gauravsh wrote:
> -d

Done.

http://codereview.chromium.org/3604001/diff/8001/9005
File scripts/image_signing/tag_image.sh (right):

http://codereview.chromium.org/3604001/diff/8001/9005#newcode86
scripts/image_signing/tag_image.sh:86: if [ "$do_enable" = ${FLAGS_TRUE} ]; then
On 2010/09/30 07:59:44, gauravsh wrote:
> nit: for consistency enclose var names within parentheses { }
> 
> Same comment for the rest of the file.

Done.