Check in tofactory script.

Check in tofactory script.

Also refactor the other scripts to move more common functions (debug output, etc.) to common.sh.

BUG=chrome-os-partner:1903
TEST=manual; ran on a Chrome notebook, verified the right things got copied.

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

[Bill Richardson, 2010-12-17 01:10:53]

A couple questions.

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common.sh
File scripts/image_signing/common.sh (right):

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common...
scripts/image_signing/common.sh:87: for i in "$(cat $TEMP_FILE_LIST)"; do
No quotes here. "${cat file}" will return the contents of the file as a single
long string.

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common...
scripts/image_signing/common.sh:91: for i in "$(cat $TEMP_DIR_LIST)"; do
No quotes here, either.

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/tofact...
File scripts/image_signing/tofactory.sh (right):

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/tofact...
scripts/image_signing/tofactory.sh:138: trap "initctl start update-engine" EXIT
You've already set a trap on EXIT in common.sh. Doing this will prevent all
post-script cleanup.

[gauravsh, 2010-12-17 01:24:37]

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common.sh
File scripts/image_signing/common.sh (right):

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common...
scripts/image_signing/common.sh:55: [ "$FLAGS_debug" = $FLAGS_TRUE ]
This and the debug_msg below were intentionally not moved to common.sh earlier
since they are shflags specific. 

My suggestion would be to either move it back or make is_debug_mode dependent on
another global defined in common.sh that must be set to value of FLAGS_debug in
any script that uses shflags is (as in make_dev_ssd or the script you just
added).

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common...
scripts/image_signing/common.sh:87: for i in "$(cat $TEMP_FILE_LIST)"; do
On 2010/12/17 01:10:53, Bill Richardson wrote:
> No quotes here. "${cat file}" will return the contents of the file as a single
> long string. 

Please, if you make this change as Bill suggested, make sure you test that this
does not break sign_official_build.sh and other scripts. Bash quoting is
somewhat confusing - and I may have added the extra quotes because something
wasn't working as expected.

[Randall Spangler, 2010-12-21 22:52:35]

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common.sh
File scripts/image_signing/common.sh (right):

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common...
scripts/image_signing/common.sh:55: [ "$FLAGS_debug" = $FLAGS_TRUE ]
On 2010/12/17 01:24:37, gauravsh wrote:
> This and the debug_msg below were intentionally not moved to common.sh earlier
> since they are shflags specific. 
> 
> My suggestion would be to either move it back or make is_debug_mode dependent
on
> another global defined in common.sh that must be set to value of FLAGS_debug
in
> any script that uses shflags is (as in make_dev_ssd or the script you just
> added). 

I've added a default value for the check so that it will default to false if
neither FLAGS_debug nor FLAGS_TRUE are defined.  This way it'll work if the
caller doesn't call load_shflags().

If the caller doesn't use shflags but wants to use debug_msg, all the caller
needs to do is define FLAGS_TRUE and FLAGS_debug.

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common...
scripts/image_signing/common.sh:87: for i in "$(cat $TEMP_FILE_LIST)"; do
On 2010/12/17 01:24:37, gauravsh wrote:
> On 2010/12/17 01:10:53, Bill Richardson wrote:
> > No quotes here. "${cat file}" will return the contents of the file as a
single
> > long string. 
> 
> Please, if you make this change as Bill suggested, make sure you test that
this
> does not break sign_official_build.sh and other scripts. Bash quoting is
> somewhat confusing - and I may have added the extra quotes because something
> wasn't working as expected.

I did some experimentation; the quotes will indeed mess things up.


rrs@a5:~/b/tmp$ for i in $(echo 1 2 3); do echo $i; done
1
2
3
rrs@a5:~/b/tmp$ for i in "$(echo 1 2 3)"; do echo $i; done
1 2 3

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common...
scripts/image_signing/common.sh:88: rm -f $i
Note that since $i is used UNquoted here, and you can specify multiple files to
rm, this undoes the damage from the previous extra quotes...

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common...
scripts/image_signing/common.sh:93: if has_needs_to_be_resigned_tag "$i"; then
If there's more than one temp dir, this check will return false, so the warning
won't show up.

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common...
scripts/image_signing/common.sh:96: sudo umount -d $i 2>/dev/null
This still works just like line 88, because umount can also take multiple mount
points on one command line.

[Bill Richardson, 2010-12-22 17:48:10]

LGTM

[gauravsh, 2010-12-22 23:04:02]

LGTM

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common.sh
File scripts/image_signing/common.sh (right):

http://codereview.chromium.org/5878005/diff/3001/scripts/image_signing/common...
scripts/image_signing/common.sh:87: for i in "$(cat $TEMP_FILE_LIST)"; do
On 2010/12/21 22:52:35, Randall Spangler wrote:
> On 2010/12/17 01:24:37, gauravsh wrote:
> > On 2010/12/17 01:10:53, Bill Richardson wrote:
> > > No quotes here. "${cat file}" will return the contents of the file as a
> single
> > > long string. 
> > 
> > Please, if you make this change as Bill suggested, make sure you test that
> this
> > does not break sign_official_build.sh and other scripts. Bash quoting is
> > somewhat confusing - and I may have added the extra quotes because something
> > wasn't working as expected.
> 
> I did some experimentation; the quotes will indeed mess things up.
> 
> 
> rrs@a5:~/b/tmp$ for i in $(echo 1 2 3); do echo $i; done
> 1
> 2
> 3
> rrs@a5:~/b/tmp$ for i in "$(echo 1 2 3)"; do echo $i; done
> 1 2 3

ahh, ok. good to know. thanks! :)