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 OSs] Android ndk-bundle along with old NDK versions will be deprecated on July, 24 #5879

Closed
10 tasks done
miketimofeev opened this issue Jul 8, 2022 · 11 comments
Closed
10 tasks done

Comments

@miketimofeev
Copy link
Contributor

miketimofeev commented Jul 8, 2022

Breaking changes

  • Symlink from LTS NDK to ndk-bundle won't be created anymore.
  • NDK version from 15 to 18 will be removed from macOS-10.15 image.

Target date

The propagation will start on July, 24 and take 2-3 days.

The motivation for the changes

  • ndk-bundle was explicitly marked as "Obsolete" by Google and no one should be using it anymore. The current symlink creation process breaks the Android SDK package manager. More details can be found here side-by-side NDK installed as the "(obsolete)" NDK #2689 (comment)
  • Old ndk versions 15-18 are not needed by xamarin-android anymore and can be deleted.

Possible impact

If your builds depend on the ndk-bundle symlink they can be broken.

Platforms affected

  • Azure DevOps
  • GitHub Actions

Virtual environments affected

  • Ubuntu 18.04
  • Ubuntu 20.04
  • Ubuntu 22.04
  • macOS 10.15
  • macOS 11
  • macOS 12
  • Windows Server 2019
  • Windows Server 2022

Mitigation ways

The symlink can be created using the following snippets:

  • macOS:
function get_full_ndk_version {
    majorVersion=$1
    ndkVersion=$(${SDKMANAGER} --list | grep "ndk;${majorVersion}.*" | awk '{gsub("ndk;", ""); print $1}' | sort -V | tail -n1)
    echo "$ndkVersion"
}
ANDROID_HOME=$HOME/Library/Android/sdk
SDKMANAGER=$ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager
ndkDefault=$(get_full_ndk_version "23")
ln -s $ANDROID_HOME/ndk/$ndkDefault $ANDROID_HOME/ndk-bundle
  • Ubuntu:
function get_full_ndk_version {
    majorVersion=$1
    ndkFullVersion=$($SDKMANAGER --list | grep "ndk;${majorVersion}.*" | awk '{gsub("ndk;", ""); print $1}' | sort -V | tail -n1)
    echo "$ndkFullVersion"
}
ANDROID_ROOT=/usr/local/lib/android
ANDROID_SDK_ROOT=${ANDROID_ROOT}/sdk
ANDROID_NDK_ROOT=${ANDROID_SDK_ROOT}/ndk-bundle
SDKMANAGER=${ANDROID_SDK_ROOT}/cmdline-tools/latest/bin/sdkmanager
ndkDefaultFullVersion=$(get_full_ndk_version "23")
ln -sf $ANDROID_SDK_ROOT/ndk/$ndkDefaultFullVersion $ANDROID_NDK_ROOT
  • Windows:
$sdkRoot = "C:\Android\android-sdk"
$sdkManager = "$sdkRoot\cmdline-tools\latest\bin\sdkmanager.bat"
$androidPackages = Get-AndroidPackages -AndroidSDKManagerPath $sdkManager
$ndkDefaultVersion = ($androidPackages | Where-Object { $_ -match "ndk;23" }).Split(';')[1]
$ndkRoot = "$sdkRoot\ndk-bundle"
New-Item -Path $ndkRoot -ItemType SymbolicLink -Value "$sdkRoot\ndk\$ndkDefaultVersion"
setx ANDROID_NDK_HOME $ndkRoot /M
setx ANDROID_NDK_PATH $ndkRoot /M
setx ANDROID_NDK_ROOT $ndkRoot /M
(Get-Content -Encoding UTF8 "${ndkRoot}\ndk-build.cmd").replace('%~dp0\build\ndk-build.cmd','"%~dp0\build\ndk-build.cmd"')|Set-Content -Encoding UTF8 "${ndkRoot}\ndk-build.cmd"
@MarijnS95
Copy link

MarijnS95 commented Jul 26, 2022

@miketimofeev Repeating what I asked in #2689 (comment), this issue seems to be about removing the ndk-bundle symlink (which was breaking sdkmanager), but couldn't you have kept something along the lines of:

export ANDROID_NDK_ROOT=$ANDROID_SDK_ROOT/ndk/$ndkDefaultFullVersion

Without creating a symlink at all, so that most software requiring just the "default" NDK (latest LTS, afaiu) keeps working?

I have a couple repositories failing because of this, and I think you could've stored the result of the suggested workaround in a much-more-easily accessible ANDROID_NDK_DEFAULT_HOME=$ANDROID_SDK_ROOT/ndk/$ndkDefaultFullVersion otherwise? For now I'm addressing this by just pointing my CI's to the latest:

    - name: Setup NDK path
      shell: bash
      run: echo "ANDROID_NDK_ROOT=$ANDROID_NDK_LATEST_HOME" >> $GITHUB_ENV

@miketimofeev
Copy link
Contributor Author

Hey @MarijnS95! Sorry for the inconvenience but I assume the variable is related only to ndk_bundle, which is obsolete now, isn't it?

@MarijnS95
Copy link

MarijnS95 commented Jul 26, 2022

@miketimofeev I've always understood ANDROID_NDK_HOME and ANDROID_NDK_ROOT to point to the NDK the user wants to use, regardless whether that's ndk-bundle or another version downloaded/extracted elsewhere. That's also how our Rust tools (cargo-apk, cargo-ndk, etc) treat these variables.

EDIT: See also #2426 originally making it available:

Add ANDROID_NDK_HOME and ANDROID_NDK_ROOT to the all images. According to discussion: ANDROID_NDK_HOME and ANDROID_NDK_ROOT are widely used by many other softwares. In most cases, If ANDROID_NDK_HOME is not defined, the value in ANDROID_NDK_ROOT is used. but some software only use ANDROID_NDK_HOME, others only use ANDROID_NDK_ROOT. so, both are preset is the best.

@miketimofeev
Copy link
Contributor Author

@MarijnS95 thanks, is it any documentation regarding setting those variables? I can't find anything here https://developer.android.com/studio/command-line/variables for some reason.

@MarijnS95
Copy link

@miketimofeev I wouldn't expect anything in the SDK something, but it should have been in the NDK-specific build-system docs where it turns out not to be... https://android.googlesource.com/platform/ndk/+/master/docs/BuildSystemMaintainers.md

At least https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html#cross-compiling-for-android:~:text=Else%2C%20if%20an%20environment%20variable%20ANDROID_NDK_ROOT%20or%20ANDROID_NDK%20is%20set%2C%20it%20will%20be%20used%20as%20the%20value%20of%20CMAKE_ANDROID_NDK%2C%20and%20the%20NDK%20will%20be%20used mentions it, and there are probably other places where we can find it being used. Android Studio manages it internally so it's really only for external tools that wish to utilize the NDK in a consistent/configurable manner.

@miketimofeev
Copy link
Contributor Author

@saurik could you provide your thoughts regarding the env variables too?

@MarijnS95
Copy link

@miketimofeev Thanks! I've meanwhile been asking around for explicit docs on this for the NDK, given that it only exists for the SDK. Hopefully that'll help clarify even further once they arrive :)

@ddobranic ddobranic unpinned this issue Jul 28, 2022
mikes-lunarg added a commit to mikes-lunarg/Vulkan-Tools that referenced this issue Jul 28, 2022
The ANDROID_NDK_HOME env var seems to have disappeared with the
deprecation of ndk-bundle:
actions/runner-images#5879
mikes-lunarg added a commit to mikes-lunarg/Vulkan-Tools that referenced this issue Jul 28, 2022
The ANDROID_NDK_HOME env var seems to have disappeared with the
deprecation of ndk-bundle:
actions/runner-images#5879
mikes-lunarg added a commit to mikes-lunarg/Vulkan-Tools that referenced this issue Jul 28, 2022
The ANDROID_NDK_HOME env var seems to have disappeared with the
deprecation of ndk-bundle:
actions/runner-images#5879
mikes-lunarg added a commit to mikes-lunarg/Vulkan-Tools that referenced this issue Jul 28, 2022
The ANDROID_NDK_HOME env var seems to have disappeared with the
deprecation of ndk-bundle:
actions/runner-images#5879
mikes-lunarg added a commit to KhronosGroup/Vulkan-Tools that referenced this issue Jul 28, 2022
The ANDROID_NDK_HOME env var seems to have disappeared with the
deprecation of ndk-bundle:
actions/runner-images#5879
gudzpoz added a commit to gudzpoz/luajava that referenced this issue Jul 29, 2022
@miketimofeev
Copy link
Contributor Author

@ThadHouse @MarijnS95 we have added three variables that point to the latest LTS NDK (25 at the moment) to all the images:
'ANDROID_NDK', 'ANDROID_NDK_HOME', 'ANDROID_NDK_ROOT'
Sorry for the inconvenience and please let me know if there is anything else left.
Thank you!

MarijnS95 added a commit to MarijnS95/winit that referenced this issue Sep 6, 2022
This reverts commit 4895a29.

GitHub Actions' runner-images readded this environment variable on my
request [1] as it wasn't strictly related to the deprecated and removed
`ndk-bundle` NDK release.  Back out of the workaround to keep CI scripts
tidy.

[1]: actions/runner-images#5879 (comment)
kchibisov pushed a commit to rust-windowing/winit that referenced this issue Sep 6, 2022
This reverts commit 4895a29.

GitHub Actions' runner-images readded this environment variable on my
request [1] as it wasn't strictly related to the deprecated and removed
`ndk-bundle` NDK release.  Back out of the workaround to keep CI scripts
tidy.

[1]: actions/runner-images#5879 (comment)
kchibisov pushed a commit to kchibisov/winit that referenced this issue Sep 9, 2022
This reverts commit 4895a29.

GitHub Actions' runner-images readded this environment variable on my
request [1] as it wasn't strictly related to the deprecated and removed
`ndk-bundle` NDK release.  Back out of the workaround to keep CI scripts
tidy.

[1]: actions/runner-images#5879 (comment)
kchibisov pushed a commit to rust-windowing/winit that referenced this issue Sep 11, 2022
This reverts commit 4895a29.

GitHub Actions' runner-images readded this environment variable on my
request [1] as it wasn't strictly related to the deprecated and removed
`ndk-bundle` NDK release.  Back out of the workaround to keep CI scripts
tidy.

[1]: actions/runner-images#5879 (comment)
vondele pushed a commit to vondele/Stockfish that referenced this issue Dec 8, 2022
Fix for the GitHub upgrade:
actions/runner-images#5879
that broke our ARM workflows because it changed the value of
the ANDROID_NDK_HOME variable referenced in our PATH.

closes official-stockfish#4267

No functional change
@saurik
Copy link

saurik commented Feb 4, 2023

@saurik could you provide your thoughts regarding the env variables too?

I took a glorious half-year break from paying attention to my GitHub inbox, but a friend of mine came across this threads and told me a few days ago that I'd been asked a question about it--which I didn't reply to--and then "they did something random" ;P.

FWIW, what you did is exactly what I would have recommended--including awkwardly having to set all three of those variables, as even Google was inconsistent with their various projects over the years and have caused us lots of confusion--so that's great! :D

The only thing I'm left wondering about is what the intended purpose of ANDROID_NDK_LATEST_HOME is (mostly as its existence seemed to confuse my friend even further as it simply looked cooler than the handful of variables everyone else uses; lol).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
@saurik @MarijnS95 @ThadHouse @miketimofeev and others