[NOT FOR COMMIT] Added GN support for Android .AAR packaged libraries

[NOT FOR COMMIT] Added GN support for Android .AAR packaged libraries

BUG=611171

[agrieve, 2016-06-16 02:00:03]

agrieve@chromium.org changed reviewers:
+ agrieve@chromium.org

[agrieve, 2016-06-16 02:00:04]

Awesome start!

https://codereview.chromium.org/2069273002/diff/1/build/android/gyp/write_bui...
File build/android/gyp/write_build_config.py (right):

https://codereview.chromium.org/2069273002/diff/1/build/android/gyp/write_bui...
build/android/gyp/write_build_config.py:45: _RESOURCE_TYPES = ('android_assets',
'android_resources', 'android_aar')
aar matches the name here, but not the comment. This will prevent one aar from
depending on another. I think maybe the thing to do is to not add to this list
and rename the variable to "_NON_CODE_TYPES" or something similar.

https://codereview.chromium.org/2069273002/diff/1/build/android/gyp/write_bui...
build/android/gyp/write_build_config.py:127: if wanted_type in ['java_library',
'android_resources']:
an android_aar may or may not have resources / assets, so it might make more
sense to do:

def DepsOfType(wanted_types, config):
  return [c for c in configs if wanted_type in c['effective_types']]

And then have all targets types have:
deps_info.effective_types = [deps_info.type]

and then have aar add in the types based on what's in the .aar file.

https://codereview.chromium.org/2069273002/diff/1/build/android/gyp/write_bui...
build/android/gyp/write_build_config.py:394: if options.type ==
'android_assets':
might want to add a TODO for allowing aars with assets.

https://codereview.chromium.org/2069273002/diff/1/build/android/gyp/write_bui...
build/android/gyp/write_build_config.py:411: if options.type in
('android_resources', 'android_aar'):
Should probably update package_resources.py to not extract the entire resource
zip, but instread just the res/ directory. build_utils.ExtractAll(filter=) can
be used for this.

https://codereview.chromium.org/2069273002/diff/1/build/android/gyp/write_bui...
build/android/gyp/write_build_config.py:415: if options.android_manifest:
I think you want to pass this flag as well from the .gni.

https://codereview.chromium.org/2069273002/diff/1/build/android/gyp/write_bui...
build/android/gyp/write_build_config.py:420: if options.r_text:
Is this being set for aar?

https://codereview.chromium.org/2069273002/diff/1/build/android/gyp/write_bui...
build/android/gyp/write_build_config.py:450: proguard_config['input_paths'] =
[options.jar_path] + java_full_classpath
Might want to add a TODO here to support adding the path of an .aar's
proguard.txt

https://codereview.chromium.org/2069273002/diff/1/build/config/android/rules.gni
File build/config/android/rules.gni (right):

https://codereview.chromium.org/2069273002/diff/1/build/config/android/rules....
build/config/android/rules.gni:2518: set_sources_assignment_filter([])
no need for this since you aren't using "sources" as an arg.

https://codereview.chromium.org/2069273002/diff/1/build/config/android/rules....
build/config/android/rules.gni:2543: copy(_copy_target_name) {
Why copy it?

https://codereview.chromium.org/2069273002/diff/1/build/config/android/unpack...
File build/config/android/unpack_aar.py (right):

https://codereview.chromium.org/2069273002/diff/1/build/config/android/unpack...
build/config/android/unpack_aar.py:26: archive.extractall(output_dir)
do you need an os.makedirs() first?

https://codereview.chromium.org/2069273002/diff/1/build/config/android/unpack...
build/config/android/unpack_aar.py:26: archive.extractall(output_dir)
Probably a good idea to shutil.rmtree() first so that old extractions are
removed first.

https://codereview.chromium.org/2069273002/diff/1/build/config/android/unpack...
build/config/android/unpack_aar.py:26: archive.extractall(output_dir)
Probably a good idea to assert that the .aar has no unsupported entries in it
(proguard.txt, assets, libs, jni)

https://codereview.chromium.org/2069273002/diff/1/build/config/android/unpack...
build/config/android/unpack_aar.py:26: archive.extractall(output_dir)
nit: you don't need to extract res/, so probably should skip that directory.
build_utils.ExtractAll(filter=) can be used for this.

https://codereview.chromium.org/2069273002/diff/1/build/config/android/unpack...
build/config/android/unpack_aar.py:26: archive.extractall(output_dir)
For extra extra bonus points, it might be useful to validate that the
AndroidManifest.xml has no entries that require manifest merging (maybe via a
cludgy contains check)? A quick look tells me this should probably be good
enough:

if not options.ignore_manifest_merging:
  assert '<application>' not in manifest_data
  assert '<uses-permission' not in manifest_data

Most have just <application /> and <uses-sdk>, which are safely ignored.

If this check ever fails, the user would be responsible for manually copying in
the manifest changes and then adding the --ignore-manifest-merging (or
implementing manifest merging)

[bajones, 2016-06-28 18:44:35]

bajones@chromium.org changed reviewers:
+ ianwen@chromium.org

[bajones, 2016-06-28 18:44:35]

Here's the latest variant of the AAR template. I'd definitely appreciate some
testing on a more reliable AAR than the one I've been using.

[agrieve, 2016-06-28 19:36:06]

https://codereview.chromium.org/2069273002/diff/20001/build/config/android/ru...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/2069273002/diff/20001/build/config/android/ru...
build/config/android/rules.gni:2518: _output_path =
"${root_out_dir}/unpacked_aar/${target_name}"
nit: should use "$target_gen_dir/$target_name"

https://codereview.chromium.org/2069273002/diff/20001/build/config/android/ru...
build/config/android/rules.gni:2536: exec_script("//build/android/gyp/find.py",
this runs at gn gen time, so won't actually find any files on the first build.

Might make sense to reuse unpack_aar.py and add a flag which just opens and
prints out the res/ directory listing.

https://codereview.chromium.org/2069273002/diff/20001/build/config/android/ru...
build/config/android/rules.gni:2544: java_prebuilt_impl(_jar_target_name) {
would android_rebuilt() work here?

https://codereview.chromium.org/2069273002/diff/20001/build/config/android/ru...
build/config/android/rules.gni:2558: resource_dirs = [ "${_output_path}/res" ]
This also results in a find.py at gn gen time. I think what you need to do is:

resource_dirs = []
generated_resource_dirs = ["${_output_path}/res"]
generated_resource_files = _resource_files

[bajones, 2016-06-28 19:55:55]

On 2016/06/28 19:36:06, agrieve wrote:
>
https://codereview.chromium.org/2069273002/diff/20001/build/config/android/ru...
> build/config/android/rules.gni:2536:
exec_script("//build/android/gyp/find.py",
> this runs at gn gen time, so won't actually find any files on the first build.

Aahhhhhh,,,,,, this explains *so much*! Thanks!