Support uploading Mojo Shell on Android.

Support uploading Mojo Shell on Android.

This change augments upload_shell_binary.py to allow uploading the shell on
Android as well as on Linux. Future work will make a similar change to
download_shell_binary.py.

R=qsr@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/6cf1799caed2b018e545eef1bf499ef5b01ec731

[blundell, 2015-01-26 15:48:14]

blundell@chromium.org changed reviewers:
+ jamesr@chromium.org, qsr@chromium.org

[qsr, 2015-01-26 15:57:47]

https://codereview.chromium.org/874243002/diff/1/mojo/tools/upload_shell_bina...
File mojo/tools/upload_shell_binary.py (right):

https://codereview.chromium.org/874243002/diff/1/mojo/tools/upload_shell_bina...
mojo/tools/upload_shell_binary.py:18: def upload(dry_run, verbose, android):
Should you take a config instead of just android?

https://codereview.chromium.org/874243002/diff/1/mojo/tools/upload_shell_bina...
mojo/tools/upload_shell_binary.py:33: zipfile_name = "android-arm32"
The usual name of android is just arm (see our gn files), and arm64 for the 64
bits version.

https://codereview.chromium.org/874243002/diff/1/mojo/tools/upload_shell_bina...
mojo/tools/upload_shell_binary.py:40: shell_path = paths.target_mojo_shell_path
Can you try to aggregate those by always setting some variable in the path?

https://codereview.chromium.org/874243002/diff/1/mojo/tools/upload_shell_bina...
mojo/tools/upload_shell_binary.py:63: action="store_true")
Should you instead pass a build_dir? Then using mopy.gn you can build a config,
and you will have all the info you need.

[blundell, 2015-01-26 16:22:43]

https://codereview.chromium.org/874243002/diff/1/mojo/tools/upload_shell_bina...
File mojo/tools/upload_shell_binary.py (right):

https://codereview.chromium.org/874243002/diff/1/mojo/tools/upload_shell_bina...
mojo/tools/upload_shell_binary.py:18: def upload(dry_run, verbose, android):
On 2015/01/26 15:57:47, qsr wrote:
> Should you take a config instead of just android?

Done.

https://codereview.chromium.org/874243002/diff/1/mojo/tools/upload_shell_bina...
mojo/tools/upload_shell_binary.py:33: zipfile_name = "android-arm32"
On 2015/01/26 15:57:47, qsr wrote:
> The usual name of android is just arm (see our gn files), and arm64 for the 64
> bits version.

Done.

https://codereview.chromium.org/874243002/diff/1/mojo/tools/upload_shell_bina...
mojo/tools/upload_shell_binary.py:40: shell_path = paths.target_mojo_shell_path
On 2015/01/26 15:57:47, qsr wrote:
> Can you try to aggregate those by always setting some variable in the path?

Done.

https://codereview.chromium.org/874243002/diff/1/mojo/tools/upload_shell_bina...
mojo/tools/upload_shell_binary.py:63: action="store_true")
On 2015/01/26 15:57:47, qsr wrote:
> Should you instead pass a build_dir? Then using mopy.gn you can build a
config,
> and you will have all the info you need.

Done.

[qsr, 2015-01-26 16:25:43]

https://codereview.chromium.org/874243002/diff/20001/mojo/tools/upload_shell_...
File mojo/tools/upload_shell_binary.py (right):

https://codereview.chromium.org/874243002/diff/20001/mojo/tools/upload_shell_...
mojo/tools/upload_shell_binary.py:32: zipfile_name = "arm"
what about using config.target_os and config.target_arch in the path? That
should handle every platform.

[blundell, 2015-01-26 16:31:34]

good suggestion, thanks

https://codereview.chromium.org/874243002/diff/20001/mojo/tools/upload_shell_...
File mojo/tools/upload_shell_binary.py (right):

https://codereview.chromium.org/874243002/diff/20001/mojo/tools/upload_shell_...
mojo/tools/upload_shell_binary.py:32: zipfile_name = "arm"
On 2015/01/26 16:25:43, qsr wrote:
> what about using config.target_os and config.target_arch in the path? That
> should handle every platform.

Done.

[qsr, 2015-01-26 16:41:43]

lgtm

https://codereview.chromium.org/874243002/diff/40001/mojo/tools/upload_shell_...
File mojo/tools/upload_shell_binary.py (right):

https://codereview.chromium.org/874243002/diff/40001/mojo/tools/upload_shell_...
mojo/tools/upload_shell_binary.py:37: zipinfo = zipfile.ZipInfo(shell_filename)
Zipping the apk is not really useful... But I don't see how to not do it without
adding back some platform specific code here. Your call.

[blundell, 2015-01-27 09:20:50]

https://codereview.chromium.org/874243002/diff/40001/mojo/tools/upload_shell_...
File mojo/tools/upload_shell_binary.py (right):

https://codereview.chromium.org/874243002/diff/40001/mojo/tools/upload_shell_...
mojo/tools/upload_shell_binary.py:37: zipinfo = zipfile.ZipInfo(shell_filename)
On 2015/01/26 16:41:43, qsr wrote:
> Zipping the apk is not really useful... But I don't see how to not do it
without
> adding back some platform specific code here. Your call.

I prefer to keep it as-is, as making platform-specific distinctions here will
also require making platform-specific distinctions on the download side.

[blundell, 2015-01-27 20:26:09]

jamesr: any comments here?

[jamesr, 2015-01-27 20:29:31]

Seems fine

https://codereview.chromium.org/874243002/diff/40001/mojo/tools/upload_shell_...
File mojo/tools/upload_shell_binary.py (right):

https://codereview.chromium.org/874243002/diff/40001/mojo/tools/upload_shell_...
mojo/tools/upload_shell_binary.py:39: zipinfo.compress_type =
zipfile.ZIP_DEFLATED
if you're worried about burning CPU zipping uselessly, change this to
compress.ZIP_STORED when you're zipping an apk.  Then the unzip step will be the
same but this will just slap a header on the archive and not much else.

[blundell, 2015-01-28 14:57:08]

Thanks!

https://codereview.chromium.org/874243002/diff/40001/mojo/tools/upload_shell_...
File mojo/tools/upload_shell_binary.py (right):

https://codereview.chromium.org/874243002/diff/40001/mojo/tools/upload_shell_...
mojo/tools/upload_shell_binary.py:39: zipinfo.compress_type =
zipfile.ZIP_DEFLATED
On 2015/01/27 20:29:31, jamesr wrote:
> if you're worried about burning CPU zipping uselessly, change this to
> compress.ZIP_STORED when you're zipping an apk.  Then the unzip step will be
the
> same but this will just slap a header on the archive and not much else.

Done.

[blundell, 2015-01-28 14:57:40]

Committed patchset #4 (id:60001) manually as
6cf1799caed2b018e545eef1bf499ef5b01ec731 (presubmit successful).