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

Make rust core use NDK >= r23 #1086

Merged
merged 13 commits into from
Jul 28, 2022
Merged

Make rust core use NDK >= r23 #1086

merged 13 commits into from
Jul 28, 2022

Conversation

GhenadiePusca
Copy link
Contributor

So this is workaround to allow building core with any NDK version >= r23. This is a little bit of magic I didn't want to do initially, but it seems that is quite safe(many projects also adopted this kind of solution).

The root cause of the issue is that newer NDK versions use -lunwind instead of -libgcc, while rust building system seems to be using still the version with -libgcc

What changed:

  • core Make will no more download the NDK, instead it will use the system one.
  • Trick rust into using -lunwind as -libgcc

@GhenadiePusca GhenadiePusca marked this pull request as draft July 21, 2022 12:59
@GhenadiePusca
Copy link
Contributor Author

GhenadiePusca commented Jul 21, 2022

Needs more work, the proposed workaround works only with r23 and r24, but not with r25... In the end it is not about NDK version, I did had a faulty test.

Copy link
Contributor

@JJ JJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, just the usual questions to understand it a bit better.

platform/core/Makefile Outdated Show resolved Hide resolved
platform/core/Makefile Outdated Show resolved Hide resolved
platform/core/Makefile Outdated Show resolved Hide resolved
@GhenadiePusca
Copy link
Contributor Author

GH action fails mostly due to GH env still using obsolete ndk-bundle. However GH is about to remove it, probably in favour of normal android NDK -> actions/runner-images#5879.

Will postpone this PR until GH makes all the changes on the env, which will happen on Monday 24th.

Copy link
Contributor

@JJ JJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. The .fbs file was simply left over, right?

.github/workflows/codeql-analysis.yml Show resolved Hide resolved
platform/core/README.md Outdated Show resolved Hide resolved
@GhenadiePusca
Copy link
Contributor Author

Looks great. The .fbs file was simply left over, right?

Right.
What I have to do to complete this, is to check if NDK r25 can be used. I know that GH env did add r25, and it should be the default, but it is not for some reason.

Ghenadie Vasiliev-Pusca added 2 commits July 27, 2022 17:51
@GhenadiePusca GhenadiePusca marked this pull request as ready for review July 28, 2022 07:12
@GhenadiePusca GhenadiePusca merged commit 4e565b8 into main Jul 28, 2022
@GhenadiePusca GhenadiePusca deleted the use-ndk-23+ branch July 28, 2022 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants