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

[all packages][Android] Migrate to Android SDK 31 and Java 11 #16941

Merged

Conversation

bbarthec
Copy link
Contributor

@bbarthec bbarthec commented Apr 6, 2022

Why

Continues Android upgrading process started in #16752 (comment)
Resolves ENG-2835

How

  • Propagated migration to compileSdkVersion 31 and targetSdkPackage 31 to every package (safeExtGet fallbacks upgrades)
  • Migrated to ndkVersion syntax for specifying Android NDK version in expo-av and expoview (fixed numerous warnings) Reference to AGP docs
  • Migrated to Java 11 by specifying the android.compileOptions.sourceCompatibility, android.compileOptions.targetCompatibility and android.kotlinOptions.jvmTarget Reference to AGP docs
  • Ensured the Expo Go Versioned compiles and run (had to backport config changes to versioned expoview packages)
  • Cleaned up a bit (deduplicated some code and unified syntax across all packages)

Test Plan

  • Compiled and run Android Expo Go (Versioned Flavour) on physical phone
  • Compiled and run Android bare-expo on physical phone

Checklist

  • Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md).
  • This diff will work correctly for expo build (eg: updated @expo/xdl).
  • This diff will work correctly for expo prebuild & EAS Build (eg: updated a module plugin).

Side notes

  • Had to remove Java 8 from the system completely as Android Studio was trying to pick old Java SDK by default
  • Had to freshly install gradle, because of Android Studio and Java upgrade (pay extra attention what is the gradle main directory because Android Studio is trying to use - I recommend default ~/.gradle)

@linear
Copy link

linear bot commented Apr 6, 2022

ENG-2835 Update Android library compileSdkVersion to 31

Hey do you know if next expo sdk will support android sdk 31? Currently having some issue with a lib that only supports 31+

actually I think its easier than I thought. I thought it would require updating gradle@7+ but seems like this is only needed when changing targetSdk (and not compileSdk)

- Janic

@bbarthec bbarthec requested a review from Kudo April 6, 2022 10:00
@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Apr 6, 2022
Copy link
Contributor

@Kudo Kudo left a comment

Choose a reason for hiding this comment

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

thanks for this update! there's just some nit from duplicated or missing changes.

do you have the detailed warnings from the AGP missing of ndkVersion?
i noticed this change from react-native upstream and it looks like they are using the default ndk version responding to the AGP version. that's why i removed the ndkVersion property from 0.68 upgrade 😅

packages/expo-face-detector/android/build.gradle Outdated Show resolved Hide resolved
packages/expo-screen-capture/android/build.gradle Outdated Show resolved Hide resolved
packages/expo-structured-headers/android/build.gradle Outdated Show resolved Hide resolved
packages/expo-updates-interface/android/build.gradle Outdated Show resolved Hide resolved
packages/expo/android/build.gradle Outdated Show resolved Hide resolved
- Propagate migration to `compileSdkVersion 31` and `targetSdkPackage 31` to every package (`safeExtGet` fallback)
- Migrated to `ndkVersion` syntax for specifying Android NDK version in `expo-av` and `expoview` (fixed numerous warnings)
- Migrated to Java 11
- Applied the chagnes for every package that required them (including versioned Expoview packages)
- Cleanedup the style/syntax in a few packages
- Deduplicated gradle code in expoview
@bbarthec bbarthec force-pushed the bbarthec/eng-2835-update-android-library-compilesdkversion branch from ce3ba9e to 9cdc304 Compare April 6, 2022 12:08
@bbarthec bbarthec marked this pull request as ready for review April 6, 2022 12:08
@bbarthec
Copy link
Contributor Author

bbarthec commented Apr 6, 2022

do you have the detailed warnings from the AGP missing of ndkVersion?
i noticed this change from react-native upstream and it looks like they are using the default ndk version responding to the AGP version. that's why i removed the ndkVersion property from 0.68 upgrade 😅

@Kudo, I was flooded with warning messages about NDK being specified in non canonical and deprecated way and there was a suggestion to migrate towards android.ndkVersion 👀
As you're suggesting we can get rid of this altogether, so I'll remove it 💪

@bbarthec bbarthec force-pushed the bbarthec/eng-2835-update-android-library-compilesdkversion branch from 94115e3 to e16283e Compare April 6, 2022 12:18
@expo-bot
Copy link
Collaborator

expo-bot commented Apr 6, 2022

Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines.

I've found some issues in your pull request that should be addressed (click on them for more details) 👇

⚠️ Suggestion: Missing changelog entries


Your changes should be noted in the changelog. Read Updating Changelogs guide and consider (it's optional) adding an appropriate entry to the following changelogs:


Generated by ExpoBot 🤖 against e16283e

Copy link
Contributor

@Kudo Kudo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@lukmccall lukmccall left a comment

Choose a reason for hiding this comment

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

LGTM ✅
Just one question:
Are you going to update the java version on the CI? or setting android.kotlinOptions.jvmTarget is enough?

@barthap
Copy link
Contributor

barthap commented Apr 6, 2022

IIRC Java on the CI is already bumped by the RN 0.68 upgrade PR

@Kudo
Copy link
Contributor

Kudo commented Apr 6, 2022

oh no, one thing i missed. eas.json supports to specify ndk version which may through the ANDROID_NDK_HOME variable. i think it is unsupported from AGP 7. should we backport the environment support back to our bare template or use difference approach to specify the ndk version on eas. cc @bbarthec @wkozyra95

@brentvatne
Copy link
Member

brentvatne commented Apr 6, 2022

oh no, one thing i missed. eas.json supports to specify ndk version which may through the ANDROID_NDK_HOME variable. i think it is unsupported from AGP 7. should we backport the environment support back to our bare template or use difference approach to specify the ndk version on eas. cc @bbarthec @wkozyra95

correct - we use ANDROID_NDK_HOME on eas build. I made a note to discuss this at the next sync meeting, but feel free to continue to discuss possible solutions

@wkozyra95
Copy link
Contributor

@Kudo You can already specify ndk version in eas.json https://docs.expo.dev/build-reference/eas-json/#ndk , If I understand correctly if ANDROID_NDK_HOME is deprecated it will be just ignored, so android build process should use the correct ndk version if it's installed. Note that it would still be user's responsibility to install that NDK, plus it increases the build time.

For classic builds, we can just add additional ndk for new SDKs and keep ANDROID_NDK_HOME pointed to sth for compatibility with older sdks

@Kudo
Copy link
Contributor

Kudo commented Apr 7, 2022

@wkozyra95 yep, that's why i am pointing this out whether we should deprecate the eas.json ndk support or backport the ANDROID_NDK_HOME support by passing it to the ndkPath property in our template. let it be discussed in eas build meeting. 😁

@bbarthec
Copy link
Contributor Author

bbarthec commented Apr 8, 2022

I'm merging it right now and if something would be decided about eas.json#ndk support we'll address it in a separate PR.

@bbarthec
Copy link
Contributor Author

bbarthec commented Apr 8, 2022

Almost all but one CI failures are unrelate to this PR.
The failure that is related narrows down to:

Could not target platform: 'Java SE 11' using tool chain: 'JDK 8 (1.8)'.

I'll look into it right now 👀

@bbarthec bbarthec merged commit 7f2f417 into main Apr 8, 2022
@bbarthec bbarthec deleted the bbarthec/eng-2835-update-android-library-compilesdkversion branch April 8, 2022 10:41
Kudo added a commit that referenced this pull request Apr 10, 2022
# Why

fix updates-e2e ci error after #16941 landed. the error job is like this one: https://github.com/expo/expo/runs/5883741338?check_suite_focus=true.

the root cause is that updates-e2e uses sdk-44 template which is far away from latest main, e.g. AGP version, `compileSdkVersion` and `targetSdkVersion`.

# How

use latest expo-template-bare-minimal for expo prebuild

# Test Plan

updates-e2e ci green

Co-authored-by: Brent Vatne <brentvatne@gmail.com>
elbywan added a commit to LedgerHQ/ledger-live that referenced this pull request May 9, 2022
3.2.0 seems incompatible with our android sdk target
see: expo/expo#16941
elbywan added a commit to LedgerHQ/ledger-live that referenced this pull request May 10, 2022
3.2.0 seems incompatible with our android sdk target
see: expo/expo#16941
elbywan added a commit to LedgerHQ/ledger-live that referenced this pull request May 12, 2022
3.2.0 seems incompatible with our android sdk target
see: expo/expo#16941
elbywan added a commit to LedgerHQ/ledger-live that referenced this pull request May 13, 2022
3.2.0 seems incompatible with our android sdk target
see: expo/expo#16941
elbywan added a commit to LedgerHQ/ledger-live that referenced this pull request May 15, 2022
3.2.0 seems incompatible with our android sdk target
see: expo/expo#16941
elbywan added a commit to LedgerHQ/ledger-live that referenced this pull request May 16, 2022
3.2.0 seems incompatible with our android sdk target
see: expo/expo#16941
valpinkman pushed a commit to LedgerHQ/ledger-live that referenced this pull request May 17, 2022
3.2.0 seems incompatible with our android sdk target
see: expo/expo#16941
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: suggestions ExpoBot has some suggestions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants