Workaround chrome-bot's special case wackiness.

Workaround chrome-bot's special case wackiness.

BUG=4547
TEST="untestable - the failure can only occur on chrome-bot, which I don't have access to "

Change-Id: Ia90eaa7c673f16533dbc397adf7f76e6404c6d2b

[Nick Sanders, 2010-10-23 01:28:15]

Second try for install shim build. Add compatibility for chrome-bot specific
naming scheme.

[David James, 2010-10-23 02:11:54]

Some nits. Please test with USER=chrome-bot, since we know that's the
special-case in chromeos_version.sh that was causing trouble for you.

http://codereview.chromium.org/3978005/diff/2001/3001
File archive_build.sh (right):

http://codereview.chromium.org/3978005/diff/2001/3001#newcode62
archive_build.sh:62: IMG_DIR="$(ls -t1 ${IMAGES_DIR}/${FLAGS_board} 2>&-| head
-1)"
Depending on timestamps is bad. Can you use $(readlink
$IMAGES_DIR/$FLAGS_board/latest) instead?

http://codereview.chromium.org/3978005/diff/2001/3001#newcode76
archive_build.sh:76: echo "test_mod requires that the default from path be
used."
Indentation looks off here. Should have two less spaces.

http://codereview.chromium.org/3978005/diff/2001/3001#newcode164
archive_build.sh:164: BUILDVER=$(ls -t1 $IMAGES_DIR/$FLAGS_board 2>&-| head -1)
Looks like you already grabbed this info. Can you reuse IMG_DIR instead?

http://codereview.chromium.org/3978005/diff/2001/3001#newcode183
archive_build.sh:183: ./enter_chroot.sh -- ./build_image --board $FLAGS_board
--factory_install \
This script is for archiving the build, not for building images. It would make
more sense to add another step to the buildbot than to muck up archive_build
with this type of stuff.

Can you add a HACK comment explaining the problems above?

http://codereview.chromium.org/3978005/diff/2001/3001#newcode184
archive_build.sh:184: --replace --build_attempt 7
How about --build_attempt factory_install_shim ? It doesn't need to be a number.

http://codereview.chromium.org/3978005/diff/2001/3001#newcode188
archive_build.sh:188: # release umages will remain in IMG_DIR, defined
previously.
s/umages/images/

http://codereview.chromium.org/3978005/diff/2001/3001#newcode189
archive_build.sh:189: SHIM_DIR="$(ls -t1 $IMAGES_DIR/$FLAGS_board 2>&-| head
-1)"
Can you use $(readlink $IMAGES_DIR/latest) instead?

http://codereview.chromium.org/3978005/diff/2001/3001#newcode197
archive_build.sh:197: [ -n "${IMG_DIR}" ] && ln -sf "${IMG_DIR}" latest
This won't work -- rather than replacing latest it'll create a link inside
latest :( You might want to just rm -f latest first.

You can also simplify this code a bunch since you know that DEFAULT_USED=1.
E.g.:
  rm -f $IMAGES_DIR/$FLAGS_board/latest
  ln -sf $FLAGS_from $IMAGES_DIR/$FLAGS_board/latest
  touch $FLAGS_from

http://codereview.chromium.org/3978005/diff/2001/3001#newcode214
archive_build.sh:214: [ -n "${SHIM_DIR}" ] && ln -sf "${SHIM_DIR}" factory_shim
The -f flags here don't work because factory_shim is a link to a directory. Can
you rm -f these symlinks first before running ln?

http://codereview.chromium.org/3978005/diff/2001/3001#newcode215
archive_build.sh:215: [ -n "${IMG_DIR}" ] && ln -sf "${IMG_DIR}" factory_test
Same here.

[Nick Sanders, 2010-10-23 02:52:01]

http://codereview.chromium.org/3978005/diff/2001/3001
File archive_build.sh (right):

http://codereview.chromium.org/3978005/diff/2001/3001#newcode62
archive_build.sh:62: IMG_DIR="$(ls -t1 ${IMAGES_DIR}/${FLAGS_board} 2>&-| head
-1)"
This is changing previous functionality unrelated to this CL. But I'll do it.

http://codereview.chromium.org/3978005/diff/2001/3001#newcode76
archive_build.sh:76: echo "test_mod requires that the default from path be
used."
On 2010/10/23 02:11:55, David James wrote:
> Indentation looks off here. Should have two less spaces.

Done.

http://codereview.chromium.org/3978005/diff/2001/3001#newcode164
archive_build.sh:164: BUILDVER=$(ls -t1 $IMAGES_DIR/$FLAGS_board 2>&-| head -1)
The earlier query is conditional, it's not guaranteed to be set at this point.

http://codereview.chromium.org/3978005/diff/2001/3001#newcode183
archive_build.sh:183: ./enter_chroot.sh -- ./build_image --board $FLAGS_board
--factory_install \
On 2010/10/23 02:11:55, David James wrote:
> This script is for archiving the build, not for building images. It would make
> more sense to add another step to the buildbot than to muck up archive_build
> with this type of stuff.
> 
> Can you add a HACK comment explaining the problems above?

Done.

http://codereview.chromium.org/3978005/diff/2001/3001#newcode184
archive_build.sh:184: --replace --build_attempt 7
It's an integer type, so it needs to be a number
"DEFINE_integer build_attempt"

http://codereview.chromium.org/3978005/diff/2001/3001#newcode188
archive_build.sh:188: # release umages will remain in IMG_DIR, defined
previously.
On 2010/10/23 02:11:55, David James wrote:
> s/umages/images/

Done.

http://codereview.chromium.org/3978005/diff/2001/3001#newcode189
archive_build.sh:189: SHIM_DIR="$(ls -t1 $IMAGES_DIR/$FLAGS_board 2>&-| head
-1)"
On 2010/10/23 02:11:55, David James wrote:
> Can you use $(readlink $IMAGES_DIR/latest) instead? 

Done.

http://codereview.chromium.org/3978005/diff/2001/3001#newcode197
archive_build.sh:197: [ -n "${IMG_DIR}" ] && ln -sf "${IMG_DIR}" latest
I noticed this while testing sequential runs. latest CL has this fix.

http://codereview.chromium.org/3978005/diff/2001/3001#newcode214
archive_build.sh:214: [ -n "${SHIM_DIR}" ] && ln -sf "${SHIM_DIR}" factory_shim
On 2010/10/23 02:11:55, David James wrote:
> The -f flags here don't work because factory_shim is a link to a directory.
Can
> you rm -f these symlinks first before running ln?

Done.

http://codereview.chromium.org/3978005/diff/2001/3001#newcode215
archive_build.sh:215: [ -n "${IMG_DIR}" ] && ln -sf "${IMG_DIR}" factory_test
On 2010/10/23 02:11:55, David James wrote:
> Same here.

Done.

[davidjames, 2010-10-23 04:14:30]

Thanks, looks much better. LGTM.

[Nick Sanders, 2010-10-23 04:14:43]

new cleaner version