Fix configuration of goma in iOS recipes.

Fix configuration of goma in iOS recipes.

The way we were configuring the chromium module so that goma
would work in compiles didn't work correctly after we switched
the location of goma on the machine; the 'goma' config no longer
gives the right directory, and the `ensure_goma` step wasn't
being called reliably and consistently.

This CL reworks things so that we call ensure_goma consistently
when needed during the read_build_config() step of the ios
recipes, rather than during the build() step.

This CL also fixes an issue where if the GYP_DEFINES were specified
as a list of strings in the build config rather than a dict, things
wouldn't work right. We need this fix so that we can use goma
even with GYP, in a way that is consistent between GYP and GN.

R=smut@chromium.org
BUG=626033
Committed: https://chromium.googlesource.com/chromium/tools/build/+/d4098d2dd3fb9ab846e3f31702afcb0843a4f4ac

[smut, 2016-07-07 22:58:27]

smut@google.com changed reviewers:
+ smut@google.com

[smut, 2016-07-07 22:58:28]

lgtm

https://codereview.chromium.org/2133483002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/ios/api.py (right):

https://codereview.chromium.org/2133483002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/ios/api.py:226: if use_goma:
Do we need the "if 'without patch' not in suffix" condition? I think there was
some issue re-applying these configs for the without patch retry. Unless it's
fixed now to be idempotent.

https://codereview.chromium.org/2133483002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/ios/api.py:234: # we gett the right
c.compile_py.goma_dir .
s/gett/get/

[Dirk Pranke, 2016-07-07 23:05:36]

https://codereview.chromium.org/2133483002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/ios/api.py (right):

https://codereview.chromium.org/2133483002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/ios/api.py:226: if use_goma:
On 2016/07/07 22:58:28, smut wrote:
> Do we need the "if 'without patch' not in suffix" condition? I think there was
> some issue re-applying these configs for the without patch retry. Unless it's
> fixed now to be idempotent.

This block is moved from build() to read_build_config(). We needed 
the if-check in build() because it might get called twice, but we can 
only apply the configs once. However, read_build_config() is only 
called once, so we don't need the check.

I.e., this change is actually doing the refactor mentioned in the TODO, below.

https://codereview.chromium.org/2133483002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/ios/api.py:228: # rest of the recipe.
These are needed for m.chromium.compile() to work properly w/ goma+ninja. I will
update the comment to make that clearer.

[Dirk Pranke, 2016-07-07 23:05:59]

https://codereview.chromium.org/2133483002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/ios/api.py (right):

https://codereview.chromium.org/2133483002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/ios/api.py:234: # we gett the right
c.compile_py.goma_dir .
On 2016/07/07 22:58:28, smut wrote:
> s/gett/get/

Done.

[Dirk Pranke, 2016-07-07 23:11:18]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2016-07-07 23:11:19]

The patchset sent to the CQ was uploaded after l-g-t-m from smut@google.com
Link to the patchset: https://codereview.chromium.org/2133483002/#ps40001
(title: "improve wording in comment")

[commit-bot: I haz the power, 2016-07-07 23:11:22]

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-07-07 23:15:39]

Description was changed from

==========
Fix configuration of goma in iOS recipes.

The way we were configuring the chromium module so that goma
would work in compiles didn't work correctly after we switched
the location of goma on the machine; the 'goma' config no longer
gives the right directory, and the `ensure_goma` step wasn't
being called reliably and consistently.

This CL reworks things so that we call ensure_goma consistently
when needed during the read_build_config() step of the ios
recipes, rather than during the build() step.

This CL also fixes an issue where if the GYP_DEFINES were specified
as a list of strings in the build config rather than a dict, things
wouldn't work right. We need this fix so that we can use goma
even with GYP, in a way that is consistent between GYP and GN.

R=smut@chromium.org
BUG=626033
==========

to

==========
Fix configuration of goma in iOS recipes.

The way we were configuring the chromium module so that goma
would work in compiles didn't work correctly after we switched
the location of goma on the machine; the 'goma' config no longer
gives the right directory, and the `ensure_goma` step wasn't
being called reliably and consistently.

This CL reworks things so that we call ensure_goma consistently
when needed during the read_build_config() step of the ios
recipes, rather than during the build() step.

This CL also fixes an issue where if the GYP_DEFINES were specified
as a list of strings in the build config rather than a dict, things
wouldn't work right. We need this fix so that we can use goma
even with GYP, in a way that is consistent between GYP and GN.

R=smut@chromium.org
BUG=626033

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

[commit-bot: I haz the power, 2016-07-07 23:15:41]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/tools/build/+/d4098d2dd3fb9ab846e3...