Closed Bug 1180461 Opened 5 years ago Closed 4 years ago

Package gaia in b2gdroid

Categories

(Firefox for Android :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 43
Tracking Status
firefox42 --- affected
firefox43 --- fixed

People

(Reporter: fabrice, Assigned: fabrice)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch b2gdroid-package-gaia.patch (obsolete) — Splinter Review
That builds gaia and add it in the .apk's asset directory.
Assignee: nobody → fabrice
Attachment #8629627 - Flags: feedback?(nalexander)
Attachment #8629760 - Flags: review?(nalexander)
Comment on attachment 8629760 [details] [diff] [review]
b2gdroid-package-gaia.patch

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

r+ because this will do what it says on the tin, provided that DIRS is ordered.

But: is there a compelling reason to route through dist/?  You could expose GAIADIR to the build system (if it isn't already), and then include GAIADIR/profile where appropriate.  I.e., not to GeckoView, but to the b2gdroid App.

::: mobile/android/b2gdroid/config/mozconfigs/common
@@ +61,5 @@
>  mk_add_options "export PATH=$topsrcdir/apache-ant/bin:$PATH"
>  
>  JS_BINARY="$topsrcdir/mobile/android/config/js_wrapper.sh"
> +
> +GAIADIR="$topsrcdir/gaia"

Is there a reason that you have GAIA_ and no underscore here?  Are you following suit with b2g/?

::: mobile/android/b2gdroid/gaia/Makefile.in
@@ +9,5 @@
> +
> +include $(topsrcdir)/config/rules.mk
> +
> +libs::
> +	+$(MAKE) -j4 -C $(GAIADIR) clean

Why clean?  Also, why force -j4?

::: mobile/android/base/moz.build
@@ +629,5 @@
>      gbjar.sources += b2gdroid_java_files
>      ANDROID_RES_DIRS += [ TOPSRCDIR + '/mobile/android/b2gdroid/base/resources']
> +    ANDROID_ASSETS_DIRS += [
> +        # Would be less ugly if we had some kind of ROOT_OBJDIR.
> +        OBJDIR + '/../../../dist/bin/gaia',

We do -- TOPOBJDIR.
Comment on attachment 8629760 [details] [diff] [review]
b2gdroid-package-gaia.patch

That was meant to be an "r+".
Attachment #8629760 - Flags: review?(nalexander) → review+
glandium: can you look at this, and confirm that DIRS is ordered?
Flags: needinfo?(mh+mozilla)
Comment on attachment 8629760 [details] [diff] [review]
b2gdroid-package-gaia.patch

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

::: mobile/android/b2gdroid/gaia/Makefile.in
@@ +9,5 @@
> +
> +include $(topsrcdir)/config/rules.mk
> +
> +libs::
> +	+$(MAKE) -j4 -C $(GAIADIR) clean

I'll venture a guess: because gaia's build system sucks. And because b2g/gaia/Makefile.in does the same.

Which bring me to thinking there's no reason these rules shouldn't be shared between b2g and b2gdroid.

::: mobile/android/base/moz.build
@@ +629,5 @@
>      gbjar.sources += b2gdroid_java_files
>      ANDROID_RES_DIRS += [ TOPSRCDIR + '/mobile/android/b2gdroid/base/resources']
> +    ANDROID_ASSETS_DIRS += [
> +        # Would be less ugly if we had some kind of ROOT_OBJDIR.
> +        OBJDIR + '/../../../dist/bin/gaia',

Time to file a bug to move ANDROID_*_DIRS to using Path classes.
(In reply to Nick Alexander :nalexander (PTO July 17-27) from comment #5)
> glandium: can you look at this, and confirm that DIRS is ordered?

DIRS is not StrictOrderingOnAppendList
Flags: needinfo?(mh+mozilla)
The try build above is some hacky WIP to build b2gdroid as a GeckoView consumer.  The build should fail because GAIADIR is not configured.

Locally, that work produces a working b2gdroid APK.  The work isn't ready to land 'cuz other bits of the build need to be updated for the java-build.mk changes.  But it shows the way.
https://hg.mozilla.org/mozilla-central/rev/bf768f510072
https://hg.mozilla.org/mozilla-central/rev/6c3aca3d7e73
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
You need to log in before you can comment on or make changes to this bug.