Add analyze steps to iOS try bot builds which use ninja

Add analyze steps to iOS try bot builds which use ninja

Note that the results of analyze are not currently used.

BUG=454416
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=294264

[smut, 2015-02-24 19:34:26]

Patchset #1 (id:1) has been deleted

[smut, 2015-02-24 19:37:00]

smut@google.com changed reviewers:
+ jam@chromium.org, phajdan.jr@chromium.org, sky@chromium.org

[smut, 2015-02-24 19:37:01]

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/filter/api.py (right):

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/filter/api.py:139: if set_env:
Added this because iOS recipes do not have a Chromium config set. I'd rather not
set it right now because iOS's gyp environment variables are very specific and
what we have is currently working.

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/ios/api.py (left):

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/ios/api.py:125: def build(self, official=False):
We were only using the official kwarg downstream. This shouldn't have been
upstreamed in the first place.

[jam, 2015-02-24 23:18:04]

Thank you for adding this. I defer to Scott for the review since he's much more
familiar with analyze than me.

[Paweł Hajdan Jr., 2015-02-25 15:23:37]

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/filter/api.py (right):

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/filter/api.py:139: if set_env:
On 2015/02/24 19:37:01, smut wrote:
> Added this because iOS recipes do not have a Chromium config set. I'd rather
not
> set it right now because iOS's gyp environment variables are very specific and
> what we have is currently working.

Before the ios recipes we had ios build on main waterfall and trybots using the
chromium config. What is preventing us from doing the same here?

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/ios/api.py (right):

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/ios/api.py:176:
self.m.filter.does_patch_require_compile(
Consider using "analyze" from chromium_tests. It already prints useful debugging
info.

[smut, 2015-02-25 18:46:50]

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/filter/api.py (right):

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/filter/api.py:139: if set_env:
On 2015/02/25 15:23:37, Paweł Hajdan Jr. wrote:
> On 2015/02/24 19:37:01, smut wrote:
> > Added this because iOS recipes do not have a Chromium config set. I'd rather
> not
> > set it right now because iOS's gyp environment variables are very specific
and
> > what we have is currently working.
> 
> Before the ios recipes we had ios build on main waterfall and trybots using
the
> chromium config. What is preventing us from doing the same here?

For one, the GYP_DEFINES are set by the developers in config files that live in
the Chromium repository. I don't want to use the Chromium recipe config because
it makes other decisions about the values of those variables.

I also don't want to regress on something that's working. We really need to get
this stuff up and running because many outstanding bugs depend on it.

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/ios/api.py (right):

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/ios/api.py:176:
self.m.filter.does_patch_require_compile(
On 2015/02/25 15:23:37, Paweł Hajdan Jr. wrote:
> Consider using "analyze" from chromium_tests. It already prints useful
debugging
> info.

Done.

[Paweł Hajdan Jr., 2015-02-26 21:42:04]

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/filter/api.py (right):

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/filter/api.py:139: if set_env:
On 2015/02/25 18:46:49, smut wrote:
> For one, the GYP_DEFINES are set by the developers in config files that live
in
> the Chromium repository. I don't want to use the Chromium recipe config
because
> it makes other decisions about the values of those variables.

I'm really concerned about this.

Using GYP_DEFINES from chromium repo sounds fine. Why not just put the value
obtained from that into GYP_DEFINES in chromium recipe module config?

This would allow us to re-use all the existing infrastructure that respects the
config (note that in most places we just read the values - you don't have to use
the "mainstream" ways of setting GYP_DEFINES in chromium config, it's fine to
put your own value there), as opposed to introducing even more special cases
throughout the modules.
 
> I also don't want to regress on something that's working. We really need to
get
> this stuff up and running because many outstanding bugs depend on it.

Makes sense to me. I'm just thinking about the best way to integrate it.

[smut, 2015-02-27 00:30:01]

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/filter/api.py (right):

https://codereview.chromium.org/957493002/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/filter/api.py:139: if set_env:
On 2015/02/26 21:42:04, Paweł Hajdan Jr. wrote:
> On 2015/02/25 18:46:49, smut wrote:
> > For one, the GYP_DEFINES are set by the developers in config files that live
> in
> > the Chromium repository. I don't want to use the Chromium recipe config
> because
> > it makes other decisions about the values of those variables.
> 
> I'm really concerned about this.
> 
> Using GYP_DEFINES from chromium repo sounds fine. Why not just put the value
> obtained from that into GYP_DEFINES in chromium recipe module config?
> 
> This would allow us to re-use all the existing infrastructure that respects
the
> config (note that in most places we just read the values - you don't have to
use
> the "mainstream" ways of setting GYP_DEFINES in chromium config, it's fine to
> put your own value there), as opposed to introducing even more special cases
> throughout the modules.

Okay, that makes sense. I thought you wanted me to change it around so we use
configs. If you just want the GYP_DEFINES that are read from the source
repository set in the config, then that should be okay. I've done that.

>  
> > I also don't want to regress on something that's working. We really need to
> get
> > this stuff up and running because many outstanding bugs depend on it.
> 
> Makes sense to me. I'm just thinking about the best way to integrate it.

[Paweł Hajdan Jr., 2015-02-27 08:08:13]

Thanks, Sana! I really like improvements in this CL.

LGTM w/comment; feel free to land with "c", and for any other solution,
including leaving "make_config()" call, please get a review from Robbie or
Aaron.

https://codereview.chromium.org/957493002/diff/60001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/ios/api.py (right):

https://codereview.chromium.org/957493002/diff/60001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/ios/api.py:127:
self.m.chromium.make_config().gyp_env.GYP_DEFINES = copy.deepcopy(
Could you replace "make_config()" with "c"? We use the latter and I haven't seen
the former much.

Please let me know if this doesn't work, and then consider asking Robbie or
Aaron for the best solution. I'll be fine with any recommendation they make.

My specific concern here is whether make_config() creates a _temporary_ config
object, and so whether other modules are going to see this change. If the
current code works, "c" should also work and would be more consistent with other
code.

[smut, 2015-02-27 19:09:06]

https://codereview.chromium.org/957493002/diff/60001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/ios/api.py (right):

https://codereview.chromium.org/957493002/diff/60001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/ios/api.py:127:
self.m.chromium.make_config().gyp_env.GYP_DEFINES = copy.deepcopy(
On 2015/02/27 08:08:13, Paweł Hajdan Jr. wrote:
> Could you replace "make_config()" with "c"? We use the latter and I haven't
seen
> the former much.
> 
> Please let me know if this doesn't work, and then consider asking Robbie or
> Aaron for the best solution. I'll be fine with any recommendation they make.
> 
> My specific concern here is whether make_config() creates a _temporary_ config
> object, and so whether other modules are going to see this change. If the
> current code works, "c" should also work and would be more consistent with
other
> code.

Okay, I figured out the actual correct way to do this.

[smut, 2015-02-27 19:23:04]

The CQ bit was checked by smut@google.com

[smut, 2015-02-27 19:23:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
phajdan.jr@chromium.org
Link to the patchset: https://codereview.chromium.org/957493002/#ps80001 (title:
" ")

[commit-bot: I haz the power, 2015-02-27 19:23:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/957493002/80001

[commit-bot: I haz the power, 2015-02-27 19:27:21]

Committed patchset #4 (id:80001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=294264