Add new stateful upload payload generator to crosutils.

Add new stateful upload payload generator to crosutils.

Change-Id: I53a24d6188711a248a1dc7c869aa9258b10970d2

BUG=chromium-os:8885
TEST=Ran by itself with latest image and current directory as my
output dir as well as with new dev server changes.

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

[sosa, 2010-11-10 22:30:37]

This is pretty much a copy-paste from the devserver code with addition of an
options parser and using python logging class.

[DaleCurtis, 2010-11-10 22:41:18]

Will patch into local code base and test for you as well.

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
File cros_generate_stateful_update_payload (right):

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload:1:
cros_generate_stateful_update_payload.py
Necessary? Could just change python code to not have an extension. Otherwise for
consistency it looks like other files in scripts use symlink.

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
File cros_generate_stateful_update_payload.py (right):

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:40:
subprocess.check_call(['%s/mount_gpt_image.sh' % crosutils_dir,
Will this take in stateful.image or just chromiumos*_image.bin ?

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:48: subprocess.check_call(['sudo',
Why /var?

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:79: parser.add_option('-i',
'--image_path',
Nit. Sync option names with cros_generate_update_payload for consistency.
--image --output, etc. Short variants are fine.

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:84: (options, unused_args) =
parser.parse_args()
Extra parens.

[petkov, 2010-11-10 22:49:35]

Should this be under crosutils/bin? LGTM otherwise on my side.

[sosa, 2010-11-10 23:31:30]

Fixed nit and commented on other comments

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
File cros_generate_stateful_update_payload (right):

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload:1:
cros_generate_stateful_update_payload.py
This IS a symlink.  This is just how retveld displays it.  I much prefer using
symlinks and using .py's because it allows other scripts to do python import to
call the code.

On 2010/11/10 22:41:18, dalec wrote:
> Necessary? Could just change python code to not have an extension. Otherwise
for
> consistency it looks like other files in scripts use symlink.

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
File cros_generate_stateful_update_payload.py (right):

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:40:
subprocess.check_call(['%s/mount_gpt_image.sh' % crosutils_dir,
This requires the image.  This is probably the best way to abstract this ... you
don't need unpack_partitions ... and for a lot of applications of this ... they
wont' have unpack_partitions / pack_partitions in the directory.

On 2010/11/10 22:41:18, dalec wrote:
> Will this take in stateful.image or just chromiumos*_image.bin ?

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:48: subprocess.check_call(['sudo',
Gmerge / portage

On 2010/11/10 22:41:18, dalec wrote:
> Why /var?

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:79: parser.add_option('-i',
'--image_path',
I'd like the ability to change the type of the payload ... that's why i use
output_dir rather than output.

On 2010/11/10 22:41:18, dalec wrote:
> Nit. Sync option names with cros_generate_update_payload for consistency.
> --image --output, etc. Short variants are fine.

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:84: (options, unused_args) =
parser.parse_args()
On 2010/11/10 22:41:18, dalec wrote:
> Extra parens.

Done.

[DaleCurtis, 2010-11-11 00:03:46]

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
File cros_generate_stateful_update_payload.py (right):

http://codereview.chromium.org/4688006/diff/2001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:79: parser.add_option('-i',
'--image_path',
Can you elaborate more? I don't understand how letting the user set an output
file restricts you.

On 2010/11/10 23:31:30, sosa wrote:
> I'd like the ability to change the type of the payload ... that's why i use
> output_dir rather than output.
> 
> On 2010/11/10 22:41:18, dalec wrote:
> > Nit. Sync option names with cros_generate_update_payload for consistency.
> > --image --output, etc. Short variants are fine.
>

http://codereview.chromium.org/4688006/diff/9001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:27: from_dir =
os.path.dirname(image_path)
This won't work if image is in the same directory and "-i
chromiumos_test_image.bin" is passed as the option.

[sosa, 2010-11-11 00:18:57]

I don't want users to misleadingly use some output suffix thinking that that's
what gets outputted.  For example, we still use update.gz because for all we
know, we think that the update payload is a gzip file ... which is no longer
true but update.gz persists because other scripts have hard-coded the suffix.

I'd much prefer that the image get generated with the right suffix so it's easy
to see what it is.  This really shouldn't be up to callers ... rather the callee

http://codereview.chromium.org/4688006/diff/9001/cros_generate_stateful_updat...
File cros_generate_stateful_update_payload.py (right):

http://codereview.chromium.org/4688006/diff/9001/cros_generate_stateful_updat...
cros_generate_stateful_update_payload.py:27: from_dir =
os.path.dirname(image_path)
On 2010/11/11 00:03:46, dalec wrote:
> This won't work if image is in the same directory and "-i
> chromiumos_test_image.bin" is passed as the option.

Done.

[DaleCurtis, 2010-11-11 01:18:36]

LGTM

On 2010/11/11 00:18:57, sosa wrote:
> I don't want users to misleadingly use some output suffix thinking that that's
> what gets outputted.  For example, we still use update.gz because for all we
> know, we think that the update payload is a gzip file ... which is no longer
> true but update.gz persists because other scripts have hard-coded the suffix.
> 
> I'd much prefer that the image get generated with the right suffix so it's
easy
> to see what it is.  This really shouldn't be up to callers ... rather the
callee
> 
>
http://codereview.chromium.org/4688006/diff/9001/cros_generate_stateful_updat...
> File cros_generate_stateful_update_payload.py (right):
> 
>
http://codereview.chromium.org/4688006/diff/9001/cros_generate_stateful_updat...
> cros_generate_stateful_update_payload.py:27: from_dir =
> os.path.dirname(image_path)
> On 2010/11/11 00:03:46, dalec wrote:
> > This won't work if image is in the same directory and "-i
> > chromiumos_test_image.bin" is passed as the option.
> 
> Done.