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

Restore NDKv21 for GitHub Actions #4077

Closed
wants to merge 1 commit into from
Closed

Restore NDKv21 for GitHub Actions #4077

wants to merge 1 commit into from

Conversation

ppigazzini
Copy link
Contributor

@ppigazzini ppigazzini commented Jun 14, 2022

GitHub updated the versions of NDK installed on the Actions runners
breaking the ARM tests.
Restore the NDKv21 using the GitHub suggested mitigation, see:
actions/runner-images#5595

@vondele vondele mentioned this pull request Jun 15, 2022
GitHub updated the versions of NDK installed on the Actions runners
breaking the ARM tests.
Restore the NDKv21 using the GitHub suggested mitigation, see:
actions/runner-images#5595
@vondele vondele added the to be merged Will be merged shortly label Jun 16, 2022
@vondele vondele closed this in d54b85b Jun 16, 2022
@vondele
Copy link
Member

vondele commented Jul 31, 2022

seems like this fix for NDKv21 stopped working. CI currently fails with

/home/runner/work/_temp/0220f0b4-7a24-47d0-8f9b-a2b9745162c3.sh: line 2: aarch64-linux-android21-clang++: command not found

https://github.com/official-stockfish/Stockfish/runs/7594674615?check_suite_focus=true

@ppigazzini
Copy link
Contributor Author

ppigazzini commented Jul 31, 2022

"21.4.7075529" is still the last r21 NDK:
https://github.com/android/ndk/wiki/Unsupported-Downloads

The GitHUb runner documentation still shows NDK r21:
https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-Readme.md#android

@ppigazzini
Copy link
Contributor Author

[all OSs] Android ndk-bundle along with old NDK versions will be deprecated on July, 24

actions/runner-images#5879

I'm testing the proposed mitigation.

@vondele
Copy link
Member

vondele commented Aug 11, 2022

any luck with this test of the mitigation?

@dav1312
Copy link
Contributor

dav1312 commented Aug 26, 2022

Should the ARM tests be turned off until the mitigation is applied so not all commits fail?

@vondele
Copy link
Member

vondele commented Aug 26, 2022

yes, I would merge such a PR now.

@ppigazzini
Copy link
Contributor Author

any luck with this test of the mitigation?

No. No other working solutions in the GH thread comments the last time that I have checked.

@vondele
Copy link
Member

vondele commented Sep 2, 2022

OK, thanks for testing. Means eventually we need to find a better way to do this, but it is not urgent.

PikaCat-OuO pushed a commit to official-pikafish/Pikafish that referenced this pull request Oct 7, 2022
GitHub updated the versions of NDK installed on the Actions runners
breaking the ARM tests.
Restore the NDKv21 using the GitHub suggested mitigation, see:
actions/runner-images#5595

closes official-stockfish/Stockfish#4077

No functional change

(cherry picked from commit d54b85b)
@ppigazzini
Copy link
Contributor Author

ppigazzini commented Dec 8, 2022

@vondele the problem is that GitHub removed NDK 21 from all the Ubuntu runners. That was the last version with the compiler able to write the TLS segment with the right alignment (with a static build) for qemu. Our workflow didn't show the missing NDK 21 because GitHub set the ANDROID_NDK_HOME variable (used by us to update the PATH) to the last NDK version, so our workflow kept using the compiler from the last NDK 25.

Some references to try to found a solution:
https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2004-Readme.md
termux/termux-packages#8273
https://github.com/Lzhiyong/termux-ndk/blob/master/patches/align_fix.py
https://reviews.llvm.org/D53906

@ppigazzini
Copy link
Contributor Author

@vondele it turn out that the problem was only the PATH, opened #4267

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

Successfully merging this pull request may close these issues.

None yet

3 participants