Opened 3 months ago

Closed 7 weeks ago

Last modified 43 hours ago

#27439 closed defect (fixed)

Configure Rust compiler to target Android Platform

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

Description

Configure Rust compiler to target Android Platform

Child Tickets

Change History (17)

comment:1 Changed 3 months ago by sisbell

Have a patch to address the page size not working due to #27441: Update Debian Image to use Stretch

comment:3 Changed 3 months ago by boklm

Keywords: TorBrowserTeam201809R added; TorBrowserTeam201808 removed
Status: newneeds_review

comment:4 in reply to:  2 Changed 3 months ago by boklm

Keywords: TorBrowserTeam201809 added; TorBrowserTeam201809R removed
Status: needs_reviewneeds_revision

Replying to sisbell:

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

Some comments:

  • instead of defining prev_rust twice (for android, and for other platforms), it is possible to define a variable var/rust_arch which defaults to var/arch, except on android where we set it to x86_64. The var/rust_arch variable can then be used in the prev_rust url, and the directory name in rust/build.
  • android-toolchain should not need to be listed in the dependencies, if it is set as var/compiler (after the change is done in #26696), as var/compiler is already listed in the dependencies.
  • the setup of the android toolchain should not be done in projects/rust/build, but in var/setup defined in project/android-toolchain/config.

comment:5 Changed 2 months ago by gk

Keywords: TorBrowserTeam201810 added; TorBrowserTeam201809 removed

Moving tickets to October

comment:6 Changed 2 months ago by sisbell

I made the following changes:

  • use '/var_rust_arch' value.
  • no longer list android_toolchain as a dependency
  • setup is done in android_toolchain

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

comment:7 Changed 8 weeks ago by boklm

Keywords: TorBrowserTeam201810R added; TorBrowserTeam201810 removed
Status: needs_revisionneeds_review

comment:8 Changed 8 weeks ago by boklm

Keywords: TorBrowserTeam201810 added; TorBrowserTeam201810R removed
Status: needs_reviewneeds_revision

Could you explain what the android.patch patch is doing, and why it is needed (for example, the error message we get without the patch)? Is it a patch we should try to get merged upstream?

comment:9 in reply to:  8 Changed 8 weeks ago by sisbell

Replying to boklm:

Could you explain what the android.patch patch is doing, and why it is needed (for example, the error message we get without the patch)? Is it a patch we should try to get merged upstream?

dl_iterate_phdr doesn't exist on debian stretch, so we are just creating dummy version everytime.
getpagesize() is also not supported on debian stretch so we are using sysconf(_SC_PAGESIZE), which is supported

comment:10 Changed 8 weeks ago by sisbell

I confirmed that we can remove this patch with the latest code now that the build is using the Android NDK for building.

comment:11 Changed 7 weeks ago by gk

I looked at commit 2d5d46f20356e7fd40fd28ceacc7512905eceb84. Could you remove the whitespace on the line after configure_opt?

comment:12 Changed 7 weeks ago by sisbell

change (android-1024)

  • Removed white space after configure_opt
  • Removed x86_64-unknown-linux-gnu as target (this is not needed)

comment:13 Changed 7 weeks ago by sisbell

Status: needs_revisionneeds_review

comment:14 Changed 7 weeks ago by boklm

Status: needs_reviewneeds_revision

In commit 992b9324097f41c9503eb4ada393cec3da03195c, the patch is removing the empty line before targets:, but I think we should keep it as it separates the different parts of the file. Other than this it looks good to me.

comment:15 in reply to:  14 Changed 7 weeks ago by sisbell

Replying to boklm:

In commit 992b9324097f41c9503eb4ada393cec3da03195c, the patch is removing the empty line before targets:, but I think we should keep it as it separates the different parts of the file. Other than this it looks good to me.

I added extra line

Last edited 7 weeks ago by sisbell (previous) (diff)

comment:16 Changed 7 weeks ago by gk

Keywords: TorBrowserTeam201810R added; TorBrowserTeam201810 removed
Resolution: fixed
Status: needs_revisionclosed

Looks good. Please use a new branch for fixups. That makes reviewing easier, thanks. Cherry-picked to master (commit 61cdddfa4b7daf96f6bd1a52fb25a3ec055cc401).

comment:17 Changed 43 hours ago by gk

Sponsor: Sponsor8

More Sponsor8 items.

Note: See TracTickets for help on using tickets.