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.

Sign up for GitHub

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

Jump to bottom

android: compileSdkVersion to 34 (Android 14) #5879

Merged
merged 1 commit into from
Sep 20, 2024
Merged

android: compileSdkVersion to 34 (Android 14) #5879

merged 1 commit into from
Sep 20, 2024

Conversation

chrisbobbe
Copy link
Contributor

As we said on the last one of these, fc2dab7:

This setting is not to be confused with the targetSdkVersion. The
latter goes into the built manifest, and affects a wide range of
behavior, so bumping it requires careful testing. This is used
purely at build time, and should have no effect on runtime
behavior.

This value needs to be at least as high as targetSdkVersion, and we need to bump that to 34 soon (#5877). So, do this to prepare for that.

No build failures or warnings appeared for me with
tools/test native --all-files
.

Related: #5877

@chrisbobbe chrisbobbe requested a review from gnprice July 10, 2024 22:07
@gnprice
Copy link
Member

gnprice commented Jul 10, 2024

No build failures or warnings appeared for me with
tools/test native --all-files

So unfortunately we have a large existing base of Android build warnings, mostly (though not entirely) caused by our dependencies. tools/test native therefore suppresses them:

    tools/gradle -q :app:assembleDebug :app:assembleDebugUnitTest \
        :app:bundleDebug || return

    # The `-q` suppresses noise from warnings about obsolete build config
    # in our dependencies from the React Native ecosystem.
    # But it also suppresses the names of tests that failed.
    # So on failure, rerun without it.
    tools/gradle -q :app:testDebugUnitTest \
        || tools/gradle :app:testDebugUnitTest

(This is a situation I hope to avoid getting into with zulip-flutter — zulip/zulip-flutter#796 causes CI to enforce that there are zero Kotlin compiler warnings, at least, and hopefully we can maintain that.)

So to see the warnings, use tools/gradle :app:assembleDebug with no -q.

You may have to add --rerun-tasks to cause Gradle to rebuild things it's previously built, and therefore report warnings there again.

I think probably the right way to check for warnings here is:

  • Run the build at main; save the output to a file like tools/gradle … >baseline.log 2>&1.
  • Run the build after the change, similarly saving it.
  • Use git diff --no-index to look for changes.
    • And hope that the order the log lines appear in is consistent. If it isn't and the diff therefore has a lot of noise… try running sort on each file and comparing the results. If they match, then great; where they don't match, it might be a bit annoying to back out what's going on because neighboring lines will have been scattered by the sort, but at least it gives a comprehensive list of things that need figuring out.

@gnprice gnprice mentioned this pull request Aug 10, 2024
Closed
@chrisbobbe

This comment was marked as outdated.

Sign in to view
@chrisbobbe
Copy link
Contributor Author

Oh wait, I overlooked this part:

You may have to add --rerun-tasks to cause Gradle to rebuild things it's previously built, and therefore report warnings there again.

Trying now.

@chrisbobbe
Copy link
Contributor Author

chrisbobbe commented Sep 19, 2024 •
edited
Loading

OK, cleared out those files and did this again, this time with --rerun-tasks.

Here are the results of:

tools/gradle :app:assembleDebug --rerun-tasks >baseline.log 2>&1
baseline.log

tools/gradle :app:assembleDebug --rerun-tasks >bumped.log 2>&1
(after bumping compileSdkVersion from 33 to 34)
bumped.log

Here's one thing that stands out to me right away, at the top of the git diff --no-index baseline.log bumped.log:

+WARNING:We recommend using a newer Android Gradle plugin to use compileSdk = 34
+
+This Android Gradle plugin (7.2.2) was tested up to compileSdk = 33
+
+This warning can be suppressed by adding
+    android.suppressUnsupportedCompileSdk=34
+to this project's gradle.properties
+
+The build will continue, but you are strongly encouraged to update your project to
+use a newer Android Gradle Plugin that has been tested with compileSdk = 34

After that, the VSCode diff viewer is showing lots of lines, mostly with the shaded background that says the lines were moved. Since it did look like the order of the lines changed a fair amount, here are those files with their lines sorted:

sort baseline.log > baseline-sorted.log
baseline-sorted.log

sort bumped.log > bumped-sorted.log
bumped-sorted.log

And:

`git diff --no-index baseline-sorted.log bumped-sorted.log` 
diff --git baseline-sorted.log bumped-sorted.log
index d48e16913..f7b6256cb 100644
--- baseline-sorted.log
+++ bumped-sorted.log
@@ -75,13 +75,13 @@
 
 
 
-
 
 
        provider#expo.modules.filesystem.FileSystemFileProvider@android:authorities was tagged at AndroidManifest.xml:23 to replace other declarations but no other declaration present
        provider#expo.modules.filesystem.FileSystemFileProvider@android:authorities was tagged at AndroidManifest.xml:8 to replace other declarations but no other declaration present
        uses-permission#android.permission.CAMERA was tagged at AndroidManifest.xml:18 to remove other declarations but no other declaration present
        uses-permission#android.permission.READ_PHONE_STATE was tagged at AndroidManifest.xml:9 to remove other declarations but no other declaration present
+    android.suppressUnsupportedCompileSdk=34
   - expo-application (4.1.0)
   - expo-constants (13.1.1)
   - expo-error-recovery (3.1.0)
@@ -942,19 +942,18 @@
 > Task :sentry_react-native:processDebugJavaRes NO-SOURCE
 > Task :sentry_react-native:processDebugManifest
 > Task :sentry_react-native:writeDebugAarMetadata
-BUILD SUCCESSFUL in 18s
+BUILD SUCCESSFUL in 17s
 Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
 Execution optimizations have been disabled for 1 invalid unit(s) of work during this build to ensure correctness.
 Execution optimizations have been disabled for task ':expo-constants:createDebugExpoConfig' to ensure correctness due to the following reasons:
+Note: /Users/chrisbobbe/dev/zulip-mobile/android/app/src/debug/java/com/zulipmobile/ReactNativeFlipper.java uses or overrides a deprecated API.
 Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/@react-native-async-storage/async-storage/android/src/main/java/com/reactnativecommunity/asyncstorage/AsyncStorageModule.java uses or overrides a deprecated API.
 Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/@react-native-async-storage/async-storage/android/src/main/java/com/reactnativecommunity/asyncstorage/AsyncStoragePackage.java uses unchecked or unsafe operations.
 Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/@react-native-camera-roll/camera-roll/android/src/main/java/com/reactnativecommunity/cameraroll/CameraRollModule.java uses or overrides a deprecated API.
-Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/@react-native-clipboard/clipboard/android/src/main/java/com/reactnativecommunity/clipboard/ClipboardModule.java uses or overrides a deprecated API.
 Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/@react-native-clipboard/clipboard/android/src/main/java/com/reactnativecommunity/clipboard/ClipboardPackage.java uses unchecked or unsafe operations.
 Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/@sentry/react-native/android/src/main/java/io/sentry/react/RNSentryOnDrawReporterManager.java uses unchecked or unsafe operations.
-Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-web-browser/android/src/main/java/expo/modules/webbrowser/InternalCustomTabsActivitiesHelper.java uses or overrides a deprecated API.
 Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/react-native-document-picker/android/src/main/java/com/reactnativedocumentpicker/DocumentPickerModule.java uses or overrides a deprecated API.
-Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/react-native-image-picker/android/src/main/java/com/imagepicker/Utils.java uses or overrides a deprecated API.
+Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/react-native-webview/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java uses or overrides a deprecated API.
 Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/react-native-webview/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java uses unchecked or unsafe operations.
 Note: Recompile with -Xlint:deprecation for details.
 Note: Recompile with -Xlint:deprecation for details.
@@ -967,8 +966,6 @@ Note: Recompile with -Xlint:deprecation for details.
 Note: Recompile with -Xlint:deprecation for details.
 Note: Recompile with -Xlint:deprecation for details.
 Note: Recompile with -Xlint:deprecation for details.
-Note: Recompile with -Xlint:deprecation for details.
-Note: Recompile with -Xlint:deprecation for details.
 Note: Recompile with -Xlint:unchecked for details.
 Note: Recompile with -Xlint:unchecked for details.
 Note: Recompile with -Xlint:unchecked for details.
@@ -981,11 +978,13 @@ Note: Some input files use or override a deprecated API.
 Note: Some input files use or override a deprecated API.
 Note: Some input files use or override a deprecated API.
 Note: Some input files use or override a deprecated API.
-Note: Some input files use or override a deprecated API.
 Note: Some input files use unchecked or unsafe operations.
 Note: Some input files use unchecked or unsafe operations.
 Please consult deprecation warnings for more details.
 See https://docs.gradle.org/7.3.3/userguide/command_line_interface.html#sec:command_line_warnings
+The build will continue, but you are strongly encouraged to update your project to
+This Android Gradle plugin (7.2.2) was tested up to compileSdk = 33
+This warning can be suppressed by adding
 Unable to strip the following libraries, packaging them as they are: libbutter.so, libc++_shared.so, libevent-2.1.so, libevent_core-2.1.so, libevent_extra-2.1.so, libfabricjni.so, libfb.so, libfbjni.so, libflipper.so, libfolly_futures.so, libfolly_json.so, libgifimage.so, libglog.so, libglog_init.so, libimagepipeline.so, libjsc.so, libjscexecutor.so, libjsi.so, libjsijniprofiler.so, libjsinspector.so, liblogger.so, libmapbufferjni.so, libnative-filters.so, libnative-imagetranscoder.so, libreact_codegen_rncore.so, libreact_config.so, libreact_debug.so, libreact_nativemodule_core.so, libreact_render_animations.so, libreact_render_attributedstring.so, libreact_render_componentregistry.so, libreact_render_core.so, libreact_render_debug.so, libreact_render_graphics.so, libreact_render_imagemanager.so, libreact_render_leakchecker.so, libreact_render_mapbuffer.so, libreact_render_mounting.so, libreact_render_runtimescheduler.so, libreact_render_scheduler.so, libreact_render_telemetry.so, libreact_render_templateprocessor.so, libreact_render_textlayoutmanager.so, libreact_render_uimanager.so, libreact_utils.so, libreactnativeblob.so, libreactnativejni.so, libreactperfloggerjni.so, libreanimated.so, librrc_image.so, librrc_root.so, librrc_text.so, librrc_unimplementedview.so, librrc_view.so, libruntimeexecutor.so, libturbomodulejsijni.so, libyoga.so.
 Using expo modules
 WARNING:Software Components will not be created automatically for Maven publishing from Android Gradle Plugin 8.0. To opt-in to the future behavior, set the Gradle property android.disableAutomaticComponentCreation=true in the `gradle.properties` file or use the new publishing DSL.
@@ -1000,17 +999,16 @@ WARNING:Software Components will not be created automatically for Maven publishi
 WARNING:Software Components will not be created automatically for Maven publishing from Android Gradle Plugin 8.0. To opt-in to the future behavior, set the Gradle property android.disableAutomaticComponentCreation=true in the `gradle.properties` file or use the new publishing DSL.
 WARNING:Software Components will not be created automatically for Maven publishing from Android Gradle Plugin 8.0. To opt-in to the future behavior, set the Gradle property android.disableAutomaticComponentCreation=true in the `gradle.properties` file or use the new publishing DSL.
 WARNING:Software Components will not be created automatically for Maven publishing from Android Gradle Plugin 8.0. To opt-in to the future behavior, set the Gradle property android.disableAutomaticComponentCreation=true in the `gradle.properties` file or use the new publishing DSL.
+WARNING:We recommend using a newer Android Gradle plugin to use compileSdk = 34
 Warning: The 'kotlin-android-extensions' Gradle plugin is deprecated. Please use this migration guide (https://goo.gle/kotlin-android-extensions-deprecation) to start working with View Binding (https://developer.android.com/topic/libraries/view-binding) and the 'kotlin-parcelize' plugin.
 You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
+to this project's gradle.properties
+use a newer Android Gradle Plugin that has been tested with compileSdk = 34
 w: /Users/chrisbobbe/dev/zulip-mobile/android/app/src/main/java/com/zulipmobile/ReactExtensions.kt: (38, 14): 'hasActiveCatalystInstance(): Boolean' is deprecated. Deprecated in Java
 w: /Users/chrisbobbe/dev/zulip-mobile/android/app/src/main/java/com/zulipmobile/sharing/SharingHelper.kt: (78, 34): 'getParcelableExtra(String!): T?' is deprecated. Deprecated in Java
 w: /Users/chrisbobbe/dev/zulip-mobile/android/app/src/main/java/com/zulipmobile/sharing/SharingHelper.kt: (84, 35): 'getParcelableArrayListExtra(String!): ArrayList<T!>?' is deprecated. Deprecated in Java
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-application/android/src/main/java/expo/modules/application/ApplicationModule.kt: (129, 14): 'versionCode: Int' is deprecated. Deprecated in Java
-w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-application/android/src/main/java/expo/modules/application/ApplicationModule.kt: (52, 34): 'getPackageInfo(String, Int): PackageInfo!' is deprecated. Deprecated in Java
-w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-application/android/src/main/java/expo/modules/application/ApplicationModule.kt: (70, 33): 'getPackageInfo(String, Int): PackageInfo!' is deprecated. Deprecated in Java
-w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-application/android/src/main/java/expo/modules/application/ApplicationModule.kt: (83, 33): 'getPackageInfo(String, Int): PackageInfo!' is deprecated. Deprecated in Java
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-constants/android/src/main/java/expo/modules/constants/ConstantsService.kt: (132, 17): 'versionCode: Int' is deprecated. Deprecated in Java
-w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-constants/android/src/main/java/expo/modules/constants/ConstantsService.kt: (58, 42): 'getPackageInfo(String, Int): PackageInfo!' is deprecated. Deprecated in Java
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt: (1028, 43): Unchecked cast: Any? to Map<String, Any>?
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt: (1040, 9): 'execute(vararg FileSystemModule.DownloadResumableTaskParams?): AsyncTask<FileSystemModule.DownloadResumab
leTaskParams?, Void?, Void?>!' is deprecated. Deprecated in Java
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt: (1101, 47): 'AsyncTask<Params : Any!, Progress : Any!, Result : Any!>' is deprecated. Deprecated in Java
@@ -1029,9 +1027,9 @@ w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-file-system/android/src/
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt: (8, 19): 'AsyncTask<Params : Any!, Progress : Any!, Result : Any!>' is deprecated. Deprecated in Java
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt: (927, 49): Unchecked cast: Any? to Map<String, Any>?
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-font/android/src/main/java/expo/modules/font/FontLoaderModule.kt: (55, 38): Type mismatch: inferred type is String? but String was expected
-w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-mail-composer/android/src/main/java/expo/modules/mailcomposer/MailComposerModule.kt: (41, 46): 'queryIntentActivities(Intent, Int): (Mutable)List<ResolveInfo!>' is deprecated. Deprecated in Java
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-mail-composer/android/src/main/java/expo/modules/mailcomposer/MailIntentBuilder.kt: (52, 14): 'fromHtml(String!): Spanned!' is deprecated. Deprecated in Java
-w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-modules-core/android/src/main/java/expo/modules/adapters/react/permissions/PermissionsService.kt: (157, 30): 'getPackageInfo(String, Int): PackageInfo!' is deprecated. Deprecated in Java
+w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-modules-core/android/src/main/java/expo/modules/kotlin/ExpoModulesHelper.kt: (11, 21): 'newInstance(): T!' is deprecated. Deprecated in Java
+w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-modules-core/android/src/main/java/expo/modules/kotlin/ModuleRegistry.kt: (33, 25): 'newInstance(): T!' is deprecated. Deprecated in Java
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-modules-core/android/src/main/java/expo/modules/kotlin/defaultmodules/ErrorManagerModule.kt: (12, 5): 'name(String): Unit' is deprecated. The 'name' component was renamed to 'Name'.
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-modules-core/android/src/main/java/expo/modules/kotlin/defaultmodules/ErrorManagerModule.kt: (13, 5): 'events(vararg String): Unit' is deprecated. The 'events' component was renamed to 'Events'.
 w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-modules-core/android/src/main/java/expo/modules/kotlin/events/KModuleEventEmitterWrapper.kt: (9, 44): 'RCTEventEmitter' is deprecated. Deprecated in Java

@chrisbobbe
Copy link
Contributor Author

chrisbobbe commented Sep 19, 2024 •
edited
Loading

The 10-line warning about the AGP version is scattered through the diff of the sorted log files. But ignoring that, and the line about the build taking 17s or 18s, there are only four green + lines:

+Note: /Users/chrisbobbe/dev/zulip-mobile/android/app/src/debug/java/com/zulipmobile/ReactNativeFlipper.java uses or overrides a deprecated API.
+Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/react-native-webview/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java uses or overrides a deprecated API.
+w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-modules-core/android/src/main/java/expo/modules/kotlin/ExpoModulesHelper.kt: (11, 21): 'newInstance(): T!' is deprecated. Deprecated in Java
+w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-modules-core/android/src/main/java/expo/modules/kotlin/ModuleRegistry.kt: (33, 25): 'newInstance(): T!' is deprecated. Deprecated in Java

All of those start with Note: or w:. I think it's also possible that some of these were popping up before, just being identified unspecifically, with something like Note: Some input files use or override a deprecated API..

So I think we're fine to go ahead and merge this?

@gnprice
Copy link
Member

gnprice commented Sep 20, 2024

Great, thanks for doing that comparison!

I agree those all seem fine. Here's those four new lines formatted a bit differently so they wrap:

Note: /Users/chrisbobbe/dev/zulip-mobile/android/app/src/debug/java/com/zulipmobile/ReactNativeFlipper.java uses or overrides a deprecated API.
Note: /Users/chrisbobbe/dev/zulip-mobile/node_modules/react-native-webview/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java uses or overrides a deprecated API.
w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-modules-core/android/src/main/java/expo/modules/kotlin/ExpoModulesHelper.kt: (11, 21): 'newInstance(): T!' is deprecated. Deprecated in Java
w: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-modules-core/android/src/main/java/expo/modules/kotlin/ModuleRegistry.kt: (33, 25): 'newInstance(): T!' is deprecated. Deprecated in Java

So all deprecation warnings, and similar in character to lots of existing warnings.

It might be a good idea to upgrade AGP as that other new warning suggests. That doesn't need to block this, though. I'll go ahead and merge.

@chrisbobbe @gnprice
android: compileSdkVersion to 34 (Android 14)
04a8646 
As we said on the last one of these, fc2dab7:

  This setting is not to be confused with the targetSdkVersion. The
  latter goes into the built manifest, and affects a wide range of
  behavior, so bumping it requires careful testing. This is used
  purely at build time, and should have no effect on runtime
  behavior.

This value needs to be at least as high as targetSdkVersion, and we
need to bump that to 34 soon (zulip#5877). So, do this to prepare for
that.

The build warnings change slightly, but the new warnings all seem
acceptable.  Details and discussion starting at:
  zulip#5879 (comment)

Related: zulip#5877
@gnprice gnprice force-pushed the pr-compile-sdk-version-34 branch from bf08fef to 04a8646 Compare September 20, 2024 01:22
@gnprice gnprice merged commit 04a8646 into zulip:main Sep 20, 2024
1 check passed
@gnprice
Copy link
Member

gnprice commented Sep 20, 2024

Merging, with this update to the commit message:

-    No build failures or warnings appeared for me with
-      tools/test native --all-files
-    .
+    The build warnings change slightly, but the new warnings all seem
+    acceptable.  Details and discussion starting at:
+      https://github.com/zulip/zulip-mobile/pull/5879#issuecomment-2362286748

@chrisbobbe chrisbobbe deleted the pr-compile-sdk-version-34 branch September 20, 2024 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnprice gnprice Awaiting requested review from gnprice

Assignees
No one assigned
Labels
a-Android P1 high-priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chrisbobbe @gnprice