927388 - [geckoview] Produce and upload Geckoview AAR during packaging

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: GRE Core, defect)

Description

[Lucas Rocha (:lucasr)]

Containing both library and assets. There's not a lot of information on the format itself right now:

http://tools.android.com/tech-docs/new-build-system/aar-format

Comment 1

[Nick Alexander :nalexander [he/him]]

See http://www.ncalexander.net/blog/2014/07/10/build-your-own-browser-a-maven-repository-for-geckoview/ and the Jenkins job at https://ci.mozilla.org/job/mozilla-central-geckoview/ for guidance here.

It would be nice to produce AARs as the TBPL build artifact, and just make Jenkins handle Maven deployment.

Comment 2

[Nick Alexander :nalexander [he/him]]

Now that Bug 1093242 has landed, this got a lot more feasible.

The AAR should be called "geckoview-$BUILDID.aar", like the geckolibs precedent.  It should

* depend on geckolibs, rather than including the .so libraries itself;
* include omni.ja;
* include all of the Java code for Fennec in a single classes.jar;
* include all of the resources (they're already packaged into a geckoview_resources.zip during the build of mobile/android/base; see https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/Makefile.in#337.

To get started on this ticket, read a little about the new AAR format (see link above) and then look at the code in https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/action/package_geckolibs_aar.py.  Try to extend that code to also write another AAR file and the metadata for it.  You'll have to figure out how to set the dependencies on geckolibs in the maven POM (and the ivy.xml, I guess).  After we have something generated locally, we can work on getting it uploaded to TBPL.

This is very open-ended, so ask for feedback early and often!

Comment 3

[Nick Alexander :nalexander [he/him]]

Ahmed: you were interested in this.  It's rather vague right now, but I can help along the way.

You can make the existing code run either by adding the line [1] to your mozconfig and running |mach package| (hard) or just running (easy)

./mach python python/mozbuild/mozbuild/action/package_geckolibs_aar.py ARGUMENTS HERE

[1] https://dxr.mozilla.org/mozilla-central/source/mobile/android/config/mozconfigs/android-api-11/nightly#18

Comment 4

[Nick Alexander :nalexander [he/him]]

> ./mach python python/mozbuild/mozbuild/action/package_geckolibs_aar.py
> ARGUMENTS HERE

You can see how the automation scripts call this by looking at https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/upload-files.mk#382.

Comment 5

[Ahmed Khalil]

I've managed to copy the omni.ja to assets inside the aar and also copied the whole res folder to the aar.
There are still classes.jar file and AndroidManifest.xml to be copied, but the problem is that I cannot find the compiled .class file, I've searched at the $OBJDIR and $TOPSRCDIR/mobile/android directories but I didn't find any .class files, any idea where would I find those files in the source tree ?

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Ahmed Khalil from comment #5)
> I've managed to copy the omni.ja to assets inside the aar and also copied
> the whole res folder to the aar.
> There are still classes.jar file and AndroidManifest.xml to be copied, but
> the problem is that I cannot find the compiled .class file, I've searched at
> the $OBJDIR and $TOPSRCDIR/mobile/android directories but I didn't find any
> .class files, any idea where would I find those files in the source tree ?

Yep -- look for build artifacts in $OBJDIR/mobile/android/base/jars, I think.  You'll want to unzip the individual JAR files and rezip them into classes.jar, I think.

Comment 7

[Ahmed Khalil]

Attachment: package_geckolibs_aar

Here is a patch that bundles fennec's resources into geckoview.aar.
I believe that there is still much room for improvement, especially in the generation of the pom and ivy xml files.
Let me know what you think about it.

Comment 8

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8572898 [details] [diff] [review]
package_geckolibs_aar

Review of attachment 8572898 [details] [diff] [review]:
-----------------------------------------------------------------

Ahmed: this is a great first cut at this!

The only substantive change I'd like to see is with the zip/rezip functionality.  For both the resources and assembling classes.jar, I think you should be able to use the JarFileFinder at [1] to save unzipping.  This will give us fine control on the inputs and outputs.

I can't wait to test this locally and get a try build up!

[1] https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozpack/files.py#857

::: python/mozbuild/mozbuild/action/package_geckolibs_aar.py
@@ +34,5 @@
>    <version>{version}</version>
>    <packaging>{packaging}</packaging>
> +  <dependencies>
> +    {dependencies}
> +    </dependencies>

nit: indenting looks a little funky.

@@ +62,4 @@
>  </ivy-module>
>  '''.lstrip()
>  
> +IVY_XML_DEPENDENCY_TEMPLATE=r'''

nit: spaces around =.

@@ +72,5 @@
> +        archive_root = root.replace(path, '')
> +        for file in files:
> +            zip.write(os.path.join(root, file), os.path.join(archive_root, file))
> +
> +#Copied from http://stackoverflow.com/a/12886818/429962

nit throughout: space after #.  We try to use full sentence comments.

@@ +119,5 @@
> +    assets = FileFinder(os.path.join(fennec_path, 'assets'), ignore=['*.so'])
> +    for p, f in assets.find('omni.ja'):
> +        jarrer.add(os.path.join('assets', p), f)
> +
> +    res_folder = FileFinder(os.path.join(fennec_path, 'res'))

I expect we will need to take these from the geckoview_resources.zip produced by mobile/android/base/Makefile.in, but the idea is good!

@@ +127,5 @@
> +    # the folder that contains fennec's jars and resources
> +    base_path = os.path.join(distdir, '..', 'mobile', 'android', 'base')
> +    base_folder = FileFinder(base_path)
> +
> +    #Unzip all jar files to $(DISTDIR)/geckoview_aar_classes

Can we extract this unzip / rezip into a little helper function?

@@ +129,5 @@
> +    base_folder = FileFinder(base_path)
> +
> +    #Unzip all jar files to $(DISTDIR)/geckoview_aar_classes
> +    geckoview_aar_classes_path = os.path.join(distdir, 'geckoview_aar_classes')
> +    for p, f in base_folder.find('*.jar'):

I think we'll want every .jar except gecko-R.jar, at least at first.

@@ +141,5 @@
> +    #Add R.txt to the aar
> +    jarrer.add('R.txt', File(os.path.join(base_path, 'R.txt')))
> +
> +    #Finally add AndroidManifest.xml
> +    srcdir = os.path.join(topsrcdir, 'mobile', 'android', 'geckoview_library', 'geckolibs')

Let's add a new directory and AndroidManifest.xml -- mobile/android/geckoview_library/geckoview/AndroidManifest.xml with the package name changed to org.mozilla.gecko.

@@ +146,5 @@
> +    jarrer.add('AndroidManifest.xml', File(os.path.join(srcdir, 'AndroidManifest.xml')))
> +
> +    jarrer.copy(output_file)
> +    return 0
> +    

nit: remove trailing whitespace (throughout).

Comment 9

[Ahmed Khalil]

(In reply to Nick Alexander :nalexander from comment #8)
> Comment on attachment 8572898 [details] [diff] [review]
> package_geckolibs_aar
> 
> Review of attachment 8572898 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Ahmed: this is a great first cut at this!
> 
> The only substantive change I'd like to see is with the zip/rezip
> functionality.  For both the resources and assembling classes.jar, I think
> you should be able to use the JarFileFinder at [1] to save unzipping.  This
> will give us fine control on the inputs and outputs.
> 
I tried to use the JarFinder class, by suppling a JarReader to it, but for some reason I get an exception http://pastebin.com/NYe202er.

I've read the test case that uses JarFinder and I think I'm doing it right [1]
Here is the client code that tries to use JarFinder http://pastebin.com/fsNHGXjC.
 
> Let's add a new directory and AndroidManifest.xml --
> mobile/android/geckoview_library/geckoview/AndroidManifest.xml with the
> package name changed to org.mozilla.gecko.

Would I add that by myself, or wait for you to add it ?

Comment 10

[Nick Alexander :nalexander [he/him]]

(In reply to Ahmed Khalil from comment #9)
> (In reply to Nick Alexander :nalexander from comment #8)
> > Comment on attachment 8572898 [details] [diff] [review]
> > package_geckolibs_aar
> > 
> > Review of attachment 8572898 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Ahmed: this is a great first cut at this!
> > 
> > The only substantive change I'd like to see is with the zip/rezip
> > functionality.  For both the resources and assembling classes.jar, I think
> > you should be able to use the JarFileFinder at [1] to save unzipping.  This
> > will give us fine control on the inputs and outputs.
> > 
> I tried to use the JarFinder class, by suppling a JarReader to it, but for
> some reason I get an exception http://pastebin.com/NYe202er.
> 
> I've read the test case that uses JarFinder and I think I'm doing it right
> [1]
> Here is the client code that tries to use JarFinder
> http://pastebin.com/fsNHGXjC.

I'll investigate this locally.

> > Let's add a new directory and AndroidManifest.xml --
> > mobile/android/geckoview_library/geckoview/AndroidManifest.xml with the
> > package name changed to org.mozilla.gecko.
> 
> Would I add that by myself, or wait for you to add it ?

Go for it!  It'll fold into your patch.

Comment 11

[Ahmed Khalil]

Attachment: package_geckolibs_aar

Hi Nick, 
I've resolved all your comments, except for the JarFinder one.
Also I've searched for geckoview_resources.zip, but I didn't find one in the $OBJDIR

Comment 12

[Ahmed Khalil]

Hi Nick,
I hope that you're alright, and everything is fine.
Any updates on reviewing the patch ?

Comment 13

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eca6802d3c48

Comment 14

[Nick Alexander :nalexander [he/him]]

Attachment: Review fixes

Comment 15

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8574834 [details] [diff] [review]
Review fixes

Hi Ahmed,

First, great work!  I made some fixes and improvements and tested this locally with the repository at https://github.com/ncalexan/geckobrowser-gradle.  The browser builds but fails immediately, but not because of this work.  We can address over time.

Major points:

* fix typo in <dependency> block;
* resources from geckoview_resources.zip;
* classes.jar was being incorrectly built (see http://stackoverflow.com/questions/12886768/how-to-unzip-file-in-python-on-all-oses/12886818#comment27680318_12886818)

Take a look and see if it works for you locally?  I've pushed a try build to see how we fare on infrastructure.  And then we can test downloading from TreeHerder in the example project.

Thanks for all your work on this!

Comment 16

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8574056 [details] [diff] [review]
package_geckolibs_aar

Review of attachment 8574056 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great!  I've pushed some review adjustments back to you -- let me know how they look.

Comment 17

[Ahmed Khalil]

(In reply to Nick Alexander :nalexander from comment #15)
> Comment on attachment 8574834 [details] [diff] [review]
> Review fixes
> 
> Hi Ahmed,
> 
> First, great work!  I made some fixes and improvements and tested this
> locally with the repository at
> https://github.com/ncalexan/geckobrowser-gradle.  The browser builds but
> fails immediately, but not because of this work.  We can address over time.
> 
> Major points:
> 
> * fix typo in <dependency> block;
> * resources from geckoview_resources.zip;
> * classes.jar was being incorrectly built (see
> http://stackoverflow.com/questions/12886768/how-to-unzip-file-in-python-on-
> all-oses/12886818#comment27680318_12886818)
> 
> Take a look and see if it works for you locally?  I've pushed a try build to
> see how we fare on infrastructure.  And then we can test downloading from
> TreeHerder in the example project.
> 
> Thanks for all your work on this!

Hi Nick,
Your code looks neat and great! but when I the script is used to generate the gecko(libs/views), for some reason the revision number is supplied in yyyyMMddHHmmss and the script fails with error:
line 167, in main
raise ValueError('Revision must be in yyyyMMddHH format: %s' % args.revision) ValueError: Revision must be in yyyyMMddHH format: 20150309231639)

Perhaps that on Mac the revision number is supplied as expected and on Linux is not ? I don't know what about Windows.

Comment 18

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=02a7e7dd112e

Comment 19

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/1707d2ad4051

Comment 20

[Nick Alexander :nalexander [he/him]]

This worked fine in automation.  I've landed with the additional files to upload (hope I didn't typo!).  I didn't include the permissions in the AndroidManifest.xml, but we can work on improving that over time.

Thanks for your work on this, Ahmed!

Comment 21

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/6119cce9ce80

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1707d2ad4051
https://hg.mozilla.org/mozilla-central/rev/6119cce9ce80

Comment 23

[Kiran]

I could build app using the AAR, but is not working on x86 processors (e.g. Acer Iconia Tab 8 A1-840FHD).
Could you help?

Thanks!

Comment 24

[Nick Alexander :nalexander [he/him]]

(In reply to Kiran from comment #23)
> I could build app using the AAR, but is not working on x86 processors (e.g.
> Acer Iconia Tab 8 A1-840FHD).
> Could you help?

We don't produce x86 AAR packages: http://ftp.mozilla.org/pub/mobile/nightly/latest-mozilla-central-android-x86/.  You could build x86 and produce the AAR yourself.  Alternately, you could grab the libraries from the ZIP files in that directory and shove them into the AAR yourself -- AARs are just ZIP files.