move Android build config into core tor

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.

changed milestone to %Tor: unspecified

added 043-deferred 044-deferred Android component::core tor/tor milestone::Tor: unspecified owner::nickm priority::medium reviewer::nickm severity::normal status::accepted tbb-mobile type::defect labels

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.

Trac:
Milestone: N/A to Tor: 0.4.3.x-final

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.

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 (moved), 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

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.

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.

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.

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

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 (moved). It might be easier to port the .travis.yml after I'm done.

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

Trac:
0001-use-standard-ANDROID-macro-for-detecting-Android.patch

Trac:
0004-gitlab-ci-purge-old-job-for-mirroring-its-unused-and.patch

Trac:
0005-gitlab-ci-add-Debian-testing-and-ubuntu-LTS.patch

Trac:
0006-gitlab-ci-add-barebones-Android-job-as-smokecheck.patch

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.

Trac:
0002-move-Android-build-setup-into-enable-android-flag.patch

Trac:
Status: new to needs_review

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.

Trac:
0003-Android-s-logging-mechanism-is-called-logcat.patch

We can just rename to libtor.so in the build file for tor-browser-build. The naming needn't be in the make file

Replying to eighthave:

The last piece, which is not implemented in the patches, is making the output filtrivialename 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.

Would it be feasible to have this as a github PR? Right now github is what we're using for our code reviews, mostly.

sure thing: https://github.com/torproject/tor/pull/1408

@sisbell one of my goals here is to remove as many of the "papercuts" as possible, building Tor apps on Android is currently "death by a million papercuts". Renaming the resulting file is one of those papercuts. The easier it is for some Android or iOS developer to work with Tor, the more people we'll have working on tor mobile. And for the existing apps, it means less complexity to manage when keeping things current. For example, there was lots of good work done in core tor to improve things on mobile. It took us a long time to integrate and ship that work, mostly because of the current vast complexity between the core tor source repo and actually shipping it in an Android app.

Trac:
Reviewer: N/A to nickm

I've had a look over the changes, except for the gitlab CI stuff which I'm not competent to evaluate. Since others asked for that, maybe they could weigh in whether it seems right to them? (I was about to suggest doing gitlab CI in a separate branch, but I see that somebody else requested it, so I probably should defer to them.)

I've left some comments on the PR. Additionally, this needs a file in the changes/* directory to explain what it is when we generate the changelog. The doc/HACKING/CodingStandards.md file explains how those look.

Please let me know if you have any questions. Thanks!

Trac:
Status: needs_review to needs_revision

I included the gitlab-ci changes there to make it easy to see which run matches the pull request. They could be merged separately, if that's easier. That's why I included all those commits on top of the code changes.

I removed the "logcat" name change in favor of #32181 (moved), and otherwise addressed the comments in github.

Trac:
Status: needs_revision to needs_review

Okay, I've responded on the github comments again.

Are there changes that you forgot to push? I don't see any changes to the code on github since the force-push 8 days ago. Am I missing them?

Trac:
Status: needs_review to needs_revision

Arg, yes, sorry about that. I pushed now.

Trac:
Status: needs_revision to needs_review

Re: https://trac.torproject.org/projects/tor/ticket/32191#comment:27

We're going to maintain the Android and iOS build systems in this fork: https://github.com/guardianproject/tor

Please take anything from there as you see fit, under the license you require.

Trac:
Status: needs_review to new

If this PR can be merged as-is, we should probably merge it.

Trac:
Status: new to needs_review

The PR can't be merged as-is. The android log parts are now #32181 (moved), where we're taking a simpler approach and just removing them. That leaves these parts:

• Changing --enable-android to imply a suite of other options
• Removing USE_ANDROID.

I think removing USE_ANDROID is fine to do after #32181 (moved) is merged, since we'll have a conflict there.

This leaves changing --enable-android to imply --disable-asciidoc --disable-html-manual --disable-manpage --enable-pic --disable-system-torrc --disable-tool-name-check --disable-systemd. For that, I'd like to survey a couple of other android integrators to see if they'd also want those defaults, and whether they'd think that a "contrib/configure_android.sh" script was adequate. If they do want something like this:

1. Then the configure option should change the defaults, not override other settings.
2. Probably it should get a new name other than --enable-android, since most of the changes have nothing to do with being android-specific, and since --enable-android used to mean something else.

I'll take that on.

Trac:
Owner: N/A to nickm
Status: needs_review to accepted

All 0.4.3.x tickets without 043-must, 043-should, or 043-can are about to be deferred.

Trac:
Keywords: N/A deleted, 043-deferred added

Trac:
Milestone: Tor: 0.4.3.x-final to Tor: 0.4.4.x-final

Bulk-remove tickets from 0.4.4. Add the 044-deferred label to them.

Trac:
Keywords: N/A deleted, 044-deferred added
Milestone: Tor: 0.4.4.x-final to Tor: unspecified

mentioned in issue #32298 (moved)

moved to tpo/core/tor#31882 (closed)

mentioned in issue tpo/core/tor#32298 (closed)