Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(28)

Issue 142393002: android: Use use_goma=1 gomadir gyp defines instead of magic GOMA_DIR env var. (Closed)

Created:
6 years, 11 months ago by Nico
Modified:
6 years, 11 months ago
Reviewers:
Isaac (away), Torne, brettw
CC:
chromium-reviews, craigdh+watch_chromium.org, bulach+watch_chromium.org, yfriedman+watch_chromium.org, ilevy-cc_chromium.org, klundberg+watch_chromium.org, frankf+watch_chromium.org, bulach, Anton, Yoshisato Yanagisawa
Visibility:
Public.

Description

android: Use use_goma=1 gomadir gyp defines instead of magic GOMA_DIR env var. For a short transition period, let envsetup.sh set the right GYP_DEFINES when GOMA_DIR is set, so that people aren't broken immediately after this lands. Also stop setting GOMA_COMPILER_PROXY_THREADS – if the value this sets is truly better, that should be the default in goma, it shouldn't be set in envsetup.sh (doing it in goma has the advantage that it works on all platforms, also abstraction barriers, etc.) BUG=332697 R=brettw@chromium.org, ilevy@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245801

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Patch Set 4 : . #

Total comments: 2

Patch Set 5 : . #

Patch Set 6 : test #

Patch Set 7 : test2 #

Patch Set 8 : test3 #

Patch Set 9 : test4 #

Patch Set 10 : rebase #

Patch Set 11 : notest #

Patch Set 12 : nowat #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -54 lines) Patch
M build/android/buildbot/bb_run_bot.py View 2 chunks +2 lines, -2 lines 0 comments Download
M build/android/envsetup.sh View 1 2 3 4 1 chunk +3 lines, -8 lines 0 comments Download
M build/common.gypi View 1 2 3 4 5 6 7 8 9 2 chunks +1 line, -12 lines 0 comments Download
D build/toolchain/android/default_goma_for_android.py View 1 1 chunk +0 lines, -17 lines 0 comments Download
M build/toolchain/goma.gni View 1 2 chunks +0 lines, -15 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Nico
6 years, 11 months ago (2014-01-18 10:07:55 UTC) #1
Isaac (away)
android change looks fine, other stuff seems minor enough for an lgtm w/ nit https://codereview.chromium.org/142393002/diff/70001/build/android/buildbot/bb_run_bot.py ...
6 years, 11 months ago (2014-01-18 10:22:17 UTC) #2
brettw
LGTM. Will this break some people?
6 years, 11 months ago (2014-01-18 17:35:17 UTC) #3
Nico
On 2014/01/18 17:35:17, brettw wrote: > LGTM. Will this break some people? Great question – ...
6 years, 11 months ago (2014-01-18 18:59:01 UTC) #4
Nico
I just spent some time debugging why the android bots came back red just to ...
6 years, 11 months ago (2014-01-18 22:08:11 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thakis@chromium.org/142393002/380001
6 years, 11 months ago (2014-01-18 22:08:39 UTC) #6
Nico
6 years, 11 months ago (2014-01-18 23:31:26 UTC) #7
Nico
6 years, 11 months ago (2014-01-18 23:54:49 UTC) #8
Message was sent while issue was closed.
Committed patchset #12 manually as r245801.

Powered by Google App Engine
This is Rietveld 408576698