Make debug builds component builds by default.

Make debug builds component builds by default.

(Except on iOS, where component builds aren't supported).

R=brettw@chromium.org
BUG=631537
Committed: https://crrev.com/5e90548f18cc7ce931c3fda659ebad2ebb9bc3f0
Cr-Commit-Position: refs/heads/master@{#409692}

[Nico, 2016-08-01 22:17:21]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2016-08-01 22:17:22]

I still have the same comment as on the other CL for this (ie I think it'd be
better if this was an explicit toggle that's just toggled to the right thing by
default)

[Dirk Pranke, 2016-08-01 22:40:09]

On 2016/08/01 22:17:22, Nico wrote:
> I still have the same comment as on the other CL for this (ie I think it'd be
> better if this was an explicit toggle that's just toggled to the right thing
by
> default)

And I continue to think that's the wrong thing to do, because that prevents us
from changing default values
for existing checkouts. But perhaps you think that's a good thing ...

[brettw, 2016-08-03 19:40:01]

The design of GN build flags requires that default value be declared with the
build flag. This gives us a canonical place to define what the value is when you
haven't set it. This value is exposed, for example, in "gn args" as the default
value. We can update this value with the build when users haven't overridden it,
and depend on this behavior for a great many things.

You're asking for a second layer of defaults on top of these that behave
differently. We would need to define some second place where yet another layer
of defaults is stored that overrides the current defaults, and these would not
be changeable once a user creates a build directory. This seems undesirable.

LGTM

[Nico, 2016-08-03 19:56:48]

On 2016/08/03 19:40:01, brettw (ping on IM after 24h) wrote:
> The design of GN build flags requires that default value be declared with the
> build flag. This gives us a canonical place to define what the value is when
you
> haven't set it. This value is exposed, for example, in "gn args" as the
default
> value. We can update this value with the build when users haven't overridden
it,
> and depend on this behavior for a great many things.
> 
> You're asking for a second layer of defaults on top of these that behave
> differently. We would need to define some second place where yet another layer
> of defaults is stored that overrides the current defaults, and these would not
> be changeable once a user creates a build directory. This seems undesirable.
> 
> LGTM

Since you disagree with my proposed solution to my perceived problem with this,
I started describing the problem again, but then I figured that probably
wouldn't go anywhere, and the root disagreement might be about which build types
we think various people use for various things. So here's a list of build types
and what I think when people use them. Do you disagree about the following list?

debug+static: Takes forever to build and is fairly slow to start. Nobody should
use this. (...except that component builds allegedly don't help much for blink
and slow down loading binaries into the debugger, so apparently some people do
want to use it, from the thread.)
debug+component: Still takes quite a while to build (apparently less bad on
Windows), but has symbols and no optimizations, so good for people who like
using debuggers
release+static: Performance characteristics much closer to official builds so
can be used for coarse performance work
release+component: Links fast, runs fast, optionally has symbols (even by
default on OS X). Imho best config for devs (with dchecks enabled)
official (always static): Can take 20 minutes for an incremental link. Same perf
characteristics as what we ship; fast. most people probably don't want to work
in this config.

[Dirk Pranke, 2016-08-03 21:57:21]

On 2016/08/03 19:56:48, Nico wrote:
> On 2016/08/03 19:40:01, brettw (ping on IM after 24h) wrote:
> > The design of GN build flags requires that default value be declared with
the
> > build flag. This gives us a canonical place to define what the value is when
> you
> > haven't set it. This value is exposed, for example, in "gn args" as the
> default
> > value. We can update this value with the build when users haven't overridden
> it,
> > and depend on this behavior for a great many things.
> > 
> > You're asking for a second layer of defaults on top of these that behave
> > differently. We would need to define some second place where yet another
layer
> > of defaults is stored that overrides the current defaults, and these would
not
> > be changeable once a user creates a build directory. This seems undesirable.
> > 
> > LGTM
> 
> Since you disagree with my proposed solution to my perceived problem with
this,
> I started describing the problem again, but then I figured that probably
> wouldn't go anywhere, and the root disagreement might be about which build
types
> we think various people use for various things. So here's a list of build
types
> and what I think when people use them. Do you disagree about the following
list?
> 
> debug+static: Takes forever to build and is fairly slow to start. Nobody
should
> use this. (...except that component builds allegedly don't help much for blink
> and slow down loading binaries into the debugger, so apparently some people do
> want to use it, from the thread.)
> debug+component: Still takes quite a while to build (apparently less bad on
> Windows), but has symbols and no optimizations, so good for people who like
> using debuggers
> release+static: Performance characteristics much closer to official builds so
> can be used for coarse performance work
> release+component: Links fast, runs fast, optionally has symbols (even by
> default on OS X). Imho best config for devs (with dchecks enabled)
> official (always static): Can take 20 minutes for an incremental link. Same
perf
> characteristics as what we ship; fast. most people probably don't want to work
> in this config.

Now you're straying more into the "stop supporting static debug builds" threads'
territory. I don't think we had consensus there that 'release shared' should be
the
default; in particular, jam@ didn't think we should support it at all.

I don't feel *that* strongly about release component, but I don't think one of
our
default configurations should be a configuration that isn't tested on the
waterfalls,
and I don't think we should add release component builds to the waterfalls, so
I would still probably not favor changing component to always be on by default.

However, if you want to try and argue for that, feel free to ping that thread or
start a new one and see if you can convince people.

[Dirk Pranke, 2016-08-03 21:57:26]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2016-08-03 21:58:15]

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

[Nico, 2016-08-03 22:06:21]

please don't land this before we've reached some kind of consensus

[Dirk Pranke, 2016-08-03 22:08:17]

The CQ bit was unchecked by dpranke@chromium.org

[Dirk Pranke, 2016-08-03 22:10:12]

On 2016/08/03 22:06:21, Nico wrote:
> please don't land this before we've reached some kind of consensus

/me un-checks the box.

There was consensus (as declared on that thread) that we should change the debug
default.

Are you saying you want to wait while you try to get consensus that we should
also change the
release default?

[Nico, 2016-08-03 22:20:38]

On 2016/08/03 22:10:12, Dirk Pranke wrote:
> On 2016/08/03 22:06:21, Nico wrote:
> > please don't land this before we've reached some kind of consensus
> 
> /me un-checks the box.
> 
> There was consensus (as declared on that thread) that we should change the
debug
> default.
> 
> Are you saying you want to wait while you try to get consensus that we should
> also change the
> release default?

I'd like the three of us to agree on some long-term plan.

But thinking about it again, you're right that that doesn't have to block
landing this CL. I think it's confusing if debug and release have different
defaults for component mode, but I might be wrong about this (we'll see) and
even if it is confusing it's less bad than having the worst-possible build
config as default. So I retract my "please don't land". Sorry!

[Dirk Pranke, 2016-08-03 22:25:52]

On 2016/08/03 22:20:38, Nico wrote:
 > I'd like the three of us to agree on some long-term plan.
> 
> But thinking about it again, you're right that that doesn't have to block
> landing this CL. I think it's confusing if debug and release have different
> defaults for component mode, but I might be wrong about this (we'll see) and
> even if it is confusing it's less bad than having the worst-possible build
> config as default. So I retract my "please don't land". Sorry!

No problem :)

I agree that it is confusing to have different defaults for component, but I
think it's important to have coverage of some shared config, some static config,
some debug config, and some release config on the waterfalls, and I think
we should also try to minimize the number of configs we have to support.

Given all that, debug+shared plus release+static seems like the best solution,
particularly since we're moving to a world where official builds take an ungodly
long time to build thanks to WPO/PGO/LTO.

/me re-checks the box.

[Dirk Pranke, 2016-08-03 22:26:01]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2016-08-03 22:26:43]

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-08-03 23:41:25]

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

[commit-bot: I haz the power, 2016-08-03 23:41:27]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Dirk Pranke, 2016-08-03 23:56:17]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2016-08-03 23:56:34]

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-08-04 01:20:11]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-08-04 01:25:04]

Description was changed from

==========
Make debug builds component builds by default.

(Except on iOS, where component builds aren't supported).

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

to

==========
Make debug builds component builds by default.

(Except on iOS, where component builds aren't supported).

R=brettw@chromium.org
BUG=631537

Committed: https://crrev.com/5e90548f18cc7ce931c3fda659ebad2ebb9bc3f0
Cr-Commit-Position: refs/heads/master@{#409692}
==========

[commit-bot: I haz the power, 2016-08-04 01:25:05]

Patchset 1 (id:??) landed as
https://crrev.com/5e90548f18cc7ce931c3fda659ebad2ebb9bc3f0
Cr-Commit-Position: refs/heads/master@{#409692}

[Nico, 2016-08-04 18:43:34]

On 2016/08/03 22:20:38, Nico wrote:
> On 2016/08/03 22:10:12, Dirk Pranke wrote:
> > On 2016/08/03 22:06:21, Nico wrote:
> > > please don't land this before we've reached some kind of consensus
> > 
> > /me un-checks the box.
> > 
> > There was consensus (as declared on that thread) that we should change the
> debug
> > default.
> > 
> > Are you saying you want to wait while you try to get consensus that we
should
> > also change the
> > release default?
> 
> I'd like the three of us to agree on some long-term plan.
> 
> But thinking about it again, you're right that that doesn't have to block
> landing this CL. I think it's confusing if debug and release have different
> defaults for component mode, but I might be wrong about this (we'll see) 

First data point: It just took me a bit to figure out why `ninja ipc_fuzzer`
told me that ipc_fuzzer doesn't exist (I wasn't explicitly setting is_debug, so
it defaulted to true, so that enabled the component build, which makes
ipc_fuzzer disappear). (Most people won't build ipc_fuzzer though; then again I
was cc'd on this change and didn't realize this immediately)