Opened 2 weeks ago

Last modified 4 days ago

#31882 needs_review defect

move Android build config into core tor

Reported by: eighthave Owned by:
Priority: Medium Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: Android, tbb-mobile
Cc: gk, sysrqb, sisbell Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Guardian Project has maintained a wrapper build system for building tor for Android since the beginning. Since many more people are now building tor for Android, I'm working to upstream as much of that as possible. The goal is that the requirements for building on Android can be fulfilled by tor's own configure.ac.

The first change is to remove the unnecessary ./configure --enable-android option.

  • Android NDK compilers define __ANDROID__ so that can be used for Android-only code without adding anything in configure.ac, (following the standard pattern like __APPLE__, __linux__, __FreeBSD__, _WIN32, etc.
  • android/log.h and __android_log_write() are always present if building for Android, so this would be like testing for printf() on UNIX.

Then move the Android config into configure.ac reusing the ./configure --enable-android flag. A working sketch for that is here:
https://trac.torproject.org/projects/tor/ticket/28766#comment:6

The last piece is including a Java JNI wrapper in tor, enabled by ./configure --enable-jni. This lets us run tor as an Android Service, which is the Android equivalent of a UNIX daemon (UNIX daemons are not really supported on Android and using them is quite problematic). This JNI API should be generic enough to be useable in Java in general, though that's not a priority for us.

I'll add patches for merging once we agree on these details. I have this all working already on my machine.

Child Tickets

Change History (18)

comment:1 Changed 2 weeks ago by nickm

Milestone: Tor: 0.4.3.x-final

This seems like a plausible line of work to me. The 0.4.2.x series is in feature-freeze, so this would have to go in 0.4.3.

On the JNI wrapper: is it possible to implement this using (and mirroring) the API in tor-api.h? I'd like to keep the number of in-process interfaces that we expose as low as possible.

comment:2 Changed 2 weeks ago by nickm

Also, for long-term stability, it would be valuable to have a CI process that tests our android builds. I wonder how hard it would be for us to get one.

comment:3 Changed 2 weeks ago by eighthave

Looks like the JNI wrapper can be entirely based on what's in tor_api.h. You can get an idea now of what this needs, it uses only the pieces in the current src/feature/api/tor_api.[ch]. This is based on @sysrqb's work in #26653, e.g:
https://gitweb.torproject.org/user/sysrqb/tor.git/commit/?h=testing_26653&id=731b7e2b176f7cf6439c1286886492998e58cd7f

My big question is now to add a new src/feature/jni section and still have its symbols exported. When I add my files to src/feature/jni, it builds fine, but then its missing the exported JNI functions. The symbols make it into the libtor-app.a but not the libtor.so. My hack for now is to just add the JNI code to src/feature/api/tor_api.[ch], that works. The idea is to have ./configure --enable-jni. Here's that in progress code with the autotools changes:
https://gitlab.com/eighthave/tor/commit/a453e9faf3e4be946506111754b18f9fcf1ac595

I already have a working Android CI build in gitlab-ci (plus I started porting the .travis.yml to .gitlab-ci.yml while I was at it and threw in a couple of distro builds). It should be relatively straightforward to port the Android CI job .travis.yml for someone who know that whole matrix setup.
https://gitlab.com/eighthave/tor/blob/dbeb6f7d8/.gitlab-ci.yml
And here's an example run on the latest master:
https://gitlab.com/eighthave/tor/pipelines/85003730

Last edited 2 weeks ago by eighthave (previous) (diff)

comment:4 Changed 2 weeks ago by nickm

I'd suggest src/app/jni instead of src/feature/jni: This is a top-level entry point into the Tor source code, so it belongs an app.

What code is responsible for making "libtor.so" ? As far as I know, that is some extension to Tor's build system that we do not currently maintain. It would be good to import that code into our build system as well, as a part of doing this.

comment:5 Changed 2 weeks ago by eighthave

Ok, as long as it can be optionally enabled and disabled, I'm fine with putting it wherever. For example, I think that this JNI code should only be included when someone specifies ./configure --enable-jni. That should be separate from ./configure --enable-android since some people might want to still build the tor daemon for Android for testing purposes.

As for making libtor.so, that's ./configure --enable-pic. Then src/app/tor is renamed to libtor.so. I think ./configure --enable-pic should change the resulting filename to libtor.so.

comment:6 Changed 2 weeks ago by nickm

Whoa, that's not how you're supposed to make an .so file, is it? I thought you were supposed to use a specific set of linker options in order to make a .so.

comment:7 Changed 2 weeks ago by eighthave

lol, yeah, not by design. I just tried ./configure --enable-pic and it works as a .so

comment:8 in reply to:  3 Changed 2 weeks ago by teor

Replying to eighthave:

...

I already have a working Android CI build in gitlab-ci (plus I started porting the .travis.yml to .gitlab-ci.yml while I was at it and threw in a couple of distro builds). It should be relatively straightforward to port the Android CI job .travis.yml for someone who know that whole matrix setup.
https://gitlab.com/eighthave/tor/blob/dbeb6f7d8/.gitlab-ci.yml
And here's an example run on the latest master:
https://gitlab.com/eighthave/tor/pipelines/85003730

I'm currently rewriting and simplifying our build matrix in #30860. It might be easier to port the .travis.yml after I'm done.

comment:9 Changed 2 weeks ago by eighthave

Also, FYI, OnionBrowser on iOS has been building and using tor like this for a while now.

comment:10 Changed 4 days ago by eighthave

Attached is the patch series to implement this all on top of master. 0003-Android-s-logging-mechanism-is-called-logcat.patch and the gitlab-ci patches could be omitted if it delays merging the first 2.

In order to make sure I didn't mess up ./configure on other platforms, I added tests to .gitlab-ci.yml for lots of GNU/Linux distros (the MinGW job isn't quite working yet, so ignore its failure). You can see a full run here:
https://gitlab.com/eighthave/tor/pipelines/87935658

I'm happy to submit the rest of the .gitlab-ci.yml changes as desired. You can see them in my gitlab fork.

comment:11 Changed 4 days ago by eighthave

Status: newneeds_review

comment:12 Changed 4 days ago by eighthave

The last piece, which is not implemented in the patches, is making the output filename be libtor.so. There is currently no support for different output file names in Makefile.am so it would be non-trivial to implement, and I'm not sure the approach that should be taken.

Note: See TracTickets for help on using tickets.