Add AAR support to Chrome and convert support libraries to using AARs

Add AAR support to Chrome and convert support libraries to using AARs

Chrome has been using Jar and resources from support library for years.
In Q1 2016, Android team stops shipping jars/res for support libray.
Instead, AAR is promoted, which is a zip that wraps jars and resources.

This CL introduces a utility python script that processes an AAR file.
In GN gen time, it lists all files in the AAR, yet it does not extract
it. Actual unpacking is postponed until compilation.

Two other things to notice:
1. In the old jar we depended on, support-v13 contains support-v4 and
support-annotations. After converting to AAR, these two libraries are no
longer part of support-v13. Thus this change needs to be reflected.
2. In the new AAR format, support-v4 now contains two jars instead of
one. All public classes are in classes.jar, and all hidden classes are
in libs/internal_impl-$VERSION.jar.

This work is not possible without bajones@'s pioneering work in
https://chromiumcodereview.appspot.com/2069273002/

BUG=611171
Committed: https://crrev.com/c84b9756d482be2561734a19138b41ca630f2701
Cr-Commit-Position: refs/heads/master@{#406603}

[Ian Wen, 2016-07-15 01:36:21]

ianwen@chromium.org changed reviewers:
+ agrieve@chromium.org, bajones@chromium.org, nyquist@chromium.org

[Ian Wen, 2016-07-15 01:36:22]

Hello reviewers,

I am excited to share the result of my 2-week-long exploration and PTAL! :)

Thanks in advance!

https://codereview.chromium.org/2156453002/diff/1/content/public/android/BUIL...
File content/public/android/BUILD.gn (right):

https://codereview.chromium.org/2156453002/diff/1/content/public/android/BUIL...
content/public/android/BUILD.gn:54:
"//third_party/android_tools:android_support_v13_java",
Q: In third_party/android, I already made v13 to depend on annotations and v4.
Why is that I have to specifically say them again in all the places that
references v13?

If I remove annotations and v4, this won't compile.

[Ian Wen, 2016-07-15 01:36:48]

The CQ bit was checked by ianwen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 01:37:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 01:54:35]

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

[commit-bot: I haz the power, 2016-07-15 01:54:36]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[agrieve, 2016-07-15 14:34:41]

Exciting stuff!

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py
File build/android/gyp/aar.py (right):

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py#ne...
build/android/gyp/aar.py:16: 
nit: 2 blank lines between top-level things

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py#ne...
build/android/gyp/aar.py:20: help=('Extract the files to output directory.'),
these braces are not required.

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py#ne...
build/android/gyp/aar.py:22: parser.add_argument('--list_res',
use - rather than _ in flags. --list-resources

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py#ne...
build/android/gyp/aar.py:26: help=('Name and path of the AAR file.'),
nit: path implies name. Just say "Path to the .aar file"

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py#ne...
build/android/gyp/aar.py:30: help=('Output dir for the extracted files.'),
nit: don't use abbreviations in help when possible. dir->directory

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py#ne...
build/android/gyp/aar.py:31: required=True,
This shouldn't be required for --list

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py#ne...
build/android/gyp/aar.py:32: metavar='PATH')
It would make more sense to say 'DIR' since your other one says 'FILE'

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py#ne...
build/android/gyp/aar.py:39: if (args.extract):
nit: drop parentheses.

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py#ne...
build/android/gyp/aar.py:40: # Clear previously extracted versions of the AAR
nit: missing . at end of sentence.

https://codereview.chromium.org/2156453002/diff/1/build/android/gyp/aar.py#ne...
build/android/gyp/aar.py:48: print os.path.join(output_dir, name)
I think it would make more sense to just output name here, and then have the
caller responsible for adding the output-dir to it (via process_file_template)

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

https://codereview.chromium.org/2156453002/diff/1/build/config/android/rules....
build/config/android/rules.gni:2570: #   internal_jar_version: The version of
the support library. This has to
I think you're supposed to treat all .jar files within libs/ as
java_prebuilt()s.

Rather than have the client pass in this variable, we should make aar.py --list
return the list of .jars and create java_prebuilt()s for them.

Also - I think this means that instead of having aar.py output a
newline-separated list, it should instead output a GN scope (see gn help
input_conversion)

It'd be something like:
print 'resources = %s' % json.dumps(resource)
print 'internal_jars = %s' % json.dumps(internal_jars)

https://codereview.chromium.org/2156453002/diff/1/build/config/android/rules....
build/config/android/rules.gni:2579:
template("android_aar_prebuilt_no_resource") {
I don't think we need to have this as a separate template. Just don't define a
resources target if aar.py --list returns an empty list.

https://codereview.chromium.org/2156453002/diff/1/build/config/android/rules....
build/config/android/rules.gni:2581: forward_variables_from(invoker,
nit: Try to use forward_variables_from only when forwarding variables to a
sub-template. In this case I don't think there's value in using it. Just use
"invoker.aar_path" below.

https://codereview.chromium.org/2156453002/diff/1/build/config/android/rules....
build/config/android/rules.gni:2698: get_path_info(rebase_path(_res_files, "",
root_out_dir), "abspath")
The get_path_info part of this is unnecessary I think.

https://codereview.chromium.org/2156453002/diff/1/content/public/android/BUIL...
File content/public/android/BUILD.gn (right):

https://codereview.chromium.org/2156453002/diff/1/content/public/android/BUIL...
content/public/android/BUILD.gn:54:
"//third_party/android_tools:android_support_v13_java",
On 2016/07/15 01:36:21, Ian Wen wrote:
> Q: In third_party/android, I already made v13 to depend on annotations and v4.
> Why is that I have to specifically say them again in all the places that
> references v13?
> 
> If I remove annotations and v4, this won't compile.

When compiling, we set the classpath only to direct dependents. If we wanted
though, we could actually make this work using:

java_group("support_v13_java") {
  deps = [ ":support_v13_for_realz", ":support_v4", ":support_annotations" ]
}

[Ian Wen, 2016-07-17 02:08:09]

All addressed.

One thing I noticed with the newest patch is that it will always recompile the
targets with two android_java_prebuilt targets. Yet this is very strange to me,
as the input of "unpack_aar" stays the same, so it should not recompile... WDYT?

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
File build/android/gyp/aar.py (right):

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
build/android/gyp/aar.py:16: 
On 2016/07/15 14:34:40, agrieve wrote:
> nit: 2 blank lines between top-level things

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
build/android/gyp/aar.py:20: help=('Extract the files to output directory.'),
On 2016/07/15 14:34:41, agrieve wrote:
> these braces are not required.

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
build/android/gyp/aar.py:22: parser.add_argument('--list_res',
On 2016/07/15 14:34:41, agrieve wrote:
> use - rather than _ in flags. --list-resources

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
build/android/gyp/aar.py:26: help=('Name and path of the AAR file.'),
On 2016/07/15 14:34:41, agrieve wrote:
> nit: path implies name. Just say "Path to the .aar file"

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
build/android/gyp/aar.py:30: help=('Output dir for the extracted files.'),
On 2016/07/15 14:34:41, agrieve wrote:
> nit: don't use abbreviations in help when possible. dir->directory

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
build/android/gyp/aar.py:31: required=True,
On 2016/07/15 14:34:40, agrieve wrote:
> This shouldn't be required for --list

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
build/android/gyp/aar.py:32: metavar='PATH')
On 2016/07/15 14:34:41, agrieve wrote:
> It would make more sense to say 'DIR' since your other one says 'FILE'

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
build/android/gyp/aar.py:39: if (args.extract):
On 2016/07/15 14:34:41, agrieve wrote:
> nit: drop parentheses. 

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
build/android/gyp/aar.py:40: # Clear previously extracted versions of the AAR
On 2016/07/15 14:34:41, agrieve wrote:
> nit: missing . at end of sentence.

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/android/gyp/aa...
build/android/gyp/aar.py:48: print os.path.join(output_dir, name)
On 2016/07/15 14:34:41, agrieve wrote:
> I think it would make more sense to just output name here, and then have the
> caller responsible for adding the output-dir to it (via process_file_template)

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/config/android...
File build/config/android/rules.gni (right):

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/config/android...
build/config/android/rules.gni:2570: #   internal_jar_version: The version of
the support library. This has to
On 2016/07/15 14:34:41, agrieve wrote:
> I think you're supposed to treat all .jar files within libs/ as
> java_prebuilt()s.
> 
> Rather than have the client pass in this variable, we should make aar.py
--list
> return the list of .jars and create java_prebuilt()s for them.
> 
> Also - I think this means that instead of having aar.py output a
> newline-separated list, it should instead output a GN scope (see gn help
> input_conversion)
> 
> It'd be something like:
> print 'resources = %s' % json.dumps(resource)
> print 'internal_jars = %s' % json.dumps(internal_jars)

Done. I used ToGNString() instead.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/config/android...
build/config/android/rules.gni:2579:
template("android_aar_prebuilt_no_resource") {
On 2016/07/15 14:34:41, agrieve wrote:
> I don't think we need to have this as a separate template. Just don't define a
> resources target if aar.py --list returns an empty list.

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/config/android...
build/config/android/rules.gni:2581: forward_variables_from(invoker,
On 2016/07/15 14:34:41, agrieve wrote:
> nit: Try to use forward_variables_from only when forwarding variables to a
> sub-template. In this case I don't think there's value in using it. Just use
> "invoker.aar_path" below.

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/1/build/config/android...
build/config/android/rules.gni:2698: get_path_info(rebase_path(_res_files, "",
root_out_dir), "abspath")
On 2016/07/15 14:34:41, agrieve wrote:
> The get_path_info part of this is unnecessary I think.

I tried to remove get_path_info but with no luck. Outputs expects files to be in
"//......" format and get_path_info() returns that format.

[Ian Wen, 2016-07-17 02:08:18]

The CQ bit was checked by ianwen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-17 02:08:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-17 02:22:54]

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

[commit-bot: I haz the power, 2016-07-17 02:22:55]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[agrieve, 2016-07-18 18:31:52]

Nothing sticks out to me as for the reason it would be detecting stale targets.
One thing I would double check is that the paths of all of the outputs actually
end up being created by the extract rule. E.g. if it lists an output that is not
created, ninja will consider it dirty and try again to build that output. May
also help to look at the paths as they exist within the .ninja files.

This change basically looks good, but should fix the bots first.

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/android/gy...
File build/android/gyp/aar.py (right):

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/android/gy...
build/android/gyp/aar.py:38: args = parser.parse_args()
nit: You should check that either --extract or --list is passed.

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/android/gy...
build/android/gyp/aar.py:53: if 'resources' not in data:
nit: I think it would slightly simplify the code on both ends if resources and
jars were always defined (as empty lists)

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
File build/config/android/rules.gni (right):

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
build/config/android/rules.gni:2610: outputs += get_path_info(
I think you could avoid get_path_info if you asked for paths relative to the
current BUILD.gn file:

outputs += rebase_path(_scanned_files.resources, ".", _output_path),

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
build/config/android/rules.gni:2624: counter = 0
nit: prefix with _

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
build/config/android/rules.gni:2627: current_target =
"${target_name}__jar_$counter"
nit: prefix with _

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
build/config/android/rules.gni:2646: "custom_package",
nit: I don't think this should be necessary. .aar files define their package
name via their AndroidManifest.xml, which is hooked up below.

[Ian Wen, 2016-07-18 23:10:02]

Description was changed from

==========
Add AAR support to Chrome and convert support libraries to using AARs

Chrome has been using Jar and resources from support library for years.
In Q1 2016, Android team stops shipping jars/res for support libray.
Instead, AAR is promoted, which is a zip that wraps jars and resources.

This CL introduces a utility python script that processes an AAR file.
In GN gen time, it lists all files in the AAR, yet it does not extract
it. Actual unpacking is postponed until compilation.

Two other things to notice:
1. In the old jar we depended on, support-v13 contains support-v4 and
support-annotations. After converting to AAR, these two libraries are no
longer part of support-v13. Thus this change needs to be reflected.
2. In the new AAR format, support-v4 now contains two jars instead of
one. All public classes are in classes.jar, and all hidden classes are
in libs/internal_impl-$VERSION.jar.

BUG=611171
==========

to

==========
Add AAR support to Chrome and convert support libraries to using AARs

Chrome has been using Jar and resources from support library for years.
In Q1 2016, Android team stops shipping jars/res for support libray.
Instead, AAR is promoted, which is a zip that wraps jars and resources.

This CL introduces a utility python script that processes an AAR file.
In GN gen time, it lists all files in the AAR, yet it does not extract
it. Actual unpacking is postponed until compilation.

Two other things to notice:
1. In the old jar we depended on, support-v13 contains support-v4 and
support-annotations. After converting to AAR, these two libraries are no
longer part of support-v13. Thus this change needs to be reflected.
2. In the new AAR format, support-v4 now contains two jars instead of
one. All public classes are in classes.jar, and all hidden classes are
in libs/internal_impl-$VERSION.jar.

This work is not possible without bajones@'s pioneering work in
https://chromiumcodereview.appspot.com/2069273002/

BUG=611171
==========

[Ian Wen, 2016-07-18 23:10:59]

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/android/gy...
File build/android/gyp/aar.py (right):

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/android/gy...
build/android/gyp/aar.py:38: args = parser.parse_args()
On 2016/07/18 18:31:52, agrieve wrote:
> nit: You should check that either --extract or --list is passed.

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/android/gy...
build/android/gyp/aar.py:53: if 'resources' not in data:
On 2016/07/18 18:31:52, agrieve wrote:
> nit: I think it would slightly simplify the code on both ends if resources and
> jars were always defined (as empty lists)

Done. If I let empty list go through to GN, I need to alter rules.gni a bit to
check if the resources list is empty, not if it is defined.

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
File build/config/android/rules.gni (right):

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
build/config/android/rules.gni:2610: outputs += get_path_info(
On 2016/07/18 18:31:52, agrieve wrote:
> I think you could avoid get_path_info if you asked for paths relative to the
> current BUILD.gn file:
> 
> outputs += rebase_path(_scanned_files.resources, ".", _output_path),

Sadly this does not work either. Outputs expect the files to start with either
$gen or $out folder. Yet rebase_path won't generate paths starting with "//".

Ideally I could just concatenate the string. Yet since this is a list, I prefer
to do it in a batch function, instead of writing a clumsy foreach loop. :)

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
build/config/android/rules.gni:2624: counter = 0
On 2016/07/18 18:31:52, agrieve wrote:
> nit: prefix with _

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
build/config/android/rules.gni:2627: current_target =
"${target_name}__jar_$counter"
On 2016/07/18 18:31:52, agrieve wrote:
> nit: prefix with _

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
build/config/android/rules.gni:2646: "custom_package",
On 2016/07/18 18:31:52, agrieve wrote:
> nit: I don't think this should be necessary. .aar files define their package
> name via their AndroidManifest.xml, which is hooked up below.

Oh yeah you are right. Done.

[Ian Wen, 2016-07-18 23:11:08]

The CQ bit was checked by ianwen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-18 23:11:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ian Wen, 2016-07-19 00:21:50]

PTAL the latest patchset :)

Tryjobs passed. Recompiling issue solved. It was because R.txt is not there in
the extracted folder for java-only targets, yet we always assume it is there.
Big thanks for your hint, agrieve@!

[agrieve, 2016-07-19 00:25:38]

lgtm!

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
File build/config/android/rules.gni (right):

https://chromiumcodereview.appspot.com/2156453002/diff/20001/build/config/and...
build/config/android/rules.gni:2610: outputs += get_path_info(
On 2016/07/18 23:10:59, Ian Wen wrote:
> On 2016/07/18 18:31:52, agrieve wrote:
> > I think you could avoid get_path_info if you asked for paths relative to the
> > current BUILD.gn file:
> > 
> > outputs += rebase_path(_scanned_files.resources, ".", _output_path),
> 
> Sadly this does not work either. Outputs expect the files to start with either
> $gen or $out folder. Yet rebase_path won't generate paths starting with "//".
> 
> Ideally I could just concatenate the string. Yet since this is a list, I
prefer
> to do it in a batch function, instead of writing a clumsy foreach loop. :)

:(. I get it now.

[Ian Wen, 2016-07-19 00:36:36]

ianwen@chromium.org changed reviewers:
+ blundell@chromium.org, dgn@chromium.org, jbudorick@chromium.org,
joedow@chromium.org, slan@chromium.org, tedchoc@chromium.org,
wangxianzhu@chromium.org, wnwen@chromium.org, yusufo@chromium.org

[Ian Wen, 2016-07-19 00:36:36]

Dear owners,

PTAL at this change about how we compile against android support library. In the
new support library format (AAR), support-v13 no longer contains annotations and
support-v4. Therefore related dependencies need to be updated.

[Ian Wen, 2016-07-19 00:36:58]

The CQ bit was checked by ianwen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 00:37:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-19 02:04:46]

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

[commit-bot: I haz the power, 2016-07-19 02:04:48]

Dry run: This issue passed the CQ dry run.

[blundell, 2016-07-19 06:25:21]

blundell@chromium.org changed reviewers:
+ mef@chromium.org
- blundell@chromium.org

[blundell, 2016-07-19 06:25:22]

blundell -> mef for //components/cronet

Prefer OWNERS of specific //components when possible. Thanks!

[jbudorick, 2016-07-19 13:14:06]

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/android/gy...
File build/android/gyp/aar.py (right):

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/android/gy...
build/android/gyp/aar.py:17:
sys.path.append(os.path.join(os.path.dirname(__file__), os.pardir, os.pardir))
nit: sys.path.append(os.path.abspath(...))

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/android/gy...
build/android/gyp/aar.py:39: assert args.extract is not None or args.list is not
None
nit: this should be calling parser.error if neither --extract nor --list is
specified, and it should better explain what a user should be doing.

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/android/gy...
build/android/gyp/aar.py:41: aar_file = args.input_file  # e.g. library.aar
nit: move these examples up into the help text.

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/config/and...
File build/config/android/rules.gni (right):

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/config/and...
build/config/android/rules.gni:2570: #   requires_android: Whether this library
must be used for compiling Android. Default to true. (Optional)
nit: this gets the requirement backwards -- requires_android means that the
target requires compiling for Android rather than vice versa.

On a larger note, though -- when would an android_aar_prebuilt ever be usable
outside of an Android compilation?

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/secondary/...
File build/secondary/third_party/android_tools/BUILD.gn (right):

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/secondary/...
build/secondary/third_party/android_tools/BUILD.gn:55: requires_android = false
This seems ... nonintuitive.

[Ian Wen, 2016-07-19 18:36:48]

Thanks for the review. All addressed.

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/android/gy...
File build/android/gyp/aar.py (right):

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/android/gy...
build/android/gyp/aar.py:17:
sys.path.append(os.path.join(os.path.dirname(__file__), os.pardir, os.pardir))
On 2016/07/19 13:14:06, jbudorick wrote:
> nit: sys.path.append(os.path.abspath(...))

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/android/gy...
build/android/gyp/aar.py:39: assert args.extract is not None or args.list is not
None
On 2016/07/19 13:14:06, jbudorick wrote:
> nit: this should be calling parser.error if neither --extract nor --list is
> specified, and it should better explain what a user should be doing.

Done.

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/android/gy...
build/android/gyp/aar.py:41: aar_file = args.input_file  # e.g. library.aar
On 2016/07/19 13:14:06, jbudorick wrote:
> nit: move these examples up into the help text.

These comments are very basic. I will simply remove them. Done.

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/config/and...
File build/config/android/rules.gni (right):

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/config/and...
build/config/android/rules.gni:2570: #   requires_android: Whether this library
must be used for compiling Android. Default to true. (Optional)
On 2016/07/19 13:14:06, jbudorick wrote:
> nit: this gets the requirement backwards -- requires_android means that the
> target requires compiling for Android rather than vice versa.
> 
> On a larger note, though -- when would an android_aar_prebuilt ever be usable
> outside of an Android compilation?

Talked offline. Added todo. Done.

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/secondary/...
File build/secondary/third_party/android_tools/BUILD.gn (right):

https://chromiumcodereview.appspot.com/2156453002/diff/60001/build/secondary/...
build/secondary/third_party/android_tools/BUILD.gn:55: requires_android = false
On 2016/07/19 13:14:06, jbudorick wrote:
> This seems ... nonintuitive.

Added TODO. Done.

[jbudorick, 2016-07-19 18:39:04]

lgtm

[Ted C, 2016-07-19 19:57:38]

On 2016/07/19 18:39:04, jbudorick wrote:
> lgtm

lgtm 

chrome*
content*
third_party/android_swipe_refresh

[Yusuf, 2016-07-19 20:04:34]

lgtm for third_party/custom_tabs_client

[mef, 2016-07-19 20:14:12]

components/cronet/* lgtm

[Xianzhu, 2016-07-19 20:33:17]

tools/android lgtm.

[Sergey Ulanov, 2016-07-19 21:18:51]

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

[Sergey Ulanov, 2016-07-19 21:18:52]

remoting lgtm

[dgn, 2016-07-19 22:02:23]

3p/android_media lgtm

https://codereview.chromium.org/2156453002/diff/80001/build/secondary/third_p...
File build/secondary/third_party/android_tools/BUILD.gn (right):

https://codereview.chromium.org/2156453002/diff/80001/build/secondary/third_p...
build/secondary/third_party/android_tools/BUILD.gn:36: lib_path =
"$android_sdk_root/extras/android/m2repository/com/android/support"
How do we get the libraries there? Do we just check in m2repository?

[Ian Wen, 2016-07-19 22:11:08]

On 2016/07/19 22:02:23, dgn wrote:
> 3p/android_media lgtm
> 
>
https://codereview.chromium.org/2156453002/diff/80001/build/secondary/third_p...
> File build/secondary/third_party/android_tools/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2156453002/diff/80001/build/secondary/third_p...
> build/secondary/third_party/android_tools/BUILD.gn:36: lib_path =
> "$android_sdk_root/extras/android/m2repository/com/android/support"
> How do we get the libraries there? Do we just check in m2repository?

I will write a doc about how to roll support library in future. The short answer
is to check out android support repo in the GUI tool called "android". And
remove all apparently unused version folders.

If you sync today, the m2repo is already in your checkout.

[Ian Wen, 2016-07-19 22:38:31]

The CQ bit was checked by ianwen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 22:39:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ian Wen, 2016-07-19 22:42:35]

Description was changed from

==========
Add AAR support to Chrome and convert support libraries to using AARs

Chrome has been using Jar and resources from support library for years.
In Q1 2016, Android team stops shipping jars/res for support libray.
Instead, AAR is promoted, which is a zip that wraps jars and resources.

This CL introduces a utility python script that processes an AAR file.
In GN gen time, it lists all files in the AAR, yet it does not extract
it. Actual unpacking is postponed until compilation.

Two other things to notice:
1. In the old jar we depended on, support-v13 contains support-v4 and
support-annotations. After converting to AAR, these two libraries are no
longer part of support-v13. Thus this change needs to be reflected.
2. In the new AAR format, support-v4 now contains two jars instead of
one. All public classes are in classes.jar, and all hidden classes are
in libs/internal_impl-$VERSION.jar.

This work is not possible without bajones@'s pioneering work in
https://chromiumcodereview.appspot.com/2069273002/

BUG=611171
==========

to

==========
Add AAR support to Chrome and convert support libraries to using AARs

Chrome has been using Jar and resources from support library for years.
In Q1 2016, Android team stops shipping jars/res for support libray.
Instead, AAR is promoted, which is a zip that wraps jars and resources.

This CL introduces a utility python script that processes an AAR file.
In GN gen time, it lists all files in the AAR, yet it does not extract
it. Actual unpacking is postponed until compilation.

Two other things to notice:
1. In the old jar we depended on, support-v13 contains support-v4 and
support-annotations. After converting to AAR, these two libraries are no
longer part of support-v13. Thus this change needs to be reflected.
2. In the new AAR format, support-v4 now contains two jars instead of
one. All public classes are in classes.jar, and all hidden classes are
in libs/internal_impl-$VERSION.jar.

This work is not possible without bajones@'s pioneering work in
https://chromiumcodereview.appspot.com/2069273002/

BUG=611171
==========

[Ian Wen, 2016-07-19 22:42:35]

ianwen@chromium.org changed reviewers:
+ hartmanng@chromium.org
- joedow@chromium.org, wnwen@chromium.org

[slan, 2016-07-19 22:47:29]

cast lgtm

[commit-bot: I haz the power, 2016-07-20 00:19:51]

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

[commit-bot: I haz the power, 2016-07-20 00:19:53]

Dry run: This issue passed the CQ dry run.

[hartmanng, 2016-07-20 14:31:23]

third_party/android_async_task/BUILD.gn LGTM

[Ian Wen, 2016-07-20 17:10:26]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-07-20 17:10:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
agrieve@chromium.org, dgn@chromium.org, sergeyu@chromium.org,
jbudorick@chromium.org, tedchoc@chromium.org, yusufo@chromium.org,
wangxianzhu@chromium.org, mef@chromium.org
Link to the patchset: https://codereview.chromium.org/2156453002/#ps100001
(title: "revert change in 3p/android_async_task/README.chromium")

[commit-bot: I haz the power, 2016-07-20 17:10:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 17:40:14]

Description was changed from

==========
Add AAR support to Chrome and convert support libraries to using AARs

Chrome has been using Jar and resources from support library for years.
In Q1 2016, Android team stops shipping jars/res for support libray.
Instead, AAR is promoted, which is a zip that wraps jars and resources.

This CL introduces a utility python script that processes an AAR file.
In GN gen time, it lists all files in the AAR, yet it does not extract
it. Actual unpacking is postponed until compilation.

Two other things to notice:
1. In the old jar we depended on, support-v13 contains support-v4 and
support-annotations. After converting to AAR, these two libraries are no
longer part of support-v13. Thus this change needs to be reflected.
2. In the new AAR format, support-v4 now contains two jars instead of
one. All public classes are in classes.jar, and all hidden classes are
in libs/internal_impl-$VERSION.jar.

This work is not possible without bajones@'s pioneering work in
https://chromiumcodereview.appspot.com/2069273002/

BUG=611171
==========

to

==========
Add AAR support to Chrome and convert support libraries to using AARs

Chrome has been using Jar and resources from support library for years.
In Q1 2016, Android team stops shipping jars/res for support libray.
Instead, AAR is promoted, which is a zip that wraps jars and resources.

This CL introduces a utility python script that processes an AAR file.
In GN gen time, it lists all files in the AAR, yet it does not extract
it. Actual unpacking is postponed until compilation.

Two other things to notice:
1. In the old jar we depended on, support-v13 contains support-v4 and
support-annotations. After converting to AAR, these two libraries are no
longer part of support-v13. Thus this change needs to be reflected.
2. In the new AAR format, support-v4 now contains two jars instead of
one. All public classes are in classes.jar, and all hidden classes are
in libs/internal_impl-$VERSION.jar.

This work is not possible without bajones@'s pioneering work in
https://chromiumcodereview.appspot.com/2069273002/

BUG=611171
==========

[commit-bot: I haz the power, 2016-07-20 17:40:16]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-07-20 17:40:22]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-20 17:44:14]

Description was changed from

==========
Add AAR support to Chrome and convert support libraries to using AARs

Chrome has been using Jar and resources from support library for years.
In Q1 2016, Android team stops shipping jars/res for support libray.
Instead, AAR is promoted, which is a zip that wraps jars and resources.

This CL introduces a utility python script that processes an AAR file.
In GN gen time, it lists all files in the AAR, yet it does not extract
it. Actual unpacking is postponed until compilation.

Two other things to notice:
1. In the old jar we depended on, support-v13 contains support-v4 and
support-annotations. After converting to AAR, these two libraries are no
longer part of support-v13. Thus this change needs to be reflected.
2. In the new AAR format, support-v4 now contains two jars instead of
one. All public classes are in classes.jar, and all hidden classes are
in libs/internal_impl-$VERSION.jar.

This work is not possible without bajones@'s pioneering work in
https://chromiumcodereview.appspot.com/2069273002/

BUG=611171
==========

to

==========
Add AAR support to Chrome and convert support libraries to using AARs

Chrome has been using Jar and resources from support library for years.
In Q1 2016, Android team stops shipping jars/res for support libray.
Instead, AAR is promoted, which is a zip that wraps jars and resources.

This CL introduces a utility python script that processes an AAR file.
In GN gen time, it lists all files in the AAR, yet it does not extract
it. Actual unpacking is postponed until compilation.

Two other things to notice:
1. In the old jar we depended on, support-v13 contains support-v4 and
support-annotations. After converting to AAR, these two libraries are no
longer part of support-v13. Thus this change needs to be reflected.
2. In the new AAR format, support-v4 now contains two jars instead of
one. All public classes are in classes.jar, and all hidden classes are
in libs/internal_impl-$VERSION.jar.

This work is not possible without bajones@'s pioneering work in
https://chromiumcodereview.appspot.com/2069273002/

BUG=611171

Committed: https://crrev.com/c84b9756d482be2561734a19138b41ca630f2701
Cr-Commit-Position: refs/heads/master@{#406603}
==========

[commit-bot: I haz the power, 2016-07-20 17:44:17]

Patchset 6 (id:??) landed as
https://crrev.com/c84b9756d482be2561734a19138b41ca630f2701
Cr-Commit-Position: refs/heads/master@{#406603}