Closed Bug 1419581 Opened 7 years ago Closed 6 years ago

Add --without-google-play-services to build Fennec without Google Play Services

Categories

(Firefox Build System :: Android Studio and Gradle Integration, enhancement)

Product:
Firefox Build System
Component:
Android Studio and Gradle Integration
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox59 wontfix, firefox60 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

(Whiteboard: [tor-mobile])

Attachments

(7 files, 10 obsolete files)

Bug 1419581 - Part 2: Guard media Fennec JNI primitives with MOZ_NATIVE_DEVICES.
59 bytes, text/x-review-board-request
jchen
: review+
Details
Bug 1419581 - Pre: Add missing excludes when building without MOZ_NATIVE_DEVICES.
59 bytes, text/x-review-board-request
nalexander
: review+
Details
Bug 1419581 - Part 1: Simplify MMA GCM sender IDs logic.
59 bytes, text/x-review-board-request
cnevinchen
: review+
Details
Bug 1419581 - Part 3: Guard PresentationMediaPlayer windows with MOZ_NATIVE_DEVICES.
59 bytes, text/x-review-board-request
jchen
: review+
Details
Bug 1419581 - Part 4: Allow setting MOZ_ANDROID_GCM in mozconfig.
59 bytes, text/x-review-board-request
chmanchester
: review+
Details
Bug 1419581 - Part 5: Add --without-google-play-services.
59 bytes, text/x-review-board-request
chmanchester
: review+
Details
Bug 1419581 - Part 6: Add Android build configuration --without-google-play-services.
59 bytes, text/x-review-board-request
dustin
: review+
Details 
When we shipped Push, we more-or-less required Google Cloud Messaging support, which requires Google Play Services.  "Free as in freedom" folks, including Tor and FDroid, don't want to ship a browser that includes GPS code.  We've regressed the build options to disable this stuff; it's easy enough to do.

This ticket tracks patching the build options to ensure we can disable GPS and anything relying on it; and figuring out if we can make this whole area better with a Gradle configuration that is at least built (if not tested) to ensure that we don't regress the build configuration again.
These are the things I think we need to turn GPS off in a mozconfig:

MOZ_ANDROID_GCM=
MOZ_NATIVE_DEVICES=
MOZ_INSTALL_TRACKING=
MOZ_ANDROID_MMA=

Actually, it should just be MOZ_NATIVE_DEVICES=, which is code for "Google Play Services" (blame that name on mfinkle!), and we should make MOZ_ANDROID_GCM depend on MOZ_NATIVE_DEVICES.  With this configuration, the build fails, 'cuz we've regressed the build configuration.  I'll try to sort it out, but it may be tricky.
Summary: Fennec can no longer be built with Google Play Services → Fennec can no longer be built without Google Play Services
Lets start?

The builds below have the above patch (8930711) applied.

Build [1] here is built with just "MOZ_NATIVE_DEVICES=", it fails.
RemotePresentationService.java and PresentationMediaPlayerManager.java are properly specified in base/moz.build.
However they are not excluded in app/build.gradle, so it still gets included.

Build [2] here is built with the following and it fails:
MOZ_ANDROID_GCM=
MOZ_NATIVE_DEVICES=
MOZ_INSTALL_TRACKING=
MOZ_ANDROID_MMA=

MmaDelegate.java appears to be deeply tied into various components and cannot be easily removed, but it should not need to be removed as there is MmaStubImp.java.
The issue is that Push is in MmaDelegate, where as it should* be instead handled in MmaLeanplumImp.java.
The offending line 76 "mmaHelper.setGcmSenderId(PushManager.getSenderIds());" can be moved into MmaLeanplumImp.java.

Build [3] has the same config as build [2], it fails.
The issue is a missing drawable, I think this is an unreleated issue.

    
    

    
  

      
      
      
      

        Attached file
          
        Error log from build 1 (obsolete)
        — 
      

    


  

    
    

    
  

      
      
      
      

        Attached file
          
        Error log from build 2 (obsolete)
        — 
      

    


  

    
    

    
  

      
      
      
      

        Attached file
          
        Error log from build 3 (obsolete)
        — 
      

    


  

    
    

    
  
Okay so the issue from build [3] is not unrelated.
The missing drawable "ic_close_light" is part of android/support/v7/mediarouter.
Mediarouter is not included when MOZ_NATIVE_DEVICES=0
"ic_close_light" is just an 'X', we can replace it with "android.R.drawable.ic_menu_close_clear_cancel" for now.

Build [4] now compiles successfully.
The resulting APK *does not* contain GMS.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch to fix errors from build 3 (obsolete)
        — 
          — Splinter Review
      

    


  

    
    

    
  
tad: thanks for working through these issues!  The MMA GCM sender ID stuff seems a little more subtle than just moving that initialization into init; it appears separate for a reason, although that code is a little convoluted.  I'll think more about it and we'll get nechen to review.

NI to me to look closer tomorrow.
Flags: needinfo?(nalexander)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Many fixes (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #8930755 -
        Attachment is obsolete: true

        Attachment #8930756 -
        Attachment is obsolete: true

        Attachment #8930757 -
        Attachment is obsolete: true

        Attachment #8930758 -
        Attachment is obsolete: true

        Attachment #8930762 -
        Attachment is obsolete: true

    
  

        Attachment #8931330 -
        Attachment description: 1419581.patch → Many fixes

    
    

    
  
With the last patches it compiled, however it did not run (crashed after intro screens). I have added a new patch that fixes those errors and squashed in the rest. This new patch edits the native code, so it cannot be built using artifact mode.

The issue however is that the build fails due to "Error: The Fennec JNI code has changed" and running "update-fennec-wrappers" will remove the PresentationMediaPlayerManager code from the wrappers, even though that code is already #ifdef'ed out. If you manually disable the check in Makefile.in, it does successfully build and run. Someone with more knowledge on how that works will have to fix that.

The resulting APK does not contain GMS and it does run/work.

And yes, that MMA setGcmSenderId can probably be handled better.

    
    

    
  
Comment on attachment 8931330 [details] [diff] [review]
Many fixes

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

OK, here's how we get this landed.  I have a question that you can answer inline.  We need to split out the:

- ActionBarPresenter icon change into a Pre: part, and get UX feedback from antlam that it's OK.  (If the icons are identical, I can stamp it.)
- build.gradle and native code changes for media presentation parts, and get review from Kilik Kuo or darchons
- MMA changes and get review from nechen

Tad, can you have a crack at that separation?  After that's done, we can add a build job that turns off all of this stuff so that we don't break it in the future.  Thanks for all your work, Tad!

::: mobile/android/base/java/org/mozilla/gecko/customtabs/ActionBarPresenter.java
@@ +188,4 @@
>          @SuppressWarnings("deprecation")
>          final Drawable icon = mActionBar.getThemedContext()
>                  .getResources()
> +                .getDrawable(R.drawable.tab_item_close_button);

Split this into a Pre: patch, since it's not right.  We should get feedback from antlam, too.

::: mobile/android/config/proguard/proguard.cfg
@@ +159,5 @@
>  # Suppress warnings about missing descriptor classes.
>  #-dontnote **,!ch.boye.**,!org.mozilla.gecko.sync.**
>  
> +# Don't print errors of missing GMS when building without
> +-dontwarn com.google.android.gms.**

This I don't understand.  What are these warnings when building with GMS?

        Attachment #8931330 -
        Flags: feedback+
Flags: needinfo?(nalexander)

    
    

    
  
jchen: I think the patch posted above won't do the right thing, 'cuz it's not setting the preprocessor flags you added in the double-preprocessor-layer patch.  Can you take a look?
Flags: needinfo?(nchen)
Blocks: 1421347

    
    

    
  
Huh, there's more history here than I anticipated.  Bug 1324331 has work in progress on this and Bug 1397348 tracks the JNI preprocessor bits.

JanH: I see you active on those tickets.  Where did this run into problems?  Can you help it across the line?
Depends on: 1324331, 1397348
Flags: needinfo?(jh+bugzilla)

    
    

    
  
(In reply to Nick Alexander :nalexander (less responsive until Jan 3, 2018) from comment #14)
> jchen: I think the patch posted above won't do the right thing, 'cuz it's
> not setting the preprocessor flags you added in the
> double-preprocessor-layer patch.  Can you take a look?

Just need to add a `@BuildFlag()` annotation in PresentationMediaPlayerManager, add the flag here [1], and regenerate the bindings.

[1] https://searchfox.org/mozilla-central/rev/7a8c667bdd2a4a32746c9862356e199627c0896d/mobile/android/base/Makefile.in#581
Flags: needinfo?(nchen)

    
    

    
  
(In reply to Nick Alexander :nalexander (less responsive until Jan 3, 2018) from comment #15)
> JanH: I see you active on those tickets.  Where did this run into problems? 

If I remember correctly, it was when the inclusion of Exoplayer pushed us across the multi-dex boundary when building with Gradle or something like that and somebody mentioned MOZ_NATIVE_DEVICES as the quickest workaround to temporarily drop some code in order to get my build unstuck again until the problem was fixed properly. Since then, I haven't needed this again.
Flags: needinfo?(jh+bugzilla)
Whiteboard: [tor-mobile]

    
    

    
  
So I split up my patch and whatnot (works), and I removed what is already done by the patches in 1324331 (not tested yet).

However I am stuck on trying to figure out what jchen said above. I added the BuildFlag annotation import to PresentationMediaPlayerManager.java, added the "@BuildFlag("MOZ_NATIVE_DEVICES")" before all the WrapForJNI annotations and added "MOZ_NATIVE_DEVICES" to BINDING_BUILD_FLAGS. Yet all the ways I've tried result in all the PMPM code being completely removed from the wrappers. Is there something I'm missing?

    
    

    
  
(In reply to Tad from comment #18)
> So I split up my patch and whatnot (works), and I removed what is already
> done by the patches in 1324331 (not tested yet).
> 
> However I am stuck on trying to figure out what jchen said above. I added
> the BuildFlag annotation import to PresentationMediaPlayerManager.java,
> added the "@BuildFlag("MOZ_NATIVE_DEVICES")" before all the WrapForJNI
> annotations and added "MOZ_NATIVE_DEVICES" to BINDING_BUILD_FLAGS. Yet all
> the ways I've tried result in all the PMPM code being completely removed
> from the wrappers. Is there something I'm missing?

I've never tried this, so I don't know off the top.  jchen may be able to help -- Jim?

In the meantime, could you post your WIP patches?  Thanks for digging into this, Tad!
Flags: needinfo?(tad)
Flags: needinfo?(nchen)

    
    

    
  
(In reply to Tad from comment #18)
> So I split up my patch and whatnot (works), and I removed what is already
> done by the patches in 1324331 (not tested yet).
> 
> However I am stuck on trying to figure out what jchen said above. I added
> the BuildFlag annotation import to PresentationMediaPlayerManager.java,
> added the "@BuildFlag("MOZ_NATIVE_DEVICES")" before all the WrapForJNI
> annotations

The `@BuildFlag` annotation only works on classes, so `@BuildFlag("MOZ_NATIVE_DEVICES")` should go before `public class PresentationMediaPlayerManager`.

> and added "MOZ_NATIVE_DEVICES" to BINDING_BUILD_FLAGS.

This is correct.

After you regenerated the bindings (building + running the "update-generated-wrappers" step), the only changes should be addition of `#ifdef MOZ_NATIVE_DEVICES` and `#endif` lines.
Flags: needinfo?(nchen)

    
    

    
  
jchen: Thanks for the quick response. Adding it before the class definition was what I tried originally. I just did a clean build again like that, and got the same result. update-generated-wrappers doesn't update anything, and running update-fennec-wrappers after just deletes the PMPM code still. I even tried doing another clean build with setting MOZ_PREPROCESSOR=1 in my MOZCONFIG.


I'll have to format the patches and attach in the morning.

    
    

    
      
  


  

    
    

    
  
Tad: I took a look at the thing that was blocking you, and I think I made some progress.  Can you look and see if that unblocks you?  I don't want this to die on the vine, as it were.  Thanks!

    
    

    
  
(In reply to Nick Alexander :nalexander (less responsive until Jan 3, 2018) from comment #23)
> Tad: I took a look at the thing that was blocking you, and I think I made
> some progress.  Can you look and see if that unblocks you?  I don't want
> this to die on the vine, as it were.  Thanks!

nalexander: Ah! I was actually appending "MOZ_NATIVE_DEVICES" with the quotes in the BINDING_BUILD_FLAGS. Thanks. I've modified what I have and am compiling now. Assuming it succeeds, I'll push the patches for that to bug 1397348 and then test the patches from bug 1324331 and then format and upload mine.

    
    

    
  
(In reply to Tad from comment #24)
> (In reply to Nick Alexander :nalexander (less responsive until Jan 3, 2018)
> from comment #23)
> > Tad: I took a look at the thing that was blocking you, and I think I made
> > some progress.  Can you look and see if that unblocks you?  I don't want
> > this to die on the vine, as it were.  Thanks!
> 
> nalexander: Ah! I was actually appending "MOZ_NATIVE_DEVICES" with the
> quotes in the BINDING_BUILD_FLAGS. Thanks. I've modified what I have and am
> compiling now. Assuming it succeeds, I'll push the patches for that to bug
> 1397348 and then test the patches from bug 1324331 and then format and
> upload mine.

Yeah, the fact that there was no existing example of a BuildFlag made this a little harder than it should have been.  Thanks for picking this up!  Given that you're working on some of the dependent tickets, I might take this ticket to do the moz.configure work to add a `--without-google-play-services` flag, which is what we really want here.

Thanks again, Tad!

    
    

    
  


  

        Attachment #8931330 -
        Attachment is obsolete: true

    
    

    
  
I have uploaded new patches.

Using all the current attachments from bug 1419581, and bug 1324331 allows Firefox to be successfully built without Google Play Services.

As discussed earlier the MmaLeanPlum changes will need to be reviewed and tested. Those are now under attachment 8937580 [details] [diff] [review].

Attachment 8935235 [details] and attachment 8937582 [details] [diff] [review] should probably be moved to bug 1397348.
Flags: needinfo?(tad)

    
    

    
      
  


  

    
    

    
      
  


  

    
    

    
      
  


  

    
    

    
      
  

    
    

    
  
Comment on attachment 8940327 [details]
Bug 1419581 - Pre: Add missing excludes when building without MOZ_NATIVE_DEVICES.

https://reviewboard.mozilla.org/r/210602/#review216326

        Attachment #8940327 -
        Flags: review?(nalexander) → review+

    
    

    
  
tad: thanks for all your work on this!  I put together a patch series from the various tickets, and have a green try build of a new build configuration at

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

(That builds on other work that should land soon-ish.)  Could you inspect the target.apk built at

https://queue.taskcluster.net/v1/task/dTgx6iPJTvKd7GqgHuf3Og/runs/0/artifacts/public/build/target.apk

and make sure it's really built --without-google-play-services?  I decompiled it and it looked good to me.  Thanks again!


droeh: there's a resource dependency from the customtabs work to a PNG in mediarouter that shouldn't be there, specifically 

https://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/customtabs/ActionBarPresenter.java#191

customtabs shouldn't depend on Chrome Cast stuff.  We have many close resources (include close.png!) in our tree -- can you either use one of those or pull in the ic_close_light.png into the tree if that's the right thing to do?

I think the Media Router asset is

https://github.com/googlecodelabs/cast-unity-plugin/blob/master/end_project/Assets/Plugins/Android/mediarouter/res/drawable-hdpi/ic_close_light.png

and our close is slightly different:

https://searchfox.org/mozilla-central/source/mobile/android/app/src/main/res/drawable-hdpi/close.png

My try build includes a patch to unconditionally include mediarouter, but that's not the correct solution.  Thanks!
Flags: needinfo?(tad)
Flags: needinfo?(droeh)

        Attachment #8935235 -
        Flags: review?(nchen)

        Attachment #8940329 -
        Flags: review?(nchen)

    
    

    
  
nalexander: Awesome to see this moving along! I can confirm that the APK doesn't contain GMS.
Flags: needinfo?(tad)

    
    

    
  
Comment on attachment 8935235 [details]
Bug 1419581 - Part 2: Guard media Fennec JNI primitives with MOZ_NATIVE_DEVICES.

https://reviewboard.mozilla.org/r/206118/#review216348

        Attachment #8935235 -
        Flags: review?(nchen) → review+

    
    

    
  
Comment on attachment 8940329 [details]
Bug 1419581 - Part 3: Guard PresentationMediaPlayer windows with MOZ_NATIVE_DEVICES.

https://reviewboard.mozilla.org/r/210606/#review216350

        Attachment #8940329 -
        Flags: review?(nchen) → review+

    
    

    
  
Comment on attachment 8940328 [details]
Bug 1419581 - Part 1: Simplify MMA GCM sender IDs logic.

https://reviewboard.mozilla.org/r/210604/#review216530

tested with MOZ_ANDROID_MMA=1 and it works.

        Attachment #8940328 -
        Flags: review?(cnevinchen) → review+

    
    

    
  
Comment on attachment 8940332 [details]
Bug 1419581 - Part 6: Add Android build configuration --without-google-play-services.

https://reviewboard.mozilla.org/r/210612/#review216798

This looks correct.  Is this something we need to build on every push, or is the idea just to prevent breaking this particular functionality?  If the latter, perhaps just running on some projects e.g., mozilla-central?

        Attachment #8940332 -
        Flags: review?(dustin) → review+

    
    

    
  
Comment on attachment 8940330 [details]
Bug 1419581 - Part 4: Allow setting MOZ_ANDROID_GCM in mozconfig.

https://reviewboard.mozilla.org/r/210608/#review216862

::: mobile/android/moz.configure:166
(Diff revision 1)
>  def check_android_pocket(android_pocket, pocket_api_keyfile):
>      if android_pocket and not pocket_api_keyfile:
>          die('You must specify --with-pocket-api-keyfile=/path/to/keyfile when'
>              ' building with MOZ_ANDROID_POCKET=1')
> +
> +# Must come after the ../../toolkit/moz.configure.

Why? MOZ_ANDROID_GCM and MOZ_NATIVE_DEVICES are both defined in this file.

        Attachment #8940330 -
        Flags: review?(cmanchester) → review+

    
    

    
  
Comment on attachment 8940331 [details]
Bug 1419581 - Part 5: Add --without-google-play-services.

https://reviewboard.mozilla.org/r/210610/#review216878

        Attachment #8940331 -
        Flags: review?(cmanchester) → review+

    
    

    
  
Comment on attachment 8940330 [details]
Bug 1419581 - Part 4: Allow setting MOZ_ANDROID_GCM in mozconfig.

https://reviewboard.mozilla.org/r/210608/#review216862

> Why? MOZ_ANDROID_GCM and MOZ_NATIVE_DEVICES are both defined in this file.

IIRC, it's the keyfile definitions that interact.
Depends on: 1429188

    
    

    
  
(In reply to Nick Alexander :nalexander (less responsive until Jan 3, 2018) from comment #38)
> droeh: there's a resource dependency from the customtabs work to a PNG in
> mediarouter that shouldn't be there, specifically 

I filed this and put up a patch in bug 1429188.
Flags: needinfo?(droeh)

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
nechen: https://reviewboard.mozilla.org/r/210604/diff/#index_header needs re-review -- I ended up changing the approach significantly.  I think the commit message explains quite well.  Thanks!
Flags: needinfo?(cnevinchen)

    
    

    
  
Comment on attachment 8940328 [details]
Bug 1419581 - Part 1: Simplify MMA GCM sender IDs logic.

https://reviewboard.mozilla.org/r/210604/#review218700

looks good to me. Thanks!
Flags: needinfo?(cnevinchen)

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/45b5309a73cb
Pre: Add missing excludes when building without MOZ_NATIVE_DEVICES. r=nalexander
https://hg.mozilla.org/integration/autoland/rev/93547276fba8
Part 1: Simplify MMA GCM sender IDs logic. r=nechen
https://hg.mozilla.org/integration/autoland/rev/bc4cda1cc57c
Part 2: Guard media Fennec JNI primitives with MOZ_NATIVE_DEVICES. r=jchen
https://hg.mozilla.org/integration/autoland/rev/d0eba5853ab6
Part 3: Guard PresentationMediaPlayer windows with MOZ_NATIVE_DEVICES. r=jchen
https://hg.mozilla.org/integration/autoland/rev/8d0855cb17b7
Part 4: Allow setting MOZ_ANDROID_GCM in mozconfig. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/c9aef37de282
Part 5: Add --without-google-play-services. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/1af5a54e829d
Part 6: Add Android build configuration --without-google-play-services. r=dustin

    
    

    
  
I don't quite understand this; I had a green N try build very recently at

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

Maybe something moved underneath me.  I'll investigate.
Flags: needinfo?(nalexander)

    
    

    
  
This looks like it was always busted, and all of my try builds were artifact builds (perhaps)?  Here's a nice green full build try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=640f4a8360789676b59a2ffee1b72add294647b8

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
hg error in cmd: hg rebase -s 45b5309a73cbde68fab5b75168bbeaba076ff238 -d 02a7d3b84dcd: abort: source and destination form a cycle

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/65195aae48d8
Pre: Add missing excludes when building without MOZ_NATIVE_DEVICES. r=nalexander
https://hg.mozilla.org/integration/autoland/rev/8f1655752d43
Part 1: Simplify MMA GCM sender IDs logic. r=nechen
https://hg.mozilla.org/integration/autoland/rev/2ea82ff4a757
Part 2: Guard media Fennec JNI primitives with MOZ_NATIVE_DEVICES. r=jchen
https://hg.mozilla.org/integration/autoland/rev/a7d75667c58b
Part 3: Guard PresentationMediaPlayer windows with MOZ_NATIVE_DEVICES. r=jchen
https://hg.mozilla.org/integration/autoland/rev/44bcb609e721
Part 4: Allow setting MOZ_ANDROID_GCM in mozconfig. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/be888fa125dc
Part 5: Add --without-google-play-services. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/bd1e3857b5ba
Part 6: Add Android build configuration --without-google-play-services. r=dustin
Assignee: nobody → nalexander
Depends on: gradle-3.0
No longer depends on: gradle-3.0

    
    

    
  
Backed out along with bug 1411654 because of bug 1431140.

https://hg.mozilla.org/mozilla-central/rev/56b6d637e647
Status: RESOLVED → REOPENED

          status-firefox59:
          fixed → wontfix
Resolution: FIXED → ---
Target Milestone: Firefox 59 → ---
Status: REOPENED → ASSIGNED

        Attachment #8930711 -
        Attachment is obsolete: true

        Attachment #8935235 -
        Attachment is obsolete: true

        Attachment #8937580 -
        Attachment is obsolete: true

        Attachment #8937581 -
        Attachment is obsolete: true

        Attachment #8937582 -
        Attachment is obsolete: true

    
    

    
  
I landed this on top of https://bugzilla.mozilla.org/show_bug.cgi?id=1411654, but there's nothing that really depends on that ticket.  I have a try build with just these changes at

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

we'll see how it does.

Pierre, with "ac_add_options --without-google-play-services" in my mozconfig, this builds fine.  I don't know what your issue from IRC is about, namely:

nalexander: I tried it last week and ended up with this error: mozbuild.configure.options.InvalidOptionError: Environment variable name must be all uppercase

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5f7645a19bf1
Pre: Add missing excludes when building without MOZ_NATIVE_DEVICES. r=nalexander
https://hg.mozilla.org/integration/autoland/rev/f8b3e95f18e4
Part 1: Simplify MMA GCM sender IDs logic. r=nechen
https://hg.mozilla.org/integration/autoland/rev/072108d16590
Part 2: Guard media Fennec JNI primitives with MOZ_NATIVE_DEVICES. r=jchen
https://hg.mozilla.org/integration/autoland/rev/5224db0c36aa
Part 3: Guard PresentationMediaPlayer windows with MOZ_NATIVE_DEVICES. r=jchen
https://hg.mozilla.org/integration/autoland/rev/78828bf781d7
Part 4: Allow setting MOZ_ANDROID_GCM in mozconfig. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/d4d42899e5cd
Part 5: Add --without-google-play-services. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/7f41dd3bbc2d
Part 6: Add Android build configuration --without-google-play-services. r=dustin
Depends on: 1432862

    
    

    
  
calixte: can I get a backout for this?
Flags: needinfo?(cdenizet)

    
    

    
  
Sigh.  I missed that we invoke the JNI wrapper preprocessor differently in moz.build and --with-gradle; we're not passing the flags correctly (at all) around

https://searchfox.org/mozilla-central/source/mobile/android/gradle/with_gecko_binaries.gradle#172

    
    

    
  
Backed out 7 changesets (bug 1419581) as requested by nalexander:

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7f41dd3bbc2d25f937fa24a54612c25bc839f5ab

Backout: https://hg.mozilla.org/mozilla-central/rev/e22e2c9eb81686e958a9448ea3d1e8cd766743e2
Status: RESOLVED → REOPENED

          status-firefox60:
          fixed → ---
Flags: needinfo?(nalexander)
Resolution: FIXED → ---
Target Milestone: Firefox 60 → ---

    
    

    
  
(In reply to Nick Alexander :nalexander from comment #104)
> Sigh.  I missed that we invoke the JNI wrapper preprocessor differently in
> moz.build and --with-gradle; we're not passing the flags correctly (at all)
> around
> 
> https://searchfox.org/mozilla-central/source/mobile/android/gradle/
> with_gecko_binaries.gradle#172

Turns out this is not quite the reason things are broken: that code is _confusing_, but doesn't seem to be _incorrect_.

What I think happened is that I forgot to include MOZ_NATIVE_DEVICES in the local DEFINES in widget/android/**.  That is, MOZ_NATIVE_DEVICES is a subst but not a define, and that means it needs special handling.  I have before and after try builds out, and with luck that'll let me reland.

I don't think this change requires additional review.
Flags: needinfo?(nalexander)
Flags: needinfo?(cdenizet)

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
https://treeherder.mozilla.org/#/jobs?repo=try&author=nalexander@mozilla.com&selectedJob=158380683 and manual inspection suggests we're good.  Running Robocop tests, and then will reland.

    
    

    
  
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a83c011fcfc1
Pre: Add missing excludes when building without MOZ_NATIVE_DEVICES. r=nalexander
https://hg.mozilla.org/integration/autoland/rev/7632333fd2ed
Part 1: Simplify MMA GCM sender IDs logic. r=nechen
https://hg.mozilla.org/integration/autoland/rev/e93f17e93618
Part 2: Guard media Fennec JNI primitives with MOZ_NATIVE_DEVICES. r=jchen
https://hg.mozilla.org/integration/autoland/rev/aa77aae43318
Part 3: Guard PresentationMediaPlayer windows with MOZ_NATIVE_DEVICES. r=jchen
https://hg.mozilla.org/integration/autoland/rev/4cc18a6f3717
Part 4: Allow setting MOZ_ANDROID_GCM in mozconfig. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/c10704fa7133
Part 5: Add --without-google-play-services. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/3d3e27e3072a
Part 6: Add Android build configuration --without-google-play-services. r=dustin
Summary: Fennec can no longer be built without Google Play Services → Add --without-google-play-services to build Fennec without Google Play Services

    
  
Depends on: 1439459

    
  
Depends on: 1439464

    
  
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 60 → mozilla60

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: