build_image, mod_image_for_*: break out make_image_bootable

build_image, mod_image_for_*: break out make_image_bootable

Breaks make_image_bootable out of build_image into a separate
script.  In addition, it make a helper .sh in the OUTPUT_DIR
to let make_image_bootable be re-run on an image with the same
arguments that were passed in via build_image.

This change also removes the use of the boot/ directory in
OUTPUT_DIR.

TEST=build_image verified; image_to_usb --test_image; booted l13
           build_image; image_to_vm; qemu booted
           build_image verified; image_to_usb; booted l13
BUG=chromium-os:5081

[Will Drewry, 2010-08-05 01:57:19]

I'm still testing this change across different combinations but if anyone else
would be willing to patch it in and test, it be much appreciated.

Being able to build a test image with a verified rootfs will mean we can make
that default, and this change will make that possible!

thanks -
will

[anush, 2010-08-05 02:48:27]

Thanks for modularizing this. 

If we expect make_image_bootable to be called by the end user as a standalone
and not just as a helper it would be good to name it with cros_* and put it in
the crosutils.git:bin/ directory.  crosutils.git is currently being installed in
the chroot and we are migrating the scripts into it. They will be installed in
/usr/bin/crosutils and /usr/lib/crosutils/.

http://codereview.chromium.org/3066037/diff/4001/5002
File make_image_bootable (right):

http://codereview.chromium.org/3066037/diff/4001/5002#newcode3
make_image_bootable:3: # Copyright (c) 2009 The Chromium OS Authors. All rights
reserved.
2010 :)

http://codereview.chromium.org/3066037/diff/4001/5002#newcode175
make_image_bootable:175: # TODO(wad) mmcblk1p3 is hardcoded for arm for now!
Just FYI we are trying to get all the per board "image finalization" into the
overlay. So we can pull in per board/variant specific image customization since
elif all the combinations wont scale.

[Will Drewry, 2010-08-05 03:00:17]

On 2010/08/05 02:48:27, anush wrote:
> Thanks for modularizing this. 

No problem!  It'd been my plan since I centralized this code in a function in
build_image.

> If we expect make_image_bootable to be called by the end user as a standalone
> and not just as a helper it would be good to name it with cros_* and put it in
> the crosutils.git:bin/ directory.  crosutils.git is currently being installed
in
> the chroot and we are migrating the scripts into it. They will be installed in
> /usr/bin/crosutils and /usr/lib/crosutils/.

So I actually expect users to call ${OUTPUT_DIR}/make_image_bootable_helper.sh
which will call this script with the right commandline arguments.

It can be called manually on any image, but the number of cmdline options to get
right is out of control.

If you'd prefer it be some where specific (bin, whereve), I'm happy to rename it
and put it there!  Unlike update_bootloader.sh, this is actually mean to be safe
for direct use.

thanks!

http://codereview.chromium.org/3066037/diff/4001/5002
File make_image_bootable (right):

http://codereview.chromium.org/3066037/diff/4001/5002#newcode3
make_image_bootable:3: # Copyright (c) 2009 The Chromium OS Authors. All rights
reserved.
On 2010/08/05 02:48:27, anush wrote:
> 2010 :)

Done.

http://codereview.chromium.org/3066037/diff/4001/5002#newcode175
make_image_bootable:175: # TODO(wad) mmcblk1p3 is hardcoded for arm for now!
On 2010/08/05 02:48:27, anush wrote:
> Just FYI we are trying to get all the per board "image finalization" into the
> overlay. So we can pull in per board/variant specific image customization
since
> elif all the combinations wont scale. 

FWIW, we won't need to specify the usb disk since we're working booting USB
using GPT UUID.  So this will go away.

However, the bootloader stuff for arm still needs to be sorted out.  Once it can
boot using our kernel partitions, it won't matter as much but for the current
approach, I can't easily make it use a verified rootfs.

[anush, 2010-08-05 03:47:05]

On Wed, Aug 4, 2010 at 8:00 PM, <wad@chromium.org> wrote:

> On 2010/08/05 02:48:27, anush wrote:
>
>> Thanks for modularizing this.
>>
>
> No problem!  It'd been my plan since I centralized this code in a function
> in
> build_image.
>
>
>  If we expect make_image_bootable to be called by the end user as a
>> standalone
>> and not just as a helper it would be good to name it with cros_* and put
>> it in
>> the crosutils.git:bin/ directory.  crosutils.git is currently being
>> installed
>>
> in
>
>> the chroot and we are migrating the scripts into it. They will be
>> installed in
>> /usr/bin/crosutils and /usr/lib/crosutils/.
>>
>
> So I actually expect users to call
> ${OUTPUT_DIR}/make_image_bootable_helper.sh
> which will call this script with the right commandline arguments.
>
> It can be called manually on any image, but the number of cmdline options
> to get
> right is out of control.
>
> If you'd prefer it be some where specific (bin, whereve), I'm happy to
> rename it
> and put it there!  Unlike update_bootloader.sh, this is actually mean to be
> safe
> for direct use.
>
>
Instead of autogenerating a script to be run from an output dir, what do you
think of creating an ${OUTPUT_DIR}/build.desc (or something) which contains
meta data about the particular build like what flags were used etc (and
could grow to include more than just what is required for
make_image_bootable.sh). And we have a bin/cros_make_image_bootable.sh
<OUTPUT_DIR> that works on the build.desc file and does the right thing for
the particular image.



> thanks!
>
>
>
> http://codereview.chromium.org/3066037/diff/4001/5002
> File make_image_bootable (right):
>
> http://codereview.chromium.org/3066037/diff/4001/5002#newcode3
> make_image_bootable:3: # Copyright (c) 2009 The Chromium OS Authors. All
> rights reserved.
> On 2010/08/05 02:48:27, anush wrote:
>
>> 2010 :)
>>
>
> Done.
>
>
> http://codereview.chromium.org/3066037/diff/4001/5002#newcode175
> make_image_bootable:175: # TODO(wad) mmcblk1p3 is hardcoded for arm for
> now!
> On 2010/08/05 02:48:27, anush wrote:
>
>> Just FYI we are trying to get all the per board "image finalization"
>>
> into the
>
>> overlay. So we can pull in per board/variant specific image
>>
> customization since
>
>> elif all the combinations wont scale.
>>
>
> FWIW, we won't need to specify the usb disk since we're working booting
> USB using GPT UUID.  So this will go away.
>
> However, the bootloader stuff for arm still needs to be sorted out.
> Once it can boot using our kernel partitions, it won't matter as much
> but for the current approach, I can't easily make it use a verified
> rootfs.
>
>
> http://codereview.chromium.org/3066037/show
>

[gauravsh, 2010-08-05 04:04:41]

http://codereview.chromium.org/3066037/diff/1/3
File make_image_bootable (right):

http://codereview.chromium.org/3066037/diff/1/3#newcode3
make_image_bootable:3: # Copyright (c) 2009 The Chromium OS Authors. All rights
reserved.
update copyright

http://codereview.chromium.org/3066037/diff/1/3#newcode102
make_image_bootable:102: root_dev=$(mount | grep -- "on
${FLAGS_rootfs_mountpoint} type" | cut -f1 -d' ' | tail -1)
80 chars

http://codereview.chromium.org/3066037/diff/1/3#newcode147
make_image_bootable:147: sudo cp "${FLAGS_output_dir}/vmlinuz_hd.vblock"
"${FLAGS_statefulfs_mountpoint}"
80 chars

http://codereview.chromium.org/3066037/diff/1/3#newcode182
make_image_bootable:182: local kpart_size="--kernel_partition_sectors="
=${ksize}?

http://codereview.chromium.org/3066037/diff/6001/7001
File build_image (right):

http://codereview.chromium.org/3066037/diff/6001/7001#newcode632
build_image:632: # Create the make_image_bootable wrapper scrupt
s/scrupt/script/

[Will Drewry, 2010-08-05 15:50:57]

On 2010/08/05 03:47:05, anush wrote:
>
> Instead of autogenerating a script to be run from an output dir, what do you
> think of creating an ${OUTPUT_DIR}/build.desc (or something) which contains
> meta data about the particular build like what flags were used etc (and
> could grow to include more than just what is required for
> make_image_bootable.sh). And we have a bin/cros_make_image_bootable.sh
> <OUTPUT_DIR> that works on the build.desc file and does the right thing for
> the particular image.

Hrm. Making a generic build.desc is a huge undertaking because shflags doesn't
ignore unknown flags and not all flags are matched perfectly across helpers. 
However, maybe a first start in the direction would be to emit:
  boot.desc
then have cros_make_image_bootable ${OUTPUT_DIR} ${IMAGE_NAME} use it.

I'll send that up and see what you think.  It could be a good starting point!

[Will Drewry, 2010-08-05 16:04:07]

Updated for comments and for anush's comments. 

PTAL! (running tests now...)

http://codereview.chromium.org/3066037/diff/1/3
File make_image_bootable (right):

http://codereview.chromium.org/3066037/diff/1/3#newcode3
make_image_bootable:3: # Copyright (c) 2009 The Chromium OS Authors. All rights
reserved.
On 2010/08/05 04:04:41, gauravsh wrote:
> update copyright
> 

Done.

http://codereview.chromium.org/3066037/diff/1/3#newcode102
make_image_bootable:102: root_dev=$(mount | grep -- "on
${FLAGS_rootfs_mountpoint} type" | cut -f1 -d' ' | tail -1)
On 2010/08/05 04:04:41, gauravsh wrote:
> 80 chars

Done.

http://codereview.chromium.org/3066037/diff/1/3#newcode147
make_image_bootable:147: sudo cp "${FLAGS_output_dir}/vmlinuz_hd.vblock"
"${FLAGS_statefulfs_mountpoint}"
On 2010/08/05 04:04:41, gauravsh wrote:
> 80 chars

Done.

http://codereview.chromium.org/3066037/diff/1/3#newcode182
make_image_bootable:182: local kpart_size="--kernel_partition_sectors="
On 2010/08/05 04:04:41, gauravsh wrote:
> =${ksize}?

The assignment continues below with partsize..., but I havent change any of this
code. This is a cut and paste and search and replace from build_image to here :)

[anush, 2010-08-05 18:53:30]

LGTM.

On 2010/08/05 16:04:07, Will Drewry wrote:
> Updated for comments and for anush's comments. 
> 
> PTAL! (running tests now...)
> 
> http://codereview.chromium.org/3066037/diff/1/3
> File make_image_bootable (right):
> 
> http://codereview.chromium.org/3066037/diff/1/3#newcode3
> make_image_bootable:3: # Copyright (c) 2009 The Chromium OS Authors. All
rights
> reserved.
> On 2010/08/05 04:04:41, gauravsh wrote:
> > update copyright
> > 
> 
> Done.
> 
> http://codereview.chromium.org/3066037/diff/1/3#newcode102
> make_image_bootable:102: root_dev=$(mount | grep -- "on
> ${FLAGS_rootfs_mountpoint} type" | cut -f1 -d' ' | tail -1)
> On 2010/08/05 04:04:41, gauravsh wrote:
> > 80 chars
> 
> Done.
> 
> http://codereview.chromium.org/3066037/diff/1/3#newcode147
> make_image_bootable:147: sudo cp "${FLAGS_output_dir}/vmlinuz_hd.vblock"
> "${FLAGS_statefulfs_mountpoint}"
> On 2010/08/05 04:04:41, gauravsh wrote:
> > 80 chars
> 
> Done.
> 
> http://codereview.chromium.org/3066037/diff/1/3#newcode182
> make_image_bootable:182: local kpart_size="--kernel_partition_sectors="
> On 2010/08/05 04:04:41, gauravsh wrote:
> > =${ksize}?
> 
> The assignment continues below with partsize..., but I havent change any of
this
> code. This is a cut and paste and search and replace from build_image to here
:)