run --mode=devstartup unconditionally if we booted developer firmware

run --mode=devstartup unconditionally if we booted developer firmware

Change-Id: Ice7d5343b50343f4cb0f323929c4e53132638cc8

BUG=none
TEST=boot system with developer mode firmware

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

[Randall Spangler, 2011-03-28 22:00:22]

LGTM with suggestion

http://codereview.chromium.org/6745022/diff/1/chromeos_startup
File chromeos_startup (right):

http://codereview.chromium.org/6745022/diff/1/chromeos_startup#newcode108
chromeos_startup:108: "$FIRMWARE_UPDATE_SCRIPT" --mode=devstartup 2>&1 |
If this returns error (for example, if 'mosys -k ec info' fails), we'll keep
going, right?  I don't see a 'set -e' above so I assume that's true.

http://codereview.chromium.org/6745022/diff/1/chromeos_startup#newcode121
chromeos_startup:121: set_startup_update_tries $(( $tries - 1 ))
I'd be tempted to move the decrement to just after line 112, so if anything goes
wrong in setting up the update (lines 113-119) we'll eventually run out of
tries.

[jrbarnette, 2011-03-29 00:14:03]

http://codereview.chromium.org/6745022/diff/1/chromeos_startup
File chromeos_startup (right):

http://codereview.chromium.org/6745022/diff/1/chromeos_startup#newcode105
chromeos_startup:105: if [ -x "$FIRMWARE_UPDATE_SCRIPT" ]; then
This is pre-existing, but can we delete it?  Is there any valid system
configuration that allows this script not to exist?

If necessary, I can file a bug to get this fixed separately.

http://codereview.chromium.org/6745022/diff/1/chromeos_startup#newcode106
chromeos_startup:106: if crossystem mainfw_type?developer; then
Ugh.  Is this really the syntax for crossystem arguments?
Using '?' in arguments is problematic, because the shell
interprets this character specially.  At minimum, this argument
needs to be quoted.  Better would be to fix the crossystem
command syntax.

http://codereview.chromium.org/6745022/diff/1/chromeos_startup#newcode108
chromeos_startup:108: "$FIRMWARE_UPDATE_SCRIPT" --mode=devstartup 2>&1 |
On 2011/03/28 22:00:25, Randall Spangler wrote:
> If this returns error (for example, if 'mosys -k ec info' fails), we'll keep
> going, right?  I don't see a 'set -e' above so I assume that's true.

There's no set -e in this script, so failures shouldn't stop things.
Moreover, the exit status of this pipeline is the exit status of the 'tee'
command, not the update script (at least with dash).  So, this command
won't fail unless 'tee' fails, which is sufficiently unlikely that we can
discount it (knock on wood).

[Stefan Reinauer, 2011-03-29 20:30:16]

ok, new version with (most of) the issues fixed

http://codereview.chromium.org/6745022/diff/1/chromeos_startup
File chromeos_startup (right):

http://codereview.chromium.org/6745022/diff/1/chromeos_startup#newcode105
chromeos_startup:105: if [ -x "$FIRMWARE_UPDATE_SCRIPT" ]; then
On 2011/03/29 00:14:03, jrbarnette wrote:
> This is pre-existing, but can we delete it?  Is there any valid system
> configuration that allows this script not to exist?
> 
> If necessary, I can file a bug to get this fixed separately.


Not sure if there is. On chromium builds the file seems to be there but empty.
There are a whole lot of use cases like this all over the init scripts. Maybe a
bug to clean them up would make sense?

http://codereview.chromium.org/6745022/diff/1/chromeos_startup#newcode106
chromeos_startup:106: if crossystem mainfw_type?developer; then
On 2011/03/29 00:14:03, jrbarnette wrote:
> Ugh.  Is this really the syntax for crossystem arguments?
> Using '?' in arguments is problematic, because the shell
> interprets this character specially.  At minimum, this argument
> needs to be quoted.  Better would be to fix the crossystem
> command syntax.

fixed

http://codereview.chromium.org/6745022/diff/1/chromeos_startup#newcode121
chromeos_startup:121: set_startup_update_tries $(( $tries - 1 ))
On 2011/03/28 22:00:25, Randall Spangler wrote:
> I'd be tempted to move the decrement to just after line 112, so if anything
goes
> wrong in setting up the update (lines 113-119) we'll eventually run out of
> tries.

done