Closed Bug 1197147 Opened 9 years ago Closed 9 years ago

Switch to Google Play Services 8.1.0 to support building with SDK 23 / Android 6.0

Categories

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

All
Android
defect
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: sebastian, Assigned: sebastian)

References

Details

Attachments

(2 files, 2 obsolete files)

The Google Play Services jar we build with contains classes that use deprecated and removed APIs.

Truncated log:
> 0:49.94 Warning: com.google.android.gms.analytics.internal.zzj: can't find referenced class org.apache.http.NameValuePair
> 0:49.94 Warning: com.google.android.gms.analytics.internal.zzj: can't find referenced class org.apache.http.NameValuePair
> 0:49.97 Warning: com.google.android.gms.common.GooglePlayServicesUtil: can't find referenced method 'void setLatestEventInfo(android.content.Context,java.lang.CharSequence,java.lang.CharSequence,android.app.PendingIntent)' in class android.app.Notification
> 0:50.10 Warning: com.google.android.gms.gcm.zza: can't find referenced method 'void setLatestEventInfo(android.content.Context,java.lang.CharSequence,java.lang.CharSequence,android.app.PendingIntent)' in class android.app.Notification
> 0:50.10 Warning: com.google.android.gms.internal.zzac: can't find referenced class android.net.http.AndroidHttpClient
> 0:50.10 Warning: com.google.android.gms.internal.zzac: can't find referenced class android.net.http.AndroidHttpClient
> 0:50.17 Warning: com.google.android.gms.internal.zzqt: can't find referenced class org.apache.http.HttpEntity
> 0:50.17 Warning: com.google.android.gms.internal.zzqt: can't find referenced class org.apache.http.HttpResponse
> 0:50.17 Warning: com.google.android.gms.internal.zzqt: can't find referenced class org.apache.http.StatusLine
> 0:50.17 Warning: com.google.android.gms.internal.zzqt: can't find referenced class org.apache.http.client.HttpClient
> 0:50.17 Warning: com.google.android.gms.internal.zzqt: can't find referenced class org.apache.http.client.methods.HttpGet

More logs here: https://pastebin.mozilla.org/8843080


Unfortunately this is partial related to our build system including the big all-inclusive library JAR instead of the more granular AARs. Some of these things are not even used by us and later stripped by ProGuard afaik (I see Google Analytics in there).

To make things worse right now there's no version of the library available that fixes these issues. It seems like we'll have to wait until Google ships an update for that.

For now this bug is mostly just blocking our "Build with Android M SDK" meta bug.
Post Bug 1108782, we're no longer using the monolith.  Is this ticket still valid?

In addition, we can roll forward our GPS versions as far as we like; and post Bug 1204260, we can also roll our target SDK and toolchain forward as far as we like.  Is there a configuration of those things that kills this off?
Depends on: 1108782, 1204260
Flags: needinfo?(s.kaspari)
I need to check. The last time I tried with a WIP patch from bug 1204260 all errors except one had disappeared:

> 0:49.97 Warning: com.google.android.gms.common.GooglePlayServicesUtil: can't find referenced method 'void setLatestEventInfo(android.content.Context,java.lang.CharSequence,java.lang.CharSequence,android.app.PendingIntent)' in class android.app.Notification

But I just saw that a new version of the Play Services library has been released so this might fix our issues. I just need to figure out where I set the SDK and dependency versions for building now. :-)
Flags: needinfo?(s.kaspari)
Assignee: nobody → s.kaspari
(In reply to Sebastian Kaspari (:sebastian) from comment #2)
> I just need to figure out where I set the SDK and dependency versions for building now. :-)

I'll test with the gradle build for now. That's easier. :)
Status: NEW → ASSIGNED
(In reply to Sebastian Kaspari (:sebastian) from comment #2)
> I need to check. The last time I tried with a WIP patch from bug 1204260 all
> errors except one had disappeared:
> 
> > 0:49.97 Warning: com.google.android.gms.common.GooglePlayServicesUtil: can't find referenced method 'void setLatestEventInfo(android.content.Context,java.lang.CharSequence,java.lang.CharSequence,android.app.PendingIntent)' in class android.app.Notification
> 
> But I just saw that a new version of the Play Services library has been
> released so this might fix our issues. I just need to figure out where I set
> the SDK and dependency versions for building now. :-)

They're... dispersed :)  See the list in Bug 1207680.
It seems like Google Play Services 8.1 solves the issue (at least I can proceed and run into other, previously hidden, build issues now.

However they now added a new "base" library called play-services-basement that we need to add to the build.

So we need:
* Use version 8.1 of the play-services libraries (base/cast)
* Add play-services-basement library
* In gradle just update cast dependency to 8.1 and it will automatically pick the right transitive dependencies
* Let the builders know
(In reply to Sebastian Kaspari (:sebastian) from comment #5)
> It seems like Google Play Services 8.1 solves the issue (at least I can
> proceed and run into other, previously hidden, build issues now.
> 
> However they now added a new "base" library called play-services-basement
> that we need to add to the build.
> 
> So we need:
> * Use version 8.1 of the play-services libraries (base/cast)
> * Add play-services-basement library

Such copy-paste :)

> * In gradle just update cast dependency to 8.1 and it will automatically
> pick the right transitive dependencies
> * Let the builders know

This is not so hard.  We use tooltool [1] for this; I'm going to request you get rights to tooltool now.  It's just a simple package upload and download thing.  You manually create the package you want, and then upload it.  Then you bump manifests [2] in mobile/android and push a try build, which picks up the new packages.

[1] https://wiki.mozilla.org/ReleaseEngineering/Applications/Tooltool#Tooltool_basics
[2] https://dxr.mozilla.org/mozilla-central/search?q=path%3Amobile%2Fandroid%2Fconfig%2F*manifest&redirect=false&case=false&limit=60&offset=0
Summary: Our Google Play Services version uses deprecated / removed APIs (API level 23) → Switch to Google Play Services 8.1.0 to support building with SDK 23 / Android 6.0
Attachment #8668430 - Attachment description: 1197147-gms8.1-gradle.patch → Gradle build: Use version 8.1 of Google Play Services
Attachment #8668430 - Flags: review?(nalexander)
Attachment #8668429 - Flags: review?(nalexander)
Splitted the changes into two patches. In theory we could land the gradle changes earlier (No build server changes needed).
Comment on attachment 8668430 [details] [diff] [review]
Gradle build: Use version 8.1 of Google Play Services

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

Nice.  I'd be pleased to see this land in advance, so we can get an early indicator of some classes of build failures from local developers.

::: mobile/android/gradle/base/build.gradle
@@ +75,5 @@
>      compile 'com.android.support:recyclerview-v7:22.2.1'
>  
>      if (mozconfig.substs.MOZ_NATIVE_DEVICES) {
>          compile 'com.android.support:mediarouter-v7:22.2.1'
> +        compile 'com.google.android.gms:play-services-cast:8.1.0'

Hmm.  Can you comment about the transitive deps?  This is a non-obvious distinction between the moz.build and Gradle approaches.
Attachment #8668430 - Flags: review?(nalexander) → review+
Comment on attachment 8668429 [details] [diff] [review]
Mach build: Use version 8.1 of Google Play Services

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

Patch looks good!  This patch is so much simpler than it would have been before AAR support!  Brings a smile to my eyes.

Is there an action needed to update |mach bootstrap|?  I imagine we need a very fresh version of the SDK and the Google Repository.  Can you comment on whether this is ready to land yet, or is blocked on other work?  (I expect it is blocked.)

::: build/autoconf/android.m4
@@ +295,5 @@
>  if test -n "$MOZ_NATIVE_DEVICES" ; then
>      AC_SUBST(MOZ_NATIVE_DEVICES)
>  
> +    MOZ_ANDROID_AAR(play-services-base, 8.1.0, google, com/google/android/gms)
> +    MOZ_ANDROID_AAR(play-services-basement, 8.1.0, google, com/google/android/gms)

Alphabetical versus dependency order?  Roll with this.
Attachment #8668429 - Flags: review?(nalexander) → review+
(In reply to Nick Alexander :nalexander from comment #11)
> Hmm.  Can you comment about the transitive deps?  This is a non-obvious
> distinction between the moz.build and Gradle approaches.

Yeah, if the artifact is pulled from a Maven repository then Gradle will read the pom.xml file and also download dependencies listed in there.

To avoid confusion it's probably better to list them in build.gradle too. With that in mind I'd prefer to re-add play-services-base and also add play-services-basement. Okay?


(In reply to Nick Alexander :nalexander from comment #12)
> Patch looks good!  This patch is so much simpler than it would have been
> before AAR support!  Brings a smile to my eyes.

Yeah, it's much simpler now! Especially when you know that you do not need to edit "configure". Wasted some time on this. :)


(In reply to Nick Alexander :nalexander from comment #12)
> Is there an action needed to update |mach bootstrap|?  I imagine we need a
> very fresh version of the SDK and the Google Repository.  Can you comment on
> whether this is ready to land yet, or is blocked on other work?  (I expect
> it is blocked.)

Oh good question. I assume bootstrap just downloads the m2 repositories and does not care about what's in there? But yeah, the builders as well as developers will have to update to have 8.1.0 in the local repository. That's all that is needed to land this. Infact I could land the gradle patch now because it does not affect the builders.


(In reply to Nick Alexander :nalexander from comment #12)
> > +    MOZ_ANDROID_AAR(play-services-base, 8.1.0, google, com/google/android/gms)
> > +    MOZ_ANDROID_AAR(play-services-basement, 8.1.0, google, com/google/android/gms)
> 
> Alphabetical versus dependency order?  Roll with this.

I changed this several times while editing the patch. I preferred dependency order but dependency graphs can be more complex to maintain than just a list. In this patch everything should be alphabetical now? But what's the preferred style?
Flags: needinfo?(nalexander)
Pushing to try (Let's see if we can update to 8.1 without updating the SDK):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4dac98e81dc4
Bug 1210755 / Bug 1197147 / Bug 1193206 - Update releng.manifest to use latest SDK and tools. r=trivial default tip 
https://hg.mozilla.org/integration/fx-team/rev/f3cb1dd809ff
https://hg.mozilla.org/mozilla-central/rev/736c3a039fce
https://hg.mozilla.org/mozilla-central/rev/e9f40a9c0154
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
This is done and dusted; clearing NI.
Flags: needinfo?(nalexander)
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 44 → mozilla44
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: