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

Explicitly decalre functions used in TorService.c to build with NDK 26+ #6

Merged
merged 1 commit into from
May 22, 2024

Conversation

bitmold
Copy link

@bitmold bitmold commented May 2, 2024

Android NDK toolchain 26 and 27 uses a newer version of clang that treats these implicitly invoked function calls as errors instead of warnings. since 25.2.9519653 is deprecated this is one of the things we need todo in order to use a supported NDK toolchain

@n8fr8 n8fr8 requested a review from eighthave May 3, 2024 17:03
@n8fr8
Copy link
Member

n8fr8 commented May 3, 2024

@eighthave thoughts?

@bitmold
Copy link
Author

bitmold commented May 3, 2024

If you stick to using NDK 25.2.9519653 which the CI uses and the scripts in tor-android recommends this won't be a problem (you just get compiler warnings), when working on code changes in tor-android I used a newer NDK by accident (27.0.11718014) and realized that tor no longer compiles due to NDK upgrading to clang 16:

https://www.redhat.com/en/blog/new-warnings-and-errors-clang-16

Of course, this code still compiles with the older clang/ndk that guardian project uses to release tor-android.

@bitmold
Copy link
Author

bitmold commented May 3, 2024

Specifically clang throws out -Wimplicit-function-declarations on the functions tor_free_all from tor code itself (declared in shutdown.h) and unset_owning_controller_socket.

guardianproject/tor-android#57

but for that matter, both were introduced to stop gap fix a crash tor had on Android with comments in guardian project's fork of tor indicating that this code was to be removed once an upstream fix was introduced in tor itself. I've only kind of cursorily researched the issue tracker on the tor codebase, but @eighthave I'm curious to know if this is still needed at all.

@bitmold
Copy link
Author

bitmold commented May 9, 2024

@n8fr8 Ideally this can be merged in before uniqx's work with #8 since releases to the tor fork are so infrequent. the remaining work to support a modern ndk toolchain is just done on the tor-android project

@bitmold bitmold changed the title Explicitly decalre functions used in TorService.c, Explicitly decalre functions used in TorService.c to build with NDK 26+ May 9, 2024
@syphyr
Copy link

syphyr commented May 9, 2024

The commit fixes the build issue with Tor and NDK 26
syphyr@75d99f8

Although, I still can't figure out why the jni in Tor is not included in libtor.so after updating to NDK 26.

@n8fr8 n8fr8 requested review from uniqx and removed request for eighthave May 10, 2024 20:20
@n8fr8 n8fr8 added the enhancement New feature or request label May 10, 2024
…ain 26

uses a newer version of clang that treats these implicitly invoked function
calls as errors instead of warngings
@uniqx
Copy link
Member

uniqx commented May 22, 2024

I've successfully finished a build of tor-android with these changes using ndk 25.2.9519653. So I think this is safe to merge.

@n8fr8 n8fr8 merged commit 5ebaf4b into guardianproject:master May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants