Update MB to use `gn analyze`.

Update MB to use `gn analyze`.

Now that GN supports a form of `analyze` directly, this patch
updates //tools/mb to use it rather than working awkwardly through
`gn refs`. The MB implementation now is mostly responsible for
mapping ninja targets to labels, calling GN, and then mapping returned
labels back to ninja targets. Eventually we should update the
recipes to just pass labels around instead.

One significant change is that now we need entries in gn_isolate_map.pyl
for every target listed in 'additional_compile_targets' and
'isolated_scripts' as well as in 'gtest_tests', because we have to remap
everything to the GN labels, not just tests for swarming.

We should switch everything to using GN labels directly, but that'll require
follow-on CLs.

R=brettw@chromium.org
BUG=555273
Committed: https://crrev.com/9aba8b21177df3ed3e897d5c500d845fbf5523e6
Cr-Commit-Position: refs/heads/master@{#419315}

[Dirk Pranke, 2016-08-31 21:20:00]

This depends on rolling `gn analyze` in first, obviously.

https://codereview.chromium.org/2298943002/

[brettw, 2016-09-06 22:35:58]

lgtm

[Dirk Pranke, 2016-09-14 04:26:14]

The CQ bit was checked by dpranke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 04:26:28]

Dry run: 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-14 05:36:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-14 05:36:07]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2016-09-16 01:36:08]

Description was changed from

==========
Update MB to use `gn analyze`.

Now that GN supports a form of `analyze` directly, this patch
updates //tools/mb to use it rather than working awkwardly through
`gn refs`. The MB implementation now is mostly responsible for
mapping ninja targets to labels, calling GN, and then mapping returned
labels back to ninja targets. Eventually we should update the
recipes to just pass labels around instead.

R=brettw@chromium.org
BUG=555273
==========

to

==========
Update MB to use `gn analyze`.

Now that GN supports a form of `analyze` directly, this patch
updates //tools/mb to use it rather than working awkwardly through
`gn refs`. The MB implementation now is mostly responsible for
mapping ninja targets to labels, calling GN, and then mapping returned
labels back to ninja targets. Eventually we should update the
recipes to just pass labels around instead.

One significant change is that now we need entries in gn_isolate_map.pyl
for every target listed in 'additional_compile_targets' and
'isolated_scripts' as well as in 'gtest_tests', because we have to remap
everything to the GN labels, not just tests for swarming.

We should switch everything to using GN labels directly, but that'll require
follow-on CLs.

R=brettw@chromium.org
BUG=555273
==========

[Dirk Pranke, 2016-09-16 01:36:08]

dpranke@chromium.org changed reviewers:
+ kbr@chromium.org, phajdan.jr@chromium.org

[Dirk Pranke, 2016-09-16 01:36:44]

@kbr, @phajdan.jr - You might be interested to the changes to `mb analyze` and
the gn_isolate_map.pyl since it affects the recipes a bit.

https://codereview.chromium.org/2298403002/diff/120001/testing/buildbot/manag...
File testing/buildbot/manage.py (right):

https://codereview.chromium.org/2298403002/diff/120001/testing/buildbot/manag...
testing/buildbot/manage.py:266: name = d['isolate_name']
@kbr - it seems like maybe for isolate_scripts I should also (or only) be
checking for override_compile_targets, but it's not clear if that actually
produces anything different, so maybe this is good enough?

https://codereview.chromium.org/2298403002/diff/120001/testing/buildbot/trybo...
File testing/buildbot/trybot_analyze_config.json (left):

https://codereview.chromium.org/2298403002/diff/120001/testing/buildbot/trybo...
testing/buildbot/trybot_analyze_config.json:24: "testing/buildbot/.*",
These changes are just for testing (since otherwise analyze is skipped), but
should not be landed.

[Paweł Hajdan Jr., 2016-09-16 17:51:30]

LGTM

[Ken Russell (switch to Gerrit), 2016-09-16 18:13:32]

Generally LGTM. One question and comment.

https://codereview.chromium.org/2298403002/diff/120001/testing/buildbot/manag...
File testing/buildbot/manage.py (right):

https://codereview.chromium.org/2298403002/diff/120001/testing/buildbot/manag...
testing/buildbot/manage.py:266: name = d['isolate_name']
On 2016/09/16 01:36:44, Dirk Pranke wrote:
> @kbr - it seems like maybe for isolate_scripts I should also (or only) be
> checking for override_compile_targets, but it's not clear if that actually
> produces anything different, so maybe this is good enough?

From examining src/testing/buildbot/chromium.gpu.json and chromium.gpu.fyi.json
it looks like override_compile_targets is being used to select the legacy _run
targets for these isolates. Since everything's on GN now we should be able to
stop using those, right?

I do think this code should probably prefer override_compile_targets though.

[Dirk Pranke, 2016-09-16 18:55:29]

On 2016/09/16 18:13:32, Ken Russell wrote:
> Generally LGTM. One question and comment.
> 
>
https://codereview.chromium.org/2298403002/diff/120001/testing/buildbot/manag...
> File testing/buildbot/manage.py (right):
> 
>
https://codereview.chromium.org/2298403002/diff/120001/testing/buildbot/manag...
> testing/buildbot/manage.py:266: name = d['isolate_name']
> On 2016/09/16 01:36:44, Dirk Pranke wrote:
> > @kbr - it seems like maybe for isolate_scripts I should also (or only) be
> > checking for override_compile_targets, but it's not clear if that actually
> > produces anything different, so maybe this is good enough?
> 
> From examining src/testing/buildbot/chromium.gpu.json and
chromium.gpu.fyi.json
> it looks like override_compile_targets is being used to select the legacy _run
> targets for these isolates. Since everything's on GN now we should be able to
> stop using those, right?

Right, in practice at the moment, on non-Android platforms, there's no
difference between
the binary target and the _run target. However, I would like to actually move
the 
management of the command lines needed to run the tests out of MB and into the
_run 
targets (which is what Android does now), and I don't think we should get rid of
them.

At that time, it would matter that we'd be using the _run targets instead, I
think.

However, I'd like to do things in steps, so let's land this as-is and continue
iterating along
those lines.

[Dirk Pranke, 2016-09-16 21:49:19]

The CQ bit was checked by dpranke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 21:50:08]

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

[Dirk Pranke, 2016-09-16 22:25:03]

The CQ bit was unchecked by dpranke@chromium.org

[Dirk Pranke, 2016-09-16 22:25:03]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2016-09-16 22:25:04]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org,
kbr@chromium.org, phajdan.jr@chromium.org
Link to the patchset: https://codereview.chromium.org/2298403002/#ps160001
(title: "back out test-related changes")

[commit-bot: I haz the power, 2016-09-16 22:25:41]

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-16 22:53:12]

Description was changed from

==========
Update MB to use `gn analyze`.

Now that GN supports a form of `analyze` directly, this patch
updates //tools/mb to use it rather than working awkwardly through
`gn refs`. The MB implementation now is mostly responsible for
mapping ninja targets to labels, calling GN, and then mapping returned
labels back to ninja targets. Eventually we should update the
recipes to just pass labels around instead.

One significant change is that now we need entries in gn_isolate_map.pyl
for every target listed in 'additional_compile_targets' and
'isolated_scripts' as well as in 'gtest_tests', because we have to remap
everything to the GN labels, not just tests for swarming.

We should switch everything to using GN labels directly, but that'll require
follow-on CLs.

R=brettw@chromium.org
BUG=555273
==========

to

==========
Update MB to use `gn analyze`.

Now that GN supports a form of `analyze` directly, this patch
updates //tools/mb to use it rather than working awkwardly through
`gn refs`. The MB implementation now is mostly responsible for
mapping ninja targets to labels, calling GN, and then mapping returned
labels back to ninja targets. Eventually we should update the
recipes to just pass labels around instead.

One significant change is that now we need entries in gn_isolate_map.pyl
for every target listed in 'additional_compile_targets' and
'isolated_scripts' as well as in 'gtest_tests', because we have to remap
everything to the GN labels, not just tests for swarming.

We should switch everything to using GN labels directly, but that'll require
follow-on CLs.

R=brettw@chromium.org
BUG=555273
==========

[commit-bot: I haz the power, 2016-09-16 22:53:13]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-09-16 22:56:27]

Description was changed from

==========
Update MB to use `gn analyze`.

Now that GN supports a form of `analyze` directly, this patch
updates //tools/mb to use it rather than working awkwardly through
`gn refs`. The MB implementation now is mostly responsible for
mapping ninja targets to labels, calling GN, and then mapping returned
labels back to ninja targets. Eventually we should update the
recipes to just pass labels around instead.

One significant change is that now we need entries in gn_isolate_map.pyl
for every target listed in 'additional_compile_targets' and
'isolated_scripts' as well as in 'gtest_tests', because we have to remap
everything to the GN labels, not just tests for swarming.

We should switch everything to using GN labels directly, but that'll require
follow-on CLs.

R=brettw@chromium.org
BUG=555273
==========

to

==========
Update MB to use `gn analyze`.

Now that GN supports a form of `analyze` directly, this patch
updates //tools/mb to use it rather than working awkwardly through
`gn refs`. The MB implementation now is mostly responsible for
mapping ninja targets to labels, calling GN, and then mapping returned
labels back to ninja targets. Eventually we should update the
recipes to just pass labels around instead.

One significant change is that now we need entries in gn_isolate_map.pyl
for every target listed in 'additional_compile_targets' and
'isolated_scripts' as well as in 'gtest_tests', because we have to remap
everything to the GN labels, not just tests for swarming.

We should switch everything to using GN labels directly, but that'll require
follow-on CLs.

R=brettw@chromium.org
BUG=555273

Committed: https://crrev.com/9aba8b21177df3ed3e897d5c500d845fbf5523e6
Cr-Commit-Position: refs/heads/master@{#419315}
==========

[commit-bot: I haz the power, 2016-09-16 22:56:30]

Patchset 9 (id:??) landed as
https://crrev.com/9aba8b21177df3ed3e897d5c500d845fbf5523e6
Cr-Commit-Position: refs/heads/master@{#419315}

[mmenke, 2016-09-16 23:10:15]

On 2016/09/16 22:56:30, commit-bot: I haz the power wrote:
> Patchset 9 (id:??) landed as
> https://crrev.com/9aba8b21177df3ed3e897d5c500d845fbf5523e6
> Cr-Commit-Position: refs/heads/master@{#419315}

Try runs all seem to be failing in the "analyze" phase on all platforms.  Is
this CL to blame?

See for example:  https://codereview.chromium.org/2346863003/

(It's not just my CL, looking around on the trybots, incrementing build numbers,
also see other CLs failing)

[Dirk Pranke, 2016-09-16 23:15:01]

On 2016/09/16 23:10:15, mmenke wrote:
> On 2016/09/16 22:56:30, commit-bot: I haz the power wrote:
> > Patchset 9 (id:??) landed as
> > https://crrev.com/9aba8b21177df3ed3e897d5c500d845fbf5523e6
> > Cr-Commit-Position: refs/heads/master@{#419315}
> 
> Try runs all seem to be failing in the "analyze" phase on all platforms.  Is
> this CL to blame?
> 
> See for example:  https://codereview.chromium.org/2346863003/
> 
> (It's not just my CL, looking around on the trybots, incrementing build
numbers,
> also see other CLs failing)

Ayup. Reverting.

[Dirk Pranke, 2016-09-16 23:15:25]

A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/2341403004/ by dpranke@chromium.org.

The reason for reverting is: Need to add "aura_builder", probably others ....