GN + Android: extract android_standalone_library rule.

GN + Android: extract android_standalone_library rule.

This patch extracts the rule needed to produce a standalone .dex.jar for
a set of Java dependencies into a separate rule. This allows to package
any android library into a standalone .dex.jar file without affecting
the target being packaged.

BUG=437290
Committed: https://crrev.com/7d7c32ec12774eaf22c901194ff9f788a1dcc9b2
Cr-Commit-Position: refs/heads/master@{#307575}

[ppi, 2014-12-05 13:03:43]

ppi@chromium.org changed reviewers:
+ cjhopman@chromium.org, qsr@chromium.org

[ppi, 2014-12-05 13:03:44]

Hi Chris, Ben - ptal.

[qsr, 2014-12-05 13:11:06]

lgtm

[cjhopman, 2014-12-05 19:11:55]

https://codereview.chromium.org/778093004/diff/1/build/config/android/interna...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/778093004/diff/1/build/config/android/interna...
build/config/android/internal_rules.gni:822: if (_java_files != [] ||
_srcjar_deps != [] || _srcjars != []) {
I don't think this is a good idea. The build_config is going to be wrong for
this target (for example, if an android_library depends on this target the build
will fail in a hard to diagnose way).

If we want a target that just does dex merging like this, it should probably not
be implemented with java_library_impl. It should write it's own build_config
(that basically just contains the list of files to dex).

[ppi, 2014-12-08 16:34:12]

Patchset #2 (id:20001) has been deleted

[ppi, 2014-12-08 16:38:27]

Thanks, Chris. Ptal.

https://codereview.chromium.org/778093004/diff/1/build/config/android/interna...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/778093004/diff/1/build/config/android/interna...
build/config/android/internal_rules.gni:822: if (_java_files != [] ||
_srcjar_deps != [] || _srcjars != []) {
On 2014/12/05 19:11:55, cjhopman wrote:
> I don't think this is a good idea. The build_config is going to be wrong for
> this target (for example, if an android_library depends on this target the
build
> will fail in a hard to diagnose way).
> 
> If we want a target that just does dex merging like this, it should probably
not
> be implemented with java_library_impl. It should write it's own build_config
> (that basically just contains the list of files to dex).

Done.

[cjhopman, 2014-12-08 21:27:52]

https://codereview.chromium.org/778093004/diff/40001/build/config/android/rul...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/778093004/diff/40001/build/config/android/rul...
build/config/android/rules.gni:1014: standalone_dex_path =
invoker.standalone_dex_path
I think you should require the invoker to specify the path for the dex.
Otherwise, the only useful way to use a target of this type would be to refer to
the generated output path and we typically don't want people doing that (because
we want to be free to change anything that the invoker doesn't specify).

Also, maybe just 'dex_path'.

[ppi, 2014-12-09 15:17:29]

Thanks, Chris. Ptal.

https://codereview.chromium.org/778093004/diff/40001/build/config/android/rul...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/778093004/diff/40001/build/config/android/rul...
build/config/android/rules.gni:1014: standalone_dex_path =
invoker.standalone_dex_path
On 2014/12/08 21:27:51, cjhopman wrote:
> I think you should require the invoker to specify the path for the dex.
> Otherwise, the only useful way to use a target of this type would be to refer
to
> the generated output path and we typically don't want people doing that
(because
> we want to be free to change anything that the invoker doesn't specify).
> 
> Also, maybe just 'dex_path'.

Done.

[cjhopman, 2014-12-09 19:00:35]

https://codereview.chromium.org/778093004/diff/60001/build/config/android/rul...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/778093004/diff/60001/build/config/android/rul...
build/config/android/rules.gni:1008: #     resources in deps will be packaged
into the resulting .dex.jar file.
nit: I don't think you get the resources

[cjhopman, 2014-12-09 19:00:39]

lgtm

[ppi, 2014-12-09 19:13:07]

Thanks!

https://codereview.chromium.org/778093004/diff/60001/build/config/android/rul...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/778093004/diff/60001/build/config/android/rul...
build/config/android/rules.gni:1008: #     resources in deps will be packaged
into the resulting .dex.jar file.
On 2014/12/09 19:00:35, cjhopman wrote:
> nit: I don't think you get the resources

Done.

[ppi, 2014-12-09 19:13:28]

The CQ bit was checked by ppi@chromium.org

[commit-bot: I haz the power, 2014-12-09 19:14:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/778093004/80001

[commit-bot: I haz the power, 2014-12-09 19:37:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-09 19:37:20]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[cjhopman, 2014-12-09 19:53:56]

https://codereview.chromium.org/778093004/diff/80001/build/android/gyp/write_...
File build/android/gyp/write_build_config.py (right):

https://codereview.chromium.org/778093004/diff/80001/build/android/gyp/write_...
build/android/gyp/write_build_config.py:218: if options.type == 'android_apk' or
'deps_dex':
Should be:
if options.type == 'android_apk' or options.type == 'deps_dex':

[ppi, 2014-12-09 21:36:46]

Thanks!

https://codereview.chromium.org/778093004/diff/80001/build/android/gyp/write_...
File build/android/gyp/write_build_config.py (right):

https://codereview.chromium.org/778093004/diff/80001/build/android/gyp/write_...
build/android/gyp/write_build_config.py:218: if options.type == 'android_apk' or
'deps_dex':
On 2014/12/09 19:53:55, cjhopman wrote:
> Should be:
> if options.type == 'android_apk' or options.type == 'deps_dex':

Done.

[ppi, 2014-12-09 21:36:53]

The CQ bit was checked by ppi@chromium.org

[commit-bot: I haz the power, 2014-12-09 21:38:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/778093004/100001

[commit-bot: I haz the power, 2014-12-09 23:09:21]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2014-12-09 23:10:05]

Patchset 5 (id:??) landed as
https://crrev.com/7d7c32ec12774eaf22c901194ff9f788a1dcc9b2
Cr-Commit-Position: refs/heads/master@{#307575}