Add MB support for the windows analyze bot.

Add MB support for the windows analyze bot.

This adds the MB config for the 'Chromium Windows Analyze' bot
on the chromium.fyi waterfall, and adds the needed flag maps
to disable precompiled headers (incl. adding a disable_precompiled_headers
flag to GN) and adding 'win_analyze' mixin (we need to actually
implement the win_analyze GN flag still, though).

R=brettw@chromium.org, brucedawson@chromium.org
BUG=597112
Committed: https://crrev.com/de9261d3072db949f60ffcaf09df404cfd7b807f
Cr-Commit-Position: refs/heads/master@{#382931}

[Dirk Pranke, 2016-03-22 23:52:54]

This needs to land before https://codereview.chromium.org/1821003005/ does.

Brett, please take a look to see if you are happy with me adding
an explicit gn arg to disable precompiled headers.

[brucedawson, 2016-03-23 00:06:21]

On 2016/03/22 23:52:54, Dirk Pranke wrote:
> This needs to land before https://codereview.chromium.org/1821003005/ does.
> 
> Brett, please take a look to see if you are happy with me adding
> an explicit gn arg to disable precompiled headers.

gn support for /analyze was actually added in crrev.com/1658903002 - I hope that
that change is MB compatible.

[Dirk Pranke, 2016-03-23 00:15:42]

On 2016/03/23 00:06:21, brucedawson wrote:
> On 2016/03/22 23:52:54, Dirk Pranke wrote:
> > This needs to land before https://codereview.chromium.org/1821003005/ does.
> > 
> > Brett, please take a look to see if you are happy with me adding
> > an explicit gn arg to disable precompiled headers.
> 
> gn support for /analyze was actually added in crrev.com/1658903002 - I hope
that
> that change is MB compatible.

Oh, yeah, that's fine then. I was looking for "win_analyze" :).

This is straightforward, then. Patch updated; please take a look?

[brucedawson, 2016-03-23 00:22:41]

Looks like black magic to me - I mean lgtm, with one nit.

https://codereview.chromium.org/1827683002/diff/20001/build/config/BUILD.gn
File build/config/BUILD.gn (right):

https://codereview.chromium.org/1827683002/diff/20001/build/config/BUILD.gn#n...
build/config/BUILD.gn:449: # exactly what's in the /FI flag below, and what
might appear in the source
Fix the line length here?

[Dirk Pranke, 2016-03-23 00:42:19]

https://codereview.chromium.org/1827683002/diff/20001/build/config/BUILD.gn
File build/config/BUILD.gn (right):

https://codereview.chromium.org/1827683002/diff/20001/build/config/BUILD.gn#n...
build/config/BUILD.gn:449: # exactly what's in the /FI flag below, and what
might appear in the source
On 2016/03/23 00:22:41, brucedawson wrote:
> Fix the line length here?

ah, good catch.

[Dirk Pranke, 2016-03-23 00:42:24]

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

[commit-bot: I haz the power, 2016-03-23 00:42:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1827683002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1827683002/40001

[commit-bot: I haz the power, 2016-03-23 00:49:51]

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

[commit-bot: I haz the power, 2016-03-23 00:49:52]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Dirk Pranke, 2016-03-23 01:03:22]

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

[commit-bot: I haz the power, 2016-03-23 01:03:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1827683002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1827683002/60001

[commit-bot: I haz the power, 2016-03-23 02:12:35]

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

[commit-bot: I haz the power, 2016-03-23 02:12:36]

Dry run: This issue passed the CQ dry run.

[brettw, 2016-03-23 18:25:50]

Why do we need to disable precompiled headers for this bot? It might be nice to
add a comment in the config where this is set about why it's needed.

[Dirk Pranke, 2016-03-23 18:45:11]

On 2016/03/23 18:25:50, brettw wrote:
> Why do we need to disable precompiled headers for this bot? It might be nice
to
> add a comment in the config where this is set about why it's needed.

Good question. I was assuming that /analyze was incompatible with either goma
or precompiled headers, since the existing bot isn't using them, but I don't
actually
see something in the MSVC docs that says so.

Bruce?

[brucedawson, 2016-03-23 21:00:12]

> Bruce?

I don't remember why goma is disabled. Perhaps an untested belief that it
wouldn't work?

I know that when I first started using /analyze in 2011 there were problems with
distributed builds due to /analyze not being part of the regular compiler, but
that was changed in VC++ 2012, so it's not applicable.

As for precompiled header files I'm also not sure - I can't actually see where
they are disabled by the current configuration. Maybe they just aren't enabled
and they are off by default?

I'd be tempted to leave both settings to minimize the number of simultaneous
changes, and file a bug for me to clean up later. But if enabling them both is
cleaner I'd be okay with that - it's an fyi builder so the risk is minimal and
they *should* work.

[Dirk Pranke, 2016-03-23 21:34:07]

On 2016/03/23 21:00:12, brucedawson wrote:
> > Bruce?
> 
> I don't remember why goma is disabled. Perhaps an untested belief that it
> wouldn't work?
> 
> I know that when I first started using /analyze in 2011 there were problems
with
> distributed builds due to /analyze not being part of the regular compiler, but
> that was changed in VC++ 2012, so it's not applicable.
> 
> As for precompiled header files I'm also not sure - I can't actually see where
> they are disabled by the current configuration. Maybe they just aren't enabled
> and they are off by default?

You can see that goma and pch are disabled here in the runhooks log here:

https://build.chromium.org/p/chromium.fyi/builders/Chromium%20Windows%20Analy...

When you call configure_bot() in your recipe, it applies the 'chromium' config
by default

https://code.google.com/p/chromium/codesearch?q=chromium_win.py#chromium/buil...

which enables goma by default:

https://code.google.com/p/chromium/codesearch?q=chromium_win.py#chromium/buil...

which disables precompiled headers (which goma + cl doesn't support):

https://code.google.com/p/chromium/codesearch?q=chromium_win.py#chromium/buil...

Then you explicitly disable goma in

https://code.google.com/p/chromium/codesearch?q=chromium_win.py#chromium/buil...
https://code.google.com/p/chromium/codesearch?q=chromium_win.py#chromium/buil...

But that doesn't have the net effect of re-enabling pch, unfortunately.

Layers upon layers!

Let's land this as-is and then see what happens when we re-enable pch and goma
in subsequent CLs.

[Dirk Pranke, 2016-03-23 21:34:16]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2016-03-23 21:34:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
brucedawson@chromium.org
Link to the patchset: https://codereview.chromium.org/1827683002/#ps60001
(title: "sheesh")

[commit-bot: I haz the power, 2016-03-23 21:34:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1827683002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1827683002/60001

[commit-bot: I haz the power, 2016-03-23 21:43:29]

Committed patchset #4 (id:60001)

[brettw, 2016-03-23 21:44:12]

What's the downside to trying this with PCH on?

[commit-bot: I haz the power, 2016-03-23 21:45:03]

Description was changed from

==========
Add MB support for the windows analyze bot.

This adds the MB config for the 'Chromium Windows Analyze' bot
on the chromium.fyi waterfall, and adds the needed flag maps
to disable precompiled headers (incl. adding a disable_precompiled_headers
flag to GN) and adding 'win_analyze' mixin (we need to actually
implement the win_analyze GN flag still, though).

R=brettw@chromium.org, brucedawson@chromium.org
BUG=597112
==========

to

==========
Add MB support for the windows analyze bot.

This adds the MB config for the 'Chromium Windows Analyze' bot
on the chromium.fyi waterfall, and adds the needed flag maps
to disable precompiled headers (incl. adding a disable_precompiled_headers
flag to GN) and adding 'win_analyze' mixin (we need to actually
implement the win_analyze GN flag still, though).

R=brettw@chromium.org, brucedawson@chromium.org
BUG=597112

Committed: https://crrev.com/de9261d3072db949f60ffcaf09df404cfd7b807f
Cr-Commit-Position: refs/heads/master@{#382931}
==========

[commit-bot: I haz the power, 2016-03-23 21:45:04]

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

[Dirk Pranke, 2016-03-23 21:55:28]

On 2016/03/23 21:44:12, brettw wrote:
> What's the downside to trying this with PCH on?

It may make it slightly harder to diagnose any problems that arise from
switching from invoking GYP directly to invoking via MB.

I generally prefer to try and switch to the equivalent gyp configs in one patch,
and then tweak in subsequent patches.

Don't worry, I won't forget about this ...

[brettw, 2016-03-23 21:58:50]

On 2016/03/23 21:55:28, Dirk Pranke wrote:
> On 2016/03/23 21:44:12, brettw wrote:
> > What's the downside to trying this with PCH on?
> 
> It may make it slightly harder to diagnose any problems that arise from
> switching from invoking GYP directly to invoking via MB.
> 
> I generally prefer to try and switch to the equivalent gyp configs in one
patch,
> and then tweak in subsequent patches.
> 
> Don't worry, I won't forget about this ...

Okay! :)