Opened 4 months ago

Closed 4 weeks ago

#26697 closed task (fixed)

Add Android toolchain

Reported by: boklm Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm, tbb-mobile, TorBrowserTeam201810
Cc: sisbell Actual Points:
Parent ID: #26693 Points:
Reviewer: Sponsor:

Description

We need to add a projects/android-toolchain directory which will download and build the Android SDK and all tools needed to be able to build Tor Browser for Android.

Child Tickets

Change History (31)

comment:1 Changed 4 months ago by gk

Priority: MediumHigh

Bumping prio.

comment:2 Changed 4 months ago by gk

Keywords: TorBrowserTeam201808 added; TorBrowserTeam201807 removed

Move our tickets to August.

comment:4 Changed 3 months ago by boklm

Keywords: TorBrowserTeam201809R added; TorBrowserTeam201808 removed
Status: newneeds_review

comment:5 in reply to:  3 Changed 3 months ago by boklm

Status: needs_reviewneeds_revision

Replying to sisbell:

commit: https://github.com/sisbell/tor-browser-build/commit/3ac1fe9c44a1ec20946896824cb06523e7370ef7

I did not try a build yet with this commit, but here are some first comments:

  • the format of the commit subjects should be Bug XXXX: description of the commit. So in this case, it should be Bug 26697: Add Android toolchain
  • it seems the var/setup in projects/android-toolchain/config is wrong:
      setup: |
        mkdir -p /var/tmp/dist
        unzip sdk.zip
    

The zip file will be extracted in the current directory instead of /var/tmp/dist. And sdk.zip is probably not the correct filename. Instead $rootdir/[% c("compiler_tarfile") %] should be used as the filename.

comment:6 Changed 3 months ago by gk

Keywords: TorBrowserTeam201809 added; TorBrowserTeam201809R removed

comment:7 Changed 8 weeks ago by sisbell

The sdkmanager is downloading packages (platform and build tools) during the build so I will need to track and add those to the rbm config.

comment:8 Changed 8 weeks ago by sisbell

I removed the setup in config and now handle this in the build file. I cleaned up the naming of sdk.zip and ndk.zip as suggested.

In addition, I collapsed issue #27438 Android Gradle Build Downloads into this same commit. I used the output from artc rust program to generate rbm config/build entries for each gradle artifact. This does lead to a large number of file entries in the config file.

If the current approach does not look clean enough, artc program can package gradle dependencies as a tar file. So its little effort to go this route instead.

The commits can be found at

https://github.com/sisbell/tor-browser-build/commits/android-rebased

comment:9 Changed 8 weeks ago by sisbell

The rbm build now downloads the android platform and build tools  (these are different than the SDK/NDK). These were previously being downloaded by the sdkmanager. 

comment:10 in reply to:  8 Changed 8 weeks ago by boklm

Replying to sisbell:

I removed the setup in config and now handle this in the build file. I cleaned up the naming of sdk.zip and ndk.zip as suggested.

There should be a var/setup in android-toolchain. This var/setup will then be used in firefox (and other components we build with the toolchain) to extract and setup the toolchain before the build.

comment:11 Changed 8 weeks ago by boklm

In commit a5e531981d5b5182277596fce92b8b7936e505e1, I think you don't need to define to define all the variables such as platform_tools_26: platform-tools_r26.0.2-linux.zip.

You could just have an input_files entry like this:

  - URL: '[% c("var/google_repo") %]/platform-tools_r26.0.2-linux.zip'
    name: platform_tools
    sha256sum: 63b15a38c2b64e6ec8b54febe9f69fce5fe6c898c554c73b826b49daf7b52519

And use c('input_files_by_name/platform_tools') when you need to use the filename in the build script.

comment:12 Changed 8 weeks ago by sisbell

Made the following changes:

  • Added var/setup
  • Changed variable names
  • Downloading maven-repo archive instead of individual files (limited sample to keep size down during testing)
  • Added gradle tool as part of android-toolchain
  • Matched directory names to be closer to what Firefox expects for its build

commit: https://github.com/sisbell/tor-browser-build/commit/e00c5e4319e67f7d17ed737b0e0c87db27547e3d

comment:13 Changed 7 weeks ago by gk

Keywords: TorBrowserTeam201810 added; TorBrowserTeam201809 removed

Moving tickets to October

comment:14 Changed 6 weeks ago by gk

Keywords: TorBrowserTeam201810R added; TorBrowserTeam201810 removed
Status: needs_revisionneeds_review

comment:15 Changed 5 weeks ago by sisbell

Created a full maven-repo archive, rather than just sample.

comment:16 in reply to:  15 Changed 5 weeks ago by boklm

Status: needs_reviewneeds_revision

Replying to sisbell:

Created a full maven-repo archive, rather than just sample.

  • could you add a comment giving instructions for generating the maven repo archive? (as said in https://trac.torproject.org/projects/tor/ticket/27438#comment:12)
  • I think the maven repo filename should include a version number, as we will probably need to update it a some point.
  • there are trailing whitespaces after the gradle_dependencies URL and sha256sum lines

comment:17 Changed 5 weeks ago by gk

Keywords: TorBrowserTeam201810 added; TorBrowserTeam201810R removed

comment:18 Changed 5 weeks ago by sisbell

Updated:

  • Added README.ANDROID file that explains how to build the maven repo. Added comment in android-toolchain/config that references the readme file.
  • Changed the name of the repo to include a version: maven-repo-1.0.tar.gz
  • Removed trailing whitespaces

Changes are on android-1017 branch.

comment:19 Changed 5 weeks ago by sisbell

Changes (android-1017)

  • Removed README.ANDROID since it looks like we are going with a different approach
  • Switched to Android platform-26 to align with latest changes in tor-browser

comment:20 Changed 5 weeks ago by sisbell

Changes (android-1017)

  • Removed reference to maven-repo archive. This is now handled within firefox project

comment:21 Changed 5 weeks ago by boklm

Keywords: TorBrowserTeam201810R added; TorBrowserTeam201810 removed
Status: needs_revisionneeds_review

comment:22 Changed 4 weeks ago by gk

Status: needs_reviewneeds_information

Why do we still have API 23 bits in this patch (like --api 23 and using 23.0.3 build tools) if we are targetting API 26?

comment:23 Changed 4 weeks ago by gk

Keywords: TorBrowserTeam201810 added; TorBrowserTeam201810R removed
Status: needs_informationneeds_revision

I guess this needs revision...

comment:24 Changed 4 weeks ago by sisbell

Changes (android-1024)

  • Removed android platform 23 from config
  • Upgraded android-tools-linux to latest version
  • Create android toolchains version 26 for arm, arm64, x86_64 (for when we add support to these platforms)

comment:25 Changed 4 weeks ago by sisbell

Status: needs_revisionneeds_review

comment:26 in reply to:  24 ; Changed 4 weeks ago by boklm

Status: needs_reviewneeds_revision

Replying to sisbell:

  • Create android toolchains version 26 for arm, arm64, x86_64 (for when we add support to these platforms)

So this includes support for all architectures in one toolchain? Usually we build one toolchain for only one architecture. Maybe we can have one that supports all android architectures, but this can be decided when we add support for arm64 and x86_64. For now I think we should build only the architecture we use.

comment:27 Changed 4 weeks ago by gk

It seems to me expect and openjdk-8-jdk is not needed in the deps: part, is it?

comment:28 in reply to:  26 Changed 4 weeks ago by sisbell

Replying to boklm:

Replying to sisbell:

  • Create android toolchains version 26 for arm, arm64, x86_64 (for when we add support to these platforms)

So this includes support for all architectures in one toolchain? Usually we build one toolchain for only one architecture. Maybe we can have one that supports all android architectures, but this can be decided when we add support for arm64 and x86_64. For now I think we should build only the architecture we use.

The Android NDK has all the architectures prebuilt within its zip file. The make_standalone_toolchain.py scripts are just copying the prebuilt binaries from various locations into one usable location. Its easy enough to remove the script calls for arm64, x86_64 until we are ready to make a decision.

comment:29 in reply to:  27 Changed 4 weeks ago by sisbell

Replying to gk:

It seems to me expect and openjdk-8-jdk is not needed in the deps: part, is it?

You are right. These were there when I was initially using sdkmanager and tools to pull the artifacts. Expect was for accepting licenses. I'll remove these.

comment:30 Changed 4 weeks ago by sisbell

Changes (android-1024)

  • Removed expect and openjdk dependencies
  • Removed arm64 and x86_64 NDK directories

comment:31 Changed 4 weeks ago by gk

Resolution: fixed
Status: needs_revisionclosed

Looks good now, thanks! I cherry-picked the patch to master (10c58d9a2f9fc37f7ba3e1b7028d2b5a730b1e6f).

Note: See TracTickets for help on using tickets.