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

Locating the android ndk path #950

Open
macalimlim opened this issue Sep 24, 2022 · 7 comments
Open

Locating the android ndk path #950

macalimlim opened this issue Sep 24, 2022 · 7 comments
Assignees
Labels

Comments

@macalimlim
Copy link
Contributor

Hi,

@orion78fr
@Bromeon

I see that there are changes to gdnative-sys/build.rs about locating the ndk path (either having an ndk directory or ndk-bundle directory). The issue with my own setup is I downloaded the ndk from Google (using wget) and extracted its contents to my file system, to which the ndk and/or ndk-bundle directories does not exist. Does the current implementation in gdnative-sys/build.rs assume it was downloaded by a package manager and/or by an Android IDE?

Mike

@orion78fr
Copy link
Contributor

Hi,

Yeah, it assumes you used the sdkmanager utility (which is part of the android SDK distribution) to download the NDK (or used Android studio which internally calls it for you).
If it is possible in your setup, I suggest using this utility (with a command line like sdkmanager --install ndk) to install the ndk, or rename your extracted folder under the sdk root as ndk-bundle.

As I said in earlier discussions on this subject, using ndk-bundle route (or direct download) makes it hard to update the ndk or maintain multiple ndk versions in parallel.
A solution may be to add directly a ANDROID_NDK_ROOT environment variable to provide a direct path but I don't know if this is really needed, as it may be confusing (and error-prone) to add many ways to configure the same folder path. I may be able to do it if maintainers want this.

@macalimlim
Copy link
Contributor Author

I think just by defining ANDROID_NDK_ROOT alone should be enough to make gdnative-sys to work. In this way it's independent on how the NDK was installed (by either manually, using a package manager or sdkmanager cli tool). Unless I misunderstood some stuff about gdnative-sys that it needs the other 2 environment variables. Thoughts?

@Bromeon
Copy link
Member

Bromeon commented Sep 25, 2022

godot-rust probably should not be opinionated about how the NDK was installed, as such I'd welcome if we also allowed an environment variable to be used. But like macalimlim, I'm not 100% sure if that's enough or users are very likely to immediately run into the next pitfall.

It probably makes sense though if we document the recommended workflow (which is sdkmanager, as I understood) and provide some defaults for that.

What do you both think?

@orion78fr
Copy link
Contributor

I don't really know what's the best, I pretty much kept what was there (the ANDROID_SDK_ROOT) and allowed it to choose between parallel ndk install or the deprecated ndk-bundle which was used, and to be honest, I don't really remember if godot-rust needs access to Java Android stuff that is in the sdk alongside the ndk.

However, rust android ndk tools recommends using sdkmanager or android studio to install the ndk too (probably so it can also be maintained up to date with the rest of the android sdk) : https://github.com/rust-windowing/android-ndk-rs#1-install-the-android-ndk-and-sdk
Plus, ANDROID_SDK_ROOT seems to be a standard environment variable that get set on all platforms when you install an android SDK.

@Bromeon
Copy link
Member

Bromeon commented Sep 26, 2022

Thanks for the update!

Would one of you guys have the motivation and time to add the sdkmanager best practices + link to the Android guide in the book?

And maybe someone to submit a PR that would allow an ANDROID_NDK_ROOT to be used? 🙂

@macalimlim
Copy link
Contributor Author

Hi @Bromeon and @orion78fr,

I can probably do both, unless @orion78fr wants to do the additions to the guidebook. I also forked this docker image by @ufoot. In the fork, I implemented the changes needed by gdnative-sys to work. You can check it on this Dockerfile, plus the addition of setting up an iOS toolchain. I can also do a separate PR for that in the guidebook. If you want to test the new docker image out, just do docker pull macalimlim/godot-rust-cross-compiler on your terminal.

Mike

@orion78fr
Copy link
Contributor

You can do both, I don't have much time to work on this these days.

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

No branches or pull requests

4 participants