Remove old logic ... and don't needlessly modifiy dev image.

Remove old logic ... and don't needlessly modifiy dev image.

Change-Id: I9b4047f2f124875553542df73e16cba6be41c561

BUG=chromium-os:chromium-os:8364
TEST=Ran it with pfq options.

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

[sosa, 2011-02-14 22:47:46]

Cleaning this up.  

Your --noinplace change already gets this done (equivalent of copying and using
copy).  We shouldn't build multiple test images

[sjg, 2011-02-15 15:19:41]

http://codereview.chromium.org/6527001/diff/1/archive_build.sh
File archive_build.sh (right):

http://codereview.chromium.org/6527001/diff/1/archive_build.sh#newcode142
archive_build.sh:142: SRC_IMAGE="${FLAGS_from}/chromiumos_image.bin"
You can use ${CHROMEOS_IMAGE_NAME} here if you like.

http://codereview.chromium.org/6527001/diff/1/archive_build.sh#newcode149
archive_build.sh:149: OUTPUT_IMAGE=$1
Great, a use for --noinplace.

I think this function could go away - $1 is not used and $2 is blank!

Also you might find that you can use prepare_test_image() here (from common.sh).

But given that you are already cleaning up code, I'm happy to LGTM if this is as
far as you want to go.

[sosa, 2011-02-15 22:06:11]

Actually what I had was broken.  I still need the OUTPUT_IMAGE to be different
and $2 is used with the factory image.  This is because we have to create both a
test image and a factory test image in the same go.  I could add this logic to
mod_image_for_test or prepare_test_image but not sure that's the right place to
put it i.e. OUTPUT_IMAGE=

[sosa, 2011-02-15 22:08:12]

http://codereview.chromium.org/6527001/diff/1/archive_build.sh
File archive_build.sh (right):

http://codereview.chromium.org/6527001/diff/1/archive_build.sh#newcode142
archive_build.sh:142: SRC_IMAGE="${FLAGS_from}/chromiumos_image.bin"
On 2011/02/15 15:19:42, sjg wrote:
> You can use ${CHROMEOS_IMAGE_NAME} here if you like.

Done.

http://codereview.chromium.org/6527001/diff/1/archive_build.sh#newcode149
archive_build.sh:149: OUTPUT_IMAGE=$1
See note.  Still want in place but I need the mv's for Factory images

On 2011/02/15 15:19:42, sjg wrote:
> Great, a use for --noinplace.
> 
> I think this function could go away - $1 is not used and $2 is blank!
> 
> Also you might find that you can use prepare_test_image() here (from
common.sh).
> 
> But given that you are already cleaning up code, I'm happy to LGTM if this is
as
> far as you want to go.

[sjg, 2011-02-15 22:25:34]

My thoughts...

http://codereview.chromium.org/6527001/diff/4004/archive_build.sh
File archive_build.sh (right):

http://codereview.chromium.org/6527001/diff/4004/archive_build.sh#newcode154
archive_build.sh:154: mv "${SRC_IMAGE}" "${OUTPUT_IMAGE}"
This overwrites the test image!

http://codereview.chromium.org/6527001/diff/4004/archive_build.sh#newcode162
archive_build.sh:162: 
Well in that case I don't really understand this. You are not doing an inplace
modify, so it will create a file called ${CHROMEOS_TEST_IMAGE_NAME}. So maybe
something like:

./enter_chroot.sh -- ./mod_image_for_test.sh --board $FLAGS_board --yes
--noinplace

is all you need. And no need for the function at all which just confuses things,
since if you happen to pass in ${CHROMEOS_TEST_IMAGE_NAME} as an argument bad
things might happen.

[sosa, 2011-02-16 00:32:39]

How about this?  This seems more clean

[sosa, 2011-02-16 00:38:23]

Ugh nvm ... won't work .. can't call prepare test image as it sorta
assumes we're in the chroot already.  Lemme see if I can find a
workaround

On Tue, Feb 15, 2011 at 4:32 PM,  <sosa@chromium.org> wrote:
> How about this?  This seems more clean
>
> http://codereview.chromium.org/6527001/
>

[sosa, 2011-02-16 01:19:47]

PTAL

I tried using prepare_test_image but ran into chroot'ing issues.  archive build
is intended to be run outside the chroot ... so without adding and refactoring
much more code I won't be able to get the right paths etc.

Since it's much less hectic to just call enter_chroot -- here This seemed like
the more clean cleanup (especially since archive_build is being deprecated
soonish).

[sjg, 2011-02-16 02:04:51]

LGTM

(Haven't tried it though)

http://codereview.chromium.org/6527001/diff/6007/archive_build.sh
File archive_build.sh (right):

http://codereview.chromium.org/6527001/diff/6007/archive_build.sh#newcode170
archive_build.sh:170: rm -f "${FLAGS_from}/${CHROMEOS_IMAGE_NAME}"
Yes this seems much better.

I guess you are wanting to remove the original image and just leave the test and
factory images, right?

[sosa, 2011-02-16 02:34:45]

Yup!

On Tue, Feb 15, 2011 at 6:04 PM,  <sjg@chromium.org> wrote:
> LGTM
>
> (Haven't tried it though)
>
>
> http://codereview.chromium.org/6527001/diff/6007/archive_build.sh
> File archive_build.sh (right):
>
> http://codereview.chromium.org/6527001/diff/6007/archive_build.sh#newcode170
> archive_build.sh:170: rm -f "${FLAGS_from}/${CHROMEOS_IMAGE_NAME}"
> Yes this seems much better.
>
> I guess you are wanting to remove the original image and just leave the
> test and factory images, right?
>
> http://codereview.chromium.org/6527001/
>