-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Description
TL;DR: The CI bots have the Android sdk and ndk folders as siblings, and set ANDROID_SDK_ROOT and ANDROID_NDK_PATH. An env-var for the NDK is non-standard, and the standard location of the ndk directory is nested inside the sdk directory. We should update our DEPS and scripts to do the same.
Long version:
The Android SDK & NDK are installed through a CIPD package: https://chrome-infra-packages.appspot.com/p/flutter/android/sdk/all
The directory structure inside the package (for every OS) is the ndk and sdk directories as siblings.
Our infra sets the ANDROID_NDK_PATH env var https://cs.opensource.google/flutter/recipes/+/main:recipe_modules/flutter_deps/api.py;l=341, and presumably we use this env var somewhere in our build (but I have not been able to find uses in flutter repos including the recipes).
When the Android SDK & NDK are installed on a users' machine through Android Studio or the SDK manager, the sdk directory contains the ndk directory. (Older SDK versions had an ndk-bundle directory.)
The official Android documentation makes no mention of using an environment variable for the NDK location: https://developer.android.com/studio/command-line/variables.html#envar
An online search reveals that project use conflicting env vars for the NDK: ANDROID_NDK_PATH, ANDROID_NDK_HOME, and ANDROID_NDK_ROOT. The latter seems the most common. But none of them is mentioned in the official Android documentation on recommendations.
#135148 adds logic for NDK discovery on top of the SDK discovery. Since there is no well defined env variable and the ndk is always nested as ndk/ inside sdk/ (old versions of the Android SDK can contain ndk-bundle instead of ndk), and we'd like to use an NDK belonging to the SDK we're using, we look for a nested ndk directory inside sdk.
This fails on our CI bots currently, because that ndk/ directory is not there. Our script for making the CIPD package explicitly moves the ndk directory out of the sdk directory:
# Special treatment for NDK to move to expected directory.
mv $upload_dir/sdk/ndk $upload_dir/ndk-bundle
ndk_sub_paths=`find $upload_dir/ndk-bundle -maxdepth 1 -type d`
ndk_sub_paths_arr=($ndk_sub_paths)
mv ${ndk_sub_paths_arr[1]} $upload_dir/ndk
rm -rf $upload_dir/ndk-bundleChanging the CIPD package to not modify the directory structure makes the tests on the above mentioned PR pass (log). (log of failing tests)
Changing the directory structure will likely require multiple changes:
- The recipes to set up
ANDROID_NDK_PATHcorrectly again (link). - The script to make the CIPD package (link).
- Stuff that pops up when trying to make this change.
Many thanks to @godofredoc for helping me triage this! 🙇
@reidbaker Do you agree we should depend the ndk directory being inside the sdk directory in the discovery in lib/src/android/android_sdk.dart in #135148?
@GaryQian You have modified the mentioned lines in create_cipd_packages.sh. Would you happen to know why we have a non-standard directory structure? And any concerns about the above proposal?