New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 616819 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 605315



Sign in to add a comment

Non-default Android toolchains do not work

Project Member Reported by agrieve@chromium.org, Jun 2 2016

Issue description

e.g. create a group:

group("foo") {
  deps = [
    "//chrome/android:chrome_public_apk(//build/toolchain/android:arm),
    "//chrome/android:chrome_public_apk",
  ]
}

Set target_cpu = "arm64"

The arm compile still tries to use the arm64 compiler. Adding:
  print("$target_name $tool_prefix")
to:
  //build/toolchain/android:android_gcc_toolchain shows:

x86 ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
clang_x86 ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
arm ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
clang_arm ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
mipsel ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
clang_mipsel ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
x64 ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
clang_x64 ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
arm64 ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
clang_arm64 ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
mips64el ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
clang_mips64el ../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-
 
Blocking: 605315
Status: Started (was: Assigned)
So, after looking at this, the main fix (bug in the toolchain definitions) is:
https://codereview.chromium.org/2036263002/

However, there are a lot of places that "target_cpu" is being used where "current_cpu" should be used instead. I've tried to fix all relevant-to-android ones:
https://codereview.chromium.org/2034133002/
https://codereview.chromium.org/2037113002/
https://codereview.chromium.org/2033393003/
https://codereview.chromium.org/2035753005/
https://codereview.chromium.org/2031953004/
https://codereview.chromium.org/2031403002/
https://codereview.chromium.org/2041443002/
https://codereview.chromium.org/2041433002/
https://codereview.chromium.org/2038113002/
https://codereview.chromium.org/2031923004/
https://codereview.chromium.org/2038563004/
https://codereview.chromium.org/2035113003/
https://codereview.chromium.org/2038083003/
https://codereview.chromium.org/2039523002/


Given how many there were, and how easy it is to use target_* rather than current_*, we should certainly proceed with caution when thinking about changing our release builders from their current approach of building twices and merging. It's almost certainly going to be the case that a current_cpu vs target_cpu change will go in unnoticed that causes monochrome to have a subtly different secondary .so when build via secondary toolchain vs merging post-build. 

Perhaps a test that builds monochrome.so in both ways and then diffs them would be a good idea.
Can I understand like this, the target_cpu always matches the default toolchain, while current_cpu is current toolchain to build the target?

If so, current_cpu should always be used, shall we have other way to warning people if target_cpu is used, like in gn format? 

of course, we should have builder to run both cases.
Cc: brettw@chromium.org
That's basically my understanding (that target_cpu should never be used). I've got a change to update the docs here:
https://codereview.chromium.org/2038733003

+brettw in case he has thoughts on this.
As I just noted in a comment on the CL, that's just not right. There are plenty of cases where branching on target_cpu is the correct thing to do.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d13c10da70c170ddbdb044ef752f98a870915eee

commit d13c10da70c170ddbdb044ef752f98a870915eee
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 02:23:09 2016

Make Android toolchains work when current_toolchain != target_toolchain

"android_tool_prefix" within the toolchain definitions was always being
keyed off of target_cpu rather than the toolchain cpu.

BUG= 616819 

Review-Url: https://codereview.chromium.org/2036263002
Cr-Commit-Position: refs/heads/master@{#398463}

[modify] https://crrev.com/d13c10da70c170ddbdb044ef752f98a870915eee/build/toolchain/android/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80377c661947d329b2fafa5e42d84982b9cac54a

commit 80377c661947d329b2fafa5e42d84982b9cac54a
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 15:40:27 2016

target_cpu -> current_cpu in components/BUILD.gn

BUG= 616819 

Review-Url: https://codereview.chromium.org/2035113003
Cr-Commit-Position: refs/heads/master@{#398563}

[modify] https://crrev.com/80377c661947d329b2fafa5e42d84982b9cac54a/components/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eae8b8fcfac94b2cea55a0801ae95cc272d97fc1

commit eae8b8fcfac94b2cea55a0801ae95cc272d97fc1
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 15:41:17 2016

target_cpu -> current_cpu in tools/perf/chrome_telemetry_build/BUILD.gn

BUG= 616819 

Review-Url: https://codereview.chromium.org/2041443002
Cr-Commit-Position: refs/heads/master@{#398564}

[modify] https://crrev.com/eae8b8fcfac94b2cea55a0801ae95cc272d97fc1/tools/perf/chrome_telemetry_build/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d3e99c39e092e6154d2f2f70c803395d1eeeb668

commit d3e99c39e092e6154d2f2f70c803395d1eeeb668
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 15:42:17 2016

target_cpu -> current_cpu in mojo/BUILD.gn

BUG= 616819 

Review-Url: https://codereview.chromium.org/2031923004
Cr-Commit-Position: refs/heads/master@{#398565}

[modify] https://crrev.com/d3e99c39e092e6154d2f2f70c803395d1eeeb668/mojo/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1dfe1f8c72091bf6b3df7a510dd77a796769bf9d

commit 1dfe1f8c72091bf6b3df7a510dd77a796769bf9d
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 15:43:14 2016

target_cpu -> current_cpu in chrome/BUILD.gn

BUG= 616819 

Review-Url: https://codereview.chromium.org/2031953004
Cr-Commit-Position: refs/heads/master@{#398566}

[modify] https://crrev.com/1dfe1f8c72091bf6b3df7a510dd77a796769bf9d/chrome/BUILD.gn

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24550c794619b7574fcfc7c266bd6441b833e532

commit 24550c794619b7574fcfc7c266bd6441b833e532
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 15:44:09 2016

target_cpu -> current_cpu in build/config/compiler/BUILD.gn

BUG= 616819 

Review-Url: https://codereview.chromium.org/2034133002
Cr-Commit-Position: refs/heads/master@{#398567}

[modify] https://crrev.com/24550c794619b7574fcfc7c266bd6441b833e532/build/config/compiler/BUILD.gn

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/efcd022d8ae52aab3dca427531b040d07e57e239

commit efcd022d8ae52aab3dca427531b040d07e57e239
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 15:48:19 2016

target_cpu -> current_cpu in cc/BUILD.gn

BUG= 616819 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2035753005
Cr-Commit-Position: refs/heads/master@{#398569}

[modify] https://crrev.com/efcd022d8ae52aab3dca427531b040d07e57e239/cc/BUILD.gn

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f0a841cc69c359dc2bb39d7fcd33e972c100c030

commit f0a841cc69c359dc2bb39d7fcd33e972c100c030
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 15:48:53 2016

target_cpu -> current_cpu in gpu/vulkan/BUILD.gn

and gpu/tools/compositor_model_bench/BUILD.gn

target_cpu does not differ based on the current toolchain,
so it should basically never be used.

BUG= 616819 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2031403002
Cr-Commit-Position: refs/heads/master@{#398570}

[modify] https://crrev.com/f0a841cc69c359dc2bb39d7fcd33e972c100c030/gpu/tools/compositor_model_bench/BUILD.gn
[modify] https://crrev.com/f0a841cc69c359dc2bb39d7fcd33e972c100c030/gpu/vulkan/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a35f7d85bc36253ff07033efeddac4d1cb188068

commit a35f7d85bc36253ff07033efeddac4d1cb188068
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 15:54:47 2016

target_cpu -> current_cpu in chrome/android/chrome_public_apk_tmpl.gni

BUG= 616819 

Review-Url: https://codereview.chromium.org/2039523002
Cr-Commit-Position: refs/heads/master@{#398573}

[modify] https://crrev.com/a35f7d85bc36253ff07033efeddac4d1cb188068/chrome/android/chrome_public_apk_tmpl.gni

Project Member

Comment 15 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea632704f3232934db1f0b21c1f8c9259d39229a

commit ea632704f3232934db1f0b21c1f8c9259d39229a
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 16:03:20 2016

target_cpu -> current_cpu in remoting/android/BUILD.gn, remoting_apk_tmpl.gni

BUG= 616819 

Review-Url: https://codereview.chromium.org/2038113002
Cr-Commit-Position: refs/heads/master@{#398577}

[modify] https://crrev.com/ea632704f3232934db1f0b21c1f8c9259d39229a/remoting/android/BUILD.gn
[modify] https://crrev.com/ea632704f3232934db1f0b21c1f8c9259d39229a/remoting/android/remoting_apk_tmpl.gni

Project Member

Comment 16 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c5f01db551857816c696b5ce556831c7a3af9d7f

commit c5f01db551857816c696b5ce556831c7a3af9d7f
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 16:12:51 2016

target_cpu -> current_cpu in base/BUILD.gn

BUG= 616819 

Review-Url: https://codereview.chromium.org/2033393003
Cr-Commit-Position: refs/heads/master@{#398586}

[modify] https://crrev.com/c5f01db551857816c696b5ce556831c7a3af9d7f/base/BUILD.gn

let me know if all patches are landed.
Project Member

Comment 18 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bfdee36d31a624d330b60b481fc26d10dff81b97

commit bfdee36d31a624d330b60b481fc26d10dff81b97
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 20:30:15 2016

Update GN docs about target_cpu to encourage use of current_cpu

BUG= 616819 

Review-Url: https://codereview.chromium.org/2038733003
Cr-Commit-Position: refs/heads/master@{#398661}

[modify] https://crrev.com/bfdee36d31a624d330b60b481fc26d10dff81b97/tools/gn/docs/cross_compiles.md
[modify] https://crrev.com/bfdee36d31a624d330b60b481fc26d10dff81b97/tools/gn/variables.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0057613281eaa604abf1443a27554518d2dbd34f

commit 0057613281eaa604abf1443a27554518d2dbd34f
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 20:55:54 2016

target_cpu -> current_cpu in build/config/sanitizers/BUILD.gn

BUG= 616819 

Review-Url: https://codereview.chromium.org/2041433002
Cr-Commit-Position: refs/heads/master@{#398666}

[modify] https://crrev.com/0057613281eaa604abf1443a27554518d2dbd34f/build/config/sanitizers/BUILD.gn

Project Member

Comment 20 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d0d82853786820b8d37a7bd55ae9a8662038592

commit 0d0d82853786820b8d37a7bd55ae9a8662038592
Author: agrieve <agrieve@chromium.org>
Date: Wed Jun 08 23:34:19 2016

target_cpu -> current_cpu in media/gpu/BUILD.gn, media/cdm/ppapi/cdm_paths.gni

BUG= 616819 

Review-Url: https://codereview.chromium.org/2038563004
Cr-Commit-Position: refs/heads/master@{#398733}

[modify] https://crrev.com/0d0d82853786820b8d37a7bd55ae9a8662038592/media/cdm/ppapi/cdm_paths.gni
[modify] https://crrev.com/0d0d82853786820b8d37a7bd55ae9a8662038592/media/gpu/BUILD.gn

Status: Fixed (was: Started)
Alight, I think I've updated enough of the target_cpu sites that it should now build. I haven't done any checks to see whether the binary produced using a secondary toolchain is the same size as one produced setting target_cpu="arm" directly, but I'll leave that to you :).
Project Member

Comment 22 by bugdroid1@chromium.org, Jun 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c50a654e54c64a127782fca76e758fb1607fad14

commit c50a654e54c64a127782fca76e758fb1607fad14
Author: agrieve <agrieve@chromium.org>
Date: Thu Jun 09 03:44:09 2016

target_cpu -> current_cpu in chromeos/BUILD.gn

BUG= 616819 

Review-Url: https://codereview.chromium.org/2038083003
Cr-Commit-Position: refs/heads/master@{#398773}

[modify] https://crrev.com/c50a654e54c64a127782fca76e758fb1607fad14/chromeos/BUILD.gn

Project Member

Comment 23 by bugdroid1@chromium.org, Jun 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5513a1a7d3a5658e192405e432f67df63acf4b0d

commit 5513a1a7d3a5658e192405e432f67df63acf4b0d
Author: agrieve <agrieve@chromium.org>
Date: Thu Jun 09 04:06:13 2016

Fix jmake root_build_dir vs root_out_dir

BUG= 616819 

Review-Url: https://codereview.chromium.org/2037113002
Cr-Commit-Position: refs/heads/master@{#398778}

[modify] https://crrev.com/5513a1a7d3a5658e192405e432f67df63acf4b0d/build/config/android/internal_rules.gni

Sign in to add a comment