Closed Bug 1247047 Opened 10 years ago Closed 10 years ago

Upgrade Google Play Services dependencies from 8.1.0 to 8.4.0

Categories

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

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

Tracking

(firefox49 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla49
Tracking Flags:
Tracking Status
firefox49 --- fixed

People

(Reporter: nalexander, Assigned: sebastian)

References

Details

Attachments

(4 files, 6 obsolete files)

MozReview Request: Bug 1247047 - Update build tools to 23.0.3 and Google Play Services to 8.4.0. r?ahunt
58 bytes, text/x-review-board-request
ahunt
: review+
Details
MozReview Request: Bug 1247047 - Add play-services-measurement, required by play-services-gcm 8.4.0. r?ahunt
58 bytes, text/x-review-board-request
ahunt
: review+
Details
MozReview Request: Bug 1247047 - Remove play-services-{analytics,appindexing}, no longer required by play-services-ads 8.4.0. r?ahunt
58 bytes, text/x-review-board-request
ahunt
: review+
Details
MozReview Request: Bug 1247047 - Update releng manifests to use new SDK. r?ahunt
58 bytes, text/x-review-board-request
ahunt
: review+
Details
In Bug 1233238, we're about to require play-services-{ads,analytics,appindexing} version 8.1.0. This is *way* more than we'd like, and it turns out that we can include just play-services-analytics if we move to 8.4.0. For the record, I computed the set of transitive dependencies manually, by inspecting ~/.mozbuild/android-sdk-macosx/extras/google/m2repository/com/google/android/gms/play-services-*/$VERSION/*.pom manually, and the POM files change from 8.1.0 to 8.4.0.
It's not clear how much APK we would save, but I expect it's a lot!
This does a few things. First, it makes non-official builds use the Adjust sandbox. Second, I observe that the fake sandbox key no longer sends anything, so it's no longer valuable; this patch instead requires an Adjust token if install tracking is enabled, since we can't provide a default any more. Third, it removes a spurious default in configure.in; without this default, builders can easily enable Adjust locally using the following in their mozconfig: ac_add_options --with-adjust-sdk-keyfile=/path/to/adjust-sdk.keyfile export MOZ_INSTALL_TRACKING=1 With the default, the "export" had no impact, because it was overwritten immediately. Review commit: https://reviewboard.mozilla.org/r/34891/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/34891/
Attachment #8719095 - Flags: review?(michael.l.comella)
Folks, this isn't quite done, but we might as well hammer out some bits early. Play Services 8.4.0 requires a new string -- the Google Application ID -- be exposed. That impacts the existing GCM work and the overall design; I'll be working out the details here shortly.
Comment on attachment 8719096 [details] MozReview Request: Bug 1247047 - Part 1: Update to build-tools 23.0.2, support libraries 23.1.1, Google Play Services 8.4.0. r?sebastian https://reviewboard.mozilla.org/r/34893/#review31509
Attachment #8719096 - Flags: review?(s.kaspari) → review+
Attachment #8719097 - Flags: review?(s.kaspari) → review+
Comment on attachment 8719097 [details] MozReview Request: Bug 1247047 - Part 2: Add play-services-measurement, required by play-services-gcm 8.4.0. r?sebastian https://reviewboard.mozilla.org/r/34895/#review31511
Comment on attachment 8719098 [details] MozReview Request: Bug 1247047 - Part 3: Remove play-services-{analytics,appindexing}, no longer required by play-services-ads 8.4.0. r?sebastian https://reviewboard.mozilla.org/r/34897/#review31513
Attachment #8719098 - Flags: review?(s.kaspari) → review+
Comment on attachment 8719094 [details] MozReview Request: Bug 1247047 - Pre: Extract Android dependency versions. r?glandium https://reviewboard.mozilla.org/r/34889/#review31585 ::: build/autoconf/android.m4:333 (Diff revision 1) > +google_play_services_version=8.1.0 > +android_support_version=23.0.1 you could do define(google_play_services_version, 8.1.0) define(android_support_version, 23.0.1) And they reference those without a $, and that would expand when running autoconf instead of being expanded by the shell when running configure.
Attachment #8719094 - Flags: review?(mh+mozilla) → review+
Assignee: nobody → nalexander
Comment on attachment 8719095 [details] MozReview Request: Bug 1247047 - Pre: Make it easier to test Adjust locally. r?mcomella https://reviewboard.mozilla.org/r/34891/#review31799 This seems reasonable to me. ::: configure.in:4918 (Diff revision 1) > + AC_MSG_ERROR([Must specify --with-adjust-sdk-keyfile when MOZ_INSTALL_TRACKING is defined!]) Can you update the documentation to specify what should be in the file specified by `--with-adjust-sdk-keyfile`? Perhaps in https://bugzilla.mozilla.org/show_bug.cgi?id=1245942 ::: mobile/android/base/java/org/mozilla/gecko/adjust/AdjustHelper.java:29 (Diff revision 1) > - AdjustConfig config = new AdjustConfig(context, appToken, environment); > + if (maybeAppToken == null) { Maybe `TextUtils.isEmpty`?
Attachment #8719095 - Flags: review?(michael.l.comella) → review+
(In reply to Nick Alexander :nalexander from comment #7) > Folks, this isn't quite done, but we might as well hammer out some bits > early. Play Services 8.4.0 requires a new string -- the Google Application > ID -- be exposed. That impacts the existing GCM work and the overall > design; I'll be working out the details here shortly. sebastian: this isn't so far off complete, and it'll be a nice win. You've done some updating of library deps; perhaps you can take a stab at pushing this one across the line? For the application ID, we expose it at https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/moz.build#944 so you should be able to use it in the manifest. Take a look at https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/strings.xml.in#26 and https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/locales/Makefile.in#67 for how to plumb a DEFINE into the strings.xml.in file directly.
(In reply to Nick Alexander :nalexander from comment #13) > (In reply to Nick Alexander :nalexander from comment #7) > > Folks, this isn't quite done, but we might as well hammer out some bits > > early. Play Services 8.4.0 requires a new string -- the Google Application > > ID -- be exposed. That impacts the existing GCM work and the overall > > design; I'll be working out the details here shortly. > > sebastian: this isn't so far off complete, and it'll be a nice win. You've > done some updating of library deps; perhaps you can take a stab at pushing > this one across the line? > > For the application ID, we expose it at > https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/moz. > build#944 so you should be able to use it in the manifest. > > Take a look at > https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/strings. > xml.in#26 and > https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/locales/ > Makefile.in#67 for how to plumb a DEFINE into the strings.xml.in file > directly. sebastian: see comment above -- forgot NI.
Flags: needinfo?(s.kaspari)
Okay, I'll push it across the line. Seems like the String is the last missing piece?
Assignee: nalexander → s.kaspari
Status: NEW → ASSIGNED
Flags: needinfo?(s.kaspari)
Depends on: 1274955
Depends on: 1274956
Attachment #8755392 - Flags: review?(mh+mozilla)
Attachment #8755392 - Attachment is obsolete: true
Attachment #8719095 - Attachment is obsolete: true
Attachment #8719094 - Attachment is obsolete: true
Attachment #8719098 - Attachment is obsolete: true
Attachment #8719097 - Attachment is obsolete: true
Attachment #8719096 - Attachment is obsolete: true
Attachment #8756396 - Flags: review?(ahunt) → review+
Comment on attachment 8756396 [details] MozReview Request: Bug 1247047 - Add play-services-measurement, required by play-services-gcm 8.4.0. r?ahunt https://reviewboard.mozilla.org/r/55130/#review51772
Comment on attachment 8756397 [details] MozReview Request: Bug 1247047 - Remove play-services-{analytics,appindexing}, no longer required by play-services-ads 8.4.0. r?ahunt https://reviewboard.mozilla.org/r/55132/#review51776
Attachment #8756397 - Flags: review?(ahunt) → review+
Comment on attachment 8756398 [details] MozReview Request: Bug 1247047 - Update releng manifests to use new SDK. r?ahunt https://reviewboard.mozilla.org/r/55134/#review51784 I don't understand why we're upgrading to build tools 23.0.2 here, whereas we're upgrading to 23.0.3 in the first commit where the build system is upgraded. Other than that this looks sane, but I'm not an expert on what these manifests do! ::: mobile/android/config/tooltool-manifests/android/releng.manifest:15 (Diff revision 1) > }, > { > "versions": [ > "Android SDK 6.0 / API 23", > "Android tools r24.4", > - "Android build tools 23.0.1", > + "Android build tools 23.0.2", Should this be 23.0.3 (not 23.0.2)? (Similarly for the rest of this commit - we seem to be replacing 23.0.1 with 23.0.2, but the first commit upgrades us to build tools 23.0.3)
Attachment #8756398 - Flags: review?(ahunt) → review+
Attachment #8756395 - Flags: review?(ahunt) → review+
Comment on attachment 8756395 [details] MozReview Request: Bug 1247047 - Update build tools to 23.0.3 and Google Play Services to 8.4.0. r?ahunt https://reviewboard.mozilla.org/r/55128/#review51786
(In reply to Andrzej Hunt :ahunt from comment #28) > Should this be 23.0.3 (not 23.0.2)? > > (Similarly for the rest of this commit - we seem to be replacing 23.0.1 with > 23.0.2, but the first commit upgrades us to build tools 23.0.3) Oh, yeah, this should be 23.0.3. This patch series started with an update to 23.0.2 but then 23.0.3 came along. :)
https://hg.mozilla.org/integration/fx-team/rev/0183f1bb4655f70a64262749b9991ba7ef0cda8b Bug 1247047 - Update build tools to 23.0.3 and Google Play Services to 8.4.0. r=ahunt https://hg.mozilla.org/integration/fx-team/rev/a38fb5ad3684b3700de19ad0c90f18ff112c03cf Bug 1247047 - Add play-services-measurement, required by play-services-gcm 8.4.0. r=ahunt https://hg.mozilla.org/integration/fx-team/rev/fac238070399620b704015d8a212693971c3a6cd Bug 1247047 - Remove play-services-{analytics,appindexing}, no longer required by play-services-ads 8.4.0. r=ahunt https://hg.mozilla.org/integration/fx-team/rev/bbd294728ecc2f4e5e838764e589220e7c288f92 Bug 1247047 - Update releng manifests to use new SDK. r=ahunt
(In reply to Sebastian H. [:aryx][:archaeopteryx] from comment #32) > Push with failure: > https://treeherder.mozilla.org/#/jobs?repo=fx- > team&revision=bbd294728ecc2f4e5e838764e589220e7c288f92 > Failure log: > https://treeherder.mozilla.org/logviewer.html#?job_id=9534920&repo=fx-team > > java.io.IOException: Please correct the above warnings first. > gmake[5]: *** [.proguard.deps] Error 1 > gmake[5]: *** Deleting file `.proguard.deps' > gmake[4]: *** [mobile/android/base/libs] Error 2 This task was successful in my try build. And all other Android builds were successful for this push. Weird.
(In reply to Sebastian Kaspari (:sebastian) from comment #33) > (In reply to Sebastian H. [:aryx][:archaeopteryx] from comment #32) > > Push with failure: > > https://treeherder.mozilla.org/#/jobs?repo=fx- > > team&revision=bbd294728ecc2f4e5e838764e589220e7c288f92 > > Failure log: > > https://treeherder.mozilla.org/logviewer.html#?job_id=9534920&repo=fx-team > > > > java.io.IOException: Please correct the above warnings first. > > gmake[5]: *** [.proguard.deps] Error 1 > > gmake[5]: *** Deleting file `.proguard.deps' > > gmake[4]: *** [mobile/android/base/libs] Error 2 > > This task was successful in my try build. And all other Android builds were > successful for this push. Weird. Clobber needed? It's possible that old .class files conflict with newer .class files.
Yeah, looks like it. A retrigger was sucessful too. I'll reland + clobber.
Flags: needinfo?(s.kaspari)
Do we need to update bootstrap to grab 23.0.3 too? (I'm unfamiliar with how bootstrap works, or if these patches affect bootstrap)
(In reply to Andrzej Hunt :ahunt from comment #36) > Do we need to update bootstrap to grab 23.0.3 too? (I'm unfamiliar with how > bootstrap works, or if these patches affect bootstrap) That's in the first patch in python/mozboot/mozboot/android.py.
https://hg.mozilla.org/integration/fx-team/rev/42f95268a3210bc372c55cdc94fdae081544aa1a Bug 1247047 - Update build tools to 23.0.3 and Google Play Services to 8.4.0. r=ahunt https://hg.mozilla.org/integration/fx-team/rev/77700ecdd9ebd10fbe93a093aeb9ce564e4655de Bug 1247047 - Add play-services-measurement, required by play-services-gcm 8.4.0. r=ahunt https://hg.mozilla.org/integration/fx-team/rev/745dee40e3398c3d73e55dd69d86e20570222ea6 Bug 1247047 - Remove play-services-{analytics,appindexing}, no longer required by play-services-ads 8.4.0. r=ahunt https://hg.mozilla.org/integration/fx-team/rev/b63d9b6ae62ef6ed140d5a286c2acd6f14ecb06c Bug 1247047 - Update releng manifests to use new SDK. r=ahunt https://hg.mozilla.org/integration/fx-team/rev/be56cb9803a91540033261d76ade0e1f06c4691c Bug 1247047 - Touch CLOBBER file. r=me
I was expecting this to reduce the APK size but it grew (very minimally though). Meh.
(In reply to Andrzej Hunt :ahunt from comment #36) > Do we need to update bootstrap to grab 23.0.3 too? (I'm unfamiliar with how > bootstrap works, or if these patches affect bootstrap) This is tricky because build-tools either run the gui or to do from a command-line it ends up being 2 steps. You need to first do: ~/.mozbuild/android-sdk-linux/tools/android list sdk --all and fine the id number of the thing you want to install and then do: ~/.mozbuild/android-sdk-linux/tools/android update sdk -u -a -t <the-number-you-determined-above>
(In reply to Bill Gianopoulos [:WG9s] from comment #41) > (In reply to Andrzej Hunt :ahunt from comment #36) > > Do we need to update bootstrap to grab 23.0.3 too? (I'm unfamiliar with how > > bootstrap works, or if these patches affect bootstrap) > > This is tricky because build-tools either run the gui or to do from a I was trying to say build-tools is not updated by default by an sdk update.
I think that something may have gotten messed up between the committing of this bug and Bug 1276043. That bug moved the definition for ANDROID_SUPPORT_LIBRARY_VERSION and ANDROID_GOOGLE_PLAY_SERVICES_VERSION out of build/autoconf/android.m4, and into mobile/android/moz.configure. However, the current (in central) mobile/android/moz.configure has ANDROID_SUPPORT_LIBRARY_VERSION="23.0.1" - *NOT* 23.0.3 like was changed here. I'm guessing that Bug 1276043 was done while this one was in progress, so the actual version bump didn't get merged correctly between the two issues? (The ANDROID_GOOGLE_PLAY_SERVICES_VERSION appears to be set correctly to 8.4.0 - it's just the library version that isn't right)
Ignore my confusion. I thought it was the build tools version - not the support library...now I'm reading the variable name correctly... :/
Looks like the build tool 23.0.3 is no longer available on google download site. STR: 1. mach bootstrap 2. Select '4. Firefox for Android' 3. Showing error log as below: Error: Ignoring unknown package filter 'build-tools-23.0.3' As a result, I run into build error as well: 0:18.45 DEBUG: configure: error: You must install the Android build-tools version 23.0.3. Try |mach bootstrap|. (Looked for /Users/astley/.mozbuild/android-sdk-macosx/build-tools/23.0.3) 0:18.45 ERROR: old-configure failed Should we upgrade the build tool version and update MDN doc?
Flags: needinfo?(s.kaspari)
(In reply to Astley Chen [:astley] (UTC+8) from comment #45) > Looks like the build tool 23.0.3 is no longer available on google download > site. It's only our bootstrap script that doesn't get it right (I think there's a bug about that). You should be able to install 23.0.3 manually though (via Android Studio - or using the 'android' tool from the SDK folder).
Flags: needinfo?(s.kaspari)
(In reply to Sebastian Kaspari (:sebastian) from comment #46) > It's only our bootstrap script that doesn't get it right (I think there's a > bug about that). You should be able to install 23.0.3 manually though (via > Android Studio - or using the 'android' tool from the SDK folder). Well, it turns out a problem when migrating homebrew. After I resolved this issue and succeed installing JDK, the build tool 23.0.3 can be found and installed now. Thanks for prompt feedback and help. Message when running 'mach bootstrap': ==> brew cask install caskroom/cask/brew-cask Warning: The default Caskroom location has moved to /usr/local/Caskroom. Please migrate your Casks to the new location and delete /opt/homebrew-cask/Caskroom, or if you would like to keep your Caskroom at /opt/homebrew-cask/Caskroom, add the following to your HOMEBREW_CASK_OPTS: --caskroom=/opt/homebrew-cask/Caskroom For more details on each of those options, see https://github.com/caskroom/homebrew-cask/issues/21913. Error: Tap caskroom/cask already tapped. Follow the instructions here: https://github.com/caskroom/homebrew-cask/blob/master/doc/reporting_bugs/pre_bug_report.md If this doesn’t fix the problem, please report this bug: https://github.com/caskroom/homebrew-cask#reporting-bugs /usr/local/Homebrew/Library/Homebrew/tap.rb:196:in `install' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/source/untapped_qualified.rb:9:in `me?' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/source.rb:28:in `block in for_query' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/source.rb:26:in `each' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/source.rb:26:in `find' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/source.rb:26:in `for_query' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc.rb:58:in `load' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli/install.rb:21:in `block in install_casks' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli/install.rb:19:in `each' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli/install.rb:19:in `install_casks' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli/install.rb:10:in `run' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli.rb:111:in `run_command' /usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli.rb:154:in `process' /usr/local/Homebrew/Library/Homebrew/cask/cmd/brew-cask.rb:8:in `<top (required)>' /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require' /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require' /usr/local/Homebrew/Library/Homebrew/brew.rb:24:in `require?' /usr/local/Homebrew/Library/Homebrew/brew.rb:100:in `<main>' Warning: The default Caskroom location has moved to /usr/local/Caskroom. Please migrate your Casks to the new location and delete /opt/homebrew-cask/Caskroom, or if you would like to keep your Caskroom at /opt/homebrew-cask/Caskroom, add the following to your HOMEBREW_CASK_OPTS: --caskroom=/opt/homebrew-cask/Caskroom For more details on each of those options, see https://github.com/caskroom/homebrew-cask/issues/21913.
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 49 → mozilla49
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: