Make android_apk template actually create an apk

Make android_apk template actually create an apk

This is essentially three steps, packaging resources, packaging code,
and then signing+zipaligning the apk.

The apk does not currently contain the native libraries.

Depends on http://crrev.com/386473002/

BUG=359249
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287729

[cjhopman, 2014-07-29 23:32:24]

newt: *

[newt (away), 2014-07-30 21:23:14]

https://codereview.chromium.org/383613002/diff/80001/build/config/android/int...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/383613002/diff/80001/build/config/android/int...
build/config/android/internal_rules.gni:155: _dex_path =
invoker.classes_dex_path
any reason not to use the same name for both of these, e.g. "_dex_path" and
"invoker.dex_path" ?

https://codereview.chromium.org/383613002/diff/80001/build/config/android/int...
build/config/android/internal_rules.gni:165: _native_libs_dir =
"//build/android/empty/res"
this path looks wrong (or at least oddly named)

https://codereview.chromium.org/383613002/diff/80001/build/config/android/int...
build/config/android/internal_rules.gni:280: group(target_name) {
do you need to use a group here? Could you just change the final action's name
to "${target_name}"?

https://codereview.chromium.org/383613002/diff/80001/build/config/android/rul...
File build/config/android/rules.gni (left):

https://codereview.chromium.org/383613002/diff/80001/build/config/android/rul...
build/config/android/rules.gni:509: assert(!defined(invoker.final_apk_path)
Do we no longer want this assert? Or is this not needed because referencing
invoker.final_apk_path down below will fail?

https://codereview.chromium.org/383613002/diff/80001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/383613002/diff/80001/build/java_apk.gypi#newc...
build/java_apk.gypi:870: '--',
why "--" ?

[cjhopman, 2014-07-31 22:15:05]

https://codereview.chromium.org/383613002/diff/80001/build/config/android/int...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/383613002/diff/80001/build/config/android/int...
build/config/android/internal_rules.gni:155: _dex_path =
invoker.classes_dex_path
On 2014/07/30 21:23:14, newt wrote:
> any reason not to use the same name for both of these, e.g. "_dex_path" and
> "invoker.dex_path" ?

Ah, this was more useful before I had come up with a pattern to prevent
template/target invocations from inheriting random stuff into 'invoker'.

https://codereview.chromium.org/383613002/diff/80001/build/config/android/int...
build/config/android/internal_rules.gni:165: _native_libs_dir =
"//build/android/empty/res"
On 2014/07/30 21:23:14, newt wrote:
> this path looks wrong (or at least oddly named)
It's just a guaranteed to be empty path. I could make a new one for this, but
once this supports native libraries it wouldn't be needed anymore. Figured I'd
just use this one for now.

https://codereview.chromium.org/383613002/diff/80001/build/config/android/int...
build/config/android/internal_rules.gni:280: group(target_name) {
On 2014/07/30 21:23:14, newt wrote:
> do you need to use a group here? Could you just change the final action's name
> to "${target_name}"?

Yeah, I could. I've basically followed the pattern where if there is more than
one action in the template, it uses a group. I find that easiest to understand.

https://codereview.chromium.org/383613002/diff/80001/build/config/android/rul...
File build/config/android/rules.gni (left):

https://codereview.chromium.org/383613002/diff/80001/build/config/android/rul...
build/config/android/rules.gni:509: assert(!defined(invoker.final_apk_path)
On 2014/07/30 21:23:14, newt wrote:
> Do we no longer want this assert? Or is this not needed because referencing
> invoker.final_apk_path down below will fail?
Yeah, it's not needed now that invoker.final_apk_path is used.

(gn will fail if you have an unused variable)

https://codereview.chromium.org/383613002/diff/80001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/383613002/diff/80001/build/java_apk.gypi#newc...
build/java_apk.gypi:870: '--',
On 2014/07/30 21:23:14, newt wrote:
> why "--" ?
Python's optparse will discard a "--" and stop processing args at it. So args
for ant.py sort of go before the "--" and args for the actual ant call go after
it.

[newt (away), 2014-07-31 22:39:55]

Sounds good. lgtm

[cjhopman, 2014-07-31 22:40:26]

brettw for OWNERS

[brettw, 2014-08-03 23:42:59]

lgtm

https://codereview.chromium.org/383613002/diff/100001/build/config/android/in...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/383613002/diff/100001/build/config/android/in...
build/config/android/internal_rules.gni:237: _rebased_resource_packaged_apk_path
= rebase_path(_resource_packaged_apk_path, root_build_dir)
Can you wrap to 80 cols?

https://codereview.chromium.org/383613002/diff/100001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/383613002/diff/100001/build/java_apk.gypi#new...
build/java_apk.gypi:891: '--',
Just to double-check. "--" usually indicates "no more switches" which seems
contrary to this usage. I assume you had a reason, just wanted to be sure you're
sure you need it.

[cjhopman, 2014-08-06 00:19:35]

https://codereview.chromium.org/383613002/diff/100001/build/config/android/in...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/383613002/diff/100001/build/config/android/in...
build/config/android/internal_rules.gni:237: _rebased_resource_packaged_apk_path
= rebase_path(_resource_packaged_apk_path, root_build_dir)
On 2014/08/03 23:42:58, brettw wrote:
> Can you wrap to 80 cols?

Done.

https://codereview.chromium.org/383613002/diff/100001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/383613002/diff/100001/build/java_apk.gypi#new...
build/java_apk.gypi:891: '--',
On 2014/08/03 23:42:58, brettw wrote:
> Just to double-check. "--" usually indicates "no more switches" which seems
> contrary to this usage. I assume you had a reason, just wanted to be sure
you're
> sure you need it.

ant.py is a fairly simple wrapper around a call to the ant executable. ant.py
will stop parsing args at the "--" and everything after that will just be passed
as args to the underlying "ant" call.

[cjhopman, 2014-08-06 00:19:37]

The CQ bit was checked by cjhopman@chromium.org

[commit-bot: I haz the power, 2014-08-06 00:21:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cjhopman@chromium.org/383613002/120001

[commit-bot: I haz the power, 2014-08-06 07:29:45]

Change committed as 287729