Opened 18 months ago

Closed 6 weeks ago

Last modified 12 days ago

#28766 closed defect (fixed)

Tor Build for Android

Reported by: sisbell Owned by: sisbell
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-mobile, tbb-rbm, tbb-parity, TorBrowserTeam202004R
Cc: sisbell, sysrqb, boklm, gk, tbb-team Actual Points:
Parent ID: #28704 Points: 1
Reviewer: boklm Sponsor:

Description


Child Tickets

Attachments (1)

0001-move-Android-build-setup-into-enable-android-flag.patch (2.3 KB) - added by eighthave 8 months ago.

Download all attachments as: .zip

Change History (36)

comment:1 Changed 18 months ago by sisbell

Tor needs rust 1.28. However for android we are using 1.26.1. How should we handle this?

comment:2 Changed 18 months ago by boklm

For the linux nightly build, we build rust 1.28.0. Maybe we can do the same.

We can also start by building Tor without rust (like on Windows and macOS).

comment:3 in reply to:  2 Changed 18 months ago by gk

Replying to boklm:

For the linux nightly build, we build rust 1.28.0. Maybe we can do the same.

We can also start by building Tor without rust (like on Windows and macOS).

Yes, let's start without building Tor on Android with the newer Rust.

comment:4 Changed 15 months ago by gk

Keywords: tbb-parity added

tbb-parity items.

comment:5 Changed 9 months ago by eighthave

Cc: hans@… added

comment:6 Changed 8 months ago by eighthave

I just added a patch to move key Android configuration to ./configure --enable-android so that it is shared across all the builds. There is more stuff that can be moved there, that is not yet:

  • Android NDK version detection and enforcement (e.g. >= r19 or only exactly r20)
  • CFLAGS and LDFLAGS to find parity with Android NDK builds done with ndk-build, which automatically sets things there.
  • Perhaps the state of --disable-linker-hardening and --disable-gcc-hardening could also be maintained there, since we're all building with the Android NDK's clang.

The patch is attached, here's the diff:

diff --git a/configure.ac b/configure.ac
index a639ffaf3..afee6940c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -88,6 +88,27 @@ else
             [Defined if we're building with OpenSSL or LibreSSL])
 fi
 
+dnl Enable Android only features.
+AC_ARG_ENABLE(android,
+     AS_HELP_STRING(--enable-android, [build with Android features enabled]))
+AM_CONDITIONAL([USE_ANDROID], [test "x$enable_android" = "xyes"])
+
+if test "x$enable_android" = "xyes"; then
+  AC_DEFINE([USE_ANDROID], [1], [Compile with Android specific features enabled])
+
+  dnl Check if the Android log library is available.
+  AC_CHECK_HEADERS([android/log.h])
+  AC_SEARCH_LIBS(__android_log_write, [log])
+
+  asciidoc=false
+  enable_html_manual=no
+  enable_manpage=no
+  enable_pic=yes
+  enable_system_torrc=no
+  enable_tool_name_check=no
+  have_systemd=no
+fi
+
 if test "$enable_static_tor" = "yes"; then
   enable_static_libevent="yes";
   enable_static_openssl="yes";
@@ -117,7 +138,7 @@ AC_ARG_ENABLE(asciidoc,
         "yes") asciidoc=true ;;
         "no")  asciidoc=false ;;
         *) AC_MSG_ERROR(bad value for --disable-asciidoc) ;;
-      esac], [asciidoc=true])
+      esac], [test "x$asciidoc" = "xtrue" && asciidoc=true])
 
 # systemd notify support
 AC_ARG_ENABLE(systemd,
@@ -227,20 +248,6 @@ if test x$enable_event_tracing_debug = xyes; then
   AC_DEFINE([TOR_EVENT_TRACING_ENABLED], [1], [Compile the event tracing instrumentation])
 fi
 
-dnl Enable Android only features.
-AC_ARG_ENABLE(android,
-     AS_HELP_STRING(--enable-android, [build with Android features enabled]))
-AM_CONDITIONAL([USE_ANDROID], [test "x$enable_android" = "xyes"])
-
-if test "x$enable_android" = "xyes"; then
-  AC_DEFINE([USE_ANDROID], [1], [Compile with Android specific features enabled])
-
-  dnl Check if the Android log library is available.
-  AC_CHECK_HEADERS([android/log.h])
-  AC_SEARCH_LIBS(__android_log_write, [log])
-
-fi
-
 dnl ---
 dnl Tor modules options. These options are namespaced with --disable-module-XXX
 dnl ---
-- 
2.20.1

comment:7 Changed 7 months ago by sysrqb

Points: 0.25

comment:8 Changed 7 months ago by gk

Points: 0.251

comment:9 Changed 7 months ago by sisbell

Do we know if this patch works with Android NDK 17b?

comment:10 Changed 7 months ago by eighthave

That configure.ac patch is now here for #31882:

I just got the gitlab-ci job for r17b working, so I can confirm that tor with my changes builds with NDK r17b, but this job does not include lzma or zstd.

comment:11 in reply to:  10 Changed 7 months ago by sisbell

Replying to eighthave:

That configure.ac patch is now here for #31882:

I just got the gitlab-ci job for r17b working, so I can confirm that tor with my changes builds with NDK r17b, but this job does not include lzma or zstd.

I'm assuming we need to open new tickets for building these libraries? Or is this something we can live without for now?

comment:12 Changed 6 months ago by sysrqb

Keywords: TorBrowserTeam201912 added; TorBrowserTeam201812 removed
Owner: changed from tbb-team to sisbell
Status: newassigned

comment:13 Changed 6 months ago by sysrqb

Cc: tbb-team added

comment:14 Changed 5 months ago by sysrqb

Keywords: TorBrowserTeam202001 added; TorBrowserTeam201912 removed

comment:15 Changed 4 months ago by pili

Keywords: TorBrowserTeam202002 added; TorBrowserTeam202001 removed

Moving tickets to February

comment:16 Changed 3 months ago by sysrqb

Status: assignedneeds_revision
-cp $distdir/share/tor/geoip $TORCONFIGDIR
-cp $distdir/share/tor/geoip6 $TORCONFIGDIR
-
+[% IF !c("var/android") -%]
+  cp $distdir/share/tor/geoip $TORCONFIGDIR
+  cp $distdir/share/tor/geoip6 $TORCONFIGDIR
+[% END -%]

Can you set TORCONFIGDIR=assets/common and use the original code instead?

In the build script, please use install instead of cp (see how files are copied within the other platform blocks).

+[% IF c("var/android") %]
+  libsdir=jniLibs/[% c("var/abi") %]
+  mkdir -p $libsdir assets/common
+  # Copy tor using naming convention that Android will recognize 
+  cp bin/tor $libsdir/libTor.so

There's a trailing space after 'recognize'

Can you move the tar command into the general platform-specific tar/zip block that immediately follows the android block?

comment:17 Changed 3 months ago by sysrqb

I should mention this is based on commit b00b0d40320431abd0d13a4fa959ce53407b35e9 from ticket:28704#comment:23.

comment:18 Changed 3 months ago by sisbell

Status: needs_revisionneeds_review

Made the following changes:

  • Set TORCONFIGDIR using original code
  • Use install command
  • Use general tar command
  • Various cleanup

https://github.com/sisbell/tor-browser-build/commits/bug-28766-1

comment:19 Changed 3 months ago by boklm

Status: needs_reviewneeds_revision

When looking at commit 4f636796fb728ce614e3223780dbdab86892eabf, I see that var/configure_opt containing options for android is defined in 3 places: tor, xz, libevent.

Instead I think we should define it only one time, in rbm.conf, as I said in ticket:28704#comment:14:

it seems we could have a var/configure_opt for android in rbm.conf containing something like CC=clang --host=[% c("var/host") %] [% c("var/configure_opt_project") %], where var/configure_opt_project is defined in each project to define options specific to this project

comment:20 Changed 3 months ago by sisbell

Keywords: TorBrowserTeam202003R added; TorBrowserTeam202002 removed
Status: needs_revisionneeds_review

I added the configure_opt_project var to the build

https://github.com/sisbell/tor-browser-build/commits/bug-28766-2

comment:21 Changed 3 months ago by sisbell

Cc: sysrqb boklm added; hans@… removed

comment:22 Changed 3 months ago by boklm

Status: needs_reviewneeds_information

In ticket:28763#comment:2 gk asked a question about zlib support:

What about zlib support which we currently have on desktop?

comment:23 Changed 2 months ago by sisbell

Status: needs_informationneeds_review

comment:24 Changed 8 weeks ago by pili

Keywords: TorBrowserTeam202004R added; TorBrowserTeam202003R removed

We are no longer in March

comment:25 Changed 7 weeks ago by pili

Reviewer: boklm

comment:26 Changed 7 weeks ago by sisbell

comment:27 in reply to:  26 Changed 7 weeks ago by boklm

Keywords: TorBrowserTeam202004 added; TorBrowserTeam202004R removed
Status: needs_reviewneeds_revision

Replying to sisbell:

Fixed copying of libevent for linux

https://github.com/sisbell/tor-browser-build/commits/bug-28766-5

The file change from libevent-2.1.so.6 to libevent-2.1.so.7 is caused by the libevent update to version 2.1.11. So this should be done in the commit for #31499. There is also no reason to change libstdc++.so.6 to libstdc++.so.7.

comment:28 Changed 7 weeks ago by sisbell

Keywords: TorBrowserTeam202004R added; TorBrowserTeam202004 removed
Status: needs_revisionneeds_review

I added the libevent version change for tor in #31499.

This new commit uses that change, otherwise is the same as before

https://github.com/sisbell/tor-browser-build/commits/bug-28766-6

comment:29 in reply to:  28 Changed 6 weeks ago by boklm

Keywords: TorBrowserTeam202004 added; TorBrowserTeam202004R removed
Status: needs_reviewneeds_revision

Replying to sisbell:

I added the libevent version change for tor in #31499.

This new commit uses that change, otherwise is the same as before

https://github.com/sisbell/tor-browser-build/commits/bug-28766-6

  • It seems we duplicate the lines for extracting zlib and setting zlibdir. I think we can them just one time behind a [% IF c("var/windows") || c("var/android") %]
  • The pkg-config symlink can be avoided by setting the --disable-tool-name-check configure flag
  • Looking at https://github.com/torproject/tor/pull/1408 other flags we can set are --disable-system-torrc and --disable-systemd.

comment:30 Changed 6 weeks ago by sysrqb

+  - name: xz
+    project: xz
+    enable: '[% c("var/android") %]'
+  - name: zstd
+    project: zstd
+    enable: '[% c("var/android") %]'

I chatted with gk and ahf about enabling xz and zstd. gk pointed me at #22341, as well, where they previously decided only starting with zstd seemed smart. Let's stick with that idea and only enable zstd (and zlib) now.

Therefore, please delete the xz parts from this patch.

comment:31 Changed 6 weeks ago by sisbell

Made the following changes:

  • removed xz/lzma support for android
  • removed pkg-config symlink and added flag to disable check
  • added '--disable-system-torrc' and '--disable-systemd' for android builds
  • removed duplicate of zlib setup
  • moved extracting of openssl and libevent projects to after the platform conditional extracting blocks (small readability cleanup)

https://github.com/sisbell/tor-browser-build/commits/bug-28766-7

comment:32 Changed 6 weeks ago by sisbell

Status: needs_revisionneeds_review

comment:33 Changed 6 weeks ago by sysrqb

I opened #33915 for enabling rust. Overall, this looks okay to me.

boklm, do you see anything that should be changed?

comment:34 in reply to:  33 ; Changed 6 weeks ago by boklm

Keywords: TorBrowserTeam202004R added; TorBrowserTeam202004 removed
Resolution: fixed
Status: needs_reviewclosed

Replying to sysrqb:

I opened #33915 for enabling rust. Overall, this looks okay to me.

boklm, do you see anything that should be changed?

This looks good to me too. This is now commit 7894190b275b412c8d4f9cf7cad281601594da24 on master.

comment:35 in reply to:  34 Changed 12 days ago by gk

Replying to boklm:

Replying to sysrqb:

I opened #33915 for enabling rust. Overall, this looks okay to me.

boklm, do you see anything that should be changed?

This looks good to me too. This is now commit 7894190b275b412c8d4f9cf7cad281601594da24 on master.

I don't think ZSTD is enabled for Android. I've filed #34219 to fix that.

Note: See TracTickets for help on using tickets.