Move to using chromeos-test to emerge test dependencies in mod_image_for_test.

Move to using chromeos-test to emerge test dependencies in mod_image_for_test.

In addition unifies changes in mount_gpt_image and one-offs in mod_image_for_test
to consolidate gpt / var mounting.

[sosa, 2010-05-13 01:56:56]

Changes to get us to emerge chromeos-test as part of mod_image_for_test and also
have mod_image_for_test uses ./mount_gpt_image.sh and not the custom code added.

@cmasone - can you look specifically at the mount_gpt_image changes.  This
should also address issues you were seeing earlier

@petkov - mod_image_for_test changes

TEST=Tested by building an image and modding it for test as well as mounting and
modifying the most recent image and an image from a usb key.

[Chris Masone, 2010-05-13 02:05:56]

mount_gpt_image lgtm

On 2010/05/13 01:56:56, sosa wrote:
> Changes to get us to emerge chromeos-test as part of mod_image_for_test and
also
> have mod_image_for_test uses ./mount_gpt_image.sh and not the custom code
added.
> 
> @cmasone - can you look specifically at the mount_gpt_image changes.  This
> should also address issues you were seeing earlier
> 
> @petkov - mod_image_for_test changes
> 
> TEST=Tested by building an image and modding it for test as well as mounting
and
> modifying the most recent image and an image from a usb key.

[petkov, 2010-05-13 05:34:55]

A couple of nits and questions...

http://codereview.chromium.org/2064001/diff/2001/3002
File src/scripts/common.sh (right):

http://codereview.chromium.org/2064001/diff/2001/3002#newcode366
src/scripts/common.sh:366: # Sets up symlinks for the developer root.
It might be good to document this function better. For example, why does it
create both $dev_image_root/usr and $dev_image_root/local? And what's
/dev_image?

http://codereview.chromium.org/2064001/diff/2001/3002#newcode378
src/scripts/common.sh:378: if [ ${var_target} = "/var" ] ; then
No space before ; -- here and below.

http://codereview.chromium.org/2064001/diff/2001/3002#newcode389
src/scripts/common.sh:389: echo "*** ERROR: ${dev_image_root}/${path} should be
a symlink if exists"
Can you use the die or warn functions here and below?

http://codereview.chromium.org/2064001/diff/2001/3003
File src/scripts/mod_image_for_test.sh (right):

http://codereview.chromium.org/2064001/diff/2001/3003#newcode17
src/scripts/mod_image_for_test.sh:17: assert_inside_chroot
I assume everybody's cool with this...

http://codereview.chromium.org/2064001/diff/2001/3003#newcode29
src/scripts/mod_image_for_test.sh:29: DEFINE_boolean installmask $FLAGS_TRUE \
Not directly part of this CL, but shouldn't all these DEFINEs be sorted
alphabetically?

http://codereview.chromium.org/2064001/diff/2001/3003#newcode82
src/scripts/mod_image_for_test.sh:82: if [[ $FLAGS_installmask -eq ${FLAGS_TRUE}
]] ; then
No space before ;

http://codereview.chromium.org/2064001/diff/2001/3003#newcode86
src/scripts/mod_image_for_test.sh:86: # Determine the root dir for test
packages.
So, mod_for_dev->mod_for_test is not the same as mod_for_test->mod_for_dev?
Maybe clarify in the comment.

[sosa, 2010-05-13 21:31:15]

http://codereview.chromium.org/2064001/diff/2001/3002
File src/scripts/common.sh (right):

http://codereview.chromium.org/2064001/diff/2001/3002#newcode366
src/scripts/common.sh:366: # Sets up symlinks for the developer root.
On 2010/05/13 05:34:55, petkov wrote:
> It might be good to document this function better. For example, why does it
> create both $dev_image_root/usr and $dev_image_root/local? And what's
> /dev_image?
> 

Done.

http://codereview.chromium.org/2064001/diff/2001/3002#newcode378
src/scripts/common.sh:378: if [ ${var_target} = "/var" ] ; then
On 2010/05/13 05:34:55, petkov wrote:
> No space before ; -- here and below.
> 

Done.

http://codereview.chromium.org/2064001/diff/2001/3002#newcode389
src/scripts/common.sh:389: echo "*** ERROR: ${dev_image_root}/${path} should be
a symlink if exists"
On 2010/05/13 05:34:55, petkov wrote:
> Can you use the die or warn functions here and below?
> 

Good call. Done.

http://codereview.chromium.org/2064001/diff/2001/3003
File src/scripts/mod_image_for_test.sh (right):

http://codereview.chromium.org/2064001/diff/2001/3003#newcode17
src/scripts/mod_image_for_test.sh:17: assert_inside_chroot
On 2010/05/13 05:34:55, petkov wrote:
> I assume everybody's cool with this...
This is already a requirement, just isn't explicit.

http://codereview.chromium.org/2064001/diff/2001/3003#newcode29
src/scripts/mod_image_for_test.sh:29: DEFINE_boolean installmask $FLAGS_TRUE \
Should they?  It doesn't hurt.  Done.

On 2010/05/13 05:34:55, petkov wrote:
> Not directly part of this CL, but shouldn't all these DEFINEs be sorted
> alphabetically?
>

http://codereview.chromium.org/2064001/diff/2001/3003#newcode86
src/scripts/mod_image_for_test.sh:86: # Determine the root dir for test
packages.
Nice catch.  I was sort of making an effort to make things backwards compatiable
(without statefuldev) but it doesn't seem wise.  I'll take out the check.


On 2010/05/13 05:34:55, petkov wrote:
> So, mod_for_dev->mod_for_test is not the same as mod_for_test->mod_for_dev?
> Maybe clarify in the comment.
>

[petkov, 2010-05-13 22:26:21]

A couple of nits.

LGTM

http://codereview.chromium.org/2064001/diff/10001/11002
File src/scripts/common.sh (right):

http://codereview.chromium.org/2064001/diff/10001/11002#newcode369
src/scripts/common.sh:369: # This avoids pacakges installing into
/usr/local/usr/local/bin.
s/pacakges/packages/

http://codereview.chromium.org/2064001/diff/10001/11003
File src/scripts/mod_image_for_test.sh (right):

http://codereview.chromium.org/2064001/diff/10001/11003#newcode22
src/scripts/mod_image_for_test.sh:22: DEFINE_boolean factory $FLAGS_FALSE
"Modify the image for manufacturing testing" f
80 chars

http://codereview.chromium.org/2064001/diff/10001/11003#newcode41
src/scripts/mod_image_for_test.sh:41: echo "*** mod_image_for_test failed.  No
board set and no image set"
This should be a die too... Unrelated to your CL though so don't worry.