Opened 8 months ago

Closed 8 months ago

Last modified 8 months ago

#33215 closed defect (fixed)

Android Toolchain: Add NDK bin path to system path

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

Description

Add the NDK Path to the system path so that each project can find the correct android tools (by host triplet).

Child Tickets

Change History (10)

comment:1 Changed 8 months ago by sysrqb

Status: newneeds_information

This is based on commit f5fdff3bf0b9e957ab011f95af9e215cca7aeed6 from ticket:28704#comment:23.

diff --git a/projects/android-toolchain/config b/projects/android-toolchain/config
index 8c9c8222..4c02b2f3 100644
--- a/projects/android-toolchain/config
+++ b/projects/android-toolchain/config
@@ -15,6 +15,7 @@ var:
     export GRADLE_HOME=/var/tmp/dist/[% project %]/gradle
     export ANDROID_HOME=$ANDROID_SDK_HOME
     export GRADLE_USER_HOME=$GRADLE_HOME
+    export PATH=$ANDROID_NDK_HOME/[% c("var/toolchain_arch") %]/bin:$PATH

If we put the toolchain at the end, are there situations where a system executable is used instead of the Android toolchain?

comment:2 in reply to:  1 ; Changed 8 months ago by sisbell

Replying to sysrqb:

This is based on commit f5fdff3bf0b9e957ab011f95af9e215cca7aeed6 from ticket:28704#comment:23.

diff --git a/projects/android-toolchain/config b/projects/android-toolchain/config
index 8c9c8222..4c02b2f3 100644
--- a/projects/android-toolchain/config
+++ b/projects/android-toolchain/config
@@ -15,6 +15,7 @@ var:
     export GRADLE_HOME=/var/tmp/dist/[% project %]/gradle
     export ANDROID_HOME=$ANDROID_SDK_HOME
     export GRADLE_USER_HOME=$GRADLE_HOME
+    export PATH=$ANDROID_NDK_HOME/[% c("var/toolchain_arch") %]/bin:$PATH

If we put the toolchain at the end, are there situations where a system executable is used instead of the Android toolchain?

I'll need to check old logs, but yes some projects just look for clang without the host info so it picks up the system executable for clang.

comment:3 Changed 8 months ago by gk

Keywords: tbb-parity removed

No parity involved in this bug.

comment:4 Changed 8 months ago by sisbell

Status: needs_informationneeds_review

I placed the ANDROID path after the system path. I did need to make a change in the zstd project to use the correct variant of clang (another commit).

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

comment:5 Changed 8 months ago by boklm

Keywords: TorBrowserTeam202002R added; TorBrowserTeam202002 removed

comment:6 Changed 8 months ago by eighthave

Cc: hans@… removed

comment:7 in reply to:  2 ; Changed 8 months ago by boklm

Status: needs_reviewneeds_information

Replying to sisbell:

Replying to sysrqb:

This is based on commit f5fdff3bf0b9e957ab011f95af9e215cca7aeed6 from ticket:28704#comment:23.

diff --git a/projects/android-toolchain/config b/projects/android-toolchain/config
index 8c9c8222..4c02b2f3 100644
--- a/projects/android-toolchain/config
+++ b/projects/android-toolchain/config
@@ -15,6 +15,7 @@ var:
     export GRADLE_HOME=/var/tmp/dist/[% project %]/gradle
     export ANDROID_HOME=$ANDROID_SDK_HOME
     export GRADLE_USER_HOME=$GRADLE_HOME
+    export PATH=$ANDROID_NDK_HOME/[% c("var/toolchain_arch") %]/bin:$PATH

If we put the toolchain at the end, are there situations where a system executable is used instead of the Android toolchain?

I'll need to check old logs, but yes some projects just look for clang without the host info so it picks up the system executable for clang.

In that case, it seems better to have the Android toolchain first in the PATH. Or are there any cases where we want a system executable to be used instead of the version from tho Android toolchain?

comment:8 in reply to:  7 ; Changed 8 months ago by sisbell

Replying to boklm:

Replying to sisbell:

Replying to sysrqb:

This is based on commit f5fdff3bf0b9e957ab011f95af9e215cca7aeed6 from ticket:28704#comment:23.

diff --git a/projects/android-toolchain/config b/projects/android-toolchain/config
index 8c9c8222..4c02b2f3 100644
--- a/projects/android-toolchain/config
+++ b/projects/android-toolchain/config
@@ -15,6 +15,7 @@ var:
     export GRADLE_HOME=/var/tmp/dist/[% project %]/gradle
     export ANDROID_HOME=$ANDROID_SDK_HOME
     export GRADLE_USER_HOME=$GRADLE_HOME
+    export PATH=$ANDROID_NDK_HOME/[% c("var/toolchain_arch") %]/bin:$PATH

If we put the toolchain at the end, are there situations where a system executable is used instead of the Android toolchain?

I'll need to check old logs, but yes some projects just look for clang without the host info so it picks up the system executable for clang.

In that case, it seems better to have the Android toolchain first in the PATH. Or are there any cases where we want a system executable to be used instead of the version from tho Android toolchain?

Yes, firefox project will use tools like make from linux. Previously I put Android PATH first but then needed to reverse the path back in the firefox build. I just needed to make the zstd project to use correct variant so everything works with the system path first.

comment:9 in reply to:  8 Changed 8 months ago by boklm

Resolution: fixed
Status: needs_informationclosed

Replying to sisbell:

Yes, firefox project will use tools like make from linux. Previously I put Android PATH first but then needed to reverse the path back in the firefox build. I just needed to make the zstd project to use correct variant so everything works with the system path first.

Ah yes, the Android toolchain contains a lot of things, so it makes sense to include it last in the PATH. I merged the patch to master with commit 457b60475f388bfb381abe3796d881fd5dbe7153, thanks!

comment:10 Changed 8 months ago by sisbell

Points: .25
Note: See TracTickets for help on using tickets.