Do not start goma in ChromeOS gclient runhook if gomadir is given.

Do not start goma in ChromeOS gclient runhook if gomadir is given.

If gomadir is given, we are very sure we will start goma compiler_proxy
in compile step.  Let me avoid starting goma compiler_proxy in
runhook step, which is redundant and make it difficult for us to
investigate the issue.

BUG=639432
Committed: https://chromium.googlesource.com/chromium/tools/build/+/60507ea6d87dfbee2b248bcdf3a90d4fa84c7592

[Yoshisato Yanagisawa, 2016-09-06 09:24:15]

yyanagisawa@chromium.org changed reviewers:
+ dnj@chromium.org, vapier@chromium.org

[Yoshisato Yanagisawa, 2016-09-06 09:24:16]

yyanagisawa@chromium.org changed required reviewers:
+ dnj@chromium.org

[Yoshisato Yanagisawa, 2016-09-06 09:24:16]

Note that this change needs
https://chromium-review.googlesource.com/#/c/381090/.

[dnj, 2016-09-07 16:09:15]

I think you need to regenerate recipe expectations?

[Yoshisato Yanagisawa, 2016-09-08 01:24:19]

On 2016/09/07 16:09:15, dnj wrote:
> I think you need to regenerate recipe expectations?

I have updated the recipe expectations.  I hope "git cl upload" warns.

[dnj, 2016-09-08 16:14:45]

Update looks like it worked, cool.

"git cl upload" should run presubmit checks, but those may not exercise the
recipe engine in the interest of brevity. The tryjob certainly would have.

https://codereview.chromium.org/2315653002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium/api.py (right):

https://codereview.chromium.org/2315653002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium/api.py:477: wrapper += ['--nostart-goma']
Can you add a comment here detailing why this flag is added? The "--gomadir" one
is obvious, but "--nostart-goma" is a little less so.

As an aside, why don't we just make it so that "chrome-sdk" is smart enough to
not start goma when "--gomadir" is provided? Is there ever a situation where
"--gomadir" will be supplied and we actually want it to start Goma? From this CL
description, it sounds like the answer is, "no".

[Yoshisato Yanagisawa, 2016-09-12 01:55:11]

https://codereview.chromium.org/2315653002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium/api.py (right):

https://codereview.chromium.org/2315653002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium/api.py:477: wrapper += ['--nostart-goma']
Wrote a comment.

I do not know a situation where "--gomadir" is supplied and we actually want it
start goma.  However, I am not confident to say no.
Then, it comes to me the question, "Is it natural to skip starting goma if
--gomadir is set?"  I can hardly say yes.  It looks like the flag to mention
which goma directory to use but it might not look like a flag to change anything
else.  Let me make the script have --nostart-goma.

[dnj, 2016-09-12 16:43:33]

lgtm

[Yoshisato Yanagisawa, 2016-09-13 02:47:18]

The CQ bit was checked by yyanagisawa@chromium.org

[commit-bot: I haz the power, 2016-09-13 02:47:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 02:51:19]

Description was changed from

==========
Do not start goma in ChromeOS gclient runhook if gomadir is given.

If gomadir is given, we are very sure we will start goma compiler_proxy
in compile step.  Let me avoid starting goma compiler_proxy in
runhook step, which is redundant and make it difficult for us to
investigate the issue.

BUG=639432
==========

to

==========
Do not start goma in ChromeOS gclient runhook if gomadir is given.

If gomadir is given, we are very sure we will start goma compiler_proxy
in compile step.  Let me avoid starting goma compiler_proxy in
runhook step, which is redundant and make it difficult for us to
investigate the issue.

BUG=639432

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/60507ea6d87dfbee2b24...
==========

[commit-bot: I haz the power, 2016-09-13 02:51:20]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/tools/build/+/60507ea6d87dfbee2b24...