Allow boot directly into mfg test.

Allow boot directly into mfg test.

Add feature to mod_image_for_test to patch rootfs.
Change initctl path to get network but no chrome.
./image_to_usb.sh --install_mfg --install_autotest --test_image
which calls ./mod_image_for_test.sh --manuf

[Nick Sanders, 2010-04-06 05:01:19]

Not really done yet, but you can take a look at how we can boot into mfg test. 

Do we want to separate qualdb check from boot into mfg?

[Tom Wai-Hong Tam, 2010-04-06 13:16:19]

Do you mind to unify all the names "mfg", "manuf", and "manufacturing" into
"factory"?

[Nick Sanders, 2010-04-09 06:24:39]

More or less working factory image build script.

http://codereview.chromium.org/1542011/diff/10001/11002
File src/scripts/mod_for_factory_scripts/factory.patch (right):

http://codereview.chromium.org/1542011/diff/10001/11002#newcode24
src/scripts/mod_for_factory_scripts/factory.patch:24: +mount -norw,remount /
writable root partition is not necessary for shipping version.

http://codereview.chromium.org/1542011/diff/10001/11002#newcode25
src/scripts/mod_for_factory_scripts/factory.patch:25: +sleep 20
sleep should be replaced with upstart messages, if I can figure out how upstart
works.

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

http://codereview.chromium.org/1542011/diff/10001/11003#newcode126
src/scripts/mod_image_for_test.sh:126: echo "NO to factory"
just debugging info

http://codereview.chromium.org/1542011/diff/10001/11003#newcode168
src/scripts/mod_image_for_test.sh:168: echo "No qualified component file found
at: ${FLAGS_qualdb}"
Is this file available somewhere?

[Tom Wai-Hong Tam, 2010-04-09 07:02:45]

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

http://codereview.chromium.org/1542011/diff/10001/11003#newcode168
src/scripts/mod_image_for_test.sh:168: echo "No qualified component file found
at: ${FLAGS_qualdb}"
This file is one of the result files of latest qual test. It's
/tmp/run_remote_tests.XXXX/hardware_Components,*/hardware_Components/results/system_components,
where XXXX is a random number. We assume that before running the
mod_image_for_test, the approved component test should be run and passed.

[Tammo Spalink, 2010-04-14 05:38:13]

LGTM

Lets get this checked in, that way we can all try it out and iterate over any
issues -- this looks to not affect non-factory scenarious, and hence should be
safe to check in regardless of completeness.

On 2010/04/09 07:02:45, Tom Wai-Hong Tam wrote:
> http://codereview.chromium.org/1542011/diff/10001/11003
> File src/scripts/mod_image_for_test.sh (right):
> 
> http://codereview.chromium.org/1542011/diff/10001/11003#newcode168
> src/scripts/mod_image_for_test.sh:168: echo "No qualified component file found
> at: ${FLAGS_qualdb}"
> This file is one of the result files of latest qual test. It's
>
/tmp/run_remote_tests.XXXX/hardware_Components,*/hardware_Components/results/system_components,
> where XXXX is a random number. We assume that before running the
> mod_image_for_test, the approved component test should be run and passed.

[kmixter1, 2010-04-14 17:00:58]

http://codereview.chromium.org/1542011/diff/7002/2002
File src/scripts/image_to_usb.sh (right):

http://codereview.chromium.org/1542011/diff/7002/2002#newcode98
src/scripts/image_to_usb.sh:98: sudo echo "Done"
Use sudo -v instead

http://codereview.chromium.org/1542011/diff/7002/2003
File src/scripts/mod_for_factory_scripts/factory.patch (right):

http://codereview.chromium.org/1542011/diff/7002/2003#newcode46
src/scripts/mod_for_factory_scripts/factory.patch:46: +sleep 10
why is this necessary?

http://codereview.chromium.org/1542011/diff/7002/2004
File src/scripts/mod_image_for_test.sh (right):

http://codereview.chromium.org/1542011/diff/7002/2004#newcode122
src/scripts/mod_image_for_test.sh:122: echo "Checking to mode for factory -
FLAGS_factory = ${FLAGS_factory}"
Seems like we're saying if factory is enabled twice.  Is this even necessary
once considering we say later that we're applying the factory patch?

http://codereview.chromium.org/1542011/diff/7002/2004#newcode133
src/scripts/mod_image_for_test.sh:133: # No Chrome UI. Just delete it.
Comment does not match code.

http://codereview.chromium.org/1542011/diff/7002/2004#newcode147
src/scripts/mod_image_for_test.sh:147: sudo bash -c "echo 'Version 0.0.0' >
${ROOT_FS_DIR}/.factory_image"
Seems strange to have another version number.  Why not use the lsb-release
version number wherever this is used?  Probably best to actually add "factory"
to the CHROMEOS_RELEASE_DESCRIPTION version instead of adding this file (see
mod_for_test_scripts/001changeBuildName).

[Nick Sanders, 2010-04-16 07:21:48]

http://codereview.chromium.org/1542011/diff/7002/2002
File src/scripts/image_to_usb.sh (right):

http://codereview.chromium.org/1542011/diff/7002/2002#newcode98
src/scripts/image_to_usb.sh:98: sudo echo "Done"
On 2010/04/14 17:00:58, kmixter1 wrote:
> Use sudo -v instead

Done.

http://codereview.chromium.org/1542011/diff/7002/2003
File src/scripts/mod_for_factory_scripts/factory.patch (right):

http://codereview.chromium.org/1542011/diff/7002/2003#newcode46
src/scripts/mod_for_factory_scripts/factory.patch:46: +sleep 10
On 2010/04/14 17:00:58, kmixter1 wrote:
> why is this necessary?

I don't know enough about how upstart works to figure out how to properly
sequence this, and I won't have time to figure it out by Friday and still get
other stuff done. I'll add a more explicit todo.

http://codereview.chromium.org/1542011/diff/7002/2004
File src/scripts/mod_image_for_test.sh (right):

http://codereview.chromium.org/1542011/diff/7002/2004#newcode122
src/scripts/mod_image_for_test.sh:122: echo "Checking to mode for factory -
FLAGS_factory = ${FLAGS_factory}"
On 2010/04/14 17:00:58, kmixter1 wrote:
> Seems like we're saying if factory is enabled twice.  Is this even necessary
> once considering we say later that we're applying the factory patch?

This is stale debug output, that has been deleted, but not CL uploaded.

http://codereview.chromium.org/1542011/diff/7002/2004#newcode133
src/scripts/mod_image_for_test.sh:133: # No Chrome UI. Just delete it.
On 2010/04/14 17:00:58, kmixter1 wrote:
> Comment does not match code.

Done.

http://codereview.chromium.org/1542011/diff/7002/2004#newcode147
src/scripts/mod_image_for_test.sh:147: sudo bash -c "echo 'Version 0.0.0' >
${ROOT_FS_DIR}/.factory_image"
On 2010/04/14 17:00:58, kmixter1 wrote:
> Seems strange to have another version number.  Why not use the lsb-release
> version number wherever this is used?  Probably best to actually add "factory"
> to the CHROMEOS_RELEASE_DESCRIPTION version instead of adding this file (see
> mod_for_test_scripts/001changeBuildName).

deleted

[Nick Sanders, 2010-04-16 07:42:16]

Including a couple of fixes to the install ehim, adding adlr

[Nick Sanders, 2010-04-16 09:40:49]

with these changes, the shim will d/l a factory+release image set, and if you
pull the usb key fast enough, then boot into the factory test sequence.

http://codereview.chromium.org/1542011/diff/28001/29002
File src/platform/factory_installer/factory_install.sh (right):

http://codereview.chromium.org/1542011/diff/28001/29002#newcode42
src/platform/factory_installer/factory_install.sh:42: for i in OEM STATE RELEASE
FACTORY; do
memento updater puts the most recent partition as the highest boot priority.
(well, marks it bootable anyways)

http://codereview.chromium.org/1542011/diff/28001/29003
File src/platform/memento_softwareupdate/memento_updater.sh (right):

http://codereview.chromium.org/1542011/diff/28001/29003#newcode173
src/platform/memento_softwareupdate/memento_updater.sh:173: if ! expr match
"$LOCAL_DEV" '^/dev/[a-z][a-z]*[123458]$' > /dev/null
allow update of part 8, for oem customization

http://codereview.chromium.org/1542011/diff/28001/29004
File src/scripts/image_to_usb.sh (right):

http://codereview.chromium.org/1542011/diff/28001/29004#newcode173
src/scripts/image_to_usb.sh:173: sudo ln -sf
/mnt/stateful_partition/dev_image${autotest_client} \
allow /usr/local/autotest

http://codereview.chromium.org/1542011/diff/28001/29004#newcode201
src/scripts/image_to_usb.sh:201: drive=$(echo "${FLAGS_to}" | sed -re
's/[0-9]+$//')
local cannot be used outside a function

[kmixter1, 2010-04-16 22:08:31]

http://codereview.chromium.org/1542011/diff/28001/29004
File src/scripts/image_to_usb.sh (right):

http://codereview.chromium.org/1542011/diff/28001/29004#newcode173
src/scripts/image_to_usb.sh:173: sudo ln -sf
/mnt/stateful_partition/dev_image${autotest_client} \
couldn't you write this as ${stateful_root}${autotest_client}?  If so I think it
would be clearer.

http://codereview.chromium.org/1542011/diff/28001/29004#newcode174
src/scripts/image_to_usb.sh:174: ${stateful_root}/autotest
btw - if i'm reading this right - mod_image_for_test is already doing this.  Can
we remove it from here?  I think it's perfectly reasonable to only support
install_autotest when test_image is also true.

http://codereview.chromium.org/1542011/diff/28001/29005
File src/scripts/mod_for_factory_scripts/factory.patch (right):

http://codereview.chromium.org/1542011/diff/28001/29005#newcode46
src/scripts/mod_for_factory_scripts/factory.patch:46: +sleep 10
Can you add an explicit TODO to fix this after the demo?  There should be a way
around it as the normal boot flow doesn't need to sleep.

[Nick Sanders, 2010-04-16 22:40:47]

http://codereview.chromium.org/1542011/diff/28001/29002
File src/platform/factory_installer/factory_install.sh (right):

http://codereview.chromium.org/1542011/diff/28001/29002#newcode16
src/platform/factory_installer/factory_install.sh:16: sleep 60
note sleep.

http://codereview.chromium.org/1542011/diff/28001/29004
File src/scripts/image_to_usb.sh (right):

http://codereview.chromium.org/1542011/diff/28001/29004#newcode173
src/scripts/image_to_usb.sh:173: sudo ln -sf
/mnt/stateful_partition/dev_image${autotest_client} \
On 2010/04/16 22:08:31, kmixter1 wrote:
> couldn't you write this as ${stateful_root}${autotest_client}?  If so I think
it
> would be clearer.

${stateful_root} has the build path in it. I could add a new variable
${client_stateful_root} set to /mnt/stateful_partition/dev_image if you think
that would be cleaner

Alternately I could make it a relative path. ./home/autotest-client

http://codereview.chromium.org/1542011/diff/28001/29004#newcode174
src/scripts/image_to_usb.sh:174: ${stateful_root}/autotest
On 2010/04/16 22:08:31, kmixter1 wrote:
> btw - if i'm reading this right - mod_image_for_test is already doing this. 
Can
> we remove it from here?  I think it's perfectly reasonable to only support
> install_autotest when test_image is also true.

Sosa changed the way /usr/local is mounted, the one in mod_image_for_test
doesn't work, and mod_image_for_test doesn't open the stateful partition.

[sosa, 2010-04-16 22:56:17]

http://codereview.chromium.org/1542011/diff/28001/29004
File src/scripts/image_to_usb.sh (right):

http://codereview.chromium.org/1542011/diff/28001/29004#newcode174
src/scripts/image_to_usb.sh:174: ${stateful_root}/autotest
If I read this correctly, you have this unusual circular thing.  You have,

/usr/local/autotest -> /usr/local/home/autotest_client ...

Why not just install directly to /usr/local/autotest? vs. home/autotest_client? 
Or is this just to get this fix in and worry about removing the symlink later?



On 2010/04/16 22:40:47, Nick Sanders wrote:
> On 2010/04/16 22:08:31, kmixter1 wrote:
> > btw - if i'm reading this right - mod_image_for_test is already doing this. 
> Can
> > we remove it from here?  I think it's perfectly reasonable to only support
> > install_autotest when test_image is also true.
> 
> Sosa changed the way /usr/local is mounted, the one in mod_image_for_test
> doesn't work, and mod_image_for_test doesn't open the stateful partition.

[Nick Sanders, 2010-04-16 23:08:20]

http://codereview.chromium.org/1542011/diff/28001/29004
File src/scripts/image_to_usb.sh (right):

http://codereview.chromium.org/1542011/diff/28001/29004#newcode174
src/scripts/image_to_usb.sh:174: ${stateful_root}/autotest
On 2010/04/16 22:56:17, sosa wrote:
> If I read this correctly, you have this unusual circular thing.  You have,
> 
> /usr/local/autotest -> /usr/local/home/autotest_client ...
> 
> Why not just install directly to /usr/local/autotest? vs.
home/autotest_client? 
> Or is this just to get this fix in and worry about removing the symlink later?

I'm not sure why autotest is installed at this location, I'd definitely prefer
moving it if no one cares about the old location.

[kmixter1, 2010-04-16 23:14:31]

/usr/local/autotest sounds good to me.

On Fri, Apr 16, 2010 at 4:08 PM,  <nsanders@chromium.org> wrote:
>
> http://codereview.chromium.org/1542011/diff/28001/29004
> File src/scripts/image_to_usb.sh (right):
>
> http://codereview.chromium.org/1542011/diff/28001/29004#newcode174
> src/scripts/image_to_usb.sh:174: ${stateful_root}/autotest
> On 2010/04/16 22:56:17, sosa wrote:
>>
>> If I read this correctly, you have this unusual circular thing.  You
>
> have,
>
>> /usr/local/autotest -> /usr/local/home/autotest_client ...
>
>> Why not just install directly to /usr/local/autotest? vs.
>
> home/autotest_client?
>>
>> Or is this just to get this fix in and worry about removing the
>
> symlink later?
>
> I'm not sure why autotest is installed at this location, I'd definitely
> prefer moving it if no one cares about the old location.
>
> http://codereview.chromium.org/1542011/show
>

[sosa, 2010-04-16 23:18:23]

sgtm 2.  I think it's an artifact from eric's changes early on that
installed autotest into /home/autotest so that autotest would be on
the stateful partition (since home is mounted from
/mnt/stateful_partition/home)

On Fri, Apr 16, 2010 at 4:14 PM, Ken Mixter <kmixter@chromium.org> wrote:
> /usr/local/autotest sounds good to me.
>
> On Fri, Apr 16, 2010 at 4:08 PM,  <nsanders@chromium.org> wrote:
>>
>> http://codereview.chromium.org/1542011/diff/28001/29004
>> File src/scripts/image_to_usb.sh (right):
>>
>> http://codereview.chromium.org/1542011/diff/28001/29004#newcode174
>> src/scripts/image_to_usb.sh:174: ${stateful_root}/autotest
>> On 2010/04/16 22:56:17, sosa wrote:
>>>
>>> If I read this correctly, you have this unusual circular thing.  You
>>
>> have,
>>
>>> /usr/local/autotest -> /usr/local/home/autotest_client ...
>>
>>> Why not just install directly to /usr/local/autotest? vs.
>>
>> home/autotest_client?
>>>
>>> Or is this just to get this fix in and worry about removing the
>>
>> symlink later?
>>
>> I'm not sure why autotest is installed at this location, I'd definitely
>> prefer moving it if no one cares about the old location.
>>
>> http://codereview.chromium.org/1542011/show
>>
>

[kmixter1, 2010-04-16 23:42:12]

LGTM

http://codereview.chromium.org/1542011/diff/28002/11008
File src/scripts/mod_for_factory_scripts/factory.patch (right):

http://codereview.chromium.org/1542011/diff/28002/11008#newcode48
src/scripts/mod_for_factory_scripts/factory.patch:48: +sleep 10
Still think this should be a TODO