From 904fbb15c61b191c5724c2e132668118dcb073f5 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Wed, 18 Jan 2023 09:49:27 -0800 Subject: [PATCH] Bump CMake to 3.22.1 to properly honor CMAKE_BUILD_TYPE (#35857) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/35857 It seems like there is an incompatibility between NDK 23 (shipped in 0.71) and the usage of custom `CMAKE_BUILD_TYPE` we do for Hermes. Specifically the `-DCMAKE_BUILD_TYPE=Release` we specify for the debug variant of Hermes is partially ignored by the new Android native build toolchain. See https://github.com/android/ndk/issues/463 for mentions on how the toolchains requires CMake 3.20+ As AGP 7.3 defaults to use CMake 3.18 unless specified, and NDK 23 unless specified. AGP 7.4 defaults to use CMake 3.22 unless specified, and NDK 23 unless specified. See: https://developer.android.com/studio/releases/gradle-plugin#7-4-0 Here I'm: 1. Bumping the docker image to an image that contains the CMake 3.22 2. Updating the logic for building `react-native` & `hermes-engine` to use 3.22 3. Provide fallbacks if the user specified `CMAKE_VERSION` Template tests will run on AGP 7.3 and will still use CMake 3.18, but I forecast no problem there as the user is not supposed to specify custom `CMAKE_BUILD_TYPE`. This is only a problem as we build `hermes-engine` with custom build types. Changelog: [Android] [Fixed] - Bump CMake to 3.22.1 to properly honor CMAKE_BUILD_TYPE Reviewed By: cipolleschi Differential Revision: D42544864 fbshipit-source-id: efd0f51120370fb808337c201df31d71f4ddfdbc --- .circleci/Dockerfiles/Dockerfile.android | 2 +- .circleci/config.yml | 2 +- ReactAndroid/build.gradle | 15 +++++++++++---- ReactAndroid/hermes-engine/build.gradle | 11 ++++++----- package.json | 2 +- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/.circleci/Dockerfiles/Dockerfile.android b/.circleci/Dockerfiles/Dockerfile.android index a4bdb0cbd0d2c8..6b54b14815ca70 100644 --- a/.circleci/Dockerfiles/Dockerfile.android +++ b/.circleci/Dockerfiles/Dockerfile.android @@ -14,7 +14,7 @@ # and build a Android application that can be used to run the # tests specified in the scripts/ directory. # -FROM reactnativecommunity/react-native-android:6.1 +FROM reactnativecommunity/react-native-android:6.2 LABEL Description="React Native Android Test Image" LABEL maintainer="Héctor Ramos " diff --git a/.circleci/config.yml b/.circleci/config.yml index 70fdbd180f920c..16ec3ff957d756 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -115,7 +115,7 @@ executors: reactnativeandroid: <<: *defaults docker: - - image: reactnativecommunity/react-native-android:6.1 + - image: reactnativecommunity/react-native-android:6.2 resource_class: "xlarge" environment: - TERM: "dumb" diff --git a/ReactAndroid/build.gradle b/ReactAndroid/build.gradle index 1e7fdbb0af2dcc..efee2a517b6a9f 100644 --- a/ReactAndroid/build.gradle +++ b/ReactAndroid/build.gradle @@ -39,6 +39,16 @@ def downloadsDir = customDownloadsDir ? new File(customDownloadsDir) : new File( def thirdPartyNdkDir = new File("$buildDir/third-party-ndk") def reactNativeRootDir = projectDir.parent +// We put the publishing version from gradle.properties inside ext. so other +// subprojects can access it as well. +ext.publishing_version = VERSION_NAME + +// This is the version of CMake we're requesting to the Android SDK to use. +// If missing it will be downloaded automatically. Only CMake versions shipped with the +// Android SDK are supported (you can find them listed in the SDK Manager of Android Studio). +def cmakeVersion = System.getenv("CMAKE_VERSION") ?: "3.22.1" +ext.cmake_version = cmakeVersion + // You need to have following folders in this directory: // - boost_1_76_0 // - double-conversion-1.1.6 @@ -224,10 +234,6 @@ final def preparePrefab = tasks.register("preparePrefab", PreparePrefabHeadersTa it.outputDir.set(prefabHeadersDir) } -// We put the publishing version from gradle.properties inside ext. so other -// subprojects can access it as well. -ext.publishing_version = VERSION_NAME - task createNativeDepsDirectories { downloadsDir.mkdirs() thirdPartyNdkDir.mkdirs() @@ -487,6 +493,7 @@ android { externalNativeBuild { cmake { + version cmakeVersion path "src/main/jni/CMakeLists.txt" } } diff --git a/ReactAndroid/hermes-engine/build.gradle b/ReactAndroid/hermes-engine/build.gradle index 551c663775c3b5..7f9bd74ffb037e 100644 --- a/ReactAndroid/hermes-engine/build.gradle +++ b/ReactAndroid/hermes-engine/build.gradle @@ -16,18 +16,19 @@ plugins { group = "com.facebook.react" version = parent.publishing_version +def cmakeVersion = parent.cmake_version -def cmakeVersion = "3.18.1" /** * We use the bundled version of CMake in the Android SDK if available, to don't force Android * users to install CMake externally. */ def findCmakePath(cmakeVersion) { - if (System.getenv("ANDROID_SDK_ROOT")) { - return "${System.getenv("ANDROID_SDK_ROOT")}/cmake/${cmakeVersion}/bin/cmake" + def cmakeRelativePath = "/cmake/${cmakeVersion}/bin/cmake" + if (System.getenv("ANDROID_SDK_ROOT") && new File("${System.getenv("ANDROID_SDK_ROOT")}/${cmakeRelativePath}").exists()) { + return "${System.getenv("ANDROID_SDK_ROOT")}/${cmakeRelativePath}" } - if (System.getenv("ANDROID_HOME")) { - return "${System.getenv("ANDROID_HOME")}/cmake/${cmakeVersion}/bin/cmake" + if (System.getenv("ANDROID_HOME") && new File("${System.getenv("ANDROID_HOME")}/${cmakeRelativePath}").exists()) { + return "${System.getenv("ANDROID_HOME")}/${cmakeRelativePath}" } return "cmake" } diff --git a/package.json b/package.json index 724be94ef2b0fc..7800fb820a33a1 100644 --- a/package.json +++ b/package.json @@ -85,7 +85,7 @@ "prettier": "prettier --write \"./**/*.{js,md,yml,ts,tsx}\"", "format-check": "prettier --list-different \"./**/*.{js,md,yml,ts,tsx}\"", "update-lock": "npx yarn-deduplicate", - "docker-setup-android": "docker pull reactnativecommunity/react-native-android:6.1", + "docker-setup-android": "docker pull reactnativecommunity/react-native-android:6.2", "docker-build-android": "docker build -t reactnativeci/android -f .circleci/Dockerfiles/Dockerfile.android .", "test-android-run-instrumentation": "docker run --cap-add=SYS_ADMIN -it reactnativeci/android bash .circleci/Dockerfiles/scripts/run-android-docker-instrumentation-tests.sh", "test-android-run-unit": "docker run --cap-add=SYS_ADMIN -it reactnativeci/android bash .circleci/Dockerfiles/scripts/run-android-docker-unit-tests.sh",