Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update android dependencies [ci full] #4345

Merged
merged 1 commit into from
Jul 26, 2021
Merged

Conversation

rfk
Copy link
Contributor

@rfk rfk commented Jul 20, 2021

This uses the new, cleaner factoring of our Android config from #4194
as an excuse to update some of our Android dependencies to bring them
inline with what's used in current android-components. It was much
easier than it has been in the past!

I'm getting some weird test failures locally that look transient, let's
see what CI has to say about them...

@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2021

Codecov Report

Merging #4345 (36693c6) into main (cf35f6c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4345   +/-   ##
=======================================
  Coverage   75.67%   75.67%           
=======================================
  Files          46       46           
  Lines        4140     4140           
=======================================
  Hits         3133     3133           
  Misses       1007     1007           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf35f6c...36693c6. Read the comment docs.

@@ -49,20 +54,21 @@ buildscript {

dependencies {
classpath "com.android.tools.build:gradle:$android_gradle_plugin_version"
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I'm not sure why things were working without this on our classpath, maybe we were just always using the "system" version of kotlin?

@@ -138,6 +144,7 @@ if (useDownloadedLibs) {
src {
switch (DefaultPlatform.RESOURCE_PREFIX) {
case 'darwin':
case 'darwin-x86-64':
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of the platform has changed in the latest release of JNA, for reasons to do with the M1 arm64 silicon stuff.

@@ -1,7 +1,6 @@
apply plugin: 'com.android.library'
apply plugin: 'org.mozilla.rust-android-gradle.rust-android'
apply plugin: 'kotlin-android'
apply plugin: 'kotlin-android-extensions'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This plugin reports being deprecated, and removing it didn't seem to break anything, so...

@rfk
Copy link
Contributor Author

rfk commented Jul 20, 2021

I'm getting some weird test failures locally that look transient, let's see what CI has to say about them...

Well, at least I got a different set of errors in TaskCluster...


[task 2021-07-20T05:39:57.106Z] Could not determine the dependencies of task ':full-megazord:publishAarPublicationToProjectBuildDirRepository'.
[task 2021-07-20T05:39:57.106Z] > java.lang.NullPointerException (no error message)

@rfk rfk force-pushed the update-android-deps-20210720 branch 2 times, most recently from da37f43 to 6fbd35c Compare July 20, 2021 07:03
@rfk
Copy link
Contributor Author

rfk commented Jul 20, 2021

This is now failing on some Nimbus error from the updated Kotlin version:

[task 2021-07-20T06:55:10.626Z] > Task :nimbus:compileReleaseKotlin FAILED
[task 2021-07-20T06:55:10.626Z] e: /builds/worker/checkouts/src/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/FeatureVariables.kt: (129, 34): Type checking has run into a recursive problem. Easiest workaround: specify types of your declarations explicitly

The line in question is:

    override fun getText(key: String) = getString(key)?.let(this::asText)

Which I don't understand enough to know how to add type declarations to it. @jhugman any chance you can help with that one?

android_components_version = '75.0.0'
kotlin_version = '1.5.20'
kotlin_coroutines_version = '1.5.0'
jna_version = '5.8.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, this version of JNA seems to fix the struct-padding bug from mozilla/uniffi-rs#334 \o/

@rfk rfk force-pushed the update-android-deps-20210720 branch 3 times, most recently from 71b334d to 2716f2b Compare July 22, 2021 02:32
@rfk
Copy link
Contributor Author

rfk commented Jul 22, 2021

Which I don't understand enough to know how to add type declarations to it. @jhugman any chance you can help with that one?

I figured it out, and also made some adjustments for deprecated Glean APIs.

RethrowCaughtException:
active: false
SwallowedException:
active: false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were newly triggering some detekt failures. I've disabled the ones that are disabled in a-c, and fixed the others.

NimbusEvents.enrollmentKeys.experiment to event.experimentSlug,
NimbusEvents.enrollmentKeys.branch to event.branchSlug,
NimbusEvents.enrollmentKeys.enrollmentId to event.enrollmentId
NimbusEvents.enrollment.record(NimbusEvents.EnrollmentExtra(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous Glean recording API is deprecated, and there's a new more strongly-typed one documented here.

@rfk rfk force-pushed the update-android-deps-20210720 branch from 2716f2b to 8e92c6c Compare July 22, 2021 04:22
@rfk
Copy link
Contributor Author

rfk commented Jul 22, 2021

OK, fingers crossed for CI, but I think this is ready to go! I've tested out the local development flow and confirmed that a Fenix build using these updates still builds and runs and syncs stuff successfully.

@travis79, could you please r? this for the Nimbus/Glean deprecation changes?

@rfk rfk marked this pull request as ready for review July 22, 2021 04:27
@rfk rfk requested review from travis79 and a team July 22, 2021 04:27
@@ -126,7 +126,7 @@ interface VariablesWithContext : Variables {
// defaults from manifest information.
fun asText(res: Int) = context.getString(res)
fun asDrawable(res: Int) = context.getDrawable(res)
fun asText(string: String) = asStringResource(string)?.let(this::asText) ?: string
fun asText(string: String): String? = asStringResource(string)?.let(this::asText) ?: string
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the magic sauce necessary to avoid a recursion error in Kotlin's type resolution.

@rfk rfk force-pushed the update-android-deps-20210720 branch from 8e92c6c to a4bd5bf Compare July 22, 2021 04:30
Copy link
Member

@travis79 travis79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Hopefully migrating to the new Glean event API wasn't too bad.

This uses the new, cleaner factoring of our Android config from #4194
as an excuse to update some of our Android dependencies to bring them
inline with what's used in current android-components. It was much
easier than it has been in the past!
@rfk rfk force-pushed the update-android-deps-20210720 branch from a4bd5bf to 36693c6 Compare July 26, 2021 00:47
@rfk
Copy link
Contributor Author

rfk commented Jul 26, 2021

I'm going to go ahead and merge this once CI goes green, since I want to Glean updates for completeness over in #4356

@rfk rfk merged commit c40fd3a into main Jul 26, 2021
@rfk rfk deleted the update-android-deps-20210720 branch July 26, 2021 01:09
lougeniaC64 pushed a commit that referenced this pull request Aug 23, 2021
This uses the new, cleaner factoring of our Android config from #4194
as an excuse to update some of our Android dependencies to bring them
inline with what's used in current android-components. It was much
easier than it has been in the past!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants